diff --git a/subcmds/sync.py b/subcmds/sync.py index bf1171dd..c6682a5b 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -21,6 +21,7 @@ import multiprocessing import netrc import optparse import os +from pathlib import Path import sys import tempfile import time @@ -87,6 +88,45 @@ _REPO_ALLOW_SHALLOW = os.environ.get("REPO_ALLOW_SHALLOW") 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 @@ later is required to fix a server side protocol bug. 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 af6bbef7..13e23e34 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py @@ -304,6 +304,32 @@ class LocalSyncState(unittest.TestCase): 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."""