From c485d5a5d990416796c1d0099ebe12a92ca7f40d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Hajdan=2C=20Jr?= Date: Fri, 2 Jun 2017 12:08:09 +0200 Subject: [PATCH] gclient: use new exec logic when validation is enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This way we can get e.g. ordered dict as needed for conditions. Only the new logic does it, not the regular python exec logic. Bug: 570091 Change-Id: Ia5554e5b018085b3b9bd876b7f28a9f8e54a7984 Reviewed-on: https://chromium-review.googlesource.com/522564 Reviewed-by: Andrii Shyshkalov Commit-Queue: Paweł Hajdan Jr. --- gclient.py | 7 +-- gclient_eval.py | 65 +++------------------------ tests/gclient_eval_unittest.py | 81 ++++++++++++++-------------------- 3 files changed, 42 insertions(+), 111 deletions(-) diff --git a/gclient.py b/gclient.py index fe2c9aa8fc..59c899dfc3 100755 --- a/gclient.py +++ b/gclient.py @@ -555,11 +555,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): } # Eval the content. try: - exec(deps_content, global_scope, local_scope) + if self._get_option('validate_syntax', False): + gclient_eval.Exec(deps_content, global_scope, local_scope, filepath) + else: + exec(deps_content, global_scope, local_scope) except SyntaxError as e: gclient_utils.SyntaxErrorToError(filepath, e) - if self._get_option('validate_syntax', False): - gclient_eval.Check(deps_content, filepath, global_scope, local_scope) if use_strict: for key, val in local_scope.iteritems(): if not isinstance(val, (dict, list, tuple, str)): diff --git a/gclient_eval.py b/gclient_eval.py index 8df68f6cc8..a5d1deeb6c 100644 --- a/gclient_eval.py +++ b/gclient_eval.py @@ -153,12 +153,9 @@ def _gclient_eval(node_or_string, global_scope, filename=''): return _convert(node_or_string) -def _gclient_exec(node_or_string, global_scope, filename=''): - """Safely execs a set of assignments. Returns resulting scope.""" - result_scope = {} - - if isinstance(node_or_string, basestring): - node_or_string = ast.parse(node_or_string, filename=filename, mode='exec') +def Exec(content, global_scope, local_scope, filename=''): + """Safely execs a set of assignments. Mutates |local_scope|.""" + node_or_string = ast.parse(content, filename=filename, mode='exec') if isinstance(node_or_string, ast.Expression): node_or_string = node_or_string.body @@ -175,12 +172,12 @@ def _gclient_exec(node_or_string, global_scope, filename=''): filename, getattr(node, 'lineno', ''))) value = _gclient_eval(node.value, global_scope, filename=filename) - if target.id in result_scope: + if target.id in local_scope: raise ValueError( 'invalid assignment: overrides var %r (file %r, line %s)' % ( target.id, filename, getattr(node, 'lineno', ''))) - result_scope[target.id] = value + local_scope[target.id] = value else: raise ValueError( 'unexpected AST node: %s %s (file %r, line %s)' % ( @@ -198,54 +195,4 @@ def _gclient_exec(node_or_string, global_scope, filename=''): filename, getattr(node_or_string, 'lineno', ''))) - return result_scope - - -class CheckFailure(Exception): - """Contains details of a check failure.""" - def __init__(self, msg, path, exp, act): - super(CheckFailure, self).__init__(msg) - self.path = path - self.exp = exp - self.act = act - - -def Check(content, path, global_scope, expected_scope): - """Cross-checks the old and new gclient eval logic. - - Safely execs |content| (backed by file |path|) using |global_scope|, - and compares with |expected_scope|. - - Throws CheckFailure if any difference between |expected_scope| and scope - returned by new gclient eval code is detected. - """ - def fail(prefix, exp, act): - raise CheckFailure( - 'gclient check for %s: %s exp %s, got %s' % ( - path, prefix, repr(exp), repr(act)), prefix, exp, act) - - def compare(expected, actual, var_path, actual_scope): - if isinstance(expected, dict): - exp = set(expected.keys()) - act = set(actual.keys()) - if exp != act: - fail(var_path, exp, act) - for k in expected: - compare(expected[k], actual[k], var_path + '["%s"]' % k, actual_scope) - return - elif isinstance(expected, list): - exp = len(expected) - act = len(actual) - if exp != act: - fail('len(%s)' % var_path, expected_scope, actual_scope) - for i in range(exp): - compare(expected[i], actual[i], var_path + '[%d]' % i, actual_scope) - else: - if expected != actual: - fail(var_path, expected_scope, actual_scope) - - result_scope = _gclient_exec(content, global_scope, filename=path) - - compare(expected_scope, result_scope, '', result_scope) - - _GCLIENT_SCHEMA.validate(result_scope) + _GCLIENT_SCHEMA.validate(local_scope) diff --git a/tests/gclient_eval_unittest.py b/tests/gclient_eval_unittest.py index 7c6787b862..5cca55ac8d 100755 --- a/tests/gclient_eval_unittest.py +++ b/tests/gclient_eval_unittest.py @@ -3,6 +3,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import collections import itertools import logging import os @@ -13,6 +14,7 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from third_party import schema +import gclient import gclient_eval @@ -67,73 +69,54 @@ class GClientEvalTest(unittest.TestCase): self.assertEqual(expected, result.items()) -class GClientExecTest(unittest.TestCase): - def test_basic(self): - self.assertEqual( - {'a': '1', 'b': '2', 'c': '3'}, - gclient_eval._gclient_exec('a = "1"\nb = "2"\nc = "3"', {})) - +class ExecTest(unittest.TestCase): def test_multiple_assignment(self): with self.assertRaises(ValueError) as cm: - gclient_eval._gclient_exec('a, b, c = "a", "b", "c"', {}) + gclient_eval.Exec('a, b, c = "a", "b", "c"', {}, {}) self.assertIn( 'invalid assignment: target should be a name', str(cm.exception)) def test_override(self): with self.assertRaises(ValueError) as cm: - gclient_eval._gclient_exec('a = "a"\na = "x"', {}) + gclient_eval.Exec('a = "a"\na = "x"', {}, {}) self.assertIn( 'invalid assignment: overrides var \'a\'', str(cm.exception)) - -class CheckTest(unittest.TestCase): - TEST_CODE=""" -include_rules = ["a", "b", "c"] - -vars = {"a": "1", "b": "2", "c": "3"} - -deps_os = { - "linux": {"a": "1", "b": "2", "c": "3"} -}""" - - def setUp(self): - self.expected = {} - exec(self.TEST_CODE, {}, self.expected) - - def test_pass(self): - gclient_eval.Check(self.TEST_CODE, '', {}, self.expected) - - def test_fail_list(self): - self.expected['include_rules'][0] = 'x' - with self.assertRaises(gclient_eval.CheckFailure): - gclient_eval.Check(self.TEST_CODE, '', {}, self.expected) - - def test_fail_dict(self): - self.expected['vars']['a'] = 'x' - with self.assertRaises(gclient_eval.CheckFailure): - gclient_eval.Check(self.TEST_CODE, '', {}, self.expected) - - def test_fail_nested(self): - self.expected['deps_os']['linux']['c'] = 'x' - with self.assertRaises(gclient_eval.CheckFailure): - gclient_eval.Check(self.TEST_CODE, '', {}, self.expected) - def test_schema_unknown_key(self): with self.assertRaises(schema.SchemaWrongKeyError): - gclient_eval.Check('foo = "bar"', '', {}, {'foo': 'bar'}) + gclient_eval.Exec('foo = "bar"', {}, {}, '') def test_schema_wrong_type(self): with self.assertRaises(schema.SchemaError): - gclient_eval.Check( - 'include_rules = {}', '', {}, {'include_rules': {}}) + gclient_eval.Exec('include_rules = {}', {}, {}, '') def test_recursedeps_list(self): - gclient_eval.Check( + local_scope = {} + gclient_eval.Exec( 'recursedeps = [["src/third_party/angle", "DEPS.chromium"]]', - '', - {}, - {'recursedeps': [['src/third_party/angle', 'DEPS.chromium']]}) - + {}, local_scope, + '') + self.assertEqual( + {'recursedeps': [['src/third_party/angle', 'DEPS.chromium']]}, + local_scope) + + def test_var(self): + local_scope = {} + global_scope = { + 'Var': gclient.GClientKeywords.VarImpl({}, local_scope).Lookup, + } + gclient_eval.Exec('\n'.join([ + 'vars = {', + ' "foo": "bar",', + '}', + 'deps = {', + ' "a_dep": "a" + Var("foo") + "b",', + '}', + ]), global_scope, local_scope, '') + self.assertEqual({ + 'vars': collections.OrderedDict([('foo', 'bar')]), + 'deps': collections.OrderedDict([('a_dep', 'abarb')]), + }, local_scope) if __name__ == '__main__': level = logging.DEBUG if '-v' in sys.argv else logging.FATAL