Revert "gclient flatten: preserve variable placeholders"

This reverts commit e79ddeaabf.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=748486

Original change's description:
> gclient flatten: preserve variable placeholders
> 
> One of the main use cases is making it clear which revision hashes
> need to be changed together. The way it's usually done is one variable
> referenced several times. With this CL, we preserve the references
> from original DEPS, as opposed to evaluating them and losing some info.
> 
> This CL actually makes Var() emit a variable placeholder
> instead of its value, and adds support for these placeholders
> to gclient.
> 
> One of possible next steps might be to deprecate Var().
> 
> Bug: 570091
> Change-Id: I9b13a691b5203cc284c33a59438720e31c9ebf7a
> Reviewed-on: https://chromium-review.googlesource.com/583617
> Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>

TBR=phajdan.jr@chromium.org,dpranke@chromium.org

Change-Id: If9c52ebfa78aba8041ce797ff842d09952d0e2ce
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 570091, 748486
Reviewed-on: https://chromium-review.googlesource.com/584907
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>

gclient.py

@@ -221,16 +221,32 @@ class Hook(object):
             gclient_utils.CommandToStr(cmd), elapsed_time))
 
 
-class DependencySettings(object):
+class GClientKeywords(object):
+  class VarImpl(object):
+    def __init__(self, custom_vars, local_scope):
+      self._custom_vars = custom_vars
+      self._local_scope = local_scope
+
+    def Lookup(self, var_name):
+      """Implements the Var syntax."""
+      if var_name in self._custom_vars:
+        return self._custom_vars[var_name]
+      elif var_name in self._local_scope.get("vars", {}):
+        return self._local_scope["vars"][var_name]
+      raise gclient_utils.Error("Var is not defined: %s" % var_name)
+
+
+class DependencySettings(GClientKeywords):
   """Immutable configuration settings."""
   def __init__(
-      self, parent, raw_url, url, managed, custom_deps, custom_vars,
+      self, parent, url, managed, custom_deps, custom_vars,
       custom_hooks, deps_file, should_process, relative,
       condition, condition_value):
+    GClientKeywords.__init__(self)
+
     # These are not mutable:
     self._parent = parent
     self._deps_file = deps_file
-    self._raw_url = raw_url
     self._url = url
     # The condition as string (or None). Useful to keep e.g. for flatten.
     self._condition = condition
@@ -308,14 +324,8 @@ class DependencySettings(object):
   def custom_hooks(self):
     return self._custom_hooks[:]
 
-  @property
-  def raw_url(self):
-    """URL before variable expansion."""
-    return self._raw_url
-
   @property
   def url(self):
-    """URL after variable expansion."""
     return self._url
 
   @property
@@ -344,12 +354,12 @@ class DependencySettings(object):
 class Dependency(gclient_utils.WorkItem, DependencySettings):
   """Object that represents a dependency checkout."""
 
