From e36c6bba14b2c2204babb60b16c45bfa5a8470a1 Mon Sep 17 00:00:00 2001 From: Joanna Wang Date: Wed, 30 Aug 2023 22:09:59 +0000 Subject: [PATCH] Make gclient getdep work with submodules. Bug: 1475770 Change-Id: I341910e75195d87b91defd98f2c6ba25262995b1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4828423 Reviewed-by: Josip Sokcevic Commit-Queue: Joanna Wang --- gclient.py | 23 ++++++++++++----------- scm.py | 17 +++++++++++++++++ tests/gclient_git_smoketest.py | 16 ++++++++++++++++ tests/gclient_test.py | 8 ++++---- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/gclient.py b/gclient.py index bf16d090bd..741f16a3ec 100755 --- a/gclient.py +++ b/gclient.py @@ -105,6 +105,7 @@ import gclient_utils import git_cache import metrics import metrics_utils +import scm as scm_git import setup_color import subcommand import subprocess2 @@ -945,17 +946,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): 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] + commit_hashes = scm_git.GIT.GetSubmoduleCommits(cwd, paths) # Structure git submodules into a dict of DEPS git url entries. submodules = {} @@ -3428,6 +3419,14 @@ def CMDgetdep(parser, args): for var in options.vars: print(gclient_eval.GetVar(local_scope, var)) + commits = {} + if local_scope.get('git_dependencies' + ) == gclient_eval.SUBMODULES and options.getdep_revisions: + commits.update( + scm_git.GIT.GetSubmoduleCommits( + os.getcwd(), + [path for path in options.getdep_revisions if ':' not in path])) + for name in options.getdep_revisions: if ':' in name: name, _, package = name.partition(':') @@ -3436,6 +3435,8 @@ def CMDgetdep(parser, args): 'Wrong CIPD format: %s:%s should be of the form path:pkg.' % (name, package)) print(gclient_eval.GetCIPD(local_scope, name, package)) + elif commits: + print(commits[name]) else: try: print(gclient_eval.GetRevision(local_scope, name)) diff --git a/scm.py b/scm.py index 85033937e4..6921824ffb 100644 --- a/scm.py +++ b/scm.py @@ -387,6 +387,23 @@ class GIT(object): # return only files return [f.split(maxsplit=3)[-1] for f in files if f.startswith('100')] + @staticmethod + def GetSubmoduleCommits(cwd, submodules): + # type: (string, List[string]) => Mapping[string][string] + """Returns a mapping of staged or committed new commits for submodules.""" + if not submodules: + return {} + result = subprocess2.check_output(['git', 'ls-files', '-s', '--'] + + submodules, + 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] + return commit_hashes + @staticmethod def GetPatchName(cwd): """Constructs a name for this patch.""" diff --git a/tests/gclient_git_smoketest.py b/tests/gclient_git_smoketest.py index 1094878926..69a669bf4b 100755 --- a/tests/gclient_git_smoketest.py +++ b/tests/gclient_git_smoketest.py @@ -810,6 +810,22 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase): 'bar_rev', ], results[0].splitlines()) + def testGetDep_Submodule(self): + self.gclient(['config', self.git_base + 'repo_20', '--name', 'src']) + subprocess2.call([ + 'git', 'update-index', '--add', '--cacheinfo', + '160000,aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,bar' + ], + cwd=self.git_base + 'repo_20') + + results = self.gclient( + ['getdep', '-r', 'foo/bar:lemur', '-r', 'bar', '--var', 'foo_checkout'], + cwd=self.git_base + 'repo_20') + + self.assertEqual( + ['True', 'version:1234', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'], + results[0].splitlines()) + def testGetDep_BuiltinVariables(self): self.gclient(['config', self.git_base + 'repo_1', '--name', 'src']) fake_deps = os.path.join(self.root_dir, 'DEPS.fake') diff --git a/tests/gclient_test.py b/tests/gclient_test.py index 35c71a0d09..d9fb6118b6 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -1500,8 +1500,8 @@ class GclientTest(trial_dir.TestCase): 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' + 'git', 'ls-files', '-s', '--', 'foo/bar', 'aaaaaa', 'a.a.a/a', + 'a_b/c', 'a b/c' ], cwd=mock.ANY) ]) @@ -1584,8 +1584,8 @@ class GclientTest(trial_dir.TestCase): 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' + 'git', 'ls-files', '-s', '--', 'foo/bar', 'aaaaaa', 'a.a.a/a', + 'a_b/c', 'a b/c' ], cwd=mock.ANY) ])