sync: Introduce git checkout levels
If a repo manifest is updated so that project B is placed within a
project A, and if project A had content in new B's location in the old
checkout, then repo sync could break depending on checkout order, since
B can't be checked out before A.
This change introduces checkout levels which enforces right sequence of
checkouts while still allowing for parallel checkout. In an example
above, A will always be checked out first before B.
BUG=b:325119758
TEST=./run_tests, manual sync on ChromeOS repository
Change-Id: Ib3b5e4d2639ca56620a1e4c6bf76d7b1ab805250
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/410421
Tested-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Greg Edelston <gredelston@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
diff --git a/subcmds/sync.py b/subcmds/sync.py
index bf1171d..c6682a5 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -21,6 +21,7 @@
import netrc
import optparse
import os
+from pathlib import Path
import sys
import tempfile
import time
@@ -87,6 +88,45 @@
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.
+
+ 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]
+
+ # depth_stack contains a current stack of parent paths.
+ depth_stack = []
+ # checkouts are iterated in asc order by relpath. That way, it can easily be
+ # determined if the previous checkout is parent of the current checkout.
+ for checkout in sorted(checkouts, key=lambda x: x.relpath):
+ checkout_path = Path(checkout.relpath)
+ while depth_stack:
+ try:
+ checkout_path.relative_to(depth_stack[-1])
+ 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)]
+ current.append(checkout)
+ depth_stack.append(checkout_path)
+
+ return res
+
+
class _FetchOneResult(NamedTuple):
"""_FetchOne return value.
@@ -1035,15 +1075,21 @@
pm.update(msg=project.name)
return ret
- proc_res = self.ExecuteInParallel(
- opt.jobs_checkout,
- functools.partial(
- self._CheckoutOne, opt.detach_head, opt.force_sync, opt.verbose
- ),
- all_projects,
- callback=_ProcessResults,
- output=Progress("Checking out", len(all_projects), quiet=opt.quiet),
- )
+ for projects in _SafeCheckoutOrder(all_projects):
+ proc_res = self.ExecuteInParallel(
+ opt.jobs_checkout,
+ functools.partial(
+ self._CheckoutOne,
+ opt.detach_head,
+ opt.force_sync,
+ opt.verbose,
+ ),
+ projects,
+ callback=_ProcessResults,
+ output=Progress(
+ "Checking out", len(all_projects), quiet=opt.quiet
+ ),
+ )
self._local_sync_state.Save()
return proc_res and not err_results
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py
index af6bbef..13e23e3 100644
--- a/tests/test_subcmds_sync.py
+++ b/tests/test_subcmds_sync.py
@@ -304,6 +304,32 @@
self.assertEqual(self.state.GetFetchTime(projA), 5)
+class SafeCheckoutOrder(unittest.TestCase):
+ def test_no_nested(self):
+ p_f = mock.MagicMock(relpath="f")
+ p_foo = mock.MagicMock(relpath="foo")
+ out = sync._SafeCheckoutOrder([p_f, p_foo])
+ self.assertEqual(out, [[p_f, p_foo]])
+
+ def test_basic_nested(self):
+ p_foo = p_foo = mock.MagicMock(relpath="foo")
+ p_foo_bar = mock.MagicMock(relpath="foo/bar")
+ out = sync._SafeCheckoutOrder([p_foo, p_foo_bar])
+ self.assertEqual(out, [[p_foo], [p_foo_bar]])
+
+ def test_complex_nested(self):
+ p_foo = mock.MagicMock(relpath="foo")
+ p_foo_bar = mock.MagicMock(relpath="foo/bar")
+ p_foo_bar_baz_baq = mock.MagicMock(relpath="foo/bar/baz/baq")
+ p_bar = mock.MagicMock(relpath="bar")
+ out = sync._SafeCheckoutOrder(
+ [p_foo_bar_baz_baq, p_foo, p_foo_bar, p_bar]
+ )
+ self.assertEqual(
+ out, [[p_bar, p_foo], [p_foo_bar], [p_foo_bar_baz_baq]]
+ )
+
+
class GetPreciousObjectsState(unittest.TestCase):
"""Tests for _GetPreciousObjectsState."""
