sync: skip the bloat check for projects without a worktree Skip shallow clone bloat checks during --network-only syncs and filter out projects whose worktree directory does not exist on disk. Also return None quietly from _GetStatusSnapshot() when a project lacks an existing worktree directory. Bug: 565047698 Change-Id: Ia4c7db36a2bd78bc89e7b72f7a492b901b50efa7 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/634425 Commit-Queue: Gavin Mak <gavinmak@google.com> Reviewed-by: Brian Gan <brgan@google.com> Tested-by: Gavin Mak <gavinmak@google.com>
diff --git a/project.py b/project.py index 4988776..6fce0e4 100644 --- a/project.py +++ b/project.py
@@ -886,9 +886,16 @@ ahead_behind: bool = False, show_stash: bool = False, ) -> Optional[git_status.StatusSnapshot]: - """Read one porcelain-v2 snapshot, or select the legacy path.""" + """Read one porcelain-v2 snapshot, or select the legacy path. + + Returns None on Git older than 2.11, when git status fails, or when + the worktree directory is missing. A missing worktree isn't logged: + status can't run there, and the legacy path raises its own error. + """ if not git_require((2, 11, 0)): return None + if not self.worktree or not platform_utils.isdir(self.worktree): + return None try: return git_status.GetStatus( self,
diff --git a/subcmds/sync.py b/subcmds/sync.py index bae3677..a0cb937 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py
@@ -1608,9 +1608,6 @@ """ project = cls.get_parallel_context()["projects"][project_index] - if not project.Exists or not project.worktree: - return None - # Only check dirty or locally modified projects. These can't be # freshly cloned and will accumulate garbage. try: @@ -1660,16 +1657,27 @@ run 'git count-objects -v' and warn if the repository is accumulating excessive pack files or garbage. """ + # --network-only promises not to touch worktrees, but git status and + # update-index --refresh can both rewrite the index. + if opt.network_only: + return + # We only care about bloated projects if we have a git version that # supports --no-auto-gc (2.23.0+) since what we use to disable auto-gc # in Project._RemoteFetch. if not git_require((2, 23, 0)): return + # Skip projects with no worktree on disk, e.g. ones only ever synced + # with --network-only. projects = [ p for p in projects - if p.clone_depth and not p.stateless_prune_needed + if p.clone_depth + and not p.stateless_prune_needed + and p.worktree + and p.Exists + and platform_utils.isdir(p.worktree) ] if not projects: return
diff --git a/tests/test_project.py b/tests/test_project.py index 2009949..46bc7f1 100644 --- a/tests/test_project.py +++ b/tests/test_project.py
@@ -713,6 +713,18 @@ ): self.assertIsNone(proj._GetStatusSnapshot()) + def test_get_status_snapshot_missing_worktree_is_quiet(self) -> None: + """A missing worktree skips git status without a warning.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + proj.worktree = os.path.join(tempdir, "missing") + with mock.patch.object(git_status, "GetStatus") as mock_get_status: + with mock.patch.object(project, "logger") as mock_logger: + self.assertIsNone(proj._GetStatusSnapshot()) + + mock_get_status.assert_not_called() + mock_logger.warning.assert_not_called() + def test_dirty_or_stash_uses_status_stash_header(self) -> None: """A normal stash is detected without a second Git process.""" with utils_for_test.TempGitTree() as tempdir:
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index 005e650..349202d 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py
@@ -981,10 +981,13 @@ self.opt = mock.Mock() self.opt.quiet = True self.opt.jobs = 1 + self.opt.network_only = False + self.tempdirobj = tempfile.TemporaryDirectory(prefix="repo_tests") + self.addCleanup(self.tempdirobj.cleanup) self.project = mock.MagicMock(clone_depth="1") self.project.name = "project" self.project.Exists = True - self.project.worktree = "worktree" + self.project.worktree = self.tempdirobj.name self.project.stateless_prune_needed = False self.cmd.git_event_log = mock.MagicMock() self.cmd._bloated_projects = [] @@ -1051,6 +1054,42 @@ self.cmd._CheckForBloatedProjects([self.project], self.opt) self.assertFalse(self.cmd.git_event_log.ErrorEvent.called) + @mock.patch("subcmds.sync.git_require", return_value=True) + @mock.patch("subcmds.sync.Progress") + def test_network_only_skips_check( + self, mock_progress: mock.Mock, mock_git_require: mock.Mock + ) -> None: + """--network-only doesn't read any worktree state.""" + self.opt.network_only = True + self.cmd.ExecuteInParallel = mock.Mock() + + self.cmd._CheckForBloatedProjects([self.project], self.opt) + + mock_progress.assert_not_called() + self.cmd.ExecuteInParallel.assert_not_called() + + @mock.patch("subcmds.sync.git_require", return_value=True) + @mock.patch("subcmds.sync.Progress") + def test_projects_without_worktree_excluded( + self, mock_progress: mock.Mock, mock_git_require: mock.Mock + ) -> None: + """Projects without a checked-out worktree are never scanned.""" + self.cmd.ExecuteInParallel = mock.Mock() + missing = os.path.join(self.tempdirobj.name, "missing") + for attr, value in ( + ("worktree", missing), + ("worktree", None), + ("Exists", False), + ): + with self.subTest(attr=attr, value=value): + mock_progress.reset_mock() + self.cmd.ExecuteInParallel.reset_mock() + with mock.patch.object(self.project, attr, value): + self.cmd._CheckForBloatedProjects([self.project], self.opt) + + mock_progress.assert_not_called() + self.cmd.ExecuteInParallel.assert_not_called() + @mock.patch("subcmds.sync.git_require") @mock.patch("subcmds.sync.Progress") def test_bloated_project_found(self, mock_progress, mock_git_require):