Skip to content

index.move failed when file being moved contains the string " to " #1283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
malsharman opened this issue Jul 2, 2021 · 10 comments
Open

index.move failed when file being moved contains the string " to " #1283

malsharman opened this issue Jul 2, 2021 · 10 comments

Comments

@malsharman
Copy link

When the filename being moved contains the string " to " this command fails with the assertion error "Too many tokens"

Steps to reproduce.

  1. Setup GitPython to point to a repository
  2. add a file called "Need to move.txt"
  3. call index.move on this file to a different file or location

Result:
Assertion error "Too Many Tokens"

Then found this line

tokens = mvlines[ln].split(' to ')

Happy to look at working on a fix, but don't exactly understand why the parsing of the string returned from git is required.

@Byron
Copy link
Member

Byron commented Jul 2, 2021

Thanks a lot for the report!

It's quite amazing to see 11 year old code do…that. Indeed, it would be great if you could attempt a fix. I would be curious if a test breaks without this kind of parsing, and if not it's probably safe to discard.

@neingeist
Copy link

neingeist commented Nov 23, 2021

Confirmed.

The code is running git mv with --dry-run and then parsing the result, e.g.:

% git mv --dry-run Need\ to\ move.txt Need\ to\ move2.txt
Checking rename of 'Need to move.txt' to 'Need to move2.txt'
Renaming Need to move.txt to Need to move2.txt

This is going to be hard to distinguish the tos here.

@neingeist
Copy link

git mv --dry-run bar foo
Checking rename of 'test/bar' to 'test/foo'
Checking rename of 'test/bar/Just to show hard this is to fix.txt' to 'test/foo/Just to show hard this is to fix.txt'
Checking rename of 'test/bar/Need to move.txt' to 'test/foo/Need to move.txt'
Renaming test/bar to test/foo
Renaming test/bar/Just to show hard this is to fix.txt to test/foo/Just to show hard this is to fix.txt
Renaming test/bar/Need to move.txt to test/foo/Need to move.txt

😬

@Byron
Copy link
Member

Byron commented Nov 23, 2021

That's a tough one indeed. One could hope there is some sort of flag that allows to change the messaging around renames. Otherwise GitPython would have to re-implement the renaming logic to be able to anticipate outputs from inputs.

@neingeist
Copy link

neingeist commented Nov 25, 2021

The question is if the Checking rename output is always equivalent to the Renaming output. The code suggests it indirectly (only checking exactly half of the output), but I wander what happens when "Checking" isn't successful?

@malsharman
Copy link
Author

It does look like some more sophisticated parsing is required. Since git --dry-run puts single quotes around the source and destination file in the output, the parser for this would have to recognise the difference between the "to" in between the single quote in 'filename to move.txt' to 'destination to move to.txt' and the "to" that is in between the two single quoted strings.
Its definitely a more complex parsing problem.

@Byron
Copy link
Member

Byron commented Dec 9, 2021

Oh indeed, there are single quotes which makes parsing this non-ambiguous. Once one verified that single-quotes within single-quotes are escaped with a backslash the parser should be able to do the right thing at all times.

@neingeist
Copy link

Note that currently the code is parsing the second half of the output, the one without the quotes. I have no idea if they are always the same.

@neingeist
Copy link

I've added a single quote to one of the files:

 git mv --dry-run bar foo                                                                                                              master +
Checking rename of 'test/bar' to 'test/foo'
Checking rename of 'test/bar/Just to show how 'hard this is to fix.txt' to 'test/foo/Just to show how 'hard this is to fix.txt'
Checking rename of 'test/bar/Need to move.txt' to 'test/foo/Need to move.txt'
Renaming test/bar to test/foo
Renaming test/bar/Just to show how 'hard this is to fix.txt to test/foo/Just to show how 'hard this is to fix.txt
Renaming test/bar/Need to move.txt to test/foo/Need to move.txt

🙄

@macfreek
Copy link

macfreek commented Oct 11, 2024

I just run in this very issue, and was stumped by it... the real fix is to use a programmatic interface instead of using Popen and parsing the raw text output. This triggered me to migrate from GitPython to pygit2 on top of libgit2. That said, I do want to give some kudos to the maintainers of this project for maintaining it for so long (Nov 2014, almost 14 years. Funily the same age as pygit2, which had its first commit in the same month). Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants