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/1085975Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

gclient.py

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

gclient_eval.py

@@ -206,8 +206,7 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({
 }))
 
 
-def _gclient_eval(node_or_string, vars_dict=None, expand_vars=False,
-                  filename='<unknown>'):
+def _gclient_eval(node_or_string, filename='<unknown>', vars_dict=None):
   """Safely evaluates a single expression. Returns the result."""
   _allowed_names = {'None': None, 'True': True, 'False': False}
   if isinstance(node_or_string, basestring):
@@ -216,12 +215,12 @@ def _gclient_eval(node_or_string, vars_dict=None, expand_vars=False,
     node_or_string = node_or_string.body
   def _convert(node):
     if isinstance(node, ast.Str):
-      if not expand_vars:
+      if vars_dict is None:
         return node.s
       try:
         return node.s.format(**vars_dict)
       except KeyError as e:
-        raise ValueError(
+        raise KeyError(
             '%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>')))
@@ -254,14 +253,10 @@ def _gclient_eval(node_or_string, vars_dict=None, expand_vars=False,
         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>')))
+        return '{' + arg + '}'
       if arg not in vars_dict:
-        raise ValueError(
+        raise KeyError(
             '%s was used as a variable, but was not declared in the vars dict '
             '(file %r, line %s)' % (
                 arg, filename, getattr(node, 'lineno', '<unknown>')))
@@ -278,7 +273,7 @@ def _gclient_eval(node_or_string, vars_dict=None, expand_vars=False,
   return _convert(node_or_string)
 
 
-def Exec(content, expand_vars=True, filename='<unknown>', vars_override=None):
+def Exec(content, filename='<unknown>', vars_override=None):
   """Safely execs a set of assignments."""
   def _validate_statement(node, local_scope):
     if not isinstance(node, ast.Assign):
@@ -330,7 +325,7 @@ def Exec(content, expand_vars=True, filename='<unknown>', vars_override=None):
   vars_dict = {}
   if 'vars' in statements:
     vars_statement = statements['vars']
-    value = _gclient_eval(vars_statement, None, False, filename)
+    value = _gclient_eval(vars_statement, 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).
@@ -342,14 +337,13 @@ def Exec(content, expand_vars=True, filename='<unknown>', vars_override=None):
         if k in vars_dict})
 
   for name, node in statements.iteritems():
-    value = _gclient_eval(node, vars_dict, expand_vars, filename)
+    value = _gclient_eval(node, filename, vars_dict)
     local_scope.SetNode(name, value, node)
 
   return _GCLIENT_SCHEMA.validate(local_scope)
 
 
-def ExecLegacy(content, expand_vars=True, filename='<unknown>',
-               vars_override=None):
+def ExecLegacy(content, filename='<unknown>', vars_override=None):
   """Executes a DEPS file |content| using exec."""
   local_scope = {}
   global_scope = {'Var': lambda var_name: '{%s}' % var_name}
@@ -360,7 +354,7 @@ def ExecLegacy(content, expand_vars=True, filename='<unknown>',
   # 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:
+  if 'vars' not in local_scope:
     return local_scope
 
   vars_dict = {}
@@ -455,7 +449,7 @@ def UpdateCondition(info_dict, op, new_condition):
     del info_dict['condition']
 
 
-def Parse(content, expand_vars, validate_syntax, filename, vars_override=None):
+def Parse(content, validate_syntax, filename, vars_override=None):
   """Parses DEPS strings.
 
   Executes the Python-like string stored in content, resulting in a Python
@@ -464,7 +458,6 @@ def Parse(content, expand_vars, 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
@@ -477,9 +470,9 @@ def Parse(content, expand_vars, validate_syntax, filename, vars_override=None):
     schema above.
   """
   if validate_syntax:
-    result = Exec(content, expand_vars, filename, vars_override)
+    result = Exec(content, filename, vars_override)
   else:
-    result = ExecLegacy(content, expand_vars, filename, vars_override)
+    result = ExecLegacy(content, 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, True, deps_path)
+    gclient_dict = gclient_eval.Exec(deps_content, 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,22 +44,19 @@ 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")', {'bar': 'foo'}, True))
+        gclient_eval._gclient_eval('Var("bar")', vars_dict={'bar': 'foo'}))
 
   def test_expands_vars_with_braces(self):
     self.assertEqual(
         'foo',
-        gclient_eval._gclient_eval('"{bar}"', {'bar': 'foo'}, True))
+        gclient_eval._gclient_eval('"{bar}"', vars_dict={'bar': 'foo'}))
 
   def test_invalid_var(self):
-    with self.assertRaises(ValueError) as cm:
-      gclient_eval._gclient_eval('"{bar}"', {}, True)
+    with self.assertRaises(KeyError) as cm:
+      gclient_eval._gclient_eval('"{bar}"', vars_dict={})
     self.assertIn('bar was used as a variable, but was not declared',
                   str(cm.exception))
 
@@ -140,20 +137,6 @@ 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)
@@ -166,14 +149,14 @@ class ExecTest(unittest.TestCase):
         'deps = {',
         '  "a_dep": "a{foo}b",',
         '}',
