mirror of
https://gerrit.googlesource.com/git-repo
synced 2026-11-13 07:47:23 +00:00
sync: don't read target tree after reprojectcmd
After repo.reprojectcmd exits 0, _Reproject ran `git diff-index --cached
--name-only <revid>^{tree}` to verify that the index matches the target
commit's tree.
Peeling <revid>^{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 <revid>^{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 <gavinmak@google.com>
Tested-by: Gavin Mak <gavinmak@google.com>
Reviewed-by: Brian Gan <brgan@google.com>
This commit is contained in:
1 parent
616396317c
commit
95d7c8c0df
3 files changed
+19
-50
No files matched your search
@@ -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.
|
||||
|
||||
|
||||
-20
@@ -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-"
|
||||
|
||||
+16
-28
@@ -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):
|
||||
|
||||
Reference in new issue
Block a user