Revert "Reland "Expand variables in gclient flattened output.""

This reverts commit ff622244.

Reason for revert: spec revert for crbug.com/849374

Original change's description:
> Reland "Expand variables in gclient flattened output."
> 
> This is a reland of a32f98e6
> 
> Original change's description:
> > Expand variables in gclient flattened output.
> >
> > Bug: 848990
> > Change-Id: I0ad7e4f965973edbc5a335bd30f9cbd7b04abff2
> > Reviewed-on: https://chromium-review.googlesource.com/1085996
> > Reviewed-by: Michael Moss <mmoss@chromium.org>
> > Reviewed-by: Aaron Gable <agable@chromium.org>
> > Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> 
> Tbr: agable@chromium.org
> Bug: 848990
> Change-Id: I7843544b79b2ab7e2046c187d13ea3eb65fc1b7d
> Reviewed-on: https://chromium-review.googlesource.com/1085975
> Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

TBR=agable@chromium.org,mmoss@chromium.org,ehmaldonado@chromium.org

Change-Id: I89ffb80893be72f60043e8a6e02f6c0fb6fe5cc1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 848990
Reviewed-on: https://chromium-review.googlesource.com/1086747Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>

gclient.py

@@ -207,7 +207,7 @@ class Hook(object):
         not gclient_eval.EvaluateCondition(self._condition, self._variables)):
       return
 
-    cmd = [arg for arg in self._action]
+    cmd = [arg.format(**self._variables) for arg in self._action]
 
     if cmd[0] == 'python':
       # If the hook specified "python" as the first item, the action is a
@@ -240,11 +240,12 @@ class Hook(object):
 class DependencySettings(object):
   """Immutable configuration settings."""
   def __init__(
-      self, parent, url, managed, custom_deps, custom_vars,
+      self, parent, raw_url, url, managed, custom_deps, custom_vars,
       custom_hooks, deps_file, should_process, relative, condition):
     # 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
