mirror of
https://gerrit.googlesource.com/git-repo
synced 2026-07-15 21:27:00 +00:00
sync: do not init sibling submodules in parallel
Initializing a submodule requires locking <parent>/.git/config. As the implementation was currently doing this in parallel for sibling submodules, it lead to a race condition causing intermittent errors like: error: could not lock config file .git/config: File exists This commit enforces that sibling submodules are initialized sequentially, eliminating the race condition. Change-Id: I5ffb3de90276ba43e262d0e279a3d34324220b63 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/591241 Tested-by: Josef Malmstrom <Josef.Malmstrom@arm.com> Commit-Queue: Josef Malmstrom <Josef.Malmstrom@arm.com> Reviewed-by: Mike Frysinger <vapier@google.com> Reviewed-by: Gavin Mak <gavinmak@google.com>
This commit is contained in:
committed by
gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com
parent
d9d86fb595
commit
4b462634e0
+28
-12
@@ -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
|
||||
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user