diff --git a/gclient.py b/gclient.py index e7da3e5a8..388799f82 100755 --- a/gclient.py +++ b/gclient.py @@ -2052,7 +2052,17 @@ it or fix the checkout. ] removed_cipd_entries = [] - for entry, prev_url in self._ReadEntries().items(): + read_entries = self._ReadEntries() + # We process entries sorted in reverse to ensure a child dir is + # always deleted before its parent dir. + # This is especially important for submodules with pinned revisions + # overwritten by a vars or custom_deps. In this case, if a parent + # submodule is encountered first in the loop, it cannot tell the + # difference between modifications from the vars or actual user + # modifications that should be kept. http://crbug/1486677#c9 for + # more details. + for entry in sorted(read_entries, reverse=True): + prev_url = read_entries[entry] if not prev_url: # entry must have been overridden via .gclient custom_deps continue @@ -2092,11 +2102,8 @@ it or fix the checkout. 'checkout, so not removing.' % entry) continue - # This is to handle the case of third_party/WebKit migrating - # from being a DEPS entry to being part of the main project. If - # the subproject is a Git project, we need to remove its .git - # folder. Otherwise git operations on that folder will have - # different effects depending on the current working directory. + versioned_state = None + # Check if this is a submodule or versioned directory. if os.path.abspath(scm_root) == os.path.abspath(e_dir): e_par_dir = os.path.join(e_dir, os.pardir) if gclient_scm.scm.GIT.IsInsideWorkTree(e_par_dir): @@ -2105,8 +2112,14 @@ it or fix the checkout. # rel_e_dir : relative path of entry w.r.t. its parent # repo. rel_e_dir = os.path.relpath(e_dir, par_scm_root) - if gclient_scm.scm.GIT.IsDirectoryVersioned( - par_scm_root, rel_e_dir): + versioned_state = gclient_scm.scm.GIT.IsVersioned( + par_scm_root, rel_e_dir) + # This is to handle the case of third_party/WebKit migrating + # from being a DEPS entry to being part of the main project. If + # the subproject is a Git project, we need to remove its .git + # folder. Otherwise git operations on that folder will have + # different effects depending on the current working directory. + if versioned_state == gclient_scm.scm.VERSIONED_DIR: save_dir = scm.GetGitBackupDirPath() # Remove any eventual stale backup dir for the same # project. @@ -2182,6 +2195,10 @@ it or fix the checkout. print('\n________ deleting \'%s\' in \'%s\'' % (entry_fixed, self.root_dir)) gclient_utils.rmtree(e_dir) + # We restore empty directories of submodule paths. + if versioned_state == gclient_scm.scm.VERSIONED_SUBMODULE: + gclient_scm.scm.GIT.Capture( + ['restore', '--', rel_e_dir], cwd=par_scm_root) # record the current list of entries for next time self._SaveEntries() return removed_cipd_entries @@ -2308,7 +2325,8 @@ it or fix the checkout. try: gclient_scm.scm.GIT.Capture(['checkout', tail], cwd=cwd) - except subprocess2.CalledProcessError: + except (subprocess2.CalledProcessError, OSError): + # repo of the deleted cipd may also have been deleted. pass if not self._options.nohooks: diff --git a/scm.py b/scm.py index 904f67c88..2c98290c5 100644 --- a/scm.py +++ b/scm.py @@ -17,6 +17,11 @@ import subprocess2 # TODO: Should fix these warnings. # pylint: disable=line-too-long +# constants used to identify the tree state of a directory. +VERSIONED_NO = 0 +VERSIONED_DIR = 1 +VERSIONED_SUBMODULE = 2 + def ValidateEmail(email): return (re.match(r"^[a-zA-Z0-9._%\-+]+@[a-zA-Z0-9._%-]+.[a-zA-Z]{2,6}$", @@ -440,9 +445,15 @@ class GIT(object): return False @staticmethod - def IsDirectoryVersioned(cwd, relative_dir): + def IsVersioned(cwd, relative_dir): + # type: (str, str) -> int """Checks whether the given |relative_dir| is part of cwd's repo.""" - return bool(GIT.Capture(['ls-tree', 'HEAD', relative_dir], cwd=cwd)) + output = GIT.Capture(['ls-tree', 'HEAD', '--', relative_dir], cwd=cwd) + if not output: + return VERSIONED_NO + if output.startswith('160000'): + return VERSIONED_SUBMODULE + return VERSIONED_DIR @staticmethod def CleanupDir(cwd, relative_dir): diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index bde905872..8e4a3f4c2 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -135,6 +135,22 @@ class GitWrapperTestCase(unittest.TestCase): scm.GIT.GetRemoteHeadRef('foo', 'proto://url', 'origin')) self.assertEqual(mockCapture.call_count, 2) + @mock.patch('scm.GIT.Capture') + def testIsVersioned(self, mockCapture): + mockCapture.return_value = ( + '160000 blob 423dc77d2182cb2687c53598a1dcef62ea2804ae dir') + actual_state = scm.GIT.IsVersioned('cwd', 'dir') + self.assertEqual(actual_state, scm.VERSIONED_SUBMODULE) + + mockCapture.return_value = '' + actual_state = scm.GIT.IsVersioned('cwd', 'dir') + self.assertEqual(actual_state, scm.VERSIONED_NO) + + mockCapture.return_value = ( + '040000 tree ef016abffb316e47a02af447bc51342dcef6f3ca dir') + actual_state = scm.GIT.IsVersioned('cwd', 'dir') + self.assertEqual(actual_state, scm.VERSIONED_DIR) + class RealGitTest(fake_repos.FakeReposTestBase): def setUp(self):