@@ -322,6 +323,11 @@ 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."""
@@ -345,6 +351,9 @@ class DependencySettings(object):
   def set_url(self, url):
     self._url = url
 
+  def set_raw_url(self, url):
+    self._raw_url = url
+
   def get_custom_deps(self, name, url):
     """Returns a custom deps if applicable."""
     if self.parent:
@@ -356,12 +365,12 @@ class DependencySettings(object):
 class Dependency(gclient_utils.WorkItem, DependencySettings):
   """Object that represents a dependency checkout."""
 
-  def __init__(self, parent, name, url, managed, custom_deps,
+  def __init__(self, parent, name, raw_url, url, managed, custom_deps,
                custom_vars, custom_hooks, deps_file, should_process,
                relative, condition, print_outbuf=False):
     gclient_utils.WorkItem.__init__(self, name)
     DependencySettings.__init__(
-        self, parent, url, managed, custom_deps, custom_vars,
+        self, parent, raw_url, url, managed, custom_deps, custom_vars,
         custom_hooks, deps_file, should_process, relative, condition)
 
     # This is in both .gclient and DEPS files:
@@ -459,15 +468,18 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       raise gclient_utils.Error('Unknown url type')
 
   def PinToActualRevision(self):
-    """Updates self.url to the revision checked out on disk."""
+    """Updates self.url and self.raw_url to the revision checked out on disk."""
     if self.url is None:
       return
-    url = None
+    url = raw_url = None
     scm = self.CreateSCM()
     if os.path.isdir(scm.checkout_path):
       revision = scm.revinfo(None, None, None)
       url = '%s@%s' % (gclient_utils.SplitUrlRevision(self.url)[0], revision)
+      raw_url = '%s@%s' % (
+          gclient_utils.SplitUrlRevision(self.raw_url)[0], revision)
     self.set_url(url)
+    self.set_raw_url(raw_url)
 
   def ToLines(self):
     s = []
@@ -476,7 +488,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     s.extend([
         '  # %s' % self.hierarchy(include_url=False),
         '  "%s": {' % (self.name,),
-        '    "url": "%s",' % (self.url,),
+        '    "url": "%s",' % (self.raw_url,),
     ] + condition_part + [
         '  },',
         '',
@@ -630,23 +642,25 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
         for package in dep_value.get('packages', []):
           if 'version' in package:
             # Matches version to vars value.
-            version = package['version']
+            raw_version = package['version']
+            version = raw_version.format(**self.get_vars())
             package['version'] = version
           deps_to_add.append(
               CipdDependency(
                   self, name, package, cipd_root, self.custom_vars,
                   should_process, use_relative_paths, condition))
       else:
-        url = dep_value.get('url')
+        raw_url = dep_value.get('url')
+        url = raw_url.format(**self.get_vars()) if raw_url else None
         deps_to_add.append(
             GitDependency(
-                self, name, url, None, None, self.custom_vars, None,
+                self, name, raw_url, url, None, None, self.custom_vars, None,
                 deps_file, should_process, use_relative_paths, condition))
 
     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
@@ -677,7 +691,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     if deps_content:
       try:
         local_scope = gclient_eval.Parse(
-            deps_content, self._get_option('validate_syntax', False),
+            deps_content, expand_vars,
+            self._get_option('validate_syntax', False),
             filepath, self.get_vars())
       except SyntaxError as e:
         gclient_utils.SyntaxErrorToError(filepath, e)
@@ -900,7 +915,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
           file_list[i] = file_list[i][1:]
 
     if self.recursion_limit:
-      self.ParseDepsFile()
+      self.ParseDepsFile(expand_vars=(command != 'flatten'))
 
     self._run_is_done(file_list or [])
 
@@ -1279,7 +1294,7 @@ solutions = %(solution_list)s
     # Do not change previous behavior. Only solution level and immediate DEPS
     # are processed.
     self._recursion_limit = 2
-    GitDependency.__init__(self, None, None, None, True, None, None, None,
+    GitDependency.__init__(self, None, None, None, None, True, None, None, None,
                            'unused', True, None, None, True)
     self._options = options
     if options.deps_os:
@@ -1371,7 +1386,7 @@ it or fix the checkout.
     for s in config_dict.get('solutions', []):
       try:
         deps_to_add.append(GitDependency(
-            self, s['name'], s['url'],
+            self, s['name'], s['url'], s['url'],
             s.get('managed', True),
             s.get('custom_deps', {}),
             s.get('custom_vars', {}),
@@ -1738,7 +1753,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')
 
@@ -1831,7 +1846,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([], [])
 
@@ -2181,7 +2196,7 @@ def _DepsOsToLines(deps_os):
       s.extend([
           '    # %s' % dep.hierarchy(include_url=False),
           '    "%s": {' % (name,),
-          '      "url": "%s",' % (dep.url,),
+          '      "url": "%s",' % (dep.raw_url,),
       ] + condition_part + [
           '    },',
           '',
@@ -2713,7 +2728,8 @@ def CMDgetdep(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, options.deps_file)
+  local_scope = gclient_eval.Exec(
+      contents, expand_vars=True, filename=options.deps_file)
 
   for var in options.vars:
     print(gclient_eval.GetVar(local_scope, var))
@@ -2762,7 +2778,8 @@ 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, options.deps_file)
+  local_scope = gclient_eval.Exec(
+      contents, expand_vars=True, filename=options.deps_file)
 
   for var in options.vars:
     name, _, value = var.partition('=')

gclient_eval.py

@@ -206,7 +206,8 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({
 }))
 
 
-def _gclient_eval(node_or_string, filename='<unknown>', vars_dict=None):
+def _gclient_eval(node_or_string, vars_dict=None, expand_vars=False,
+                  filename='<unknown>'):
   """Safely evaluates a single expression. Returns the result."""
   _allowed_names = {'None': None, 'True': True, 'False': False}
   if isinstance(node_or_string, basestring):
@@ -215,12 +216,12 @@ def _gclient_eval(node_or_string, filename='<unknown>', vars_dict=None):
     node_or_string = node_or_string.body
   def _convert(node):
     if isinstance(node, ast.Str):
-      if vars_dict is None:
+      if not expand_vars:
         return node.s
       try:
         return node.s.format(**vars_dict)
       except KeyError as e:
-        raise KeyError(
+        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>')))
@@ -253,10 +254,14 @@ def _gclient_eval(node_or_string, filename='<unknown>', vars_dict=None):
         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:
-        return '{' + arg + '}'
+        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 KeyError(
+        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>')))
@@ -273,7 +278,7 @@ def _gclient_eval(node_or_string, filename='<unknown>', vars_dict=None):
   return _convert(node_or_string)
 
 
-def Exec(content, filename='<unknown>', vars_override=None):
+def Exec(content, expand_vars=True, filename='<unknown>', vars_override=None):
   """Safely execs a set of assignments."""
   def _validate_statement(node, local_scope):
     if not isinstance(node, ast.Assign):
@@ -325,7 +330,7 @@ def Exec(content, filename='<unknown>', vars_override=None):
   vars_dict = {}
   if 'vars' in statements:
     vars_statement = statements['vars']
-    value = _gclient_eval(vars_statement, filename)
+    value = _gclient_eval(vars_statement, None, False, filename)
     local_scope.SetNode('vars', value, vars_statement)
     # Update the parsed vars with the overrides, but only if they are already
     # present (overrides do not introduce new variables).
@@ -337,13 +342,14 @@ def Exec(content, filename='<unknown>', vars_override=None):
         if k in vars_dict})
 
   for name, node in statements.iteritems():
-    value = _gclient_eval(node, filename, vars_dict)
+    value = _gclient_eval(node, vars_dict, expand_vars, filename)
     local_scope.SetNode(name, value, node)
 
   return _GCLIENT_SCHEMA.validate(local_scope)
 
 
-def ExecLegacy(content, filename='<unknown>', vars_override=None):
+def ExecLegacy(content, expand_vars=True, filename='<unknown>',
+               vars_override=None):
   """Executes a DEPS file |content| using exec."""
   local_scope = {}
   global_scope = {'Var': lambda var_name: '{%s}' % var_name}
@@ -354,7 +360,7 @@ def ExecLegacy(content, filename='<unknown>', vars_override=None):
   # 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:
+  if 'vars' not in local_scope or not expand_vars:
     return local_scope
 
   vars_dict = {}
@@ -449,7 +455,7 @@ def UpdateCondition(info_dict, op, new_condition):
     del info_dict['condition']
 
 
-def Parse(content, validate_syntax, filename, vars_override=None):
+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
@@ -458,6 +464,7 @@ def Parse(content, validate_syntax, filename, vars_override=None):
 
   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
@@ -470,9 +477,9 @@ def Parse(content, validate_syntax, filename, vars_override=None):
     schema above.
   """
   if validate_syntax:
