sync: skip submodules removed from their parent The submodules to sync are derived from their parent before it is fetched, so one that the parent's new revision no longer holds a gitlink for is still synced. Interleaved sync then fails to check it out, because `git submodule init` no longer knows that path: error: Cannot checkout a/c error: pathspec '.../a/c' did not match any file(s) known to git Report such submodules while the gitlinks are read again, and leave them out of the fetch and of the checkout, so the usual project list update removes them from the working tree. Phased sync already left them out of the checkout, but still fetched them. Nothing is reported as removed while the gitlinks of a parent cannot be read, since a missing gitlink cannot be told apart from a fetch that did not happen. Bug: 550074864 Change-Id: Ia429b35c12758a68e4df73665e5512a201af658a Signed-off-by: kimhappy <hwanhee.kim@laplacian.cc> Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/621681 Reviewed-by: Gavin Mak <gavinmak@google.com> Reviewed-by: Brian Gan <brgan@google.com>
diff --git a/subcmds/sync.py b/subcmds/sync.py index e713300..bedc1f1 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py
@@ -179,7 +179,7 @@ def _RefreshDerivedRevisions( projects: List[Project], submodule_revisions: Optional[Dict[Project, Dict[str, str]]] = None, -) -> None: +) -> List[Project]: """Re-resolve the gitlinks of the discovered submodules in |projects|. The revision of a discovered submodule is read from its parent when the @@ -193,6 +193,9 @@ holding them. Passing the same dict for project sets that follow the same fetches, e.g. the levels of one checkout order, keeps a project from being read more than once. + + Returns: + The submodules that their parent no longer holds a gitlink for. """ if submodule_revisions is None: submodule_revisions = {} @@ -202,6 +205,7 @@ if project.Derived and project.parent: subprojects_by_parent[project.parent].append(project) + removed = [] for parent, subprojects in subprojects_by_parent.items(): revisions = submodule_revisions.get(parent) if revisions is None: @@ -214,6 +218,19 @@ rev = revisions.get(subproject.gitlink_path) if rev: subproject.SetRevision(rev, revisionId=rev) + else: + removed.append(subproject) + return removed + + +def _WithoutProjects( + projects: List[Project], unwanted: List[Project] +) -> List[Project]: + """Return |projects| without the projects in |unwanted|.""" + if not unwanted: + return projects + dropped = set(unwanted) + return [p for p in projects if p not in dropped] def _chunksize(projects: int, jobs: int) -> int: @@ -1150,7 +1167,9 @@ success = True fetched = set() for batch in _ParentFirstBatches(projects): - _RefreshDerivedRevisions(batch) + batch = _WithoutProjects(batch, _RefreshDerivedRevisions(batch)) + if not batch: + continue batch.sort(key=self._fetch_times.Get, reverse=True) result = self._Fetch(batch, opt, err_event, ssh_proxy, errors) success = success and result.success @@ -3076,9 +3095,14 @@ if not level_projects: continue - _RefreshDerivedRevisions( - level_projects, submodule_revisions + level_projects = _WithoutProjects( + level_projects, + _RefreshDerivedRevisions( + level_projects, submodule_revisions + ), ) + if not level_projects: + continue objdir_project_map = collections.defaultdict( list
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index e854a19..b921069 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py
@@ -708,6 +708,14 @@ p_a.GetSubmoduleRevisions.assert_not_called() + def test_reports_submodules_removed_from_their_parent(self) -> None: + p_a = self._parent_with_submodules(b="beef1234") + p_a_c = self._submodule(p_a, "c") + + removed = sync._RefreshDerivedRevisions([p_a, p_a_c]) + + self.assertEqual(removed, [p_a_c]) + def test_keeps_submodules_of_an_unreadable_parent(self) -> None: p_a = FakeProject("a") p_a.GetSubmoduleRevisions = mock.Mock(return_value=None) @@ -719,8 +727,9 @@ gitlink_path="b", ) - sync._RefreshDerivedRevisions([p_a, p_a_b]) + removed = sync._RefreshDerivedRevisions([p_a, p_a_b]) + self.assertEqual(removed, []) self.assertEqual(p_a_b.revisionId, "stale") @@ -764,6 +773,23 @@ ) +class WithoutProjects(unittest.TestCase): + def test_drops_the_unwanted_projects(self) -> None: + p_a = FakeProject("a") + p_a_b = FakeProject("a/b") + self.assertEqual(sync._WithoutProjects([p_a, p_a_b], [p_a_b]), [p_a]) + self.assertEqual(sync._WithoutProjects([p_a, p_a_b], []), [p_a, p_a_b]) + + def test_keeps_projects_with_the_same_path(self) -> None: + # Paths are relative to their own (sub)manifest, so two projects can + # share one. + first = FakeProject("a/b") + second = FakeProject("a/b") + self.assertEqual( + sync._WithoutProjects([first, second], [second]), [first] + ) + + class Chunksize(unittest.TestCase): """Tests for _chunksize.""" @@ -1364,6 +1390,60 @@ self.assertIn(("projA/sub", "fetched"), synced) + def test_interleaved_skips_removed_submodule(self) -> None: + """Test submodules dropped by their parent are not checked out.""" + opt, args = self.cmd.OptionParser.parse_args(["--interleaved", "-j4"]) + opt.quiet = True + + submodule = FakeProject( + "projA/sub", + name="projA_sub", + objdir="objA_sub", + parent=self.projA, + is_derived=True, + revisionId="stale", + gitlink_path="sub", + ) + # The parent no longer holds a gitlink for the submodule. + self.projA.GetSubmoduleRevisions = mock.Mock(return_value={}) + # The reloaded manifest no longer derives the removed submodule. + mock.patch.object( + self.cmd, "GetProjects", return_value=[self.projA] + ).start() + + synced = [] + + def execute_side_effect( + jobs: int, + target: object, + work_items: List[List[int]], + **kwargs: object, + ) -> bool: + synced_relpaths_set = kwargs["callback"].args[0] + projects_in_pass = self.cmd.get_parallel_context()["projects"] + for item in work_items: + for project_idx in item: + project = projects_in_pass[project_idx] + synced.append(project.relpath) + synced_relpaths_set.add(project.relpath) + return True + + mock.patch.object( + self.cmd, "ExecuteInParallel", side_effect=execute_side_effect + ).start() + + self.cmd._SyncInterleaved( + opt, + args, + [], + self.manifest, + self.manifest.manifestProject, + [self.projA, submodule], + {}, + ) + + self.assertEqual(synced, ["projA"]) + def test_interleaved_shared_objdir_serial(self): """Test that projects with shared objdir are processed serially.""" opt, args = self.cmd.OptionParser.parse_args(["--interleaved", "-j4"])