sync: skip shallow bloat check when using fetchcmd

Skip projects fetched via repo.fetchcmd in _CheckForBloatedProjects,
and expose Project.UseFetchCmd alongside Project.UseReprojectCmd.

The bloat check exists because Project._RemoteFetch passes --no-auto-gc
for clone-depth="1" repositories. Projects using repo.fetchcmd bypass
_RemoteFetch and manage object storage externally, so scanning their
worktrees for bloat is unnecessary and recommending git repack can
materialize virtualized objects locally.

Bug: 543101585
Change-Id: I16510ad3b5b39c3a5e2a1a75df706439252a088e
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/655083
Tested-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Brian Gan <brgan@google.com>

project.py

@@ -1965,15 +1965,11 @@ class Project:
         else:
             alt_dir = None
 
-        custom_fetch_configured = (
-            self.manifest.manifestProject.use_local_gitdirs
-            and self.manifest.manifestProject.fetch_cmd
-            and not isinstance(self, MetaProject)
-        )
+        use_fetch_cmd = self.UseFetchCmd
 
         if (
             clone_bundle
-            and not custom_fetch_configured
+            and not use_fetch_cmd
             and alt_dir is None
             and self._ApplyCloneBundle(
                 initial=is_new, quiet=quiet, verbose=verbose
@@ -2008,7 +2004,7 @@ class Project:
             clone_filter = clone_filter_for_depth
 
         remote_fetched = False
-        if custom_fetch_configured:
+        if use_fetch_cmd:
             # The custom fetch command is executed unconditionally on every sync
             # because it is responsible for updating tracking refs and
             # FETCH_HEAD.
@@ -2179,6 +2175,18 @@ class Project:
 
         self.revisionId = revisionId
 
+    @property
+    def UseFetchCmd(self) -> bool:
+        """Whether repo.fetchcmd fetches this project's objects.
+
+        MetaProjects (repo itself and the manifests) always use Git. See
+        docs/fetch-cmd.md.
+        """
+        if isinstance(self, MetaProject):
+            return False
+        mp = self.manifest.manifestProject
+        return bool(mp.use_local_gitdirs and mp.fetch_cmd)
+
     @property
     def UseReprojectCmd(self) -> bool:
         """Whether repo.reprojectcmd materializes this project's tree.
@@ -5923,7 +5931,7 @@ class ManifestProject(MetaProject):
         return self.config.GetBoolean("repo.uselocalgitdirs")
 
     @property
-    def fetch_cmd(self):
+    def fetch_cmd(self) -> Optional[str]:
         """The fetch command to use."""
         return self.config.GetString("repo.fetchcmd")
 

subcmds/sync.py

@@ -1747,7 +1747,9 @@ later is required to fix a server side protocol bug.
             return project.name
         return None
 
-    def _CheckForBloatedProjects(self, projects, opt):
+    def _CheckForBloatedProjects(
+        self, projects: List[Project], opt: optparse.Values
+    ) -> None:
         """Check for shallow projects that are accumulating unoptimized data.
 
         For projects with clone-depth="1" that are dirty (have local changes),
@@ -1765,12 +1767,14 @@ later is required to fix a server side protocol bug.
         if not git_command.git_require((2, 23, 0)):
             return
 
-        # Skip projects with no worktree on disk, e.g. ones only ever synced
-        # with --network-only.
+        # Skip projects fetched via repo.fetchcmd (which bypasses _RemoteFetch
+        # and manages object storage externally) or 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.UseFetchCmd
             and not p.stateless_prune_needed
             and p.worktree
             and p.Exists

tests/test_project.py

@@ -3722,6 +3722,24 @@ class FetchCmdTests(unittest.TestCase):
                     proj._CustomFetch()
                 mock_run.assert_not_called()
 
+    def test_use_fetch_cmd_property(self) -> None:
+        """Test UseFetchCmd requires use_local_gitdirs and skips MetaProject."""
+        with utils_for_test.TempGitTree() as tempdir:
+            proj = self._get_project(tempdir)
+            self.assertTrue(proj.UseFetchCmd)
+
+            proj.manifest.manifestProject.use_local_gitdirs = False
+            self.assertFalse(proj.UseFetchCmd)
+
+            proj.manifest.manifestProject.use_local_gitdirs = True
+            proj.manifest.manifestProject.fetch_cmd = None
+            self.assertFalse(proj.UseFetchCmd)
+
+            fakeproj = _create_meta_project(tempdir)
+            fakeproj.config.SetString("repo.fetchcmd", "echo hi")
+            fakeproj.config.SetBoolean("repo.uselocalgitdirs", True)
+            self.assertFalse(fakeproj.UseFetchCmd)
+
 
 class ReprojectCmdTests(unittest.TestCase):
     """Tests for the repo.reprojectcmd feature."""

tests/test_subcmds_sync.py

@@ -987,6 +987,7 @@ class FakeProject:
 
         self.use_git_worktrees = False
         self.UseAlternates = False
+        self.UseFetchCmd = False
         self.UseReprojectCmd = False
         self.manifest = mock.MagicMock()
         self.manifest.GetProjectsWithName.return_value = [self]
@@ -1433,6 +1434,7 @@ class CheckForBloatedProjects(unittest.TestCase):
         self.project.name = "project"
         self.project.Exists = True
         self.project.worktree = self.tempdirobj.name
+        self.project.UseFetchCmd = False
         self.project.stateless_prune_needed = False
         self.cmd.git_event_log = mock.MagicMock()
         self.cmd._bloated_projects = []
@@ -1573,6 +1575,22 @@ class CheckForBloatedProjects(unittest.TestCase):
         self.assertFalse(self.cmd.ExecuteInParallel.called)
         self.assertEqual(self.cmd._bloated_projects, [])
 
+    @mock.patch.object(git_command, "git_require", return_value=True)
+    @mock.patch("subcmds.sync.Progress")
+    def test_fetch_cmd_excluded(
+        self, mock_progress: mock.Mock, mock_git_require: mock.Mock
+    ) -> None:
+        """Projects using repo.fetchcmd are excluded from the bloat check."""
+        self.project.UseFetchCmd = True
+        self.cmd.ExecuteInParallel = mock.Mock()
+
+        with mock.patch.object(self.cmd, "ParallelContext"):
+            self.cmd._CheckForBloatedProjects([self.project], self.opt)
+
+        mock_progress.assert_not_called()
+        self.cmd.ExecuteInParallel.assert_not_called()
+        self.assertEqual(self.cmd._bloated_projects, [])
+
 
 class GCProjectsTest(unittest.TestCase):
     """Tests for Sync._GCProjects."""
@@ -1662,6 +1680,7 @@ class SyncCommand(unittest.TestCase):
         self.project = p = mock.MagicMock(
             use_git_worktrees=False,
             UseAlternates=False,
+            UseFetchCmd=False,
             UseReprojectCmd=False,
             name="project",
             relpath="rel_path",