-    result = Exec(content, filename, vars_override)
+    result = Exec(content, expand_vars, filename, vars_override)
   else:
-    result = ExecLegacy(content, filename, vars_override)
+    result = ExecLegacy(content, expand_vars, filename, vars_override)
 
   vars_dict = result.get('vars', {})
   if 'deps' in result:

recipes/trigger_recipe_roller.txt

@@ -9,4 +9,4 @@ As the CI needs of the browser grew, Batty, the Build and Test Yeti, got
 a new friend:
 
 
-The End.
+The End!

roll_dep.py

@@ -225,7 +225,7 @@ def main():
     # First gather all the information without modifying anything, except for a
     # git fetch.
     deps_path, deps_content = get_deps(current_dir)
-    gclient_dict = gclient_eval.Exec(deps_content, deps_path)
+    gclient_dict = gclient_eval.Exec(deps_content, True, deps_path)
     is_relative = gclient_dict.get('use_relative_paths', False)
     root_dir = current_dir if is_relative else gclient_root
     rolls = {}

tests/gclient_eval_unittest.py

@@ -44,19 +44,22 @@ class GClientEvalTest(unittest.TestCase):
       gclient_eval._gclient_eval('Foo("bar")')
     self.assertIn('Var is the only allowed function', str(cm.exception))
 
+  def test_call(self):
+    self.assertEqual('{bar}', gclient_eval._gclient_eval('Var("bar")'))
+
   def test_expands_vars(self):
     self.assertEqual(
         'foo',
-        gclient_eval._gclient_eval('Var("bar")', vars_dict={'bar': 'foo'}))
+        gclient_eval._gclient_eval('Var("bar")', {'bar': 'foo'}, True))
 
   def test_expands_vars_with_braces(self):
     self.assertEqual(
         'foo',
-        gclient_eval._gclient_eval('"{bar}"', vars_dict={'bar': 'foo'}))
+        gclient_eval._gclient_eval('"{bar}"', {'bar': 'foo'}, True))
 
   def test_invalid_var(self):
-    with self.assertRaises(KeyError) as cm:
-      gclient_eval._gclient_eval('"{bar}"', vars_dict={})
+    with self.assertRaises(ValueError) as cm:
+      gclient_eval._gclient_eval('"{bar}"', {}, True)
     self.assertIn('bar was used as a variable, but was not declared',
                   str(cm.exception))
 
