gclient: use new exec logic when validation is enabled

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 <tandrii@chromium.org>
Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>

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)):

gclient_eval.py

@@ -153,12 +153,9 @@ def _gclient_eval(node_or_string, global_scope, filename='<unknown>'):
   return _convert(node_or_string)
 
 
-def _gclient_exec(node_or_string, global_scope, filename='<unknown>'):
+def Exec(content, global_scope, local_scope, filename='<unknown>'):
-  """Safely execs a set of assignments. Returns resulting scope."""
+  """Safely execs a set of assignments. Mutates |local_scope|."""
-  result_scope = {}
+  node_or_string = ast.parse(content, filename=filename, mode='exec')
-
-  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
 
@@ -175,12 +172,12 @@ def _gclient_exec(node_or_string, global_scope, filename='<unknown>'):
                 filename, getattr(node, 'lineno', '<unknown>')))
       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', '<unknown>')))
 
-      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='<unknown>'):
             filename,
             getattr(node_or_string, 'lineno', '<unknown>')))
 
-  return result_scope
+  _GCLIENT_SCHEMA.validate(local_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)

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):
+class ExecTest(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"', {})
+      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, '<string>', {}, 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, '<string>', {}, self.expected)
-
-  def test_fail_dict(self):
-    self.expected['vars']['a'] = 'x'
-    with self.assertRaises(gclient_eval.CheckFailure):
-      gclient_eval.Check(self.TEST_CODE, '<string>', {}, 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, '<string>', {}, self.expected)
-
   def test_schema_unknown_key(self):
     with self.assertRaises(schema.SchemaWrongKeyError):
-      gclient_eval.Check('foo = "bar"', '<string>', {}, {'foo': 'bar'})
+      gclient_eval.Exec('foo = "bar"', {}, {}, '<string>')
 
   def test_schema_wrong_type(self):
     with self.assertRaises(schema.SchemaError):
-      gclient_eval.Check(
+      gclient_eval.Exec('include_rules = {}', {}, {}, '<string>')
-          'include_rules = {}', '<string>', {}, {'include_rules': {}})
 
   def test_recursedeps_list(self):
-    gclient_eval.Check(
+    local_scope = {}
+    gclient_eval.Exec(
         'recursedeps = [["src/third_party/angle", "DEPS.chromium"]]',
-        '<string>',
+        {}, local_scope,
-        {},
+        '<string>')
-        {'recursedeps': [['src/third_party/angle', 'DEPS.chromium']]})
+    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, '<string>')
+    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