-    ]), True, vars_override={'foo': 'baz'})
+    ]), 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(ValueError) as cm:
+    with self.assertRaises(KeyError) as cm:
       gclient_eval.Exec('\n'.join([
           'vars = {',
           '  "foo": "bar",',
@@ -181,7 +164,7 @@ class ExecTest(unittest.TestCase):
           'deps = {',
           '  "a_dep": "a{baz}b",',
           '}',
-      ]), True, vars_override={'baz': 'lalala'})
+      ]), vars_override={'baz': 'lalala'})
     self.assertIn('baz was used as a variable, but was not declared',
                   str(cm.exception))
 
@@ -601,8 +584,7 @@ class RevisionTest(unittest.TestCase):
 
 
 class ParseTest(unittest.TestCase):
-  def callParse(self, expand_vars=True, validate_syntax=True,
-                vars_override=None):
+  def callParse(self, validate_syntax=True, vars_override=None):
     return gclient_eval.Parse('\n'.join([
         'vars = {',
         '  "foo": "bar",',
@@ -610,24 +592,39 @@ class ParseTest(unittest.TestCase):
         'deps = {',
         '  "a_dep": "a{foo}b",',
         '}',
-    ]), expand_vars, validate_syntax, '<unknown>', vars_override)
+    ]), validate_syntax, '<unknown>', vars_override)
 
-  def test_expands_vars(self):
-    for validate_syntax in True, False:
-      local_scope = self.callParse(validate_syntax=validate_syntax)
+  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'},
-          'deps': {'a_dep': {'url': 'abarb',
-                             'dep_type': 'git'}},
+          'vars': {'foo': 'bar',
+                   'baz': '"bar" == "bar"'},
+          'deps': {'src/baz': {'url': 'baz_url',
+                               'dep_type': 'git',
+                               'condition': 'baz'}},
       }, local_scope)
 
-  def test_no_expands_vars(self):
+
+  def test_expands_vars(self):
     for validate_syntax in True, False:
-      local_scope = self.callParse(False,
-                                   validate_syntax=validate_syntax)
+      local_scope = self.callParse(validate_syntax=validate_syntax)
       self.assertEqual({
           'vars': {'foo': 'bar'},
-          'deps': {'a_dep': {'url': 'a{foo}b',
+          'deps': {'a_dep': {'url': 'abarb',
                              'dep_type': 'git'}},
       }, local_scope)
 
@@ -651,17 +648,15 @@ class ParseTest(unittest.TestCase):
         '}',
     ])
 
-    with self.assertRaises(ValueError) as cm:
+    with self.assertRaises(KeyError) as cm:
       gclient_eval.Parse(
-          deps_file, True, True,
-          '<unknown>', {'baz': 'lalala'})
+          deps_file, 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, True, False,
-          '<unknown>', {'baz': 'lalala'})
+          deps_file, False, '<unknown>', {'baz': 'lalala'})
     self.assertIn('baz', str(cm.exception))
 
   def test_standardizes_deps_string_dep(self):
@@ -670,7 +665,7 @@ class ParseTest(unittest.TestCase):
         'deps = {',
         '  "a_dep": "a_url@a_rev",',
         '}',
-      ]), False, validate_syntax, '<unknown>')
+      ]), validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git'}},
@@ -685,7 +680,7 @@ class ParseTest(unittest.TestCase):
         '     "condition": "checkout_android",',
         '  },',
         '}',
-      ]), False, validate_syntax, '<unknown>')
+      ]), validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git',
@@ -703,7 +698,7 @@ class ParseTest(unittest.TestCase):
         '     "a_dep": None,',
         '  },',
         '}',
-      ]), False, validate_syntax, '<unknown>')
+      ]), validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git'}},
@@ -720,7 +715,7 @@ class ParseTest(unittest.TestCase):
         '     "b_dep": "b_url@b_rev"',
         '  },',
         '}',
-      ]), False, validate_syntax, '<unknown>')
+      ]), validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git'},
@@ -740,7 +735,7 @@ class ParseTest(unittest.TestCase):
         '     "a_dep": "a_url@a_rev"',
         '  },',
         '}',
-      ]), False, validate_syntax, '<unknown>')
+      ]), validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {'a_dep': {'url': 'a_url@a_rev',
                              'dep_type': 'git'}},
@@ -760,7 +755,7 @@ class ParseTest(unittest.TestCase):
         '     "a_dep": "a_url@a_rev"',
         '  },',
         '}',