@@ -137,6 +140,20 @@ class ExecTest(unittest.TestCase):
         '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",',
+        '}',
+    ]), False)
+    self.assertEqual({
+        'vars': collections.OrderedDict([('foo', 'bar')]),
+        'deps': collections.OrderedDict([('a_dep', 'a{foo}b')]),
+    }, local_scope)
+
   def test_empty_deps(self):
     local_scope = gclient_eval.Exec('deps = {}')
     self.assertEqual({'deps': {}}, local_scope)
@@ -149,14 +166,14 @@ class ExecTest(unittest.TestCase):
         'deps = {',
         '  "a_dep": "a{foo}b",',
         '}',
-    ]), vars_override={'foo': 'baz'})
+    ]), True, vars_override={'foo': 'baz'})
     self.assertEqual({
         'vars': collections.OrderedDict([('foo', 'bar')]),
         'deps': collections.OrderedDict([('a_dep', 'abazb')]),
     }, local_scope)
 
   def test_doesnt_override_undeclared_vars(self):
-    with self.assertRaises(KeyError) as cm:
+    with self.assertRaises(ValueError) as cm:
       gclient_eval.Exec('\n'.join([
           'vars = {',
           '  "foo": "bar",',
@@ -164,7 +181,7 @@ class ExecTest(unittest.TestCase):
           'deps = {',
           '  "a_dep": "a{baz}b",',
           '}',
-      ]), vars_override={'baz': 'lalala'})
+      ]), True, vars_override={'baz': 'lalala'})
     self.assertIn('baz was used as a variable, but was not declared',
                   str(cm.exception))
 
@@ -584,7 +601,8 @@ class RevisionTest(unittest.TestCase):
 
 
 class ParseTest(unittest.TestCase):
-  def callParse(self, validate_syntax=True, vars_override=None):
+  def callParse(self, expand_vars=True, validate_syntax=True,
+                vars_override=None):
     return gclient_eval.Parse('\n'.join([
         'vars = {',
         '  "foo": "bar",',
@@ -592,32 +610,7 @@ class ParseTest(unittest.TestCase):
         'deps = {',
         '  "a_dep": "a{foo}b",',
         '}',
-    ]), validate_syntax, '<unknown>', vars_override)
-
-  def test_supports_vars_inside_vars(self):
-    deps_file = '\n'.join([
-        'vars = {',
-        '  "foo": "bar",',
-        '  "baz": "\\"{foo}\\" == \\"bar\\"",',
-        '}',
-        'deps = {',
-        '  "src/baz": {',
-        '    "url": "baz_url",',
-        '    "condition": "baz",',
-        '  },',
-        '}',
-    ])
-    for validate_syntax in False, True:
-      local_scope = gclient_eval.Parse(
-          deps_file, validate_syntax, '<unknown>', None)
-      self.assertEqual({
-          'vars': {'foo': 'bar',
-                   'baz': '"bar" == "bar"'},
-          'deps': {'src/baz': {'url': 'baz_url',
-                               'dep_type': 'git',
-                               'condition': 'baz'}},
-      }, local_scope)
-
+    ]), expand_vars, validate_syntax, '<unknown>', vars_override)
 
   def test_expands_vars(self):
     for validate_syntax in True, False:
@@ -628,6 +621,16 @@ class ParseTest(unittest.TestCase):
                              'dep_type': 'git'}},
       }, local_scope)
 
+  def test_no_expands_vars(self):
+    for validate_syntax in True, False:
+      local_scope = self.callParse(False,
+                                   validate_syntax=validate_syntax)
+      self.assertEqual({
+          'vars': {'foo': 'bar'},
+          'deps': {'a_dep': {'url': 'a{foo}b',
+                             'dep_type': 'git'}},
+      }, local_scope)
+
   def test_overrides_vars(self):
     for validate_syntax in True, False:
       local_scope = self.callParse(validate_syntax=validate_syntax,
@@ -648,15 +651,17 @@ class ParseTest(unittest.TestCase):
         '}',
     ])
 
