From 4978203b074c66af0a10211f7d6ccdffcaefc109 Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Fri, 6 Nov 2026 22:26:33 +0000 Subject: [PATCH] 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 Commit-Queue: Gavin Mak Reviewed-by: Brian Gan --- project.py | 24 ++++++++++++++++-------- subcmds/sync.py | 10 +++++++--- tests/test_project.py | 18 ++++++++++++++++++ tests/test_subcmds_sync.py | 19 +++++++++++++++++++ 4 files changed, 60 insertions(+), 11 deletions(-) diff --git a/project.py b/project.py index 25455297c..4d0bd3d41 100644 --- a/project.py +++ b/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") diff --git a/subcmds/sync.py b/subcmds/sync.py index 54e206ed4..acef8ea6c 100644 --- a/subcmds/sync.py +++ b/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 diff --git a/tests/test_project.py b/tests/test_project.py index d7ce0a641..3d848838c 100644 --- a/tests/test_project.py +++ b/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.""" diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index be8bcde02..8e8eda252 100644 --- a/tests/test_subcmds_sync.py +++ b/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",