-  def __init__(self, parent, name, raw_url, url, managed, custom_deps,
+  def __init__(self, parent, name, url, managed, custom_deps,
                custom_vars, custom_hooks, deps_file, should_process,
                relative, condition, condition_value):
     gclient_utils.WorkItem.__init__(self, name)
     DependencySettings.__init__(
-        self, parent, raw_url, url, managed, custom_deps, custom_vars,
+        self, parent, url, managed, custom_deps, custom_vars,
         custom_hooks, deps_file, should_process, relative,
         condition, condition_value)
 
@@ -626,24 +636,21 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       condition = None
       condition_value = True
       if isinstance(dep_value, basestring):
-        raw_url = dep_value
+        url = dep_value
       else:
         # This should be guaranteed by schema checking in gclient_eval.
         assert isinstance(dep_value, collections.Mapping)
-        raw_url = dep_value['url']
+        url = dep_value['url']
         # Take into account should_process metadata set by MergeWithOsDeps.
         should_process = (should_process and
                           dep_value.get('should_process', True))
         condition = dep_value.get('condition')
-
-      url = raw_url.format(**self.get_vars())
-
       if condition:
         condition_value = gclient_eval.EvaluateCondition(
             condition, self.get_vars())
         should_process = should_process and condition_value
       deps_to_add.append(Dependency(
-          self, name, raw_url, url, None, None, self.custom_vars, None,
+          self, name, url, None, None, self.custom_vars, None,
           deps_file, should_process, use_relative_paths, condition,
           condition_value))
     deps_to_add.sort(key=lambda x: x.name)
@@ -678,8 +685,10 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     local_scope = {}
     if deps_content:
+      # One thing is unintuitive, vars = {} must happen before Var() use.
+      var = self.VarImpl(self.custom_vars, local_scope)
       global_scope = {
-        'Var': lambda var_name: '{%s}' % var_name,
+        'Var': var.Lookup,
         'deps_os': {},
       }
       # Eval the content.
@@ -1194,7 +1203,7 @@ solutions = [
     # Do not change previous behavior. Only solution level and immediate DEPS
     # are processed.
     self._recursion_limit = 2
-    Dependency.__init__(self, None, None, None, None, True, None, None, None,
+    Dependency.__init__(self, None, None, None, True, None, None, None,
                         'unused', True, None, None, True)
     self._options = options
     if options.deps_os:
@@ -1277,7 +1286,7 @@ it or fix the checkout.
     for s in config_dict.get('solutions', []):
       try:
         deps_to_add.append(Dependency(
-            self, s['name'], s['url'], s['url'],
+            self, s['name'], s['url'],
             s.get('managed', True),
             s.get('custom_deps', {}),
             s.get('custom_vars', {}),
@@ -1729,7 +1738,7 @@ class Flattener(object):
           continue
         scm = gclient_scm.CreateSCM(
             dep.parsed_url, self._client.root_dir, dep.name, dep.outbuf)
-        dep._parsed_url = dep._raw_url = dep._url = '%s@%s' % (
+        dep._parsed_url = dep._url = '%s@%s' % (
             url, scm.revinfo(self._client._options, [], None))
 
     self._deps_string = '\n'.join(
@@ -1866,7 +1875,7 @@ def _DepsToLines(deps):
     s.extend([
         '  # %s' % dep.hierarchy(include_url=False),
         '  "%s": {' % (name,),
-        '    "url": "%s",' % (dep.raw_url,),
+        '    "url": "%s",' % (dep.url,),
     ] + condition_part + [
         '  },',
         '',

testing_support/fake_repos.py

@@ -458,7 +458,6 @@ pre_deps_hooks = [
       'DEPS': """
 vars = {
   'DummyVariable': 'repo',
-  'git_base': '%(git_base)s',
 }
 gclient_gn_args_file = 'src/gclient.args'
 gclient_gn_args = ['DummyVariable']
@@ -467,7 +466,7 @@ allowed_hosts = [
 ]
 deps = {
   'src/repo2': {
-    'url': Var('git_base') + 'repo_2@%(hash)s',
+    'url': '%(git_base)srepo_2@%(hash)s',
     'condition': 'True',
   },
   'src/repo4': {

tests/gclient_eval_unittest.py

@@ -103,7 +103,7 @@ class ExecTest(unittest.TestCase):
   def test_var(self):
     local_scope = {}
     global_scope = {
-        'Var': lambda var_name: '{%s}' % var_name,
+        'Var': gclient.GClientKeywords.VarImpl({}, local_scope).Lookup,
     }
     gclient_eval.Exec('\n'.join([
         'vars = {',
@@ -115,7 +115,7 @@ class ExecTest(unittest.TestCase):
     ]), global_scope, local_scope, '<string>')
     self.assertEqual({
         'vars': collections.OrderedDict([('foo', 'bar')]),
-        'deps': collections.OrderedDict([('a_dep', 'a{foo}b')]),
+        'deps': collections.OrderedDict([('a_dep', 'abarb')]),
     }, local_scope)
 
 

tests/gclient_smoketest.py

@@ -602,7 +602,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '  # src -> src/repo2',
         '  "src/repo2": {',
-        '    "url": "{git_base}repo_2@%s",' % (
+        '    "url": "git://127.0.0.1:20000/git/repo_2@%s",' % (
                  self.githash('repo_2', 1)[:7]),
         '    "condition": "True",',
         '  },',
@@ -704,9 +704,6 @@ class GClientSmokeGIT(GClientSmokeBase):
         '  # src',
         '  "DummyVariable": \'repo\',',
         '',
-        '  # src',
-        '  "git_base": \'git://127.0.0.1:20000/git/\',',
-        '',
         '}',
         '',
     ], deps_contents.splitlines())
@@ -748,7 +745,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '  # src -> src/repo2',
         '  "src/repo2": {',
-        '    "url": "{git_base}repo_2@%s",' % (
+        '    "url": "git://127.0.0.1:20000/git/repo_2@%s",' % (
                  self.githash('repo_2', 1)[:7]),
         '    "condition": "True",',
         '  },',
@@ -851,9 +848,6 @@ class GClientSmokeGIT(GClientSmokeBase):
         '  # src',
         '  "DummyVariable": \'repo\',',
         '',
-        '  # src',
-        '  "git_base": \'git://127.0.0.1:20000/git/\',',
-        '',
         '}',
         '',
     ], deps_contents.splitlines())

tests/gclient_test.py

@@ -200,9 +200,8 @@ class GclientTest(trial_dir.TestCase):
   def testAutofix(self):
     # Invalid urls causes pain when specifying requirements. Make sure it's
     # auto-fixed.
-    url = 'proto://host/path/@revision'
     d = gclient.Dependency(
-        None, 'name', url, url, None, None, None,
+        None, 'name', 'proto://host/path/@revision', None, None, None,
         None, '', True, False, None, True)
     self.assertEquals('proto://host/path@revision', d.url)
 
@@ -213,17 +212,17 @@ class GclientTest(trial_dir.TestCase):
     obj.add_dependencies_and_close(
       [
         gclient.Dependency(
-          obj, 'foo', 'raw_url', 'url', None, None, None, None, 'DEPS', True,
-          False, None, True),
+          obj, 'foo', 'url', None, None, None, None, 'DEPS', True, False,
+          None, True),
         gclient.Dependency(
-          obj, 'bar', 'raw_url', 'url', None, None, None, None, 'DEPS', True,
-          False, None, True),
+          obj, 'bar', 'url', None, None, None, None, 'DEPS', True, False,
+          None, True),
       ],
       [])
     obj.dependencies[0].add_dependencies_and_close(
       [
         gclient.Dependency(
-          obj.dependencies[0], 'foo/dir1', 'raw_url', 'url', None, None, None,
+          obj.dependencies[0], 'foo/dir1', 'url', None, None, None,
           None, 'DEPS', True, False, None, True),
       ],
       [])
@@ -621,7 +620,7 @@ class GclientTest(trial_dir.TestCase):
   def testLateOverride(self):
     """Verifies expected behavior of LateOverride."""
     url = "git@github.com:dart-lang/spark.git"
-    d = gclient.Dependency(None, 'name', 'raw_url', 'url',
+    d = gclient.Dependency(None, 'name', 'url',
                            None, None, None, None, '', True, False, None, True)
     late_url = d.LateOverride(url)
     self.assertEquals(url, late_url)