gclient eval: Expand vars while parsing DEPS files

Introduce a Parse function that takes care of expanding vars while parsing
the DEPS file.

It wraps Exec and exec calls, and supports deferring the expansion until
later, so gclient flatten gets access to the unexpanded version.

Bug: 821199
Change-Id: I943b021cc4474c9cda67b3816b841dd8ada3f5b2
Reviewed-on: https://chromium-review.googlesource.com/973749
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>

gclient.py

@@ -736,7 +736,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     deps_to_add.sort(key=lambda x: x.name)
     return deps_to_add
 
-  def ParseDepsFile(self):
+  def ParseDepsFile(self, expand_vars=True):
     """Parses the DEPS file for this dependency."""
     assert not self.deps_parsed
     assert not self.dependencies
@@ -765,15 +765,14 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     local_scope = {}
     if deps_content:
-      # Eval the content.
       try:
-        if self._get_option('validate_syntax', False):
-          local_scope = gclient_eval.Exec(deps_content, filepath)
-        else:
-          global_scope = {
-            'Var': lambda var_name: '{%s}' % var_name,
-          }
-          exec(deps_content, global_scope, local_scope)
+        vars_override = self.get_vars()
+        if self.parent:
+          vars_override.update(self.parent.get_vars())
+        local_scope = gclient_eval.Parse(
+            deps_content, expand_vars,
+            self._get_option('validate_syntax', False),
+            filepath, vars_override)
       except SyntaxError as e:
         gclient_utils.SyntaxErrorToError(filepath, e)
 
@@ -988,7 +987,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
           file_list[i] = file_list[i][1:]
 
     # Always parse the DEPS file.
-    self.ParseDepsFile()
+    self.ParseDepsFile(expand_vars=(command != 'flatten'))
     self._run_is_done(file_list or [], parsed_url)
     if command in ('update', 'revert') and not options.noprehooks:
       self.RunPreDepsHooks()
@@ -1864,7 +1863,7 @@ it or fix the checkout.
           print('%s: %s' % (x, entries[x]))
     logging.info(str(self))
 
-  def ParseDepsFile(self):
+  def ParseDepsFile(self, expand_vars=None):
     """No DEPS to parse for a .gclient file."""
     raise gclient_utils.Error('Internal error')
 
@@ -1973,7 +1972,7 @@ class CipdDependency(Dependency):
       self._cipd_package = self._cipd_root.add_package(
           self._cipd_subdir, self._package_name, self._package_version)
 
-  def ParseDepsFile(self):
+  def ParseDepsFile(self, expand_vars=None):
     """CIPD dependencies are not currently allowed to have nested deps."""
     self.add_dependencies_and_close([], [])
 
@@ -2930,7 +2929,9 @@ def CMDsetdep(parser, args):
         '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)
+  local_scope = gclient_eval.Parse(
+      contents, expand_vars=True, validate_syntax=True,
+      filename=options.deps_file)
 
   for var in options.vars:
     name, _, value = var.partition('=')

gclient_eval.py

@@ -37,7 +37,7 @@ class _NodeDict(collections.MutableMapping):
   def GetNode(self, key):
     return self.data[key][1]
 
-  def _SetNode(self, key, value, node):
+  def SetNode(self, key, value, node):
     self.data[key] = (value, node)
 
 
@@ -183,8 +183,9 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({
 }))
 
 
-def _gclient_eval(node_or_string, filename='<unknown>'):
+def _gclient_eval(node_or_string, vars_dict, expand_vars, filename):
   """Safely evaluates a single expression. Returns the result."""
+  vars_dict = vars_dict or {}
   _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')
@@ -192,7 +193,15 @@ def _gclient_eval(node_or_string, filename='<unknown>'):
     node_or_string = node_or_string.body
   def _convert(node):
     if isinstance(node, ast.Str):
-      return node.s
+      if not expand_vars:
+        return node.s
+      try:
+        return node.s.format(**vars_dict)
+      except KeyError as e:
+        raise ValueError(
+            '%s was used as a variable, but was not declared in the vars dict '
+            '(file %r, line %s)' % (
+                e.message, filename, getattr(node, 'lineno', '<unknown>')))
     elif isinstance(node, ast.Num):
       return node.n
     elif isinstance(node, ast.Tuple):
