mirrors/git-repo
1
0
Fork 0
mirror of https://gerrit.googlesource.com/git-repo synced 2026-05-07 11:29:27 +00:00
Code Issues Projects Releases Wiki Activity

sync: Refactor to use _RunOneGC and fix config leakage

Browse Source 
Extract _RunOneGC to handle GC on a single project. This refactoring
makes it easier to invoke GC from parallel worker tasks.

Also, avoid modifying the passed-in config dictionary in _RunOneGC by
creating a local copy, preventing unintended side effects on other
commands sharing the same config.

Bug: 498290329
Change-Id: I7b77ed6629b14b5ee3322870b9c6c8ce2bfd6ea2
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/574923
Reviewed-by: Becky Siegel <beckysiegel@google.com>
Tested-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
This commit is contained in:
Gavin Mak
2026-04-20 17:57:20 +00:00
committed by gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com
parent 7e9079b7cf
commit baa281d99e
2 changed files with 42 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
subcmds/sync.py
+23 -8
View File
@@ -1244,14 +1244,15 @@ later is required to fix a server side protocol bug.
return False return False
def _SetPreciousObjectsState(self, project: Project, opt): @classmethod
def _SetPreciousObjectsState(cls, project: Project, opt):
"""Correct the preciousObjects state for the project. """Correct the preciousObjects state for the project.
Args: Args:
project: the project to examine, and possibly correct. project: the project to examine, and possibly correct.
opt: options given to sync. opt: options given to sync.
""" """
expected = self._GetPreciousObjectsState(project, opt) expected = cls._GetPreciousObjectsState(project, opt)
actual = ( actual = (
project.config.GetBoolean("extensions.preciousObjects") or False project.config.GetBoolean("extensions.preciousObjects") or False
) )
@@ -1285,7 +1286,22 @@ later is required to fix a server side protocol bug.
project.config.SetString("extensions.preciousObjects", None) project.config.SetString("extensions.preciousObjects", None)
project.config.SetString("gc.pruneExpire", None) project.config.SetString("gc.pruneExpire", None)
def _GCProjects(self, projects, opt, err_event): @staticmethod
def _RunOneGC(project: Project, config: Optional[dict] = None) -> None:
"""Run auto GC on a single project."""
local_config = {}
if config:
local_config.update(config)
local_config["gc.autoDetach"] = "false"
project.bare_git.gc("--auto", config=local_config)
@classmethod
def _GCProjects(
cls,
projects: List[Project],
opt: optparse.Values,
err_event: _threading.Event,
) -> None:
"""Perform garbage collection. """Perform garbage collection.
If We are skipping garbage collection (opt.auto_gc not set), we still If We are skipping garbage collection (opt.auto_gc not set), we still
@@ -1295,7 +1311,7 @@ later is required to fix a server side protocol bug.
if not opt.auto_gc: if not opt.auto_gc:
# Just repair preciousObjects state, and return. # Just repair preciousObjects state, and return.
for project in projects: for project in projects:
self._SetPreciousObjectsState(project, opt) cls._SetPreciousObjectsState(project, opt)
return return
pm = Progress( pm = Progress(
@@ -1305,9 +1321,8 @@ later is required to fix a server side protocol bug.
tidy_dirs = {} tidy_dirs = {}
for project in projects: for project in projects:
self._SetPreciousObjectsState(project, opt) cls._SetPreciousObjectsState(project, opt)
project.config.SetString("gc.autoDetach", "false")
# Only call git gc once per objdir, but call pack-refs for the # Only call git gc once per objdir, but call pack-refs for the
# remainder. # remainder.
if project.objdir not in tidy_dirs: if project.objdir not in tidy_dirs:
@@ -1328,7 +1343,7 @@ later is required to fix a server side protocol bug.
pm.update(msg=bare_git._project.name) pm.update(msg=bare_git._project.name)
if run_gc: if run_gc:
bare_git.gc("--auto") cls._RunOneGC(bare_git._project)
else: else:
bare_git.pack_refs() bare_git.pack_refs()
pm.end() pm.end()
@@ -1345,7 +1360,7 @@ later is required to fix a server side protocol bug.
try: try:
try: try:
if run_gc: if run_gc:
bare_git.gc("--auto", config=config) cls._RunOneGC(bare_git._project, config=config)
else: else:
bare_git.pack_refs(config=config) bare_git.pack_refs(config=config)
except GitError: except GitError:
tests/test_subcmds_sync.py
+19 -18
View File
@@ -552,35 +552,36 @@ class GCProjectsTest(unittest.TestCase):
def test_GCProjects_skip_gc(self, mock_progress): def test_GCProjects_skip_gc(self, mock_progress):
"""Test that it skips GC if opt.auto_gc is False.""" """Test that it skips GC if opt.auto_gc is False."""
self.opt.auto_gc = False self.opt.auto_gc = False
self.cmd._SetPreciousObjectsState = mock.Mock() with mock.patch.object(
self.cmd._GCProjects([self.project], self.opt, None) sync.Sync, "_SetPreciousObjectsState"
self.cmd._SetPreciousObjectsState.assert_called_once_with( ) as mock_set_state:
self.project, self.opt self.cmd._GCProjects([self.project], self.opt, None)
) mock_set_state.assert_called_once_with(self.project, self.opt)
self.assertFalse(self.project.bare_git.gc.called) self.assertFalse(self.project.bare_git.gc.called)
@mock.patch("subcmds.sync.Progress") @mock.patch("subcmds.sync.Progress")
def test_GCProjects_sequential(self, mock_progress): def test_GCProjects_sequential(self, mock_progress):
"""Test sequential GC (jobs < 2).""" """Test sequential GC (jobs < 2)."""
self.cmd._SetPreciousObjectsState = mock.Mock() with mock.patch.object(sync.Sync, "_SetPreciousObjectsState"):
self.cmd._GCProjects([self.project], self.opt, None) self.cmd._GCProjects([self.project], self.opt, None)
self.project.config.SetString.assert_called_once_with( self.project.bare_git.gc.assert_called_once_with(
"gc.autoDetach", "false" "--auto", config={"gc.autoDetach": "false"}
) )
self.project.bare_git.gc.assert_called_once_with("--auto") # Verify that gc.autoDetach was not permanently set in config.
for call in self.project.config.SetString.call_args_list:
self.assertNotEqual(call.args[0], "gc.autoDetach")
@mock.patch("subcmds.sync.Progress") @mock.patch("subcmds.sync.Progress")
def test_GCProjects_parallel(self, mock_progress): def test_GCProjects_parallel(self, mock_progress):
"""Test parallel GC (jobs >= 2).""" """Test parallel GC (jobs >= 2)."""
self.opt.jobs = 2 self.opt.jobs = 2
self.cmd._SetPreciousObjectsState = mock.Mock() with mock.patch.object(sync.Sync, "_SetPreciousObjectsState"):
with mock.patch("subcmds.sync._threading.Thread") as mock_thread:
with mock.patch("subcmds.sync._threading.Thread") as mock_thread: mock_t = mock.MagicMock()
mock_t = mock.MagicMock() mock_thread.return_value = mock_t
mock_thread.return_value = mock_t err_event = mock.Mock()
err_event = mock.Mock() err_event.is_set.return_value = False
err_event.is_set.return_value = False self.cmd._GCProjects([self.project], self.opt, err_event)
self.cmd._GCProjects([self.project], self.opt, err_event)
self.assertTrue(mock_thread.called) self.assertTrue(mock_thread.called)