diff --git a/gclient.py b/gclient.py index 396b950e3..f6294ff7f 100755 --- a/gclient.py +++ b/gclient.py @@ -884,18 +884,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): # If dependencies are configured within git submodules, add them to deps. if self.git_dependencies_state in (gclient_eval.SUBMODULES, gclient_eval.SYNC): - gitsubmodules = self.ParseGitSubmodules() - # TODO(crbug.com/1471685): Temporary hack. In case of applied patches - # where the changes are staged but not committed, any gitlinks from - # the patch are not returned in ParseGitSubmodules. In these cases, - # DEPS does have the commits from the patch, so we use those commits - # instead. - if self.git_dependencies_state == gclient_eval.SYNC: - for sub in gitsubmodules: - if sub not in deps: - deps[sub] = gitsubmodules[sub] - elif self.git_dependencies_state == gclient_eval.SUBMODULES: - deps.update(gitsubmodules) + deps.update(self.ParseGitSubmodules()) deps_to_add = self._deps_to_objects( self._postprocess_deps(deps, rel_prefix), self._use_relative_paths) @@ -952,18 +941,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): filepath) return {} - # Get submodule commit hashes - # Output Format: ` SP SP TAB `. - result = subprocess2.check_output(['git', 'ls-tree', '-r', 'HEAD'], - cwd=cwd).decode('utf-8') - - commit_hashes = {} - for r in result.splitlines(): - # ['', '', '', '']. - record = r.strip().split(maxsplit=3) # path can contain spaces. - if record[0] == '160000': # Only add gitlinks - commit_hashes[record[3]] = record[2] - # Get .gitmodules fields gitmodules_entries = subprocess2.check_output( ['git', 'config', '--file', filepath, '-l']).decode('utf-8') @@ -988,6 +965,19 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): if sub_key in ('url', 'gclient-condition', 'path'): gitmodules[submodule][sub_key] = value + paths = [module['path'] for module in gitmodules.values()] + # Get submodule commit hashes. We use `gitt ls-files -s` to ensure + # we capture gitlink changes from staged applied patches. + # Output Format: ` SP SP TAB `. + result = subprocess2.check_output(['git', 'ls-files', '-s'] + paths, + cwd=cwd).decode('utf-8') + commit_hashes = {} + for r in result.splitlines(): + # ['', '', '', '']. + record = r.strip().split(maxsplit=3) # path can contain spaces. + assert record[0] == '160000', 'file is not a gitlink: %s' % record + commit_hashes[record[3]] = record[1] + # Structure git submodules into a dict of DEPS git url entries. submodules = {} for module in gitmodules.values(): diff --git a/tests/gclient_test.py b/tests/gclient_test.py index 5156926cf..69086835b 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -1432,12 +1432,11 @@ class GclientTest(trial_dir.TestCase): }] write('.gclient', 'solutions = %s' % repr(solutions)) - ls_tree = """160000 commit be8c5114d606692dc783b60cf256690b62fbad17\tfoo/bar - 100644 blob daf2de9caad4a70e6bb1047a6b50c412066f68b1\tREADME.txt - 160000 commit 3ad3b564f8ae456f286446d091709f5a09fa4a93\taaaaaa - 160000 commit 956df937508b65b5e72a4cf02696255be3631b78\ta.a.a/a - 160000 commit b9f77763f0fab67eeeb6371492166567a8b7a3d2\ta_b/c - 160000 commit b9f77763f0fab67eeeb6371492166567a8b7a3d2\ta b/c""" + ls_files = """160000 be8c5114d606692dc783b60cf256690b62fbad17 0\tfoo/bar + 160000 3ad3b564f8ae456f286446d091709f5a09fa4a93 0\taaaaaa + 160000 956df937508b65b5e72a4cf02696255be3631b78 0\ta.a.a/a + 160000 b9f77763f0fab67eeeb6371492166567a8b7a3d2 0\ta_b/c + 160000 b9f77763f0fab67eeeb6371492166567a8b7a3d2 0\ta b/c""" git_config = """submodule.foo/bar.path=foo/bar submodule.foo/bar.url=http://example.com/foo/bar @@ -1453,7 +1452,8 @@ class GclientTest(trial_dir.TestCase): os_path_isfile_mock = mock.MagicMock(return_value=True) subprocess2_check_output_mock = mock.MagicMock( - side_effect=[ls_tree.encode(), git_config.encode()]) + side_effect=[git_config.encode(), + ls_files.encode()]) options, _ = gclient.OptionParser().parse_args([]) client = gclient.GClient.LoadCurrentConfig(options) @@ -1498,6 +1498,14 @@ class GclientTest(trial_dir.TestCase): 'b9f77763f0fab67eeeb6371492166567a8b7a3d2'), } }) + subprocess2_check_output_mock.assert_has_calls([ + mock.call(['git', 'config', '--file', mock.ANY, '-l']), + mock.call([ + 'git', 'ls-files', '-s', 'foo/bar', 'aaaaaa', 'a.a.a/a', 'a_b/c', + 'a b/c' + ], + cwd=mock.ANY) + ]) def testParseGitSubmodules_UsesAbsolutePath(self): """ParseGitSubmodules uses absolute path when use_relative_path is not @@ -1509,12 +1517,11 @@ class GclientTest(trial_dir.TestCase): }] write('.gclient', 'solutions = %s' % repr(solutions)) - ls_tree = """160000 commit be8c5114d606692dc783b60cf256690b62fbad17\tfoo/bar - 100644 blob daf2de9caad4a70e6bb1047a6b50c412066f68b1\tREADME.txt - 160000 commit 3ad3b564f8ae456f286446d091709f5a09fa4a93\taaaaaa - 160000 commit 956df937508b65b5e72a4cf02696255be3631b78\ta.a.a/a - 160000 commit b9f77763f0fab67eeeb6371492166567a8b7a3d2\ta_b/c - 160000 commit b9f77763f0fab67eeeb6371492166567a8b7a3d2\ta b/c""" + ls_files = """160000 be8c5114d606692dc783b60cf256690b62fbad17 0\tfoo/bar + 160000 3ad3b564f8ae456f286446d091709f5a09fa4a93 0\taaaaaa + 160000 956df937508b65b5e72a4cf02696255be3631b78 0\ta.a.a/a + 160000 b9f77763f0fab67eeeb6371492166567a8b7a3d2 0\ta_b/c + 160000 b9f77763f0fab67eeeb6371492166567a8b7a3d2 0\ta b/c""" git_config = """submodule.foo/bar.path=foo/bar submodule.foo/bar.url=http://example.com/foo/bar @@ -1530,7 +1537,8 @@ class GclientTest(trial_dir.TestCase): os_path_isfile_mock = mock.MagicMock(return_value=True) subprocess2_check_output_mock = mock.MagicMock( - side_effect=[ls_tree.encode(), git_config.encode()]) + side_effect=[git_config.encode(), + ls_files.encode()]) options, _ = gclient.OptionParser().parse_args([]) client = gclient.GClient.LoadCurrentConfig(options) @@ -1574,6 +1582,14 @@ class GclientTest(trial_dir.TestCase): 'b9f77763f0fab67eeeb6371492166567a8b7a3d2'), } }) + subprocess2_check_output_mock.assert_has_calls([ + mock.call(['git', 'config', '--file', mock.ANY, '-l']), + mock.call([ + 'git', 'ls-files', '-s', 'foo/bar', 'aaaaaa', 'a.a.a/a', 'a_b/c', + 'a b/c' + ], + cwd=mock.ANY) + ]) def testSameDirAllowMultipleCipdDeps(self): """Verifies gclient allow multiple cipd deps under same directory."""