Reland "resolve CIPD package names in gclient"

This CL relands the below CL with the addition of handling situations where the CIPD package name has a variable=value and does not equate to true. In this case just return the original value in DEPS.

This is a reland of commit 8faa5514ec0c4a36d65b44acbd98b2eb66b91405

Original change's description:
> 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>

Bug: b/279097790
Change-Id: Ib08dac506ce221243c595dde5cb00e8e0d7dc7ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4501582
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Dan Le Febvre <dlf@google.com>

gclient.py

@@ -2216,7 +2216,16 @@ class CipdDependency(Dependency):
   def __init__(
       self, parent, name, dep_value, cipd_root,
       custom_vars, should_process, relative, condition):
+
     package = dep_value['package']
+    # TODO(dlf): Switch to a batch mechanism using cipd ensure file instead
+    # of describe and expand all packages not just should_process.
+    if should_process:
+      expanded_package = cipd_root.expand_package_name(dep_value['package'])
+      # If package is not suitable for the platform an empty string will be
+      # returned. Just use original package name from DEPS file in that case.
+      if expanded_package:
+        package = expanded_package
     version = dep_value['version']
     url = urlparse.urljoin(
         cipd_root.service_url, '%s@%s' % (package, version))

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:

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.

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,32 @@ def parse_cipd(root, contents):
   return tree
 
 
+def expand_package_name_cmd(package_name):
+  package_split = package_name.split("/")
+  suffix = package_split[-1]
+  # Any use of var equality should return empty for testing.
+  if "=" in suffix:
+    if suffix != "${platform=fake-platform-ok}":
+      return ""
+    package_name = "/".join(package_split[:-1] + ["${platform}"])
+
+  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')

testing_support/fake_repos.py

@@ -657,8 +657,10 @@ hooks = [{
       'origin': 'git/repo_13@3\n'
     })
 
-    self._commit_git('repo_14', {
-      'DEPS': textwrap.dedent("""\
+    self._commit_git(
+        'repo_14', {
+            'DEPS':
+            textwrap.dedent("""\
         vars = {}
         deps = {
           'src/cipd_dep': {
@@ -692,9 +694,28 @@ hooks = [{
             ],
             'dep_type': 'cipd',
           },
+          'src/cipd_dep_with_cipd_variable_equation_not_ok': {
+            'packages': [
+              {
+                'package': 'package3/${{platform=fake-platform-not-ok}}',
+                'version': '1.2',
+              },
+            ],
+            'dep_type': 'cipd',
+          },
+          'src/cipd_dep_with_cipd_variable_equation_ok': {
+            'packages': [
+              {
+                'package': 'package3/${{platform=fake-platform-ok}}',
+                'version': '1.4',
+              },
+            ],
+            'dep_type': 'cipd',
+          },
         }"""),
-      'origin': 'git/repo_14@2\n'
-    })
+            'origin':
+            'git/repo_14@2\n'
+        })
 
     # A repo with a hook to be recursed in, without use_relative_paths
     self._commit_git('repo_15', {

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,29 @@ 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',
+            '',
+            '@Subdir src/cipd_dep_with_cipd_variable_equation_not_ok',
+            'package3/${platform=fake-platform-not-ok} 1.2',
+            '',
+            '@Subdir src/cipd_dep_with_cipd_variable_equation_ok',
+            'package3/platform-expanded-test-only 1.4',
             '',
             '',
         ]),
-        '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',
+        'src/cipd_dep_with_cipd_variable_equation_not_ok/_cipd':
+        'package3/${platform=fake-platform-not-ok} 1.2',
+        'src/cipd_dep_with_cipd_variable_equation_ok/_cipd':
+        'package3/platform-expanded-test-only 1.4',
     })
     self.assertTree(tree)
 

tests/gclient_git_smoketest.py

@@ -1261,13 +1261,35 @@ 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",',
         '      },',
         '    ],',
         '    "dep_type": "cipd",',
         '  },',
         '',
+        '  # "src" -> src/cipd_dep_with_cipd_variable_equation_not_ok',
+        '  "src/cipd_dep_with_cipd_variable_equation_not_ok": {',
+        '    "packages": [',
+        '      {',
+        '        "package": "package3/${{platform=fake-platform-not-ok}}",',
+        '        "version": "1.2",',
+        '      },',
+        '    ],',
+        '    "dep_type": "cipd",',
+        '  },',
+        '',
+        '  # "src" -> src/cipd_dep_with_cipd_variable_equation_ok',
+        '  "src/cipd_dep_with_cipd_variable_equation_ok": {',
+        '    "packages": [',
+        '      {',
+        '        "package": "package3/platform-expanded-test-only",',
+        '        "version": "1.4",',
+        '      },',
+        '    ],',
+        '    "dep_type": "cipd",',
+        '  },',
+        '',
         '}',
         '',
         '# ' + self.git_base + 'repo_14, DEPS',

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,