-    with self.assertRaises(KeyError) as cm:
+    with self.assertRaises(ValueError) as cm:
       gclient_eval.Parse(
-          deps_file, True, '<unknown>', {'baz': 'lalala'})
+          deps_file, True, True,
+          '<unknown>', {'baz': 'lalala'})
     self.assertIn('baz was used as a variable, but was not declared',
                   str(cm.exception))
 
     with self.assertRaises(KeyError) as cm:
       gclient_eval.Parse(
-          deps_file, False, '<unknown>', {'baz': 'lalala'})
+          deps_file, True, False,
+          '<unknown>', {'baz': 'lalala'})
     self.assertIn('baz', str(cm.exception))
 
   def test_standardizes_deps_string_dep(self):
@@ -665,7 +670,7 @@ class ParseTest(unittest.TestCase):
         'deps = {',
         '  "a_dep": "a_url@a_rev",',
         '}',
-      ]), validate_syntax, '<unknown>')
+      ]), False, validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git'}},
@@ -680,7 +685,7 @@ class ParseTest(unittest.TestCase):
         '     "condition": "checkout_android",',
         '  },',
         '}',
-      ]), validate_syntax, '<unknown>')
+      ]), False, validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git',
@@ -698,7 +703,7 @@ class ParseTest(unittest.TestCase):
         '     "a_dep": None,',
         '  },',
         '}',
-      ]), validate_syntax, '<unknown>')
+      ]), False, validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git'}},
@@ -715,7 +720,7 @@ class ParseTest(unittest.TestCase):
         '     "b_dep": "b_url@b_rev"',
         '  },',
         '}',
-      ]), validate_syntax, '<unknown>')
+      ]), False, validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git'},
@@ -735,7 +740,7 @@ class ParseTest(unittest.TestCase):
         '     "a_dep": "a_url@a_rev"',
         '  },',
         '}',
-      ]), validate_syntax, '<unknown>')
+      ]), False, validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git'}},
@@ -755,7 +760,7 @@ class ParseTest(unittest.TestCase):
         '     "a_dep": "a_url@a_rev"',
         '  },',
         '}',
-      ]), validate_syntax, '<unknown>')
+      ]), False, validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {
               'a_dep': {'url': 'a_url@a_rev',
@@ -775,7 +780,7 @@ class ParseTest(unittest.TestCase):
         '     "a_dep": "a_url@a_rev"',
         '  },',
         '}',
-      ]), validate_syntax, '<unknown>')
+      ]), False, validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {
               'a_dep': {'url': 'a_url@a_rev',
@@ -799,7 +804,7 @@ class ParseTest(unittest.TestCase):
           '     "a_dep": "a_url@b_rev"',
           '  },',
           '}',
-        ]), validate_syntax, '<unknown>')
+        ]), False, validate_syntax, '<unknown>')
       self.assertIn('conflicts with existing deps', str(cm.exception))
 
   def test_merges_hooks_os(self):
@@ -817,7 +822,7 @@ class ParseTest(unittest.TestCase):
         '    },',
         '  ]',
         '}',
-      ]), validate_syntax, '<unknown>')
+      ]), False, validate_syntax, '<unknown>')
       self.assertEqual({
           "hooks": [{"action": ["a", "action"]},
                     {"action": ["b", "action"], "condition": "checkout_mac"}],

tests/gclient_smoketest.py

@@ -839,15 +839,13 @@ class GClientSmokeGIT(GClientSmokeBase):
           '}',
       ]))
 
