diff --git a/subcmds/sync.py b/subcmds/sync.py index 7e0e7418c..8c5332761 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -96,19 +96,27 @@ logger = RepoLogger(__file__) def _SafeCheckoutOrder(checkouts: List[Project]) -> List[List[Project]]: - """Generate a sequence of checkouts that is safe to perform. The client - should checkout everything from n-th index before moving to n+1. + """Generate a sequence of checkouts that is safe to perform. + + The client should checkout everything from n-th index before moving to + n+1. This is only useful if manifest contains nested projects. E.g. if foo, foo/bar and foo/bar/baz are project paths, then foo needs to finish before foo/bar can proceed, and foo/bar needs to finish before - foo/bar/baz.""" - res = [[]] - current = res[0] + foo/bar/baz. - # depth_stack contains a current stack of parent paths. + Discovered submodules have an additional constraint: sibling submodules in + the same parent repository must not be checked out in parallel because they + all run `git submodule init` against the same parent .git/config. + """ + res = [[]] + + # depth_stack contains the current stack of parent paths together with the + # effective checkout level assigned to each path. depth_stack = [] + submodule_parent_level = {} # Checkouts are iterated in the hierarchical order. That way, it can easily # be determined if the previous checkout is parent of the current checkout. # We are splitting by the path separator so the final result is @@ -119,21 +127,29 @@ def _SafeCheckoutOrder(checkouts: List[Project]) -> List[List[Project]]: checkout_path = Path(checkout.relpath) while depth_stack: try: - checkout_path.relative_to(depth_stack[-1]) + checkout_path.relative_to(depth_stack[-1][0]) except ValueError: # Path.relative_to returns ValueError if paths are not relative. # TODO(sokcevic): Switch to is_relative_to once min supported # version is py3.9. depth_stack.pop() else: - if len(depth_stack) >= len(res): - # Another depth created. - res.append([]) break - current = res[len(depth_stack)] + level = depth_stack[-1][1] + 1 if depth_stack else 0 + parent = checkout.parent + if parent is not None: + level = max( + level, + submodule_parent_level.get(parent.worktree, level - 1) + 1, + ) + submodule_parent_level[parent.worktree] = level + if level >= len(res): + res.extend([] for _ in range(level + 1 - len(res))) + + current = res[level] current.append(checkout) - depth_stack.append(checkout_path) + depth_stack.append((checkout_path, level)) return res diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index 785c0e629..70b9684a7 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py @@ -340,6 +340,7 @@ class FakeProject: self.name = name or relpath self.objdir = objdir or relpath self.worktree = relpath + self.parent = None self.use_git_worktrees = False self.UseAlternates = False @@ -397,6 +398,65 @@ class SafeCheckoutOrder(unittest.TestCase): ], ) + def test_sibling_submodules_with_shared_parent_are_serialized(self): + parent = mock.Mock(worktree="/worktree/parent") + other_parent = mock.Mock(worktree="/worktree/other") + p_parent = FakeProject("parent") + p_other = FakeProject("other") + p_parent_sub1 = FakeProject("parent/sub1") + p_parent_sub1.parent = parent + p_parent_sub2 = FakeProject("parent/sub2") + p_parent_sub2.parent = parent + p_other_sub = FakeProject("other/sub") + p_other_sub.parent = other_parent + + out = sync._SafeCheckoutOrder( + [p_parent_sub2, p_other_sub, p_parent, p_parent_sub1, p_other] + ) + + self.assertEqual( + out, + [ + [p_other, p_parent], + [p_other_sub, p_parent_sub1], + [p_parent_sub2], + ], + ) + + def test_nested_submodules_respect_delayed_parent_level(self): + parent = mock.Mock(worktree="/worktree/parent") + sub1 = mock.Mock(worktree="/worktree/parent/sub1") + sub2 = mock.Mock(worktree="/worktree/parent/sub2") + p_parent = FakeProject("parent") + p_parent_sub1 = FakeProject("parent/sub1") + p_parent_sub1.parent = parent + p_parent_sub1_nested = FakeProject("parent/sub1/nested") + p_parent_sub1_nested.parent = sub1 + p_parent_sub2 = FakeProject("parent/sub2") + p_parent_sub2.parent = parent + p_parent_sub2_nested = FakeProject("parent/sub2/nested") + p_parent_sub2_nested.parent = sub2 + + out = sync._SafeCheckoutOrder( + [ + p_parent_sub2_nested, + p_parent_sub2, + p_parent_sub1_nested, + p_parent, + p_parent_sub1, + ] + ) + + self.assertEqual( + out, + [ + [p_parent], + [p_parent_sub1], + [p_parent_sub1_nested, p_parent_sub2], + [p_parent_sub2_nested], + ], + ) + class Chunksize(unittest.TestCase): """Tests for _chunksize."""