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>

project.py

@@ -886,9 +886,16 @@ class Project:
         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,

subcmds/sync.py

@@ -1608,9 +1608,6 @@ later is required to fix a server side protocol bug.
         """
         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 @@ later is required to fix a server side protocol bug.
         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

tests/test_project.py

@@ -713,6 +713,18 @@ class ProjectTests(unittest.TestCase):
             ):
                 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:

tests/test_subcmds_sync.py

@@ -981,10 +981,13 @@ class CheckForBloatedProjects(unittest.TestCase):
         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 @@ class CheckForBloatedProjects(unittest.TestCase):
         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):