diff --git a/git/diff.py b/git/diff.py index 123c22840..48ed863f4 100644 --- a/git/diff.py +++ b/git/diff.py @@ -9,16 +9,9 @@ import re import warnings -from git.cmd import Git, handle_process_output -from git.compat import defenc -from git.objects.base import IndexObject -from git.objects.blob import Blob -from git.objects.util import mode_str_to_int -from git.util import finalize_process, hex_to_bin - # typing ------------------------------------------------------------------ - from typing import ( + TYPE_CHECKING, Any, Iterator, List, @@ -26,12 +19,18 @@ Optional, Sequence, Tuple, - TYPE_CHECKING, TypeVar, Union, cast, ) -from git.types import PathLike, Literal + +from git.cmd import Git, handle_process_output +from git.compat import defenc +from git.objects.base import IndexObject +from git.objects.blob import Blob +from git.objects.util import mode_str_to_int +from git.types import Literal, PathLike +from git.util import finalize_process, hex_to_bin if TYPE_CHECKING: from subprocess import Popen @@ -410,7 +409,8 @@ class Diff: re_header = re.compile( rb""" ^diff[ ]--git - [ ](?P"?[ab]/.+?"?)[ ](?P"?[ab]/.+?"?)\n + [ ](?P(?P"?[ab]/.+?"?) + [ ](?P"?[ab]/.+?"?))\n (?:^old[ ]mode[ ](?P\d+)\n ^new[ ]mode[ ](?P\d+)(?:\n|$))? (?:^similarity[ ]index[ ]\d+%\n @@ -419,7 +419,7 @@ class Diff: (?:^new[ ]file[ ]mode[ ](?P.+)(?:\n|$))? (?:^deleted[ ]file[ ]mode[ ](?P.+)(?:\n|$))? (?:^similarity[ ]index[ ]\d+%\n - ^copy[ ]from[ ].*\n + ^copy[ ]from[ ](?P.*)\n ^copy[ ]to[ ](?P.*)(?:\n|$))? (?:^index[ ](?P[0-9A-Fa-f]+) \.\.(?P[0-9A-Fa-f]+)[ ]?(?P.+)?(?:\n|$))? @@ -626,6 +626,42 @@ def _pick_best_path(cls, path_match: bytes, rename_match: bytes, path_fallback_m return None + @staticmethod + def _same_path_in_header(names: bytes) -> Optional[bytes]: + """Return the path that a ``diff --git a/ b/`` line names twice. + + git separates the two names with a space but does not quote a path for + containing one, so once the path contains ``" a/"`` or ``" b/"`` the line alone + does not show where the first name ends. When no ``---``, ``+++``, rename or + copy line names the file, both names are the same path, and the split is + accepted only where the two halves agree, as ``git_header_name`` in git's + ``apply.c`` does. A quoted first name ends at its closing quote. + + :return: + The path without its ``a/`` prefix, or ``None`` if no split agrees. + """ + if names.startswith(b'"'): + end = 1 + while end < len(names) and names[end : end + 1] != b'"': + end += 2 if names[end : end + 1] == b"\\" else 1 + split = end + 1 + else: + # Equal unquoted paths have equal-length names, so only the midpoint + # can separate them. Trying every space would copy the header quadratically. + if len(names) % 2 == 0: + return None + split = len(names) // 2 + if names[split : split + 1] != b" ": + return None + a_name, b_name = names[:split], names[split + 1 :] + if a_name.startswith(b'"'): + if not (a_name.endswith(b'"') and b_name.startswith(b'"') and b_name.endswith(b'"')): + return None + a_name, b_name = _unquote_path(a_name[1:-1]), _unquote_path(b_name[1:-1]) + if a_name.startswith(b"a/") and b_name.startswith(b"b/") and a_name[2:] == b_name[2:]: + return a_name[2:] + return None + @classmethod def _index_from_patch_format(cls, repo: "Repo", proc: Union["Popen", "Git.AutoInterrupt"]) -> DiffIndex["Diff"]: """Create a new :class:`DiffIndex` from the given process output which must be @@ -662,6 +698,7 @@ def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> N a_mode, b_mode = None, None # For mypy. for _header in cls.re_header.finditer(text): ( + header_names, a_path_fallback, b_path_fallback, old_mode, @@ -670,6 +707,7 @@ def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> N rename_to, new_file_mode, deleted_file_mode, + copied_from_name, copied_file_name, a_blob_id, b_blob_id, @@ -684,8 +722,15 @@ def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> N bool(copied_file_name), ) - a_path = cls._pick_best_path(a_path, rename_from, a_path_fallback) - b_path = cls._pick_best_path(b_path, rename_to, b_path_fallback) + # Mode changes, binary files and empty files have no `---` and `+++` lines, + # so only the `diff --git` line names them, and it names the same path twice. + named_elsewhere = any((a_path, b_path, rename_from, rename_to, copied_from_name, copied_file_name)) + same_path = None if named_elsewhere else cls._same_path_in_header(header_names) + if same_path is None: + a_path = cls._pick_best_path(a_path, rename_from or copied_from_name, a_path_fallback) + b_path = cls._pick_best_path(b_path, rename_to or copied_file_name, b_path_fallback) + else: + a_path = b_path = same_path # Our only means to find the actual text is to see what has not been matched # by our regex, and then retro-actively assign it to our index. diff --git a/test/test_diff.py b/test/test_diff.py index 06399b823..07e2011d7 100644 --- a/test/test_diff.py +++ b/test/test_diff.py @@ -4,23 +4,21 @@ # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ import gc -from io import BytesIO import os.path as osp import shutil import sys import tempfile +from io import BytesIO import ddt import pytest - from gitdb.base import IStream -from git import NULL_TREE, Blob, Diff, DiffIndex, Diffable, GitCommandError, Repo, Submodule +from git import NULL_TREE, Blob, Diff, Diffable, DiffIndex, GitCommandError, Repo, Submodule from git.cmd import Git from git.diff import decode_path from git.exc import UnsafeOptionError from git.index.typ import BaseIndexEntry - from test.lib import StringProcessAdapter, TestBase, fixture, with_rw_directory @@ -270,6 +268,92 @@ def test_diff_raw_paths_with_colon_or_line_feed(self, rw_dir): self.assertEqual([d.b_path for d in diffs], sorted(paths)) self.assertEqual({d.change_type for d in diffs}, {"A"}) + @with_rw_directory + def test_diff_patch_paths_named_only_by_the_header_line(self, rw_dir): + # Binary files, empty files, mode changes and copies have no `---` and `+++` + # lines, so their paths come from the `diff --git a/ b/` line, where + # git does not quote a space. Each path contains " a/" or " b/" so that the line + # alone does not show where the first name ends. + repo = Repo.init(rw_dir) + binary = repo.odb.store(IStream(Blob.type, 1, BytesIO(b"\0"))).binsha + empty = repo.odb.store(IStream(Blob.type, 0, BytesIO(b""))).binsha + paths = ["assets b/logo.png", "docs a/empty", "quoted\t a/tab"] + repo.index.add( + [ + BaseIndexEntry((0o100644, binary, 0, paths[0])), + BaseIndexEntry((0o100644, empty, 0, paths[1])), + BaseIndexEntry((0o100644, binary, 0, paths[2])), + ] + ) + commit = repo.index.commit("paths") + + diffs = commit.diff(NULL_TREE, create_patch=True) + self.assertEqual({(d.a_path, d.b_path) for d in diffs}, {(path, path) for path in paths}) + + repo.index.add( + [ + BaseIndexEntry((0o100755, binary, 0, paths[0])), + BaseIndexEntry((0o100644, binary, 0, "copy b/logo.png")), + ] + ) + changed = repo.index.commit("mode change and copy") + + diffs = commit.diff(changed, create_patch=True, C=True, find_copies_harder=True) + self.assertEqual( + {(d.a_path, d.b_path, d.copied_file) for d in diffs}, + {(paths[0], paths[0], False), (paths[0], "copy b/logo.png", True)}, + ) + + @ddt.data( + (b"a/x b/x", b"x"), + (b"a/x b/x ", b"x "), + (b"a/x b/y", None), + (b"a/x b/xx", None), + (b"a/x-b/x", None), + (b"", None), + (b'"a/x\\t" "b/x\\t"', b"x\t"), + (b'"a/x\\t" "b/x\\011"', b"x\t"), + (b'"a/x\\t" "b/y\\t"', None), + (b'"a/x\\t" "b/x\\t', None), + ) + def test_same_path_in_header(self, case): + names, expected = case + self.assertEqual(Diff._same_path_in_header(names), expected) + + def test_same_path_in_header_copies_linear_bytes(self): + class SliceCountingBytes(bytes): + sliced_bytes = 0 + + def __getitem__(self, key): + result = super().__getitem__(key) + if isinstance(key, slice): + self.sliced_bytes += len(result) + return result + + path = b"x " * 1000 + names = SliceCountingBytes(b"a/" + path + b" b/" + path) + self.assertEqual(Diff._same_path_in_header(names), path) + # Count copied bytes rather than asserting a machine-dependent time limit. + self.assertLessEqual(names.sliced_bytes, 2 * len(names)) + + @with_rw_directory + def test_diff_patch_mode_only_with_long_space_heavy_path(self, rw_dir): + repo = Repo.init(rw_dir) + path = b"x " * 250000 + blob = repo.odb.store(IStream(Blob.type, 0, BytesIO(b""))).binsha + trees = [] + # Store trees directly: Git accepts paths longer than filesystem or index limits. + for mode in (b"100644", b"100755"): + data = mode + b" " + path + b"\0" + blob + tree = repo.odb.store(IStream(b"tree", len(data), BytesIO(data))) + trees.append(repo.tree(tree.binsha.hex())) + + diffs = trees[0].diff(trees[1], create_patch=True) + self.assertEqual(len(diffs), 1) + self.assertEqual((diffs[0].a_rawpath, diffs[0].b_rawpath), (path, path)) + self.assertEqual((diffs[0].a_mode, diffs[0].b_mode), (0o100644, 0o100755)) + self.assertEqual(diffs[0].diff, b"") + def test_empty_diff(self): res = [] Diff._handle_diff_line(b"", None, res)