From 0b89935ad6fee0109212969cbaccdce822a7632c Mon Sep 17 00:00:00 2001 From: Edward Lesmes Date: Mon, 19 Mar 2018 21:59:55 +0000 Subject: [PATCH] Revert "gclient: Add commands to edit dependencies and variables in DEPS" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 7f4c905fc53e7cbcc3277074c6a339ade8bc0f66. Reason for revert: When running "gclient sync" on a V8 checkout, it says:   File "/work/chrome/depot_tools/gclient.py", line 995, in run    self.ParseDepsFile()  File "/work/chrome/depot_tools/gclient.py", line 874, in ParseDepsFile    self._postprocess_deps(deps, rel_prefix), use_relative_paths)  File "/work/chrome/depot_tools/gclient.py", line 660, in _postprocess_deps    dval['condition'], self.condition) TypeError: '_NodeDict' object does not support item assignment on the recursive descent into third_party/android_tools/DEPS. Any chance that's related to https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/950405 ? Original change's description: > gclient: Add commands to edit dependencies and variables in DEPS > > Adds 'gclient setvar' and 'gclient setdep' commands to edit variables > and dependencies in a DEPS file. > > Bug: 760633 > Change-Id: I6c0712cc079dbbbaee6541b7eda71f4b4813b77b > Reviewed-on: https://chromium-review.googlesource.com/950405 > Commit-Queue: Edward Lesmes > Reviewed-by: Aaron Gable TBR=agable@chromium.org,ehmaldonado@chromium.org Change-Id: If58f6b15d31b19fc53294f1e41d26b4e684a2cf9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 760633 Reviewed-on: https://chromium-review.googlesource.com/969165 Reviewed-by: Edward Lesmes Commit-Queue: Edward Lesmes --- gclient.py | 58 +------------ gclient_eval.py | 143 ++++++--------------------------- tests/gclient_eval_unittest.py | 99 ++--------------------- 3 files changed, 32 insertions(+), 268 deletions(-) diff --git a/gclient.py b/gclient.py index 4c02db451..21eeeb5a1 100755 --- a/gclient.py +++ b/gclient.py @@ -804,7 +804,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): self._gn_args_file = local_scope.get('gclient_gn_args_file') self._gn_args = local_scope.get('gclient_gn_args', []) - self._vars = dict(local_scope.get('vars', {})) + self._vars = local_scope.get('vars', {}) if self.parent: for key, value in self.parent.get_vars().iteritems(): if key in self._vars: @@ -2822,62 +2822,6 @@ def CMDrevinfo(parser, args): return 0 -def CMDsetdep(parser, args): - parser.add_option('--var', action='append', - dest='vars', metavar='VAR=VAL', default=[], - help='Sets a variable to the given value with the format ' - 'name=value.') - parser.add_option('-r', '--revision', action='append', - dest='revisions', metavar='DEP@REV', default=[], - help='Sets the revision/version for the dependency with ' - 'the format dep@rev. If it is a git dependency, dep ' - 'must be a path and rev must be a git hash or ' - 'reference (e.g. src/dep@deadbeef). If it is a CIPD ' - 'dependency, dep must be of the form path:package and ' - 'rev must be the package version ' - '(e.g. src/pkg:chromium/pkg@2.1-cr0).') - parser.add_option('--deps-file', default='DEPS', - # TODO(ehmaldonado): Try to find the DEPS file pointed by - # .gclient first. - help='The DEPS file to be edited. Defaults to the DEPS ' - 'file in the current directory.') - (options, args) = parser.parse_args(args) - - global_scope = {'Var': lambda var: '{%s}' % var} - - if not os.path.isfile(options.deps_file): - raise gclient_utils.Error( - 'DEPS file %s does not exist.' % options.deps_file) - with open(options.deps_file) as f: - contents = f.read() - local_scope = gclient_eval.Exec(contents, global_scope, {}) - - for var in options.vars: - name, _, value = var.partition('=') - if not name or not value: - raise gclient_utils.Error( - 'Wrong var format: %s should be of the form name=value.' % var) - gclient_eval.SetVar(local_scope, name, value) - - for revision in options.revisions: - name, _, value = revision.partition('@') - if not name or not value: - raise gclient_utils.Error( - 'Wrong dep format: %s should be of the form dep@rev.' % revision) - if ':' in name: - name, _, package = name.partition(':') - if not name or not package: - raise gclient_utils.Error( - 'Wrong CIPD format: %s:%s should be of the form path:pkg@version.' - % (name, package)) - gclient_eval.SetCIPD(local_scope, name, package, value) - else: - gclient_eval.SetRevision(local_scope, global_scope, name, value) - - with open(options.deps_file) as f: - f.write(gclient_eval.RenderDEPSFile(local_scope)) - - def CMDverify(parser, args): """Verifies the DEPS file deps are only from allowed_hosts.""" (options, args) = parser.parse_args(args) diff --git a/gclient_eval.py b/gclient_eval.py index d816725e1..38d37e672 100644 --- a/gclient_eval.py +++ b/gclient_eval.py @@ -3,49 +3,17 @@ # found in the LICENSE file. import ast -import cStringIO import collections -import tokenize from third_party import schema -class _NodeDict(collections.Mapping): - """Dict-like type that also stores information on AST nodes and tokens.""" - def __init__(self, data, tokens=None): - self.data = collections.OrderedDict(data) - self.tokens = tokens - - def __str__(self): - return str({k: v[0] for k, v in self.data.iteritems()}) - - def __getitem__(self, key): - return self.data[key][0] - - def __iter__(self): - return iter(self.data) - - def __len__(self): - return len(self.data) - - def GetNode(self, key): - return self.data[key][1] - - def _SetNode(self, key, value, node): - self.data[key] = (value, node) - - -def _NodeDictSchema(dict_schema): - """Validate dict_schema after converting _NodeDict to a regular dict.""" - return lambda d: schema.Schema(dict_schema).validate(dict(d)) - - # See https://github.com/keleshev/schema for docs how to configure schema. -_GCLIENT_DEPS_SCHEMA = _NodeDictSchema({ +_GCLIENT_DEPS_SCHEMA = { schema.Optional(basestring): schema.Or( None, basestring, - _NodeDictSchema({ + { # Repo and revision to check out under the path # (same as if no dict was used). 'url': basestring, @@ -55,25 +23,25 @@ _GCLIENT_DEPS_SCHEMA = _NodeDictSchema({ schema.Optional('condition'): basestring, schema.Optional('dep_type', default='git'): basestring, - }), + }, # CIPD package. - _NodeDictSchema({ + { 'packages': [ - _NodeDictSchema({ + { 'package': basestring, 'version': basestring, - }) + } ], schema.Optional('condition'): basestring, schema.Optional('dep_type', default='cipd'): basestring, - }), + }, ), -}) +} -_GCLIENT_HOOKS_SCHEMA = [_NodeDictSchema({ +_GCLIENT_HOOKS_SCHEMA = [{ # Hook action: list of command-line arguments to invoke. 'action': [basestring], @@ -91,9 +59,9 @@ _GCLIENT_HOOKS_SCHEMA = [_NodeDictSchema({ # Optional condition string. The hook will only be run # if the condition evaluates to True. schema.Optional('condition'): basestring, -})] +}] -_GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({ +_GCLIENT_SCHEMA = schema.Schema({ # List of host names from which dependencies are allowed (whitelist). # NOTE: when not present, all hosts are allowed. # NOTE: scoped to current DEPS file, not recursive. @@ -111,9 +79,9 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({ # Similar to 'deps' (see above) - also keyed by OS (e.g. 'linux'). # Also see 'target_os'. - schema.Optional('deps_os'): _NodeDictSchema({ + schema.Optional('deps_os'): { schema.Optional(basestring): _GCLIENT_DEPS_SCHEMA, - }), + }, # Path to GN args file to write selected variables. schema.Optional('gclient_gn_args_file'): basestring, @@ -127,9 +95,9 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({ schema.Optional('hooks'): _GCLIENT_HOOKS_SCHEMA, # Similar to 'hooks', also keyed by OS. - schema.Optional('hooks_os'): _NodeDictSchema({ + schema.Optional('hooks_os'): { schema.Optional(basestring): _GCLIENT_HOOKS_SCHEMA - }), + }, # Rules which #includes are allowed in the directory. # Also see 'skip_child_includes' and 'specific_include_rules'. @@ -155,9 +123,9 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({ # Mapping from paths to include rules specific for that path. # See 'include_rules' for more details. - schema.Optional('specific_include_rules'): _NodeDictSchema({ + schema.Optional('specific_include_rules'): { schema.Optional(basestring): [basestring] - }), + }, # List of additional OS names to consider when selecting dependencies # from deps_os. @@ -168,10 +136,10 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({ schema.Optional('use_relative_paths'): bool, # Variables that can be referenced using Var() - see 'deps'. - schema.Optional('vars'): _NodeDictSchema({ + schema.Optional('vars'): { schema.Optional(basestring): schema.Or(basestring, bool), - }), -})) + }, +}) def _gclient_eval(node_or_string, global_scope, filename=''): @@ -191,7 +159,8 @@ def _gclient_eval(node_or_string, global_scope, filename=''): elif isinstance(node, ast.List): return list(map(_convert, node.elts)) elif isinstance(node, ast.Dict): - return _NodeDict((_convert(k), (_convert(v), v)) + return collections.OrderedDict( + (_convert(k), _convert(v)) for k, v in zip(node.keys, node.values)) elif isinstance(node, ast.Name): if node.id not in _allowed_names: @@ -228,7 +197,6 @@ def Exec(content, global_scope, local_scope, filename=''): if isinstance(node_or_string, ast.Expression): node_or_string = node_or_string.body - defined_variables = set() def _visit_in_module(node): if isinstance(node, ast.Assign): if len(node.targets) != 1: @@ -242,13 +210,12 @@ def Exec(content, global_scope, local_scope, filename=''): filename, getattr(node, 'lineno', ''))) value = _gclient_eval(node.value, global_scope, filename=filename) - if target.id in defined_variables: + if target.id in local_scope: raise ValueError( 'invalid assignment: overrides var %r (file %r, line %s)' % ( target.id, filename, getattr(node, 'lineno', ''))) - defined_variables.add(target.id) - return target.id, (value, node.value) + local_scope[target.id] = value else: raise ValueError( 'unexpected AST node: %s %s (file %r, line %s)' % ( @@ -256,15 +223,8 @@ def Exec(content, global_scope, local_scope, filename=''): getattr(node, 'lineno', ''))) if isinstance(node_or_string, ast.Module): - data = [] for stmt in node_or_string.body: - data.append(_visit_in_module(stmt)) - tokens = { - token[2]: list(token) - for token in tokenize.generate_tokens( - cStringIO.StringIO(content).readline) - } - local_scope = _NodeDict(data, tokens) + _visit_in_module(stmt) else: raise ValueError( 'unexpected AST node: %s %s (file %r, line %s)' % ( @@ -373,56 +333,3 @@ def EvaluateCondition(condition, variables, referenced_variables=None): 'unexpected AST node: %s %s (inside %r)' % ( node, ast.dump(node), condition)) return _convert(main_node) - - -def RenderDEPSFile(gclient_dict): - contents = sorted(gclient_dict.tokens.values(), key=lambda token: token[2]) - return tokenize.untokenize(contents) - - -def _UpdateAstString(tokens, node, value): - position = node.lineno, node.col_offset - tokens[position][1] = repr(value) - node.s = value - - -def SetVar(gclient_dict, var_name, value): - node = gclient_dict['vars'].GetNode(var_name) - tokens = gclient_dict.tokens - _UpdateAstString(tokens, node, value) - gclient_dict['vars']._SetNode(var_name, value, node) - - -def SetCIPD(gclient_dict, dep_name, package_name, new_version): - packages = [ - package - for package in gclient_dict['deps'][dep_name]['packages'] - if package['package'] == package_name - ] - assert len(packages) == 1 - node = packages[0].GetNode('version') - # TODO(ehmaldonado): Support Var in package's version. - tokens = gclient_dict.tokens - new_version = 'version:' + new_version - _UpdateAstString(tokens, node, new_version) - packages[0]._SetNode('version', new_version, node) - - -def SetRevision(gclient_dict, global_scope, dep_name, new_revision): - def _UpdateRevision(dep_dict, dep_key): - dep_node = dep_dict.GetNode(dep_key) - node = dep_node - if isinstance(node, ast.BinOp): - node = node.right - if isinstance(node, ast.Call): - SetVar(gclient_dict, node.args[0].s, new_revision) - else: - _UpdateAstString(gclient_dict.tokens, node, new_revision) - value = _gclient_eval(dep_node, global_scope) - dep_dict._SetNode(dep_key, value, dep_node) - - # TODO(ehmaldonado): Support Var in dep names. - if isinstance(gclient_dict['deps'][dep_name], _NodeDict): - _UpdateRevision(gclient_dict['deps'][dep_name], 'url') - else: - _UpdateRevision(gclient_dict['deps'], dep_name) diff --git a/tests/gclient_eval_unittest.py b/tests/gclient_eval_unittest.py index 5205eb60d..bc0b5923d 100755 --- a/tests/gclient_eval_unittest.py +++ b/tests/gclient_eval_unittest.py @@ -8,7 +8,6 @@ import itertools import logging import os import sys -import textwrap import unittest sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -19,45 +18,6 @@ import gclient import gclient_eval -_SAMPLE_DEPS_FILE = textwrap.dedent("""\ -deps = { - 'src/dep': Var('git_repo') + '/dep' + '@' + 'deadbeef', - # Some comment - 'src/android/dep_2': { - 'url': Var('git_repo') + '/dep_2' + '@' + Var('dep_2_rev'), - 'condition': 'checkout_android', - }, - - 'src/dep_3': Var('git_repo') + '/dep_3@' + Var('dep_3_rev'), - - 'src/cipd/package': { - 'packages': [ - { - 'package': 'some/cipd/package', - 'version': 'version:1234', - }, - { - 'package': 'another/cipd/package', - 'version': 'version:5678', - }, - ], - 'condition': 'checkout_android', - 'dep_type': 'cipd', - }, -} - -vars = { - 'git_repo': 'https://example.com/repo.git', - # Some comment with bad indentation - 'dep_2_rev': '1ced', - # Some more comments - # 1 - # 2 - # 3 - 'dep_3_rev': '5p1e5', -}""") - - class GClientEvalTest(unittest.TestCase): def test_str(self): self.assertEqual('foo', gclient_eval._gclient_eval('"foo"', {})) @@ -122,13 +82,17 @@ class ExecTest(unittest.TestCase): self.assertIn( 'invalid assignment: overrides var \'a\'', str(cm.exception)) + def test_schema_unknown_key(self): + with self.assertRaises(schema.SchemaWrongKeyError): + gclient_eval.Exec('foo = "bar"', {}, {}, '') + def test_schema_wrong_type(self): with self.assertRaises(schema.SchemaError): gclient_eval.Exec('include_rules = {}', {}, {}, '') def test_recursedeps_list(self): local_scope = {} - local_scope = gclient_eval.Exec( + gclient_eval.Exec( 'recursedeps = [["src/third_party/angle", "DEPS.chromium"]]', {}, local_scope, '') @@ -141,7 +105,7 @@ class ExecTest(unittest.TestCase): global_scope = { 'Var': lambda var_name: '{%s}' % var_name, } - local_scope = gclient_eval.Exec('\n'.join([ + gclient_eval.Exec('\n'.join([ 'vars = {', ' "foo": "bar",', '}', @@ -205,57 +169,6 @@ class EvaluateConditionTest(unittest.TestCase): str(cm.exception)) -class SetVarTest(unittest.TestCase): - def testSetVar(self): - local_scope = gclient_eval.Exec(_SAMPLE_DEPS_FILE, {'Var': str}, {}) - - gclient_eval.SetVar(local_scope, 'dep_2_rev', 'c0ffee') - result = gclient_eval.RenderDEPSFile(local_scope) - - self.assertEqual( - result, - _SAMPLE_DEPS_FILE.replace('1ced', 'c0ffee')) - - -class SetCipdTest(unittest.TestCase): - def testSetCIPD(self): - local_scope = gclient_eval.Exec(_SAMPLE_DEPS_FILE, {'Var': str}, {}) - - gclient_eval.SetCIPD( - local_scope, 'src/cipd/package', 'another/cipd/package', '6.789') - result = gclient_eval.RenderDEPSFile(local_scope) - - self.assertEqual(result, _SAMPLE_DEPS_FILE.replace('5678', '6.789')) - - -class SetRevisionTest(unittest.TestCase): - def setUp(self): - self.global_scope = {'Var': str} - self.local_scope = gclient_eval.Exec( - _SAMPLE_DEPS_FILE, self.global_scope, {}) - - def testSetRevision(self): - gclient_eval.SetRevision( - self.local_scope, self.global_scope, 'src/dep', 'deadfeed') - result = gclient_eval.RenderDEPSFile(self.local_scope) - - self.assertEqual(result, _SAMPLE_DEPS_FILE.replace('deadbeef', 'deadfeed')) - - def testSetRevisionInUrl(self): - gclient_eval.SetRevision( - self.local_scope, self.global_scope, 'src/dep_3', '0ff1ce') - result = gclient_eval.RenderDEPSFile(self.local_scope) - - self.assertEqual(result, _SAMPLE_DEPS_FILE.replace('5p1e5', '0ff1ce')) - - def testSetRevisionInVars(self): - gclient_eval.SetRevision( - self.local_scope, self.global_scope, 'src/android/dep_2', 'c0ffee') - result = gclient_eval.RenderDEPSFile(self.local_scope) - - self.assertEqual(result, _SAMPLE_DEPS_FILE.replace('1ced', 'c0ffee')) - - if __name__ == '__main__': level = logging.DEBUG if '-v' in sys.argv else logging.FATAL logging.basicConfig(