From ccf9ce39b8228403d94007d3a1e9f2b153df7e8c Mon Sep 17 00:00:00 2001 From: Victor Pushkarev Date: Thu, 17 Sep 2026 10:47:00 +0300 Subject: [PATCH] sync: keep local sync state paths relative to repo root `LocalSyncState` uses `Project.relpath` to identify projects, but that path is relative to each project's own manifest. Projects in different manifests can have the same relpath, so their fetch and checkout timestamps overwrite each other. This can make a later partial sync look like a full sync. Pruning has a related problem. It treats the stored paths as relative to the current manifest's topdir, so when sync starts from the outermost manifest it can look for submanifest projects in the wrong place and remove valid state. Use `Project.RelPath(local=False)` to identify projects in local sync state and check stored paths from the repo root when pruning. Add tests for same-relpath projects, partial-sync detection, and pruning when sync starts from either the outermost manifest or a submanifest. Change-Id: I24485c315f1277a14913443674245cedd20820fe Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/632421 Reviewed-by: Gavin Mak Reviewed-by: Mike Frysinger Tested-by: Victor Pushkarev Commit-Queue: Victor Pushkarev --- subcmds/sync.py | 7 +-- tests/test_subcmds_sync.py | 91 +++++++++++++++++++++++++++++++++----- 2 files changed, 83 insertions(+), 15 deletions(-) diff --git a/subcmds/sync.py b/subcmds/sync.py index 523fac7df..32fd7f0a2 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -3365,13 +3365,13 @@ class LocalSyncState: def _Get(self, project, key): self._Load() - p = project.relpath + p = project.RelPath(local=False) if p not in self._state: return return self._state[p].get(key) def _Set(self, project, key): - p = project.relpath + p = project.RelPath(local=False) if p not in self._state: self._state[p] = {} self._state[p][key] = self._time @@ -3399,8 +3399,9 @@ class LocalSyncState: if not self._state: return delete = set() + outer_topdir = self._manifest.outer_client.topdir for path in self._state: - gitdir = os.path.join(self._manifest.topdir, path, ".git") + gitdir = os.path.join(outer_topdir, path, ".git") if not os.path.exists(gitdir) or os.path.islink(gitdir): delete.add(path) if not delete: diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index 349202d3c..7203414aa 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py @@ -338,8 +338,9 @@ class LocalSyncState(unittest.TestCase): self.manifest = mock.MagicMock( topdir=self.topdir, repodir=self.repodir, - repoProject=mock.MagicMock(relpath=".repo/repo"), + repoProject=FakeProject(".repo/repo"), ) + self.manifest.outer_client = self.manifest self.state = self._new_state() def tearDown(self): @@ -352,7 +353,7 @@ class LocalSyncState(unittest.TestCase): def test_set(self): """Times are set.""" - p = mock.MagicMock(relpath="projA") + p = FakeProject("projA") self.state.SetFetchTime(p) self.state.SetCheckoutTime(p) self.assertEqual(self.state.GetFetchTime(p), self._TIME) @@ -374,8 +375,8 @@ class LocalSyncState(unittest.TestCase): # Initialize state to read from the new file. self.state = self._new_state() - projA = mock.MagicMock(relpath="projA") - projB = mock.MagicMock(relpath="projB") + projA = FakeProject("projA") + projB = FakeProject("projB") self.assertEqual(self.state.GetFetchTime(projA), None) self.assertEqual(self.state.GetFetchTime(projB), 5) self.assertEqual(self.state.GetCheckoutTime(projB), 7) @@ -386,9 +387,43 @@ class LocalSyncState(unittest.TestCase): self.assertEqual(self.state.GetFetchTime(projB), self._TIME) self.assertEqual(self.state.GetCheckoutTime(projB), 7) + def test_same_relpath_projects_keep_separate_state(self) -> None: + """Projects with the same relpath keep separate sync state.""" + outer = FakeProject("proj") + child = FakeProject("proj", path_prefix="sub") + + self.state.SetFetchTime(outer) + self.state.Save() + + self.state = self._new_state(self._TIME + 1) + self.state.SetFetchTime(child) + + self.assertEqual(self.state.GetFetchTime(outer), self._TIME) + self.assertEqual( + self.state.GetFetchTime(child), + self._TIME + 1, + ) + + def test_partial_sync_with_same_relpath_projects(self) -> None: + """Projects with the same relpath keep independent partial sync.""" + outer = FakeProject("proj") + child = FakeProject("proj", path_prefix="sub") + + for project in (outer, child): + self.state.SetFetchTime(project) + self.state.SetCheckoutTime(project) + self.state.Save() + self.assertFalse(self.state.IsPartiallySynced()) + + self.state = self._new_state(self._TIME + 1) + self.state.SetFetchTime(outer) + self.state.SetCheckoutTime(outer) + + self.assertTrue(self.state.IsPartiallySynced()) + def test_save_to_file(self): """Data is saved under repodir.""" - p = mock.MagicMock(relpath="projA") + p = FakeProject("projA") self.state.SetFetchTime(p) self.state.Save() self.assertEqual( @@ -415,7 +450,7 @@ class LocalSyncState(unittest.TestCase): # Initialize state to read from the new file. self.state = self._new_state() - projB = mock.MagicMock(relpath="projB") + projB = FakeProject("projB") self.assertEqual(self.state.IsPartiallySynced(), False) self.state.SetFetchTime(projB) @@ -424,7 +459,7 @@ class LocalSyncState(unittest.TestCase): def test_ignore_repo_project(self): """Sync data for repo project is ignored when checking partial sync.""" - p = mock.MagicMock(relpath="projA") + p = FakeProject("projA") self.state.SetFetchTime(p) self.state.SetCheckoutTime(p) self.state.SetFetchTime(self.manifest.repoProject) @@ -441,7 +476,7 @@ class LocalSyncState(unittest.TestCase): def test_nonexistent_project(self): """Unsaved projects don't have data.""" - p = mock.MagicMock(relpath="projC") + p = FakeProject("projC") self.assertEqual(self.state.GetFetchTime(p), None) self.assertEqual(self.state.GetCheckoutTime(p), None) @@ -466,8 +501,8 @@ class LocalSyncState(unittest.TestCase): return False return True - projA = mock.MagicMock(relpath="projA") - projB = mock.MagicMock(relpath="projB") + projA = FakeProject("projA") + projB = FakeProject("projB") self.state = self._new_state() self.assertEqual(self.state.GetFetchTime(projA), 5) self.assertEqual(self.state.GetFetchTime(projB), 7) @@ -479,6 +514,38 @@ class LocalSyncState(unittest.TestCase): self.assertIsNone(self.state.GetFetchTime(projA)) self.assertEqual(self.state.GetFetchTime(projB), 7) + def test_prune_keeps_submanifest_project(self) -> None: + """Existing submanifest projects are not pruned.""" + project = FakeProject("proj", path_prefix="sub") + os.makedirs( + os.path.join(self.topdir, "sub", "proj", ".git"), + ) + + self.state.SetFetchTime(project) + self.state.PruneRemovedProjects() + + self.assertEqual(self.state.GetFetchTime(project), self._TIME) + + def test_prune_from_submanifest_keeps_project(self) -> None: + """Existing projects are not pruned during a submanifest sync.""" + child_topdir = os.path.join(self.topdir, "sub") + project = FakeProject("proj", path_prefix="sub") + os.makedirs(os.path.join(child_topdir, "proj", ".git")) + + child_manifest = mock.MagicMock( + topdir=child_topdir, + repodir=self.repodir, + outer_client=self.manifest, + ) + + with mock.patch("time.time", return_value=self._TIME): + state = sync.LocalSyncState(child_manifest) + + state.SetFetchTime(project) + state.PruneRemovedProjects() + + self.assertEqual(state.GetFetchTime(project), self._TIME) + def test_prune_removed_and_symlinked_projects(self): """Removed projects that still exists on disk as symlink are pruned.""" with open(self.state._path, "w") as f: @@ -503,8 +570,8 @@ class LocalSyncState(unittest.TestCase): return True return False - projA = mock.MagicMock(relpath="projA") - projB = mock.MagicMock(relpath="projB") + projA = FakeProject("projA") + projB = FakeProject("projB") self.state = self._new_state() self.assertEqual(self.state.GetFetchTime(projA), 5) self.assertEqual(self.state.GetFetchTime(projB), 7)