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 <gavinmak@google.com> Reviewed-by: Mike Frysinger <vapier@google.com> Tested-by: Victor Pushkarev <corvinus.v@gmail.com> Commit-Queue: Victor Pushkarev <corvinus.v@gmail.com>
diff --git a/subcmds/sync.py b/subcmds/sync.py index 523fac7..32fd7f0 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py
@@ -3365,13 +3365,13 @@ 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 @@ 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 349202d..7203414 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py
@@ -338,8 +338,9 @@ 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 @@ 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 @@ # 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 @@ 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 @@ # 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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)