project: avoid unnecessary sync revision enumeration Skip the detached-history walk when its count will not be printed, ask rev-list --count for upstream gain, and cap the published-but-unmerged existence probe at one commit. These preserve the decisions while reducing traversal and output work. Bug: 553599402 Change-Id: I30f769c9aa5269428ece15ffcb45c50993d7c4b8 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/634004 Commit-Queue: Gavin Mak <gavinmak@google.com> Tested-by: Gavin Mak <gavinmak@google.com> Reviewed-by: Brian Gan <brgan@google.com>
diff --git a/project.py b/project.py index a10de99..75d722d 100644 --- a/project.py +++ b/project.py
@@ -2053,13 +2053,13 @@ def Sync_LocalHalf( self, - syncbuf, - force_sync=False, - force_checkout=False, - force_rebase=False, - submodules=False, - verbose=False, - ): + syncbuf: Any, + force_sync: bool = False, + force_checkout: bool = False, + force_rebase: bool = False, + submodules: bool = False, + verbose: bool = False, + ) -> None: """Perform only the local IO portion of the sync process. Network access is not required. @@ -2203,9 +2203,11 @@ self._CopyAndLinkFiles() return else: - lost = self._revlist(not_rev(revid), HEAD) - if lost and verbose: - syncbuf.info(self, "discarding %d commits", len(lost)) + if verbose: + lost_output = self._revlist("--count", not_rev(revid), HEAD) + lost = int(lost_output[0]) if lost_output else 0 + if lost: + syncbuf.info(self, "discarding %d commits", lost) try: _checkout() @@ -2244,7 +2246,8 @@ self._CopyAndLinkFiles() return - upstream_gain = self._revlist(not_rev(HEAD), revid) + gain_output = self._revlist("--count", not_rev(HEAD), revid) + upstream_gain = int(gain_output[0]) if gain_output else 0 # See if we can perform a fast forward merge. This can happen if our # branch isn't in the exact same state as we last published. @@ -2258,7 +2261,7 @@ pub = self.WasPublished(branch.name, all_refs) if pub: - not_merged = self._revlist(not_rev(revid), pub) + not_merged = self._revlist("-1", not_rev(revid), pub) if not_merged: if upstream_gain: if force_rebase: @@ -2274,7 +2277,7 @@ "branch %s is published (but not merged) and " "is now %d commits behind. Fix this manually " "or rerun with the --rebase option to force a " - "rebase." % (branch.name, len(upstream_gain)), + "rebase." % (branch.name, upstream_gain), project=self.name, ) )
diff --git a/tests/test_project.py b/tests/test_project.py index ab20105..d02dabf 100644 --- a/tests/test_project.py +++ b/tests/test_project.py
@@ -2248,6 +2248,97 @@ ) proj._CopyAndLinkFiles.assert_called_once_with() + def test_sync_local_half_skips_unused_detached_history_walk(self) -> None: + """Non-verbose detached sync does not enumerate discarded commits.""" + with utils_for_test.TempGitTree() as tempdir: + proj = self._get_project(tempdir) + proj._InitWorkTree = mock.MagicMock() + proj.CleanPublishedCache = mock.MagicMock() + proj.GetRevisionId = mock.MagicMock(return_value="new") + proj._Checkout = mock.MagicMock() + proj._CopyAndLinkFiles = mock.MagicMock() + proj.IsRebaseInProgress = mock.MagicMock(return_value=False) + proj.IsCherryPickInProgress = mock.MagicMock(return_value=False) + proj._revlist = mock.MagicMock() + proj.bare_ref = mock.MagicMock() + proj.bare_ref.all = {"HEAD": "old"} + proj.bare_ref.head = "old" + proj.work_git = mock.MagicMock() + proj.work_git.GetHead.return_value = "old" + + syncbuf = project.SyncBuffer(proj.config) + proj.Sync_LocalHalf(syncbuf, verbose=False) + + proj._revlist.assert_not_called() + proj._Checkout.assert_called_once_with( + "new", force_checkout=False, quiet=True + ) + + def test_sync_local_half_verbose_detached_uses_count(self) -> None: + """Verbose detached sync queries commit count with --count.""" + with utils_for_test.TempGitTree() as tempdir: + proj = self._get_project(tempdir) + proj._InitWorkTree = mock.MagicMock() + proj.CleanPublishedCache = mock.MagicMock() + proj.GetRevisionId = mock.MagicMock(return_value="new") + proj._Checkout = mock.MagicMock() + proj._CopyAndLinkFiles = mock.MagicMock() + proj.IsRebaseInProgress = mock.MagicMock(return_value=False) + proj.IsCherryPickInProgress = mock.MagicMock(return_value=False) + proj._revlist = mock.MagicMock(return_value=["3"]) + proj.bare_ref = mock.MagicMock() + proj.bare_ref.all = {"HEAD": "old"} + proj.bare_ref.head = "old" + proj.work_git = mock.MagicMock() + proj.work_git.GetHead.return_value = "old" + + syncbuf = mock.MagicMock() + syncbuf.detach_head = False + proj.Sync_LocalHalf(syncbuf, verbose=True) + + proj._revlist.assert_called_once_with("--count", "^new", "HEAD") + syncbuf.info.assert_called_once_with( + proj, "discarding %d commits", 3 + ) + + def test_sync_local_half_uses_count_and_limit_for_published_branch( + self, + ) -> None: + """Published sync uses --count for gain and -1 for probe.""" + with utils_for_test.TempGitTree() as tempdir: + proj = self._get_project(tempdir) + proj._InitWorkTree = mock.MagicMock() + proj.CleanPublishedCache = mock.MagicMock() + proj.GetRevisionId = mock.MagicMock(return_value="new") + proj._CopyAndLinkFiles = mock.MagicMock() + proj.IsRebaseInProgress = mock.MagicMock(return_value=False) + proj.IsCherryPickInProgress = mock.MagicMock(return_value=False) + proj._revlist = mock.MagicMock(side_effect=[["2"], ["pub-sha"]]) + proj.bare_ref = mock.MagicMock() + proj.bare_ref.all = {"refs/heads/topic": "old"} + proj.bare_ref.head = "refs/heads/topic" + proj.work_git = mock.MagicMock() + proj.work_git.GetHead.return_value = "refs/heads/topic" + proj.work_git.merge_base.side_effect = project.GitError("diverged") + branch = mock.MagicMock() + branch.name = "topic" + branch.LocalMerge = "refs/remotes/origin/main" + proj.GetBranch = mock.MagicMock(return_value=branch) + proj.WasPublished = mock.MagicMock(return_value="pub-sha") + + syncbuf = mock.MagicMock() + syncbuf.detach_head = False + proj.Sync_LocalHalf(syncbuf, force_rebase=False) + + self.assertEqual( + proj._revlist.call_args_list, + [ + mock.call("--count", "^HEAD", "new"), + mock.call("-1", "^new", "pub-sha"), + ], + ) + syncbuf.fail.assert_called_once() + def test_sync_network_half_stateless_skips_if_stash(self): """Test stateless sync skips if stash exists.""" with utils_for_test.TempGitTree() as tempdir: @@ -3351,9 +3442,11 @@ def _revlist(*args: Any, **kwargs: Any) -> List[str]: if kwargs.get("format"): return list(local_changes) - if args[0] == project.not_rev(project.HEAD): + if project.not_rev(project.HEAD) in args: + if "--count" in args: + return [str(len(upstream_gain))] return list(upstream_gain) - if args[1] == self.PUB_ID: + if self.PUB_ID in args: return [self.PUB_ID] return []