@@ -222,7 +231,18 @@ def _gclient_eval(node_or_string, filename='<unknown>'):
         raise ValueError(
             'Var\'s argument must be a variable name (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
-      return '{%s}' % arg
+      if not expand_vars:
+        return '{%s}' % arg
+      if vars_dict is None:
+        raise ValueError(
+            'vars must be declared before Var can be used (file %r, line %s)'
+            % (filename, getattr(node, 'lineno', '<unknown>')))
+      if arg not in vars_dict:
+        raise ValueError(
+            '%s was used as a variable, but was not declared in the vars dict '
+            '(file %r, line %s)' % (
+                arg, filename, getattr(node, 'lineno', '<unknown>')))
+      return vars_dict[arg]
     elif isinstance(node, ast.BinOp) and isinstance(node.op, ast.Add):
       return _convert(node.left) + _convert(node.right)
     elif isinstance(node, ast.BinOp) and isinstance(node.op, ast.Mod):
@@ -235,13 +255,18 @@ def _gclient_eval(node_or_string, filename='<unknown>'):
   return _convert(node_or_string)
 
 
-def Exec(content, filename='<unknown>'):
-  """Safely execs a set of assignments. Mutates |local_scope|."""
+def Exec(content, expand_vars=True, filename='<unknown>'):
+  """Safely execs a set of assignments."""
   node_or_string = ast.parse(content, filename=filename, mode='exec')
   if isinstance(node_or_string, ast.Expression):
     node_or_string = node_or_string.body
 
-  defined_variables = set()
+  tokens = {
+      token[2]: list(token)
+      for token in tokenize.generate_tokens(
+          cStringIO.StringIO(content).readline)
+  }
+  local_scope = _NodeDict({}, tokens)
   def _visit_in_module(node):
     if isinstance(node, ast.Assign):
       if len(node.targets) != 1:
@@ -253,15 +278,15 @@ def Exec(content, filename='<unknown>'):
         raise ValueError(
             'invalid assignment: target should be a name (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
-      value = _gclient_eval(node.value, filename=filename)
+      value = _gclient_eval(node.value, local_scope.get('vars', None),
+                            expand_vars, 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', '<unknown>')))
 
-      defined_variables.add(target.id)
-      return target.id, (value, node.value)
+      local_scope.SetNode(target.id, value, node.value)
     else:
       raise ValueError(
           'unexpected AST node: %s %s (file %r, line %s)' % (
@@ -269,15 +294,8 @@ def Exec(content, filename='<unknown>'):
               getattr(node, 'lineno', '<unknown>')))
 
   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)' % (
@@ -289,6 +307,69 @@ def Exec(content, filename='<unknown>'):
   return _GCLIENT_SCHEMA.validate(local_scope)
 
 
+def Parse(content, expand_vars, validate_syntax, filename, vars_override=None):
+  """Parses DEPS strings.
+
+  Executes the Python-like string stored in content, resulting in a Python
+  dictionary specifyied by the schema above. Supports syntax validation and
+  variable expansion.
+
+  Args:
+    content: str. DEPS file stored as a string.
+    expand_vars: bool. Whether variables should be expanded to their values.
+    validate_syntax: bool. Whether syntax should be validated using the schema
+      defined above.
+    filename: str. The name of the DEPS file, or a string describing the source
+      of the content, e.g. '<string>', '<unknown>'.
+    vars_override: dict, optional. A dictionary with overrides for the variables
+      defined by the DEPS file.
+
+  Returns:
+    A Python dict with the parsed contents of the DEPS file, as specified by the
+    schema above.
+  """
+  # TODO(ehmaldonado): Make validate_syntax = True the only case
+  if validate_syntax:
+    return Exec(content, expand_vars, filename)
+
+  vars_dict = {}
+  def _DeepFormat(node):
+    if isinstance(node, basestring):
+      return node.format(**vars_dict)
+    elif isinstance(node, dict):
+      return {
+          k.format(**vars_dict): _DeepFormat(v)
+          for k, v in node.iteritems()
+      }
+    elif isinstance(node, list):
+      return [
+          _DeepFormat(elem)
+          for elem in node
+      ]
+    elif isinstance(node, tuple):
+      return tuple(
+          _DeepFormat(elem)
+          for elem in node
+      )
+    else:
+      return node
+
+  local_scope = {}
+  global_scope = {'Var': lambda var_name: '{%s}' % var_name}
+  exec(content, global_scope, local_scope)
+
+  if 'vars' not in local_scope or not expand_vars:
+    return local_scope
+
+  vars_dict.update(local_scope['vars'])
+  vars_override = vars_override or {}
+  for var, value in vars_override.iteritems():
+    if var in vars_dict:
+      vars_dict[var] = value
+
+  return _DeepFormat(local_scope)
+
+
 def EvaluateCondition(condition, variables, referenced_variables=None):
   """Safely evaluates a boolean condition. Returns the result."""
   if not referenced_variables:
@@ -416,7 +497,7 @@ def SetVar(gclient_dict, var_name, value):
         "The vars entry for %s has no formatting information." % var_name)
 
   _UpdateAstString(tokens, node, value)
-  gclient_dict['vars']._SetNode(var_name, value, node)
+  gclient_dict['vars'].SetNode(var_name, value, node)
 
 
 def SetCIPD(gclient_dict, dep_name, package_name, new_version):
@@ -450,7 +531,7 @@ def SetCIPD(gclient_dict, dep_name, package_name, new_version):
 
   new_version = 'version:' + new_version
   _UpdateAstString(tokens, node, new_version)
-  packages[0]._SetNode('version', new_version, node)
+  packages[0].SetNode('version', new_version, node)
 
 
 def SetRevision(gclient_dict, dep_name, new_revision):
@@ -477,8 +558,9 @@ def SetRevision(gclient_dict, dep_name, new_revision):
       SetVar(gclient_dict, node.args[0].s, new_revision)
     else:
       _UpdateAstString(tokens, node, new_revision)
-      value = _gclient_eval(dep_node)
-      dep_dict._SetNode(dep_key, value, dep_node)
+      value = _gclient_eval(dep_node, gclient_dict.get('vars', None),
+                            expand_vars=True, filename='<unknown>')
+      dep_dict.SetNode(dep_key, value, dep_node)
 
   if isinstance(gclient_dict['deps'][dep_name], _NodeDict):
     _UpdateRevision(gclient_dict['deps'][dep_name], 'url')

tests/gclient_eval_unittest.py

@@ -20,6 +20,17 @@ import gclient_eval
 
 
 _SAMPLE_DEPS_FILE = textwrap.dedent("""\
+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',
+}
+
 deps = {
     'src/dep': Var('git_repo') + '/dep' + '@' + 'deadbeef',
     # Some comment
@@ -45,59 +56,82 @@ deps = {
         '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"'))
+    self.assertEqual(
+        'foo',
+        gclient_eval._gclient_eval('"foo"', vars_dict=None, expand_vars=False,
+                                   filename='<unknown>'))
 
   def test_tuple(self):
-    self.assertEqual(('a', 'b'), gclient_eval._gclient_eval('("a", "b")'))
+    self.assertEqual(
+        ('a', 'b'),
+        gclient_eval._gclient_eval('("a", "b")', vars_dict=None,
+                                   expand_vars=False, filename='<unknown>'))
 
   def test_list(self):
-    self.assertEqual(['a', 'b'], gclient_eval._gclient_eval('["a", "b"]'))
+    self.assertEqual(
+        ['a', 'b'],
+        gclient_eval._gclient_eval('["a", "b"]', vars_dict=None,
+                                   expand_vars=False, filename='<unknown>'))
 
   def test_dict(self):
-    self.assertEqual({'a': 'b'}, gclient_eval._gclient_eval('{"a": "b"}'))
+    self.assertEqual(
+        {'a': 'b'},
+        gclient_eval._gclient_eval('{"a": "b"}', vars_dict=None,
+                                   expand_vars=False, filename='<unknown>'))
 
   def test_name_safe(self):
-    self.assertEqual(True, gclient_eval._gclient_eval('True'))
+    self.assertEqual(
+        True,
+        gclient_eval._gclient_eval('True', vars_dict=None,
+                                   expand_vars=False, filename='<unknown>'))
 
   def test_name_unsafe(self):
     with self.assertRaises(ValueError) as cm:
-      gclient_eval._gclient_eval('UnsafeName')
+      gclient_eval._gclient_eval('UnsafeName', vars_dict=None,
+                                 expand_vars=False, filename='<unknown>')
     self.assertIn('invalid name \'UnsafeName\'', str(cm.exception))
 
+  def test_invalid_call(self):
+    with self.assertRaises(ValueError) as cm:
+      gclient_eval._gclient_eval('Foo("bar")', vars_dict=None,
+                                 expand_vars=False, filename='<unknown>')
+    self.assertIn('Var is the only allowed function', str(cm.exception))
+
   def test_call(self):
     self.assertEqual(
         '{bar}',
-        gclient_eval._gclient_eval('Var("bar")'))
+        gclient_eval._gclient_eval('Var("bar")', vars_dict=None,
+                                   expand_vars=False, filename='<unknown>'))
 
   def test_plus(self):
-    self.assertEqual('foo', gclient_eval._gclient_eval('"f" + "o" + "o"'))
+    self.assertEqual(
+        'foo',
+        gclient_eval._gclient_eval('"f" + "o" + "o"', vars_dict=None,
+                                   expand_vars=False, filename='<unknown>'))
 
   def test_format(self):
-    self.assertEqual('foo', gclient_eval._gclient_eval('"%s" % "foo"'))
+    self.assertEqual(
+        'foo',
+        gclient_eval._gclient_eval('"%s" % "foo"', vars_dict=None,
+                                   expand_vars=False, filename='<unknown>'))
 
   def test_not_expression(self):
     with self.assertRaises(SyntaxError) as cm:
-      gclient_eval._gclient_eval('def foo():\n  pass')
+      gclient_eval._gclient_eval(
+          'def foo():\n  pass', vars_dict=None, expand_vars=False,
+          filename='<unknown>')
     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]]')
+      gclient_eval._gclient_eval(
+          '[x for x in [1, 2, 3]]', vars_dict=None, expand_vars=False,
+          filename='<unknown>')
     self.assertIn(
         'unexpected AST node: <_ast.ListComp object', str(cm.exception))
 
@@ -105,7 +139,9 @@ class GClientEvalTest(unittest.TestCase):
     for test_case in itertools.permutations(range(4)):
       input_data = ['{'] + ['"%s": "%s",' % (n, n) for n in test_case] + ['}']
       expected = [(str(n), str(n)) for n in test_case]
-      result = gclient_eval._gclient_eval(''.join(input_data))
+      result = gclient_eval._gclient_eval(
+          ''.join(input_data), vars_dict=None, expand_vars=False,
+          filename='<unknown>')
       self.assertEqual(expected, result.items())
 
 
@@ -124,12 +160,11 @@ class ExecTest(unittest.TestCase):
 
   def test_schema_wrong_type(self):
     with self.assertRaises(schema.SchemaError):
-      gclient_eval.Exec('include_rules = {}', '<string>')
+      gclient_eval.Exec('include_rules = {}')
 
   def test_recursedeps_list(self):
     local_scope = gclient_eval.Exec(
-        'recursedeps = [["src/third_party/angle", "DEPS.chromium"]]',
-        '<string>')
+        'recursedeps = [["src/third_party/angle", "DEPS.chromium"]]')
     self.assertEqual(
         {'recursedeps': [['src/third_party/angle', 'DEPS.chromium']]},
         local_scope)
@@ -142,7 +177,35 @@ class ExecTest(unittest.TestCase):
         'deps = {',
         '  "a_dep": "a" + Var("foo") + "b",',
         '}',
-    ]), '<string>')
+    ]))
+    self.assertEqual({
+        'vars': collections.OrderedDict([('foo', 'bar')]),
+        'deps': collections.OrderedDict([('a_dep', 'abarb')]),
+    }, local_scope)
+
+  def test_braces_var(self):
+    local_scope = gclient_eval.Exec('\n'.join([
+        'vars = {',
+        '  "foo": "bar",',
+        '}',
+        'deps = {',
+        '  "a_dep": "a{foo}b",',
+        '}',
+    ]))
+    self.assertEqual({
+        'vars': collections.OrderedDict([('foo', 'bar')]),
+        'deps': collections.OrderedDict([('a_dep', 'abarb')]),
+    }, local_scope)
+
+  def test_var_unexpanded(self):
+    local_scope = gclient_eval.Exec('\n'.join([
+        'vars = {',
+        '  "foo": "bar",',
+        '}',
+        'deps = {',
+        '  "a_dep": "a" + Var("foo") + "b",',
+        '}',
+    ]), expand_vars=False)
     self.assertEqual({
         'vars': collections.OrderedDict([('foo', 'bar')]),
         'deps': collections.OrderedDict([('a_dep', 'a{foo}b')]),