-      ]), False, validate_syntax, '<unknown>')
+      ]), validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {
               'a_dep': {'url': 'a_url@a_rev',
@@ -780,7 +775,7 @@ class ParseTest(unittest.TestCase):
         '     "a_dep": "a_url@a_rev"',
         '  },',
         '}',
-      ]), False, validate_syntax, '<unknown>')
+      ]), validate_syntax, '<unknown>')
       self.assertEqual({
           'deps': {
               'a_dep': {'url': 'a_url@a_rev',
@@ -804,7 +799,7 @@ class ParseTest(unittest.TestCase):
           '     "a_dep": "a_url@b_rev"',
           '  },',
           '}',
-        ]), False, validate_syntax, '<unknown>')
+        ]), validate_syntax, '<unknown>')
       self.assertIn('conflicts with existing deps', str(cm.exception))
 
   def test_merges_hooks_os(self):
@@ -822,7 +817,7 @@ class ParseTest(unittest.TestCase):
         '    },',
         '  ]',
         '}',
-      ]), False, validate_syntax, '<unknown>')
+      ]), validate_syntax, '<unknown>')
       self.assertEqual({
           "hooks": [{"action": ["a", "action"]},
                     {"action": ["b", "action"], "condition": "checkout_mac"}],

tests/gclient_smoketest.py

@@ -839,13 +839,15 @@ class GClientSmokeGIT(GClientSmokeBase):
           '}',
       ]))
 
