From 8faa5514ec0c4a36d65b44acbd98b2eb66b91405 Mon Sep 17 00:00:00 2001 From: Dan Le Febvre Date: Tue, 2 May 2023 23:10:32 +0000 Subject: [PATCH] resolve CIPD package names in gclient Currently the package names are not resolved in gclient output, see example below. This is part of broader work to improve CIPD output, the next CL will replace 'None'. ``` $~ gclient revinfo -a | grep '${platform}\|${arch}' src/buildtools/linux64:gn/gn/linux-${arch}: None src/buildtools/reclient:infra/rbe/client/${platform}: None src/third_party/dawn/third_party/ninja:infra/3pp/tools/ninja/${platform}: None src/third_party/dawn/tools/golang:infra/3pp/tools/go/${platform}: None src/third_party/devtools-frontend-internal/devtools-frontend/third_party/esbuild:infra/3pp/tools/esbuild/${platform}: None src/third_party/devtools-frontend/src/third_party/esbuild:infra/3pp/tools/esbuild/${platform}: None src/third_party/ninja:infra/3pp/tools/ninja/${platform}: None src/tools/luci-go:infra/tools/luci/isolate/${platform}: None src/tools/luci-go:infra/tools/luci/swarming/${platform}: None src/tools/resultdb:infra/tools/result_adapter/${platform}: None ``` Bug:b/279097790 Change-Id: I21abf2579910e9d79d9ee0dcd018ee761e3d3c1c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4463983 Reviewed-by: Rachael Newitt Reviewed-by: Gavin Mak Commit-Queue: Dan Le Febvre --- gclient.py | 2 +- gclient_scm.py | 14 ++++++++++++++ testing_support/cipd.bat | 2 +- testing_support/fake_cipd.py | 24 +++++++++++++++++++++++- tests/gclient_cipd_smoketest.py | 14 +++++++++----- tests/gclient_git_smoketest.py | 2 +- tests/gclient_test.py | 25 ++++++++++++++++--------- 7 files changed, 65 insertions(+), 18 deletions(-) diff --git a/gclient.py b/gclient.py index 8ca899caa..d8a14a8e3 100755 --- a/gclient.py +++ b/gclient.py @@ -2216,7 +2216,7 @@ class CipdDependency(Dependency): def __init__( self, parent, name, dep_value, cipd_root, custom_vars, should_process, relative, condition): - package = dep_value['package'] + package = cipd_root.expand_package_name(dep_value['package']) version = dep_value['version'] url = urlparse.urljoin( cipd_root.service_url, '%s@%s' % (package, version)) diff --git a/gclient_scm.py b/gclient_scm.py index 8acc4250c..e0a5a4157 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -1596,6 +1596,20 @@ class CipdRoot(object): if os.path.exists(cipd_cache_dir): raise + def expand_package_name(self, package_name_string, **kwargs): + """Run `cipd expand-package-name`. + + CIPD package names can be declared with placeholder variables + such as '${platform}', this cmd will return the package name + with the variables resolved. The resolution is based on the host + the command is executing on. + """ + + kwargs.setdefault('stderr', subprocess2.PIPE) + cmd = ['cipd', 'expand-package-name', package_name_string] + ret = subprocess2.check_output(cmd, **kwargs).decode('utf-8') + return ret.strip() + @contextlib.contextmanager def _create_ensure_file(self): try: diff --git a/testing_support/cipd.bat b/testing_support/cipd.bat index f5b6df7dd..a5d0c55c0 100755 --- a/testing_support/cipd.bat +++ b/testing_support/cipd.bat @@ -1,4 +1,4 @@ -echo off +@echo off :: Copyright (c) 2018 The Chromium Authors. All rights reserved. :: Use of this source code is governed by a BSD-style license that can be :: found in the LICENSE file. diff --git a/testing_support/fake_cipd.py b/testing_support/fake_cipd.py index 2244e6045..e57c1ba35 100644 --- a/testing_support/fake_cipd.py +++ b/testing_support/fake_cipd.py @@ -10,8 +10,14 @@ import re import shutil import sys +ARCH_VAR = 'arch' +OS_VAR = 'os' +PLATFORM_VAR = 'platform' CIPD_SUBDIR_RE = '@Subdir (.*)' +CIPD_ENSURE = 'ensure' +CIPD_EXPAND_PKG = 'expand-package-name' +CIPD_EXPORT = 'export' def parse_cipd(root, contents): @@ -29,8 +35,24 @@ def parse_cipd(root, contents): return tree +def expand_package_name_cmd(package_name): + for v in [ARCH_VAR, OS_VAR, PLATFORM_VAR]: + var = "${%s}" % v + if package_name.endswith(var): + package_name = package_name.replace(var, "%s-expanded-test-only" % v) + return package_name + + def main(): - assert sys.argv[1] in ['ensure', 'export'] + cmd = sys.argv[1] + assert cmd in [CIPD_ENSURE, CIPD_EXPAND_PKG, CIPD_EXPORT] + # Handle cipd expand-package-name + if cmd == CIPD_EXPAND_PKG: + # Expecting argument after cmd + assert len(sys.argv) == 3 + # Write result to stdout + sys.stdout.write(expand_package_name_cmd(sys.argv[2])) + return 0 parser = argparse.ArgumentParser() parser.add_argument('-ensure-file') parser.add_argument('-root') diff --git a/tests/gclient_cipd_smoketest.py b/tests/gclient_cipd_smoketest.py index 289023fc3..4ddf84471 100644 --- a/tests/gclient_cipd_smoketest.py +++ b/tests/gclient_cipd_smoketest.py @@ -33,7 +33,8 @@ class GClientSmokeCipd(gclient_smoketest_base.GClientSmokeBase): tree = self.mangle_git_tree(('repo_14@1', 'src')) tree.update({ - '_cipd': '\n'.join([ + '_cipd': + '\n'.join([ '$ParanoidMode CheckPresence', '$OverrideInstallMode copy', '', @@ -45,16 +46,19 @@ class GClientSmokeCipd(gclient_smoketest_base.GClientSmokeBase): 'package0 0.1', '', '@Subdir src/cipd_dep_with_cipd_variable', - 'package3/${platform} 1.2', + 'package3/platform-expanded-test-only 1.2', '', '', ]), - 'src/another_cipd_dep/_cipd': '\n'.join([ + 'src/another_cipd_dep/_cipd': + '\n'.join([ 'package1 1.1-cr0', 'package2 1.13', ]), - 'src/cipd_dep/_cipd': 'package0 0.1', - 'src/cipd_dep_with_cipd_variable/_cipd': 'package3/${platform} 1.2', + 'src/cipd_dep/_cipd': + 'package0 0.1', + 'src/cipd_dep_with_cipd_variable/_cipd': + 'package3/platform-expanded-test-only 1.2', }) self.assertTree(tree) diff --git a/tests/gclient_git_smoketest.py b/tests/gclient_git_smoketest.py index a50086b0c..2283fa895 100755 --- a/tests/gclient_git_smoketest.py +++ b/tests/gclient_git_smoketest.py @@ -1261,7 +1261,7 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase): ' "src/cipd_dep_with_cipd_variable": {', ' "packages": [', ' {', - ' "package": "package3/${{platform}}",', + ' "package": "package3/platform-expanded-test-only",', ' "version": "1.2",', ' },', ' ],', diff --git a/tests/gclient_test.py b/tests/gclient_test.py index 252014dec..5de4c8d36 100755 --- a/tests/gclient_test.py +++ b/tests/gclient_test.py @@ -46,6 +46,15 @@ def write(filename, content): f.write(content) +class CIPDRootMock(object): + def __init__(self, root_dir, service_url): + self.root_dir = root_dir + self.service_url = service_url + + def expand_package_name(self, package_name): + return package_name + + class SCMMock(object): unit_test = None def __init__(self, parsed_url, root_dir, name, out_fh=None, out_cb=None, @@ -1082,6 +1091,7 @@ class GclientTest(trial_dir.TestCase): options, _ = gclient.OptionParser().parse_args([]) options.ignore_dep_type = 'git' obj = gclient.GClient.LoadCurrentConfig(options) + obj._cipd_root = CIPDRootMock('src', 'https://example.com') self.assertEqual(1, len(obj.dependencies)) sol = obj.dependencies[0] @@ -1092,9 +1102,7 @@ class GclientTest(trial_dir.TestCase): dep = sol.dependencies[0] self.assertIsInstance(dep, gclient.CipdDependency) - self.assertEqual( - 'https://chrome-infra-packages.appspot.com/lemur@version:1234', - dep.url) + self.assertEqual('https://example.com/lemur@version:1234', dep.url) def testDepsFromNotAllowedHostsUnspecified(self): """Verifies gclient works fine with DEPS without allowed_hosts.""" @@ -1239,6 +1247,7 @@ class GclientTest(trial_dir.TestCase): '}') options, _ = gclient.OptionParser().parse_args([]) obj = gclient.GClient.LoadCurrentConfig(options) + obj._cipd_root = CIPDRootMock('src', 'https://example.com') self.assertEqual(1, len(obj.dependencies)) sol = obj.dependencies[0] @@ -1249,9 +1258,7 @@ class GclientTest(trial_dir.TestCase): dep = sol.dependencies[0] self.assertIsInstance(dep, gclient.CipdDependency) - self.assertEqual( - 'https://chrome-infra-packages.appspot.com/lemur@version:1234', - dep.url) + self.assertEqual('https://example.com/lemur@version:1234', dep.url) def testIgnoresCipdDependenciesWhenFlagIsSet(self): """Verifies that CIPD deps are ignored if --ignore-dep-type cipd is set.""" @@ -1404,8 +1411,8 @@ class GclientTest(trial_dir.TestCase): parser = gclient.OptionParser() options, _ = parser.parse_args([]) obj = gclient.GClient('foo', options) - cipd_root = gclient_scm.CipdRoot( - os.path.join(self.root_dir, 'dir1'), 'https://example.com') + cipd_root = CIPDRootMock(os.path.join(self.root_dir, 'dir1'), + 'https://example.com') obj.add_dependencies_and_close( [ gclient.Dependency( @@ -1458,7 +1465,7 @@ class GclientTest(trial_dir.TestCase): parser = gclient.OptionParser() options, _ = parser.parse_args([]) obj = gclient.GClient('src', options) - cipd_root = obj.GetCipdRoot() + cipd_root = CIPDRootMock('src', 'https://example.com') cipd_dep = gclient.CipdDependency( parent=obj,