Revert "resolve CIPD package names in gclient"

This reverts commit 8faa5514ec.

Reason for revert: cipd expand-package-name errors on wrong platform

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: I0beba8606d8cceeb815629d6eae6d8021c7731a8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4501579
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Auto-Submit: Dan Le Febvre <dlf@google.com>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Owners-Override: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
changes/79/4501579/2
Dan Le Febvre 2 years ago committed by LUCI CQ
parent 8faa5514ec
commit f2da906faf

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

@ -1596,20 +1596,6 @@ 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:

@ -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.

@ -10,14 +10,8 @@ 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):
@ -35,24 +29,8 @@ 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():
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
assert sys.argv[1] in ['ensure', 'export']
parser = argparse.ArgumentParser()
parser.add_argument('-ensure-file')
parser.add_argument('-root')

@ -33,8 +33,7 @@ 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',
'',
@ -46,19 +45,16 @@ class GClientSmokeCipd(gclient_smoketest_base.GClientSmokeBase):
'package0 0.1',
'',
'@Subdir src/cipd_dep_with_cipd_variable',
'package3/platform-expanded-test-only 1.2',
'package3/${platform} 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-expanded-test-only 1.2',
'src/cipd_dep/_cipd': 'package0 0.1',
'src/cipd_dep_with_cipd_variable/_cipd': 'package3/${platform} 1.2',
})
self.assertTree(tree)

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

@ -46,15 +46,6 @@ 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,
@ -1091,7 +1082,6 @@ 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]
@ -1102,7 +1092,9 @@ class GclientTest(trial_dir.TestCase):
dep = sol.dependencies[0]
self.assertIsInstance(dep, gclient.CipdDependency)
self.assertEqual('https://example.com/lemur@version:1234', dep.url)
self.assertEqual(
'https://chrome-infra-packages.appspot.com/lemur@version:1234',
dep.url)
def testDepsFromNotAllowedHostsUnspecified(self):
"""Verifies gclient works fine with DEPS without allowed_hosts."""
@ -1247,7 +1239,6 @@ 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]
@ -1258,7 +1249,9 @@ class GclientTest(trial_dir.TestCase):
dep = sol.dependencies[0]
self.assertIsInstance(dep, gclient.CipdDependency)
self.assertEqual('https://example.com/lemur@version:1234', dep.url)
self.assertEqual(
'https://chrome-infra-packages.appspot.com/lemur@version:1234',
dep.url)
def testIgnoresCipdDependenciesWhenFlagIsSet(self):
"""Verifies that CIPD deps are ignored if --ignore-dep-type cipd is set."""
@ -1411,8 +1404,8 @@ class GclientTest(trial_dir.TestCase):
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
obj = gclient.GClient('foo', options)
cipd_root = CIPDRootMock(os.path.join(self.root_dir, 'dir1'),
'https://example.com')
cipd_root = gclient_scm.CipdRoot(
os.path.join(self.root_dir, 'dir1'), 'https://example.com')
obj.add_dependencies_and_close(
[
gclient.Dependency(
@ -1465,7 +1458,7 @@ class GclientTest(trial_dir.TestCase):
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
obj = gclient.GClient('src', options)
cipd_root = CIPDRootMock('src', 'https://example.com')
cipd_root = obj.GetCipdRoot()
cipd_dep = gclient.CipdDependency(
parent=obj,

Loading…
Cancel
Save