From 95d7c8c0df43e1d7fea97e61a38ff2bc908ebd24 Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Tue, 13 Oct 2026 00:19:02 +0000 Subject: [PATCH] sync: don't read target tree after reprojectcmd After repo.reprojectcmd exits 0, _Reproject ran `git diff-index --cached --name-only ^{tree}` to verify that the index matches the target commit's tree. Peeling ^{tree} and comparing every entry against the index forces Git to read the target commit and walk every tree object in the target revision from .git/objects. reprojectcmd exists for virtualized filesystems such as ABFS whose checkout command materializes the worktree and writes .git/index from its own tree metadata without reading .git/objects/pack/*-trees-*.pack. Running diff-index against ^{tree} afterward forces Git to fault in the tree packfiles over the network (260 FetchTrees pack downloads / 1.64 GiB across a 3,296-project Android checkout). Trust reprojectcmd's exit status for index materialization instead of re-walking the target tree in Git, while keeping the HEAD postcondition check that verifies the command did not modify refs. Bug: 543101585 Change-Id: Ie5feea41a4191245232c6763a489d01596370c55 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/643004 Commit-Queue: Gavin Mak Tested-by: Gavin Mak Reviewed-by: Brian Gan --- docs/reproject-cmd.md | 5 +++-- project.py | 20 -------------------- tests/test_project.py | 44 ++++++++++++++++--------------------------- 3 files changed, 19 insertions(+), 50 deletions(-) diff --git a/docs/reproject-cmd.md b/docs/reproject-cmd.md index 76e1de822..95d0ba639 100644 --- a/docs/reproject-cmd.md +++ b/docs/reproject-cmd.md @@ -91,8 +91,9 @@ Detecting collisions with untracked files or unstaged working-tree modifications After the command exits with status 0, `repo` expects the following postconditions to be met: -1. `git diff-index --quiet --cached REPO_TREV^{tree}` exits 0 (the index - matches the target tree). +1. The index matches `REPO_TREV`'s tree (`repo` trusts the command's exit + status and does not run `git diff-index` against `REPO_TREV^{tree}`, which + would read every tree object from `.git/objects`). 2. `HEAD` still names what it did before the command, and its resolved commit object ID has not changed. diff --git a/project.py b/project.py index ffa56a7c7..00655e7bc 100644 --- a/project.py +++ b/project.py @@ -4367,26 +4367,6 @@ class Project: project=self.name, ) - p = git_command.GitCommand( - self, - ["diff-index", "--cached", "--name-only", f"{revid}^{{tree}}"], - capture_stdout=True, - capture_stderr=True, - ) - if p.Wait() != 0: - raise LocalSyncFail( - f"cannot compare the index against {revid}: " - f"{p.stderr.strip()}", - project=self.name, - ) - mismatched = p.stdout.splitlines() - if mismatched: - raise LocalSyncFail( - f"reprojectcmd left the index different from {revid}:\n" - + _FirstLines(mismatched), - project=self.name, - ) - def _InitGitDir(self, mirror_git=None, force_sync=False, quiet=False): # Prefix for temporary directories created during gitdir initialization. TMP_GITDIR_PREFIX = ".tmp-project-initgitdir-" diff --git a/tests/test_project.py b/tests/test_project.py index 60d01ff39..7d67be23f 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -4008,13 +4008,12 @@ class ReprojectCmdTests(unittest.TestCase): @staticmethod def _git_command( - staged: str = "", diff: str = "", returncode: int = 0 + staged: str = "", returncode: int = 0 ) -> Callable[..., mock.MagicMock]: """Return a GitCommand stand-in answering the reproject queries. Args: staged: `git diff-index -z --cached` or `git ls-files -z` output. - diff: `git diff-index --cached` postcondition output. returncode: exit code of GitCommand. """ @@ -4027,12 +4026,7 @@ class ReprojectCmdTests(unittest.TestCase): return real_git_command(project, cmdv, **kwargs) cmd = mock.MagicMock() cmd.stderr = "" - if cmdv[0] == "diff-index": - if any("^{tree}" in arg for arg in cmdv): - cmd.stdout = diff - else: - cmd.stdout = staged - elif cmdv[0] == "ls-files": + if cmdv[0] in ("diff-index", "ls-files"): cmd.stdout = staged else: cmd.stdout = "" @@ -4168,19 +4162,23 @@ class ReprojectCmdTests(unittest.TestCase): proj._Reproject(self.REVID) self.assertIn("moved HEAD", str(e.exception)) - def test_reproject_rejects_an_index_mismatch(self) -> None: - """Test a command that left the index off the target fails the sync.""" + def test_reproject_does_not_read_target_tree(self) -> None: + """Test _Reproject does not diff the index against REPO_TREV^{tree}.""" with utils_for_test.TempGitTree() as tempdir: proj = self._get_project(tempdir) with mock.patch( "git_command.GitCommand", - side_effect=self._git_command(diff="lib/a.c\n"), - ), mock.patch("subprocess.run") as mock_run: + side_effect=self._git_command(), + ) as mock_git_cmd, mock.patch("subprocess.run") as mock_run: mock_run.return_value = mock.MagicMock(returncode=0, stdout="") - with self.assertRaises(project.LocalSyncFail) as e: - proj._Reproject(self.REVID) - self.assertIn(self.REVID, str(e.exception)) - self.assertIn("lib/a.c", str(e.exception)) + proj._Reproject(self.REVID) + + self.assertEqual(mock_git_cmd.call_count, 1) + cmdv = mock_git_cmd.call_args[0][1] + self.assertEqual( + cmdv, + ["diff-index", "-z", "--cached", "--name-only", self.HEAD_TREE], + ) def _get_synced_project( self, @@ -4999,7 +4997,7 @@ class ReprojectCmdGitTests(unittest.TestCase): ) def test_reproject_unborn_empty_skips_redundant_git_checks(self) -> None: - """Clean unborn files-backend checkout runs only post-check diff.""" + """Clean unborn files-backend checkout runs zero GitCommand checks.""" with tempfile.TemporaryDirectory() as topdir, mock.patch.dict( os.environ, {"GIT_DEFAULT_REF_FORMAT": "files"} ): @@ -5020,17 +5018,7 @@ class ReprojectCmdGitTests(unittest.TestCase): with mock.patch.object(git_command, "GitCommand", SpyGitCommand): proj._Reproject(revid) - self.assertEqual( - [ - [ - "diff-index", - "--cached", - "--name-only", - f"{revid}^{{tree}}", - ] - ], - seen_cmds, - ) + self.assertEqual([], seen_cmds) class DownloadPatchSetTests(unittest.TestCase):