Skip to content

Commit

Permalink
Fix bug where colons in paths raise a ValueError on diff() calls.
Browse files Browse the repository at this point in the history
This commit introduces a potential fix for #1490 and #1483, in which an
`invalid literal for int() with base 10: 'n'` exception was raised
within a diff operation. Within `_handle_diff_line()`, we split the
output of `git diff-tree` on colons (`:` characters), under the
assumption that there are no colons within the paths of the files being
diffed. On POSIX systems this is not a valid assumption. The fix is to
split on `\x00:`, since a null character always precedes the colons we
actually need to split on.

A test already existed for this case (`test_diff_file_with_colon()`),
but it was marked as skipped.

* Split on `\x00:` instead of `:` in `_handle_diff_line()`.
* Unskip `test_diff_file_with_colon()`.
  • Loading branch information
langfield committed Sep 13, 2022
1 parent bec6157 commit db392ae
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
5 changes: 4 additions & 1 deletion git/diff.py
Expand Up @@ -570,7 +570,10 @@ def _index_from_patch_format(cls, repo: "Repo", proc: Union["Popen", "Git.AutoIn
def _handle_diff_line(lines_bytes: bytes, repo: "Repo", index: DiffIndex) -> None:
lines = lines_bytes.decode(defenc)

for line in lines.split(":")[1:]:
# Discard everything before the first colon, and the colon itself.
_, _, lines = lines.partition(":")

for line in lines.split("\x00:"):
meta, _, path = line.partition("\x00")
path = path.rstrip("\x00")
a_blob_id: Optional[str]
Expand Down
1 change: 0 additions & 1 deletion test/test_diff.py
Expand Up @@ -236,7 +236,6 @@ def test_diff_index_raw_format(self):
res[0].b_path,
)

@unittest.skip("This currently fails and would need someone to improve diff parsing")
def test_diff_file_with_colon(self):
output = fixture("diff_file_with_colon")
res = []
Expand Down

0 comments on commit db392ae

Please sign in to comment.