-    self.gclient([
+    results = 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",',
@@ -879,11 +881,13 @@ 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:
@@ -928,7 +932,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         'deps = {',
         '  # src -> src/repo2 -> foo/bar',
         '  "foo/bar": {',
-        '    "url": "/repo_3",',
+        '    "url": "' + self.git_base + 'repo_3",',
         '    "condition": \'(repo2_false_var) and (true_str_var)\',',
         '  },',
         '',
@@ -939,43 +943,43 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '  # src -> src/mac_repo',
         '  "src/mac_repo": {',
-        '    "url": "{repo5_var}",',
+        '    "url": "' + self.git_base + 'repo_5",',
         '    "condition": \'checkout_mac\',',
         '  },',
         '',
         '  # src -> src/repo8 -> src/recursed_os_repo',
         '  "src/recursed_os_repo": {',
-        '    "url": "/repo_5",',
+        '    "url": "' + self.git_base + 'repo_5",',
         '    "condition": \'(checkout_linux) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo2',
         '  "src/repo2": {',
-        '    "url": "{git_base}repo_2@%s",' % (
+        '    "url": "' + self.git_base + 'repo_2@%s",' % (
                  self.githash('repo_2', 1)[:7]),
         '    "condition": \'true_str_var\',',
         '  },',
         '',
         '  # src -> src/repo4',
         '  "src/repo4": {',
-        '    "url": "/repo_4",',
+        '    "url": "' + self.git_base + 'repo_4",',
         '    "condition": \'False\',',
         '  },',
         '',
         '  # src -> src/repo8',
         '  "src/repo8": {',
-        '    "url": "/repo_8",',
+        '    "url": "' + self.git_base + 'repo_8",',
         '  },',
         '',
         '  # src -> src/unix_repo',
         '  "src/unix_repo": {',
-        '    "url": "{repo5_var}",',
+        '    "url": "' + self.git_base + 'repo_5",',
         '    "condition": \'checkout_linux\',',
         '  },',
         '',
         '  # src -> src/win_repo',
         '  "src/win_repo": {',
-        '    "url": "{repo5_var}",',
+        '    "url": "' + self.git_base + 'repo_5",',
         '    "condition": \'checkout_win\',',
         '  },',
         '',
@@ -991,7 +995,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '        "python",',
         '        "-c",',
         '        "open(\'src/git_hooked1\', \'w\')'
-            '.write(\'{hook1_contents}\')",',
+            '.write(\'git_hooked1\')",',
         '    ]',
         '  },',
         '',
@@ -1089,7 +1093,8 @@ class GClientSmokeGIT(GClientSmokeBase):
         'deps = {',
         '  # src -> src/repo2 -> foo/bar',
         '  "foo/bar": {',
-        '    "url": "/repo_3@%s",' % (self.githash('repo_3', 2)),
+        '    "url": "' + self.git_base + 'repo_3@%s",' % (
+                self.githash('repo_3', 2)),
         '    "condition": \'(repo2_false_var) and (true_str_var)\',',
         '  },',
         '',
@@ -1101,43 +1106,49 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '  # src -> src/mac_repo',
         '  "src/mac_repo": {',
-        '    "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)),
+        '    "url": "' + self.git_base + 'repo_5@%s",' % (
+                self.githash('repo_5', 3)),
         '    "condition": \'checkout_mac\',',
         '  },',
         '',
         '  # src -> src/repo8 -> src/recursed_os_repo',
         '  "src/recursed_os_repo": {',
-        '    "url": "/repo_5@%s",' % (self.githash('repo_5', 3)),
+        '    "url": "' + self.git_base + 'repo_5@%s",' % (
+                self.githash('repo_5', 3)),
         '    "condition": \'(checkout_linux) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo2',
         '  "src/repo2": {',
-        '    "url": "{git_base}repo_2@%s",' % (
+        '    "url": "' + self.git_base + 'repo_2@%s",' % (
                  self.githash('repo_2', 1)),
         '    "condition": \'true_str_var\',',
         '  },',
         '',
         '  # src -> src/repo4',
         '  "src/repo4": {',
-        '    "url": "/repo_4@%s",' % (self.githash('repo_4', 2)),
+        '    "url": "' + self.git_base + 'repo_4@%s",' % (
+                self.githash('repo_4', 2)),
         '    "condition": \'False\',',
         '  },',
         '',
         '  # src -> src/repo8',
         '  "src/repo8": {',
-        '    "url": "/repo_8@%s",' % (self.githash('repo_8', 1)),
+        '    "url": "' + self.git_base + 'repo_8@%s",' % (
+                self.githash('repo_8', 1)),
         '  },',
         '',
         '  # src -> src/unix_repo',
         '  "src/unix_repo": {',
-        '    "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)),
+        '    "url": "' + self.git_base + 'repo_5@%s",' % (
+                self.githash('repo_5', 3)),
         '    "condition": \'checkout_linux\',',
         '  },',
         '',
         '  # src -> src/win_repo',
         '  "src/win_repo": {',
-        '    "url": "{repo5_var}@%s",' % (self.githash('repo_5', 3)),
+        '    "url": "' + self.git_base + 'repo_5@%s",' % (
+                self.githash('repo_5', 3)),
         '    "condition": \'checkout_win\',',
         '  },',
         '',
@@ -1153,7 +1164,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '        "python",',
         '        "-c",',
         '        "open(\'src/git_hooked1\', \'w\')'
-            '.write(\'{hook1_contents}\')",',
+            '.write(\'git_hooked1\')",',
         '    ]',
         '  },',
         '',
@@ -1256,46 +1267,46 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '  # src -> src/repo9 -> src/repo8 -> src/recursed_os_repo',
         '  "src/recursed_os_repo": {',
-        '    "url": "/repo_5",',
+        '    "url": "' + self.git_base + 'repo_5",',
         '    "condition": \'(checkout_linux) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo11',
         '  "src/repo11": {',
-        '    "url": "/repo_11",',
+        '    "url": "' + self.git_base + 'repo_11",',
         '    "condition": \'(checkout_ios) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo11 -> src/repo12',
         '  "src/repo12": {',
-        '    "url": "/repo_12",',
+        '    "url": "' + self.git_base + 'repo_12",',
         '    "condition": \'(checkout_ios) or (checkout_mac)\',',
         '  },',
         '',
         '  # src -> src/repo9 -> src/repo4',
         '  "src/repo4": {',
-        '    "url": "/repo_4",',
+        '    "url": "' + self.git_base + 'repo_4",',
         '    "condition": \'checkout_android\',',
         '  },',
         '',
         '  # src -> src/repo6',
         '  "src/repo6": {',
-        '    "url": "/repo_6",',
+        '    "url": "' + self.git_base + 'repo_6",',
         '  },',
         '',
         '  # src -> src/repo9 -> src/repo7',
         '  "src/repo7": {',
-        '    "url": "/repo_7",',
+        '    "url": "' + self.git_base + 'repo_7",',
         '  },',
         '',
         '  # src -> src/repo9 -> src/repo8',
         '  "src/repo8": {',
-        '    "url": "/repo_8",',
+        '    "url": "' + self.git_base + 'repo_8",',
         '  },',
         '',
         '  # src -> src/repo9',
         '  "src/repo9": {',
-        '    "url": "/repo_9",',
+        '    "url": "' + self.git_base + '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, url, None, None, None,
+        None, 'name', 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', 'svn://example.com/foo', None,
+            obj, 'foo', 'svn://example.com/foo', None,
             None, None, None, 'DEPS', True, False, None, True),
         gclient.Dependency(
-            obj, 'bar', 'svn://example.com/bar', 'svn://example.com/bar', None,
+            obj, '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',
-            'svn://example.com/foo/dir1', None, None, None, None, 'DEPS', True,
+            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', 'svn://example.com/foo', None,
+            obj, 'foo', 'svn://example.com/foo', None,
             None, None, None, 'DEPS', True,
             False, None, True),
       ],