From e2f9feecaf4326cef699624d5f52b9f31ecbc104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Hajdan=2C=20Jr?= Date: Tue, 9 May 2017 10:04:02 +0200 Subject: [PATCH] Add validate command to gclient MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inspired by https://chromium-review.googlesource.com/c/411515/ . Bug: 570091 Change-Id: I7bf9653178c06125ae8be1dee021acfc187b2bdc Reviewed-on: https://chromium-review.googlesource.com/497848 Commit-Queue: Paweł Hajdan Jr. Reviewed-by: Dirk Pranke --- gclient.py | 31 ++++++- gclient_eval.py | 142 +++++++++++++++++++++++++++++++++ tests/gclient_eval_unittest.py | 116 +++++++++++++++++++++++++++ 3 files changed, 286 insertions(+), 3 deletions(-) create mode 100644 gclient_eval.py create mode 100755 tests/gclient_eval_unittest.py diff --git a/gclient.py b/gclient.py index 6d7988afa..1270e03e6 100755 --- a/gclient.py +++ b/gclient.py @@ -96,6 +96,7 @@ import urllib import urlparse import fix_encoding +import gclient_eval import gclient_scm import gclient_utils import git_cache @@ -606,6 +607,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): 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)): @@ -721,6 +724,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): self.add_dependencies_and_close(deps_to_add, hooks_to_run) logging.info('ParseDepsFile(%s) done' % self.name) + def _get_option(self, attr, default): + obj = self + while not hasattr(obj, '_options'): + obj = obj.parent + return getattr(obj._options, attr, default) + def add_dependencies_and_close(self, deps_to_add, hooks): """Adds the dependencies, hooks and mark the parsing as done.""" for dep in deps_to_add: @@ -757,7 +766,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings): # When running runhooks, there's no need to consult the SCM. # All known hooks are expected to run unconditionally regardless of working # copy state, so skip the SCM status check. - run_scm = command not in ('runhooks', 'recurse', None) + run_scm = command not in ('runhooks', 'recurse', 'validate', None) parsed_url = self.LateOverride(self.url) file_list = [] if not options.nohooks else None revision_override = revision_overrides.pop(self.name, None) @@ -1346,7 +1355,8 @@ it or fix the checkout. revision_overrides = {} # It's unnecessary to check for revision overrides for 'recurse'. # Save a few seconds by not calling _EnforceRevisions() in that case. - if command not in ('diff', 'recurse', 'runhooks', 'status', 'revert'): + if command not in ('diff', 'recurse', 'runhooks', 'status', 'revert', + 'validate'): self._CheckConfig() revision_overrides = self._EnforceRevisions() pm = None @@ -1354,7 +1364,7 @@ it or fix the checkout. if (setup_color.IS_TTY and not self._options.verbose and progress): if command in ('update', 'revert'): pm = Progress('Syncing projects', 1) - elif command == 'recurse': + elif command in ('recurse', 'validate'): pm = Progress(' '.join(args), 1) work_queue = gclient_utils.ExecutionQueue( self._options.jobs, pm, ignore_requirements=ignore_requirements, @@ -1862,6 +1872,8 @@ def CMDsync(parser, args): help='DEPRECATED: This is a no-op.') parser.add_option('-m', '--manually_grab_svn_rev', action='store_true', help='DEPRECATED: This is a no-op.') + parser.add_option('--validate-syntax', action='store_true', + help='Validate the .gclient and DEPS syntax') (options, args) = parser.parse_args(args) client = GClient.LoadCurrentConfig(options) @@ -1892,6 +1904,19 @@ def CMDsync(parser, args): CMDupdate = CMDsync +def CMDvalidate(parser, args): + """Validates the .gclient and DEPS syntax.""" + options, args = parser.parse_args(args) + options.validate_syntax = True + client = GClient.LoadCurrentConfig(options) + rv = client.RunOnDeps('validate', args) + if rv == 0: + print('validate: SUCCESS') + else: + print('validate: FAILURE') + return rv + + def CMDdiff(parser, args): """Displays local diff for every dependencies.""" parser.add_option('--deps', dest='deps_os', metavar='OS_LIST', diff --git a/gclient_eval.py b/gclient_eval.py new file mode 100644 index 000000000..7364a9542 --- /dev/null +++ b/gclient_eval.py @@ -0,0 +1,142 @@ +# Copyright 2017 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. + +import ast + + +def _gclient_eval(node_or_string, global_scope, filename=''): + """Safely evaluates a single expression. Returns the result.""" + _allowed_names = {'None': None, 'True': True, 'False': False} + if isinstance(node_or_string, basestring): + node_or_string = ast.parse(node_or_string, filename=filename, mode='eval') + if isinstance(node_or_string, ast.Expression): + node_or_string = node_or_string.body + def _convert(node): + if isinstance(node, ast.Str): + return node.s + elif isinstance(node, ast.Tuple): + return tuple(map(_convert, node.elts)) + elif isinstance(node, ast.List): + return list(map(_convert, node.elts)) + elif isinstance(node, ast.Dict): + return dict((_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: + raise ValueError( + 'invalid name %r (file %r, line %s)' % ( + node.id, filename, getattr(node, 'lineno', ''))) + return _allowed_names[node.id] + elif isinstance(node, ast.Call): + if not isinstance(node.func, ast.Name): + raise ValueError( + 'invalid call: func should be a name (file %r, line %s)' % ( + filename, getattr(node, 'lineno', ''))) + if node.keywords or node.starargs or node.kwargs: + raise ValueError( + 'invalid call: use only regular args (file %r, line %s)' % ( + filename, getattr(node, 'lineno', ''))) + args = map(_convert, node.args) + return global_scope[node.func.id](*args) + elif isinstance(node, ast.BinOp) and isinstance(node.op, ast.Add): + return _convert(node.left) + _convert(node.right) + else: + raise ValueError( + 'unexpected AST node: %s (file %r, line %s)' % ( + node, filename, getattr(node, 'lineno', ''))) + 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') + if isinstance(node_or_string, ast.Expression): + node_or_string = node_or_string.body + + def _visit_in_module(node): + if isinstance(node, ast.Assign): + if len(node.targets) != 1: + raise ValueError( + 'invalid assignment: use exactly one target (file %r, line %s)' % ( + filename, getattr(node, 'lineno', ''))) + target = node.targets[0] + if not isinstance(target, ast.Name): + raise ValueError( + 'invalid assignment: target should be a name (file %r, line %s)' % ( + filename, getattr(node, 'lineno', ''))) + value = _gclient_eval(node.value, global_scope, filename=filename) + + if target.id in result_scope: + raise ValueError( + 'invalid assignment: overrides var %r (file %r, line %s)' % ( + target.id, filename, getattr(node, 'lineno', ''))) + + result_scope[target.id] = value + else: + raise ValueError( + 'unexpected AST node: %s (file %r, line %s)' % ( + node, filename, getattr(node, 'lineno', ''))) + + if isinstance(node_or_string, ast.Module): + for stmt in node_or_string.body: + _visit_in_module(stmt) + else: + raise ValueError( + 'unexpected AST node: %s (file %r, line %s)' % ( + node_or_string, + 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) diff --git a/tests/gclient_eval_unittest.py b/tests/gclient_eval_unittest.py new file mode 100755 index 000000000..adde6a720 --- /dev/null +++ b/tests/gclient_eval_unittest.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python +# Copyright 2017 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. + +import logging +import os +import sys +import unittest + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import gclient_eval + + +class GClientEvalTest(unittest.TestCase): + def test_str(self): + self.assertEqual('foo', gclient_eval._gclient_eval('"foo"', {})) + + def test_tuple(self): + self.assertEqual(('a', 'b'), gclient_eval._gclient_eval('("a", "b")', {})) + + def test_list(self): + self.assertEqual(['a', 'b'], gclient_eval._gclient_eval('["a", "b"]', {})) + + def test_dict(self): + self.assertEqual({'a': 'b'}, gclient_eval._gclient_eval('{"a": "b"}', {})) + + def test_name_safe(self): + self.assertEqual(True, gclient_eval._gclient_eval('True', {})) + + def test_name_unsafe(self): + with self.assertRaises(ValueError) as cm: + gclient_eval._gclient_eval('UnsafeName', {'UnsafeName': 'foo'}) + self.assertIn('invalid name \'UnsafeName\'', str(cm.exception)) + + def test_call(self): + self.assertEqual( + 'bar', + gclient_eval._gclient_eval('Foo("bar")', {'Foo': lambda x: x})) + + def test_plus(self): + self.assertEqual('foo', gclient_eval._gclient_eval('"f" + "o" + "o"', {})) + + def test_not_expression(self): + with self.assertRaises(SyntaxError) as cm: + gclient_eval._gclient_eval('def foo():\n pass', {}) + self.assertIn('invalid syntax', str(cm.exception)) + + def test_not_whitelisted(self): + with self.assertRaises(ValueError) as cm: + gclient_eval._gclient_eval('[x for x in [1, 2, 3]]', {}) + self.assertIn( + 'unexpected AST node: <_ast.ListComp object', str(cm.exception)) + + +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"', {})) + + def test_multiple_assignment(self): + with self.assertRaises(ValueError) as cm: + gclient_eval._gclient_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"', {}) + self.assertIn( + 'invalid assignment: overrides var \'a\'', str(cm.exception)) + + +class CheckTest(unittest.TestCase): + TEST_CODE=""" +list_var = ["a", "b", "c"] + +dict_var = {"a": "1", "b": "2", "c": "3"} + +nested_var = { + "list": ["a", "b", "c"], + "dict": {"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['list_var'][0] = 'x' + with self.assertRaises(gclient_eval.CheckFailure): + gclient_eval.Check(self.TEST_CODE, '', {}, self.expected) + + def test_fail_dict(self): + self.expected['dict_var']['a'] = 'x' + with self.assertRaises(gclient_eval.CheckFailure): + gclient_eval.Check(self.TEST_CODE, '', {}, self.expected) + + def test_fail_nested(self): + self.expected['nested_var']['dict']['c'] = 'x' + with self.assertRaises(gclient_eval.CheckFailure): + gclient_eval.Check(self.TEST_CODE, '', {}, self.expected) + + +if __name__ == '__main__': + level = logging.DEBUG if '-v' in sys.argv else logging.FATAL + logging.basicConfig( + level=level, + format='%(asctime).19s %(levelname)s %(filename)s:' + '%(lineno)s %(message)s') + unittest.main()