-    results = self.gclient([
+    self.gclient([
         'setdep', '-r', 'foo@new_foo', '-r', 'bar@new_bar',
         '--var', 'foo_var=new_val', '--deps-file', fake_deps])
 
     with open(fake_deps) as f:
       contents = f.read().splitlines()
 
-    self.assertEqual('', results[1])
-    self.assertEqual(0, results[2])
     self.assertEqual([
           'vars = { ',
           '  "foo_var": "new_val",',
@@ -881,13 +879,11 @@ class GClientSmokeGIT(GClientSmokeBase):
         'getdep', '-r', 'foo', '-r', 'bar','--var', 'foo_var',
         '--deps-file', fake_deps])
 
-    self.assertEqual('', results[1])
     self.assertEqual([
         'foo_val',
         'foo_rev',
         'bar_rev',
     ], results[0].splitlines())
-    self.assertEqual(0, results[2])
 
   def testFlatten(self):
     if not self.enabled:
@@ -932,7 +928,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         'deps = {',
         '  # src -> src/repo2 -> foo/bar',
         '  "foo/bar": {',
-        '    "url": "' + self.git_base + 'repo_3",',
+        '    "url": "/repo_3",',
         '    "condition": \'(repo2_false_var) and (true_str_var)\',',
         '  },',
         '',
@@ -943,43 +939,43 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '  # src -> src/mac_repo',
         '  "src/mac_repo": {',
-        '    "url": "' + self.git_base + 'repo_5",',
+        '    "url": "{repo5_var}",',
         '    "condition": \'checkout_mac\',',
         '  },',
         '',
         '  # src -> src/repo8 -> src/recursed_os_repo',
         '  "src/recursed_os_repo": {',
-        '    "url": "' + self.git_base + 'repo_5",',
+        '    "url": "/repo_5",',
         '    "condition": \'(checkout_linux) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo2',
         '  "src/repo2": {',
-        '    "url": "' + self.git_base + 'repo_2@%s",' % (
+        '    "url": "{git_base}repo_2@%s",' % (
                  self.githash('repo_2', 1)[:7]),
         '    "condition": \'true_str_var\',',
         '  },',
         '',
         '  # src -> src/repo4',
         '  "src/repo4": {',
-        '    "url": "' + self.git_base + 'repo_4",',
+        '    "url": "/repo_4",',
         '    "condition": \'False\',',
         '  },',
         '',
         '  # src -> src/repo8',
         '  "src/repo8": {',
-        '    "url": "' + self.git_base + 'repo_8",',
+        '    "url": "/repo_8",',
         '  },',
         '',
         '  # src -> src/unix_repo',
         '  "src/unix_repo": {',
-        '    "url": "' + self.git_base + 'repo_5",',
+        '    "url": "{repo5_var}",',
         '    "condition": \'checkout_linux\',',
         '  },',
         '',
         '  # src -> src/win_repo',
         '  "src/win_repo": {',
-        '    "url": "' + self.git_base + 'repo_5",',
+        '    "url": "{repo5_var}",',
         '    "condition": \'checkout_win\',',
         '  },',
         '',
@@ -995,7 +991,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '        "python",',
         '        "-c",',
         '        "open(\'src/git_hooked1\', \'w\')'
-            '.write(\'git_hooked1\')",',
+            '.write(\'{hook1_contents}\')",',
         '    ]',
         '  },',
         '',
@@ -1093,8 +1089,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         'deps = {',
         '  # src -> src/repo2 -> foo/bar',
         '  "foo/bar": {',
-        '    "url": "' + self.git_base + 'repo_3@%s",' % (
-                self.githash('repo_3', 2)),
+        '    "url": "/repo_3@%s",' % (self.githash('repo_3', 2)),
         '    "condition": \'(repo2_false_var) and (true_str_var)\',',
         '  },',
         '',
@@ -1106,49 +1101,43 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '  # src -> src/mac_repo',
         '  "src/mac_repo": {',
-        '    "url": "' + self.git_base + 'repo_5@%s",' % (
-                self.githash('repo_5', 3)),
+        '    "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)),
         '    "condition": \'checkout_mac\',',
         '  },',
         '',
         '  # src -> src/repo8 -> src/recursed_os_repo',
         '  "src/recursed_os_repo": {',
-        '    "url": "' + self.git_base + 'repo_5@%s",' % (
-                self.githash('repo_5', 3)),
+        '    "url": "/repo_5@%s",' % (self.githash('repo_5', 3)),
         '    "condition": \'(checkout_linux) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo2',
         '  "src/repo2": {',
-        '    "url": "' + self.git_base + 'repo_2@%s",' % (
+        '    "url": "{git_base}repo_2@%s",' % (
                  self.githash('repo_2', 1)),
         '    "condition": \'true_str_var\',',
         '  },',
         '',
         '  # src -> src/repo4',
         '  "src/repo4": {',
-        '    "url": "' + self.git_base + 'repo_4@%s",' % (
-                self.githash('repo_4', 2)),
+        '    "url": "/repo_4@%s",' % (self.githash('repo_4', 2)),
         '    "condition": \'False\',',
         '  },',
         '',
         '  # src -> src/repo8',
         '  "src/repo8": {',
-        '    "url": "' + self.git_base + 'repo_8@%s",' % (
-                self.githash('repo_8', 1)),
+        '    "url": "/repo_8@%s",' % (self.githash('repo_8', 1)),
         '  },',
         '',
         '  # src -> src/unix_repo',
         '  "src/unix_repo": {',
-        '    "url": "' + self.git_base + 'repo_5@%s",' % (
-                self.githash('repo_5', 3)),
+        '    "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)),
         '    "condition": \'checkout_linux\',',
         '  },',
         '',
         '  # src -> src/win_repo',
         '  "src/win_repo": {',
-        '    "url": "' + self.git_base + 'repo_5@%s",' % (
-                self.githash('repo_5', 3)),
+        '    "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)),
         '    "condition": \'checkout_win\',',
         '  },',
         '',
@@ -1164,7 +1153,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '        "python",',
         '        "-c",',
         '        "open(\'src/git_hooked1\', \'w\')'
-            '.write(\'git_hooked1\')",',
+            '.write(\'{hook1_contents}\')",',
         '    ]',
         '  },',
         '',
@@ -1267,46 +1256,46 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '  # src -> src/repo9 -> src/repo8 -> src/recursed_os_repo',
         '  "src/recursed_os_repo": {',
-        '    "url": "' + self.git_base + 'repo_5",',
+        '    "url": "/repo_5",',
         '    "condition": \'(checkout_linux) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo11',
         '  "src/repo11": {',
-        '    "url": "' + self.git_base + 'repo_11",',
+        '    "url": "/repo_11",',
         '    "condition": \'(checkout_ios) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo11 -> src/repo12',
         '  "src/repo12": {',
-        '    "url": "' + self.git_base + 'repo_12",',
+        '    "url": "/repo_12",',
         '    "condition": \'(checkout_ios) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo9 -> src/repo4',
         '  "src/repo4": {',
-        '    "url": "' + self.git_base + 'repo_4",',
+        '    "url": "/repo_4",',
         '    "condition": \'checkout_android\',',
         '  },',
         '',
         '  # src -> src/repo6',
         '  "src/repo6": {',
-        '    "url": "' + self.git_base + 'repo_6",',
+        '    "url": "/repo_6",',
         '  },',
         '',
         '  # src -> src/repo9 -> src/repo7',
         '  "src/repo7": {',
-        '    "url": "' + self.git_base + 'repo_7",',
+        '    "url": "/repo_7",',
         '  },',
         '',
         '  # src -> src/repo9 -> src/repo8',
         '  "src/repo8": {',
-        '    "url": "' + self.git_base + 'repo_8",',
+        '    "url": "/repo_8",',
         '  },',
         '',
         '  # src -> src/repo9',
         '  "src/repo9": {',
-        '    "url": "' + self.git_base + 'repo_9",',
+        '    "url": "/repo_9",',
         '  },',
         '',
         '}',

tests/gclient_test.py

@@ -201,7 +201,7 @@ class GclientTest(trial_dir.TestCase):
     # auto-fixed.
     url = 'proto://host/path/@revision'
     d = gclient.Dependency(
-        None, 'name', url, None, None, None,
+        None, 'name', url, url, None, None, None,
         None, '', True, False, None, True)
     self.assertEquals('proto://host/path@revision', d.url)
 
@@ -212,10 +212,10 @@ class GclientTest(trial_dir.TestCase):
     obj.add_dependencies_and_close(
       [
         gclient.Dependency(
-            obj, 'foo', 'svn://example.com/foo', None,
+            obj, 'foo', 'svn://example.com/foo', 'svn://example.com/foo', None,
             None, None, None, 'DEPS', True, False, None, True),
         gclient.Dependency(
-            obj, 'bar', 'svn://example.com/bar', None,
+            obj, 'bar', 'svn://example.com/bar', 'svn://example.com/bar', None,
             None, None, None, 'DEPS', True, False, None, True),
       ],
       [])
@@ -223,7 +223,7 @@ class GclientTest(trial_dir.TestCase):
       [
         gclient.Dependency(
             obj.dependencies[0], 'foo/dir1', 'svn://example.com/foo/dir1',
-            None, None, None, None, 'DEPS', True,
+            'svn://example.com/foo/dir1', None, None, None, None, 'DEPS', True,
             False, None, True),
       ],
       [])
@@ -1130,7 +1130,7 @@ class GclientTest(trial_dir.TestCase):
     obj.add_dependencies_and_close(
       [
         gclient.Dependency(
-            obj, 'foo', 'svn://example.com/foo', None,
+            obj, 'foo', 'svn://example.com/foo', 'svn://example.com/foo', None,
             None, None, None, 'DEPS', True,
             False, None, True),
       ],