Browse Source

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 <renewitt@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Dan Le Febvre <dlf@google.com>
Dan Le Febvre 2 years ago
parent
commit
8faa5514ec

+ 1 - 1
gclient.py

@@ -2216,7 +2216,7 @@ class CipdDependency(Dependency):
   def __init__(
   def __init__(
       self, parent, name, dep_value, cipd_root,
       self, parent, name, dep_value, cipd_root,
       custom_vars, should_process, relative, condition):
       custom_vars, should_process, relative, condition):
-    package = dep_value['package']
+    package = cipd_root.expand_package_name(dep_value['package'])
     version = dep_value['version']
     version = dep_value['version']
     url = urlparse.urljoin(
     url = urlparse.urljoin(
         cipd_root.service_url, '%s@%s' % (package, version))
         cipd_root.service_url, '%s@%s' % (package, version))

+ 14 - 0
gclient_scm.py

@@ -1596,6 +1596,20 @@ class CipdRoot(object):
         if os.path.exists(cipd_cache_dir):
         if os.path.exists(cipd_cache_dir):
           raise
           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
   @contextlib.contextmanager
   def _create_ensure_file(self):
   def _create_ensure_file(self):
     try:
     try:

+ 1 - 1
testing_support/cipd.bat

@@ -1,4 +1,4 @@
-echo off
+@echo off
 :: Copyright (c) 2018 The Chromium Authors. All rights reserved.
 :: Copyright (c) 2018 The Chromium Authors. All rights reserved.
 :: Use of this source code is governed by a BSD-style license that can be
 :: Use of this source code is governed by a BSD-style license that can be
 :: found in the LICENSE file.
 :: found in the LICENSE file.

+ 23 - 1
testing_support/fake_cipd.py

@@ -10,8 +10,14 @@ import re
 import shutil
 import shutil
 import sys
 import sys
 
 
+ARCH_VAR = 'arch'
+OS_VAR = 'os'
+PLATFORM_VAR = 'platform'
 
 
 CIPD_SUBDIR_RE = '@Subdir (.*)'
 CIPD_SUBDIR_RE = '@Subdir (.*)'
+CIPD_ENSURE = 'ensure'
+CIPD_EXPAND_PKG = 'expand-package-name'
+CIPD_EXPORT = 'export'
 
 
 
 
 def parse_cipd(root, contents):
 def parse_cipd(root, contents):
@@ -29,8 +35,24 @@ def parse_cipd(root, contents):
   return tree
   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():
 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 = argparse.ArgumentParser()
   parser.add_argument('-ensure-file')
   parser.add_argument('-ensure-file')
   parser.add_argument('-root')
   parser.add_argument('-root')

+ 9 - 5
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 = self.mangle_git_tree(('repo_14@1', 'src'))
     tree.update({
     tree.update({
-        '_cipd': '\n'.join([
+        '_cipd':
+        '\n'.join([
             '$ParanoidMode CheckPresence',
             '$ParanoidMode CheckPresence',
             '$OverrideInstallMode copy',
             '$OverrideInstallMode copy',
             '',
             '',
@@ -45,16 +46,19 @@ class GClientSmokeCipd(gclient_smoketest_base.GClientSmokeBase):
             'package0 0.1',
             'package0 0.1',
             '',
             '',
             '@Subdir src/cipd_dep_with_cipd_variable',
             '@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',
             'package1 1.1-cr0',
             'package2 1.13',
             '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)
     self.assertTree(tree)
 
 

+ 1 - 1
tests/gclient_git_smoketest.py

@@ -1261,7 +1261,7 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
         '  "src/cipd_dep_with_cipd_variable": {',
         '  "src/cipd_dep_with_cipd_variable": {',
         '    "packages": [',
         '    "packages": [',
         '      {',
         '      {',
-        '        "package": "package3/${{platform}}",',
+        '        "package": "package3/platform-expanded-test-only",',
         '        "version": "1.2",',
         '        "version": "1.2",',
         '      },',
         '      },',
         '    ],',
         '    ],',

+ 16 - 9
tests/gclient_test.py

@@ -46,6 +46,15 @@ def write(filename, content):
     f.write(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):
 class SCMMock(object):
   unit_test = None
   unit_test = None
   def __init__(self, parsed_url, root_dir, name, out_fh=None, out_cb=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, _ = gclient.OptionParser().parse_args([])
     options.ignore_dep_type = 'git'
     options.ignore_dep_type = 'git'
     obj = gclient.GClient.LoadCurrentConfig(options)
     obj = gclient.GClient.LoadCurrentConfig(options)
+    obj._cipd_root = CIPDRootMock('src', 'https://example.com')
 
 
     self.assertEqual(1, len(obj.dependencies))
     self.assertEqual(1, len(obj.dependencies))
     sol = obj.dependencies[0]
     sol = obj.dependencies[0]
@@ -1092,9 +1102,7 @@ class GclientTest(trial_dir.TestCase):
     dep = sol.dependencies[0]
     dep = sol.dependencies[0]
 
 
     self.assertIsInstance(dep, gclient.CipdDependency)
     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):
   def testDepsFromNotAllowedHostsUnspecified(self):
     """Verifies gclient works fine with DEPS without allowed_hosts."""
     """Verifies gclient works fine with DEPS without allowed_hosts."""
@@ -1239,6 +1247,7 @@ class GclientTest(trial_dir.TestCase):
         '}')
         '}')
     options, _ = gclient.OptionParser().parse_args([])
     options, _ = gclient.OptionParser().parse_args([])
     obj = gclient.GClient.LoadCurrentConfig(options)
     obj = gclient.GClient.LoadCurrentConfig(options)
+    obj._cipd_root = CIPDRootMock('src', 'https://example.com')
 
 
     self.assertEqual(1, len(obj.dependencies))
     self.assertEqual(1, len(obj.dependencies))
     sol = obj.dependencies[0]
     sol = obj.dependencies[0]
@@ -1249,9 +1258,7 @@ class GclientTest(trial_dir.TestCase):
     dep = sol.dependencies[0]
     dep = sol.dependencies[0]
 
 
     self.assertIsInstance(dep, gclient.CipdDependency)
     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):
   def testIgnoresCipdDependenciesWhenFlagIsSet(self):
     """Verifies that CIPD deps are ignored if --ignore-dep-type cipd is set."""
     """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()
     parser = gclient.OptionParser()
     options, _ = parser.parse_args([])
     options, _ = parser.parse_args([])
     obj = gclient.GClient('foo', options)
     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(
     obj.add_dependencies_and_close(
       [
       [
         gclient.Dependency(
         gclient.Dependency(
@@ -1458,7 +1465,7 @@ class GclientTest(trial_dir.TestCase):
     parser = gclient.OptionParser()
     parser = gclient.OptionParser()
     options, _ = parser.parse_args([])
     options, _ = parser.parse_args([])
     obj = gclient.GClient('src', options)
     obj = gclient.GClient('src', options)
-    cipd_root = obj.GetCipdRoot()
+    cipd_root = CIPDRootMock('src', 'https://example.com')
 
 
     cipd_dep = gclient.CipdDependency(
     cipd_dep = gclient.CipdDependency(
         parent=obj,
         parent=obj,