Skip to content

C
Chromium_Depot_tools
Commit 0c91147d authored by Edward Lemur's avatar Edward Lemur Committed by Commit Bot
Browse files
Options

gclient: Don't allow None URLs (except in .gclient files) 

This reverts commit crrev.com/4e9b50ab
and thus relands the following commits:

  ebdd0db4: "gclient: Remove URLs from hierarchy."
  54a5c2ba: "gclient: Refactor PrintRevInfo"
  083eb25f: "gclient: Don't allow URL to be None."

When a None URL is specified in a .gclient file, and a DEPS file is
given, the DEPS file is treated as a .gclient file and its dependencies
are added.

Bug: 839925

Change-Id: I1068b66487874bfa0a788bf9da5273714b6ad39e
Reviewed-on: https://chromium-review.googlesource.com/1083340
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: 's avatarAaron Gable <agable@chromium.org>
Reviewed-by: 's avatarMichael Moss <mmoss@chromium.org>
parent 425d9ce2
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 463 additions and 246 deletions
gclient.py
View file @ 0c91147d
This diff is collapsed.
recipes/trigger_recipe_roller.txt
View file @ 0c91147d
......@@ -9,4 +9,4 @@ As the CI needs of the browser grew, Batty, the Build and Test Yeti, got
a new friend:
The End!
The End.
tests/gclient_smoketest.py
View file @ 0c91147d
......@@ -246,9 +246,14 @@ class GClientSmoke(GClientSmokeBase):
']\n'
'cache_dir = None\n') % self.git_base)
test(['config', '--spec', '["blah blah"]'], '["blah blah"]')
os.remove(p)
results = self.gclient(['config', '--spec', '["blah blah"]'])
self.assertEqual(('', 'Error: No solution specified\n', 1), results)
results = self.gclient(['config', '--spec',
'solutions=[{"name": "./", "url": None}]'])
self.assertEqual(('', 'Error: No solution specified\n', 1), results)
results = self.gclient(['config', 'foo', 'faa', 'fuu'])
err = ('Usage: gclient.py config [options] [url]\n\n'
'gclient.py: error: Inconsistent arguments. Use either --spec or one'
......@@ -256,24 +261,6 @@ class GClientSmoke(GClientSmokeBase):
self.check(('', err, 2), results)
self.assertFalse(os.path.exists(join(self.root_dir, '.gclient')))
def testSolutionNone(self):
results = self.gclient(['config', '--spec',
'solutions=[{"name": "./", "url": None}]'])
self.check(('', '', 0), results)
results = self.gclient(['sync'])
self.check(('', '', 0), results)
self.assertTree({})
results = self.gclient(['revinfo'])
self.check(('./: None\n', '', 0), results)
self.check(('', '', 0), self.gclient(['diff']))
self.assertTree({})
self.check(('', '', 0), self.gclient(['pack']))
self.check(('', '', 0), self.gclient(['revert']))
self.assertTree({})
self.check(('', '', 0), self.gclient(['runhooks']))
self.assertTree({})
self.check(('', '', 0), self.gclient(['status']))
def testDifferentTopLevelDirectory(self):
# Check that even if the .gclient file does not mention the directory src
# itself, but it is included via dependencies, the .gclient file is used.
......@@ -809,8 +796,10 @@ class GClientSmokeGIT(GClientSmokeBase):
with open(output_json) as f:
output_json = json.load(f)
self.maxDiff = None
out = [{
'solution_url': self.git_base + 'repo_1',
'solution_url': '%srepo_1@%s' % (
self.git_base, self.githash('repo_1', 2)),
'managed': True,
'name': 'src',
'deps_file': 'DEPS',
......@@ -1239,6 +1228,118 @@ class GClientSmokeGIT(GClientSmokeBase):
self.githash('repo_8', 1)),
], deps_contents.splitlines())
def testFlattenLocalDepsFile(self):
if not self.enabled:
return
output_deps = os.path.join(self.root_dir, 'DEPS.flattened')
self.assertFalse(os.path.exists(output_deps))
output_deps_files = os.path.join(self.root_dir, 'DEPS.files')
self.assertFalse(os.path.exists(output_deps_files))
write(
os.path.join(self.root_dir, 'DEPS'),
'deps = {\n'
' "src": "' + self.git_base + 'repo_10",\n'
'}\n'
'recursedeps = ["src"]')
self.gclient(['config', '--spec', 'solutions=[{"deps_file": "DEPS"}]'])
self.gclient(['sync', '--process-all-deps'])
self.gclient(['flatten', '-v', '-v', '-v',
'--output-deps', output_deps,
'--output-deps-files', output_deps_files])
with open(output_deps) as f:
deps_contents = f.read()
self.maxDiff = None
self.assertEqual([
'gclient_gn_args_file = "src/repo2/gclient.args"',
"gclient_gn_args = ['str_var']",
'deps = {',
' # src',
' "src": {',
' "url": "' + self.git_base + 'repo_10",',
' },',
'',
' # src -> src/repo9 -> src/repo8 -> src/recursed_os_repo',
' "src/recursed_os_repo": {',
' "url": "' + self.git_base + 'repo_5",',
' "condition": \'(checkout_linux) or (checkout_mac)\',',
' },',
'',
' # src -> src/repo11',
' "src/repo11": {',
' "url": "' + self.git_base + 'repo_11",',
' "condition": \'(checkout_ios) or (checkout_mac)\',',
' },',
'',
' # src -> src/repo11 -> src/repo12',
' "src/repo12": {',
' "url": "' + self.git_base + 'repo_12",',
' "condition": \'(checkout_ios) or (checkout_mac)\',',
' },',
'',
' # src -> src/repo9 -> src/repo4',
' "src/repo4": {',
' "url": "' + self.git_base + 'repo_4",',
' "condition": \'checkout_android\',',
' },',
'',
' # src -> src/repo6',
' "src/repo6": {',
' "url": "' + self.git_base + 'repo_6",',
' },',
'',
' # src -> src/repo9 -> src/repo7',
' "src/repo7": {',
' "url": "' + self.git_base + 'repo_7",',
' },',
'',
' # src -> src/repo9 -> src/repo8',
' "src/repo8": {',
' "url": "' + self.git_base + 'repo_8",',
' },',
'',
' # src -> src/repo9',
' "src/repo9": {',
' "url": "' + self.git_base + 'repo_9",',
' },',
'',
'}',
'',
'vars = {',
' # src -> src/repo9',
' "str_var": \'xyz\',',
'',
'}',
'',
'# ' + self.git_base + 'repo_10, DEPS',
'# ' + self.git_base + 'repo_11, DEPS',
'# ' + self.git_base + 'repo_8, DEPS',
'# ' + self.git_base + 'repo_9, DEPS',
], deps_contents.splitlines())
with open(output_deps_files) as f:
deps_files_contents = json.load(f)
self.assertEqual([
{'url': self.git_base + 'repo_10', 'deps_file': 'DEPS',
'hierarchy': [['src', self.git_base + 'repo_10']]},
{'url': self.git_base + 'repo_11', 'deps_file': 'DEPS',
'hierarchy': [['src', self.git_base + 'repo_10'],
['src/repo11', self.git_base + 'repo_11']]},
{'url': self.git_base + 'repo_8', 'deps_file': 'DEPS',
'hierarchy': [['src', self.git_base + 'repo_10'],
['src/repo9', self.git_base + 'repo_9'],
['src/repo8', self.git_base + 'repo_8']]},
{'url': self.git_base + 'repo_9', 'deps_file': 'DEPS',
'hierarchy': [['src', self.git_base + 'repo_10'],
['src/repo9', self.git_base + 'repo_9']]},
], deps_files_contents)
def testFlattenRecursedeps(self):
if not self.enabled:
return
......@@ -1366,7 +1467,7 @@ class GClientSmokeGIT(GClientSmokeBase):
' "url": "' + self.git_base + 'repo_14",',
' },',
'',
' # src -> src/another_cipd_dep:package1',
' # src -> src/another_cipd_dep',
' "src/another_cipd_dep": {',
' "packages": [',
' {',
......@@ -1381,7 +1482,7 @@ class GClientSmokeGIT(GClientSmokeBase):
' "dep_type": "cipd",',
' },',
'',
' # src -> src/cipd_dep:package0',
' # src -> src/cipd_dep',
' "src/cipd_dep": {',
' "packages": [',
' {',
......
tests/gclient_test.py
View file @ 0c91147d
......@@ -77,6 +77,166 @@ class GclientTest(trial_dir.TestCase):
os.chdir(self.previous_dir)
super(GclientTest, self).tearDown()
def testLocalDepsFileRespectsRecursedeps(self):
"""Test that we only recurse into specified dependencies.
When parsing a local DEPS file, don't recurse into all of its dependencies,
only on the ones specified by the DEPS file.
"""
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
write(
'.gclient',
'solutions = [\n'
' {\n'
' "name": None,\n'
' "url": None,\n'
' "deps_file": "DEPS",\n'
' },\n'
']')
write(
'DEPS',
'deps = {\n'
' "foo": "svn://example.com/foo",\n'
' "bar": "svn://example.com/bar",\n'
'}\n'
'recursedeps=["foo"]')
write(
os.path.join('foo', 'DEPS'),
'deps = {\n'
' "foo/dir1": "svn://example.com/dir1",\n'
'}')
# We shouldn't process bar/DEPS, since it wasn't specifed as a recursedep.
write(
os.path.join('bar', 'DEPS'),
'ERROR ERROR ERROR')
obj = gclient.GClient.LoadCurrentConfig(options)
obj.RunOnDeps('None', [])
self.assertEqual(
[('bar', 'svn://example.com/bar'),
('foo', 'svn://example.com/foo'),
('foo/dir1', 'svn://example.com/dir1')],
sorted(self._get_processed()))
def testLocalDepsFilePreservesCustomVars(self):
"""Test that the processed dependencies respect custom_vars.
"""
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
write(
'.gclient',
'solutions = [\n'
' {\n'
' "name": None,\n'
' "url": None,\n'
' "deps_file": "DEPS",\n'
' "custom_vars": {\n'
' "custom_var": "custom_value",\n'
' },\n'
' },\n'
']')
write(
'DEPS',
'deps = {\n'
' "foo": "svn://example.com/foo",\n'
'}')
obj = gclient.GClient.LoadCurrentConfig(options)
obj.RunOnDeps('None', [])
self.assertEqual(
[('foo', 'svn://example.com/foo')],
sorted(self._get_processed()))
# Verify that custom_var is set correctly in foo/dir1 and foo/dir2.
foo_vars = obj.dependencies[0].get_vars()
self.assertEqual(foo_vars['custom_var'], 'custom_value')
def testLocalDepsFilePreservesCustomDeps(self):
"""Test that the processed dependencies respect custom_deps.
"""
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
write(
'.gclient',
'solutions = [\n'
' {\n'
' "name": None,\n'
' "url": None,\n'
' "deps_file": "DEPS",\n'
' "custom_deps": {\n'
' "foo/dir1": "svn://example.com/custom",'
' },\n'
' },\n'
']')
write(
'DEPS',
'deps = {\n'
' "foo": "svn://example.com/foo",\n'
'}\n'
'recursedeps = ["foo"]')
write(
os.path.join('foo', 'DEPS'),
'deps = {\n'
' "foo/dir1": "svn://example.com/dir1",\n'
'}')
obj = gclient.GClient.LoadCurrentConfig(options)
obj.RunOnDeps('None', [])
self.assertEqual(
[('foo', 'svn://example.com/foo'),
('foo/dir1', 'svn://example.com/custom')],
sorted(self._get_processed()))
def testLocalDepsFileInheritsVars(self):
"""Tests that variables in the local DEPS file are preserved.
We want to test that the variables in the local DEPS files are preserved
when interpreting the DEPS file as a .gclient file.
In particular, checkout_dir1 should be True in foo/, since it was set in the
local DEPS file.
"""
parser = gclient.OptionParser()
options, _ = parser.parse_args([])
write(
'.gclient',
'solutions = [\n'
' {\n'
' "name": None,\n'
' "url": None,\n'
' "deps_file": "DEPS",\n'
' },\n'
']')
write(
'DEPS',
'vars = {\n'
' "checkout_dir1": True,\n'
'}\n'
'deps = {\n'
' "foo": "svn://example.com/foo",\n'
'}\n'
'recursedeps = ["foo"]\n'
)
write(
os.path.join('foo', 'DEPS'),
'vars = {\n'
' "checkout_dir1": False,\n'
'}\n'
'deps = {\n'
' "foo/dir1": {\n'
' "url": "svn://example.com/dir1",\n'
' "condition": "checkout_dir1",\n'
' },\n'
'}')
obj = gclient.GClient.LoadCurrentConfig(options)
obj.RunOnDeps('None', [])
self.assertEqual(
[('foo', 'svn://example.com/foo'),
('foo/dir1', 'svn://example.com/dir1')],
sorted(self._get_processed()))
def testDependencies(self):
self._dependencies('1')
......@@ -209,7 +369,6 @@ class GclientTest(trial_dir.TestCase):
custom_vars=None,
custom_hooks=None,
deps_file='',
should_process=True,
should_recurse=False,
relative=False,
condition=None,
......@@ -231,7 +390,6 @@ class GclientTest(trial_dir.TestCase):
custom_vars=None,
custom_hooks=None,
deps_file='DEPS',
should_process=True,
should_recurse=True,
relative=False,
condition=None,
......@@ -245,7 +403,6 @@ class GclientTest(trial_dir.TestCase):
custom_vars=None,
custom_hooks=None,
deps_file='DEPS',
should_process=True,
should_recurse=False,
relative=False,
condition=None,
......@@ -263,7 +420,6 @@ class GclientTest(trial_dir.TestCase):
custom_vars=None,
custom_hooks=None,
deps_file='DEPS',
should_process=True,
should_recurse=False,
relative=False,
condition=None,
......@@ -275,7 +431,7 @@ class GclientTest(trial_dir.TestCase):
# pylint: disable=protected-access
obj.dependencies[0]._file_list.append('foo')
str_obj = str(obj)
self.assertEquals(322, len(str_obj), '%d\n%s' % (len(str_obj), str_obj))
self.assertEquals(240, len(str_obj), '%d\n%s' % (len(str_obj), str_obj))
def testHooks(self):
topdir = self.root_dir
......@@ -586,14 +742,12 @@ class GclientTest(trial_dir.TestCase):
('foo/rel', 'svn://example.com/rel'),
], self._get_processed())
self.assertEqual(6, len(sol.dependencies))
self.assertEqual(4, len(sol.dependencies))
self.assertEqual([
('foo/bar', 'svn://example.com/override'),
('foo/baz', 'svn://example.com/baz'),
('foo/new', 'svn://example.com/new'),
('foo/rel', 'svn://example.com/rel'),
('foo/skip', None),
('foo/skip2', None),
], [(dep.name, dep.url) for dep in sol.dependencies])
def testDepsOsOverrideDepsInDepsFile(self):
......@@ -1042,7 +1196,6 @@ class GclientTest(trial_dir.TestCase):
custom_vars=None,
custom_hooks=None,
deps_file='DEPS',
should_process=True,
should_recurse=True,
relative=False,
condition=None,
......@@ -1058,7 +1211,6 @@ class GclientTest(trial_dir.TestCase):
'version': 'foo_version'},
cipd_root=cipd_root,
custom_vars=None,
should_process=True,
relative=False,
condition='fake_condition'),
gclient.CipdDependency(
......@@ -1068,7 +1220,6 @@ class GclientTest(trial_dir.TestCase):
'version': 'bar_version'},
cipd_root=cipd_root,
custom_vars=None,
should_process=True,
relative=False,
condition='fake_condition'),
],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment