Revert (Reland "gclient eval: Expand vars while parsing DEPS files")

This reverts commits:
  dbbf350a Fix trailing whitespace test.
  b4f1d449 Fix actual pylint errors.
  0d9ecc92 Reland "gclient eval: Expand vars while parsing DEPS files"

Reason: http://crbug.com/825724 Clank Official Build Failure - 65.0.3325.193

TBR=agable@chromium.org

Bug: 825724
Change-Id: Icef2d0ccc8a13b782a8e3d6b474b1281820947b5
Reviewed-on: https://chromium-review.googlesource.com/980334
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@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, expand_vars=True):
+  def ParseDepsFile(self):
     """Parses the DEPS file for this dependency."""
     assert not self.deps_parsed
     assert not self.dependencies
@@ -765,15 +765,15 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     local_scope = {}
     if deps_content:
+      # Eval the content.
       try:
-        vars_override = {}
-        if self.parent:
-          vars_override = self.parent.get_vars()
-        vars_override.update(self.get_vars())
-        local_scope = gclient_eval.Parse(
-            deps_content, expand_vars,
-            self._get_option('validate_syntax', False),
-            filepath, vars_override)
+        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)
       except SyntaxError as e:
         gclient_utils.SyntaxErrorToError(filepath, e)
 
@@ -988,7 +988,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
           file_list[i] = file_list[i][1:]
 
     # Always parse the DEPS file.
-    self.ParseDepsFile(expand_vars=(command != 'flatten'))
+    self.ParseDepsFile()
     self._run_is_done(file_list or [], parsed_url)
     if command in ('update', 'revert') and not options.noprehooks:
       self.RunPreDepsHooks()
@@ -1864,7 +1864,7 @@ it or fix the checkout.
           print('%s: %s' % (x, entries[x]))
     logging.info(str(self))
 
-  def ParseDepsFile(self, expand_vars=None):
+  def ParseDepsFile(self):
     """No DEPS to parse for a .gclient file."""
     raise gclient_utils.Error('Internal error')
 
@@ -1973,7 +1973,7 @@ class CipdDependency(Dependency):
       self._cipd_package = self._cipd_root.add_package(
           self._cipd_subdir, self._package_name, self._package_version)
 
-  def ParseDepsFile(self, expand_vars=None):
+  def ParseDepsFile(self):
     """CIPD dependencies are not currently allowed to have nested deps."""
     self.add_dependencies_and_close([], [])
 
@@ -2930,9 +2930,7 @@ 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.Parse(
-      contents, expand_vars=True, validate_syntax=True,
-      filename=options.deps_file)
+  local_scope = gclient_eval.Exec(contents)
 
   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,7 +183,7 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({
 }))
 
 
-def _gclient_eval(node_or_string, vars_dict, expand_vars, filename):
+def _gclient_eval(node_or_string, filename='<unknown>'):
   """Safely evaluates a single expression. Returns the result."""
   _allowed_names = {'None': None, 'True': True, 'False': False}
   if isinstance(node_or_string, basestring):
@@ -192,15 +192,7 @@ def _gclient_eval(node_or_string, vars_dict, expand_vars, filename):
     node_or_string = node_or_string.body
   def _convert(node):
     if isinstance(node, ast.Str):
-      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>')))
+      return node.s
     elif isinstance(node, ast.Num):
       return node.n
     elif isinstance(node, ast.Tuple):
@@ -230,18 +222,7 @@ def _gclient_eval(node_or_string, vars_dict, expand_vars, filename):
         raise ValueError(
             'Var\'s argument must be a variable name (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
-      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]
+      return '{%s}' % 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):
@@ -254,19 +235,13 @@ def _gclient_eval(node_or_string, vars_dict, expand_vars, filename):
   return _convert(node_or_string)
 
 
-def Exec(content, expand_vars, filename='<unknown>', vars_override=None):
-  """Safely execs a set of assignments."""
+def Exec(content, filename='<unknown>'):
+  """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
 
-  tokens = {
-      token[2]: list(token)
-      for token in tokenize.generate_tokens(
-          cStringIO.StringIO(content).readline)
-  }
-  local_scope = _NodeDict({}, tokens)
-  vars_dict = {}
+  defined_variables = set()
   def _visit_in_module(node):
     if isinstance(node, ast.Assign):
       if len(node.targets) != 1:
@@ -278,22 +253,15 @@ def Exec(content, expand_vars, filename='<unknown>', vars_override=None):
         raise ValueError(
             'invalid assignment: target should be a name (file %r, line %s)' % (
                 filename, getattr(node, 'lineno', '<unknown>')))
-      value = _gclient_eval(node.value, vars_dict, expand_vars, filename)
+      value = _gclient_eval(node.value, filename=filename)
 
-      if target.id in local_scope:
+      if target.id in defined_variables:
         raise ValueError(
             'invalid assignment: overrides var %r (file %r, line %s)' % (
                 target.id, filename, getattr(node, 'lineno', '<unknown>')))
 
-      if target.id == 'vars':
-        vars_dict.update(value)
-        if vars_override:
-          vars_dict.update({
-            k: v
-            for k, v in vars_override.iteritems()
-            if k in vars_dict})
-
-      local_scope.SetNode(target.id, value, node.value)
+      defined_variables.add(target.id)
+      return target.id, (value, node.value)
     else:
       raise ValueError(
           'unexpected AST node: %s %s (file %r, line %s)' % (
@@ -301,8 +269,15 @@ def Exec(content, expand_vars, filename='<unknown>', vars_override=None):
               getattr(node, 'lineno', '<unknown>')))
 
   if isinstance(node_or_string, ast.Module):
+    data = []
     for stmt in node_or_string.body:
-      _visit_in_module(stmt)
+      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)
   else:
     raise ValueError(
         'unexpected AST node: %s %s (file %r, line %s)' % (
@@ -314,70 +289,6 @@ def Exec(content, expand_vars, filename='<unknown>', vars_override=None):
   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_override)
-
-  local_scope = {}
-  global_scope = {'Var': lambda var_name: '{%s}' % var_name}
-
-  # If we use 'exec' directly, it complains that 'Parse' contains a nested
-  # function with free variables.
-  # This is because on versions of Python < 2.7.9, "exec(a, b, c)" not the same
-  # as "exec a in b, c" (See https://bugs.python.org/issue21591).
-  eval(compile(content, filename, 'exec'), global_scope, local_scope)
-
-  if 'vars' not in local_scope or not expand_vars:
-    return local_scope
-
-  vars_dict = {}
-  vars_dict.update(local_scope['vars'])
-  if vars_override:
-    vars_dict.update({
-        k: v
-        for k, v in vars_override.iteritems()
-        if k in 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
-
-  return _DeepFormat(local_scope)
-
-
 def EvaluateCondition(condition, variables, referenced_variables=None):
   """Safely evaluates a boolean condition. Returns the result."""
   if not referenced_variables:
@@ -505,7 +416,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):
@@ -539,7 +450,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):
@@ -566,9 +477,8 @@ 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, gclient_dict.get('vars', None),
-                            expand_vars=True, filename='<unknown>')
-      dep_dict.SetNode(dep_key, value, dep_node)
+      value = _gclient_eval(dep_node)
+      dep_dict._SetNode(dep_key, value, dep_node)
 
   if isinstance(gclient_dict['deps'][dep_name], _NodeDict):
     _UpdateRevision(gclient_dict['deps'][dep_name], 'url')