Revert "gclient: Don't allow None URLs (except in .gclient files)"

This reverts commit 0c91147d50.

Reason for revert: This is causing 'gclient revinfo' to fail on the release builders, and appears to be somehow related to the "--output-json" flag.

Original change's description:
> gclient: Don't allow None URLs (except in .gclient files)
> 
> This reverts commit crrev.com/4e9b50ab86b9b9f8ebf0b9ba6bd4954217ebeff9
> and thus relands the following commits:
> 
>   ebdd0db493: "gclient: Remove URLs from hierarchy."
>   54a5c2ba8a: "gclient: Refactor PrintRevInfo"
>   083eb25f9a: "gclient: Don't allow URL to be None."
> 
> When a None URL is specified in a .gclient file, and a DEPS file is
> given, the DEPS file is treated as a .gclient file and its dependencies
> are added.
> 
> Bug: 839925
> 
> Change-Id: I1068b66487874bfa0a788bf9da5273714b6ad39e
> Reviewed-on: https://chromium-review.googlesource.com/1083340
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Aaron Gable <agable@chromium.org>
> Reviewed-by: Michael Moss <mmoss@chromium.org>

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

Change-Id: I46785bd272b16b3672e553b6443cee6d6b370ec1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 839925, 853093
Reviewed-on: https://chromium-review.googlesource.com/1101978
Reviewed-by: Michael Moss <mmoss@chromium.org>
Commit-Queue: Michael Moss <mmoss@chromium.org>

gclient.py

@@ -241,7 +241,7 @@ class DependencySettings(object):
   """Immutable configuration settings."""
   def __init__(
       self, parent, url, managed, custom_deps, custom_vars,
-      custom_hooks, deps_file, relative, condition):
+      custom_hooks, deps_file, should_process, relative, condition):
     # These are not mutable:
     self._parent = parent
     self._deps_file = deps_file
@@ -249,9 +249,13 @@ class DependencySettings(object):
     # The condition as string (or None). Useful to keep e.g. for flatten.
     self._condition = condition
     # 'managed' determines whether or not this dependency is synced/updated by
-    # gclient after gclient checks it out initially. The user specifies
-    # 'managed' via the --unmanaged command-line flag or a .gclient config.
+    # gclient after gclient checks it out initially.  The difference between
+    # 'managed' and 'should_process' is that the user specifies 'managed' via
+    # the --unmanaged command-line flag or a .gclient config, where
+    # 'should_process' is dynamically set by gclient if it goes over its
+    # recursion limit and controls gclient's behavior so it does not misbehave.
     self._managed = managed
+    self._should_process = should_process
     # If this is a recursed-upon sub-dependency, and the parent has
     # use_relative_paths set, then this dependency should check out its own
     # dependencies relative to that parent's path for this, rather than
@@ -266,10 +270,15 @@ class DependencySettings(object):
     self._custom_deps = custom_deps or {}
     self._custom_hooks = custom_hooks or []
 
-    if self.url is not None:
+    # Post process the url to remove trailing slashes.
+    if isinstance(self.url, basestring):
       # urls are sometime incorrectly written as proto://host/path/@rev. Replace
       # it to proto://host/path@rev.
       self.set_url(self.url.replace('/@', '@'))
+    elif not isinstance(self.url, (None.__class__)):
+      raise gclient_utils.Error(
+          ('dependency url must be either string or None, '
+           'instead of %s') % self.url.__class__.__name__)
 
     # Make any deps_file path platform-appropriate.
     if self._deps_file:
@@ -296,6 +305,11 @@ class DependencySettings(object):
       return self or GClient(None, None)
     return self.parent.root
 
+  @property
+  def should_process(self):
+    """True if this dependency should be processed, i.e. checked out."""
+    return self._should_process
+
   @property
   def custom_vars(self):
     return self._custom_vars.copy()
@@ -343,20 +357,12 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
   """Object that represents a dependency checkout."""
 
   def __init__(self, parent, name, url, managed, custom_deps,
-               custom_vars, custom_hooks, deps_file, should_recurse, relative,
-               condition, print_outbuf=False):
+               custom_vars, custom_hooks, deps_file, should_process,
+               should_recurse, relative, condition, print_outbuf=False):
     gclient_utils.WorkItem.__init__(self, name)
     DependencySettings.__init__(
-        self,
-        parent=parent,
-        url=url,
-        managed=managed,
-        custom_deps=custom_deps,
-        custom_vars=custom_vars,
-        custom_hooks=custom_hooks,
-        deps_file=deps_file,
-        relative=relative,
-        condition=condition)
+        self, parent, url, managed, custom_deps, custom_vars,
+        custom_hooks, deps_file, should_process, relative, condition)
 
     # This is in both .gclient and DEPS files:
     self._deps_hooks = []
@@ -405,8 +411,9 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     # Whether we should process this dependency's DEPS file.
     self._should_recurse = should_recurse
 
-    if self.url:
-      # This is inherited from WorkItem.  We want the URL to be a resource.
+    self._OverrideUrl()
+    # This is inherited from WorkItem.  We want the URL to be a resource.
+    if self.url and isinstance(self.url, basestring):
       # The url is usually given to gclient either as https://blah@123
       # or just https://blah.  The @123 portion is irrelevant.
       self.resources.append(self.url.split('@')[0])
@@ -415,8 +422,43 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     # dependency
     self.print_outbuf = print_outbuf
 
+    if not self.name and self.parent:
+      raise gclient_utils.Error('Dependency without name')
+
+  def _OverrideUrl(self):
+    """Resolves the parsed url from the parent hierarchy."""
+    parsed_url = self.get_custom_deps(self._name, self.url)
+    if parsed_url != self.url:
+      logging.info('Dependency(%s)._OverrideUrl(%s) -> %s', self._name,
+                   self.url, parsed_url)
+      self.set_url(parsed_url)
+
+    elif isinstance(self.url, basestring):
+      parsed_url = urlparse.urlparse(self.url)
+      if (not parsed_url[0] and
+          not re.match(r'^\w+\@[\w\.-]+\:[\w\/]+', parsed_url[2])):
+        path = parsed_url[2]
+        if not path.startswith('/'):
+          raise gclient_utils.Error(
+              'relative DEPS entry \'%s\' must begin with a slash' % self.url)
+        # A relative url. Get the parent url, strip from the last '/'
+        # (equivalent to unix basename), and append the relative url.
+        parent_url = self.parent.url
+        parsed_url = parent_url[:parent_url.rfind('/')] + self.url
+        logging.info('Dependency(%s)._OverrideUrl(%s) -> %s', self.name,
+                     self.url, parsed_url)
+        self.set_url(parsed_url)
+
+    elif self.url is None:
+      logging.info('Dependency(%s)._OverrideUrl(None) -> None', self._name)
+
+    else:
+      raise gclient_utils.Error('Unknown url type')
+
   def PinToActualRevision(self):
     """Updates self.url to the revision checked out on disk."""
+    if self.url is None:
+      return
     url = None
     scm = self.CreateSCM()
     if os.path.isdir(scm.checkout_path):
@@ -460,7 +502,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     if self.name:
       requirements |= set(
-          obj.name for obj in self.root.subtree()
+          obj.name for obj in self.root.subtree(False)
           if (obj is not self
               and obj.name and
               self.name.startswith(posixpath.join(obj.name, ''))))
@@ -483,11 +525,15 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       raise gclient_utils.Error(
           'The same name "%s" appears multiple times in the deps section' %
               self.name)
+    if not self.should_process:
+      # Return early, no need to set requirements.
+      return True
 
     # This require a full tree traversal with locks.
-    siblings = [d for d in self.root.subtree() if d.name == self.name]
+    siblings = [d for d in self.root.subtree(False) if d.name == self.name]
     for sibling in siblings:
-      if self.url != sibling.url:
+      # Allow to have only one to be None or ''.
+      if self.url != sibling.url and bool(self.url) == bool(sibling.url):
         raise gclient_utils.Error(
             ('Dependency %s specified more than once:\n'
             '  %s [%s]\n'
@@ -537,47 +583,28 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
 
     return deps
 
-  def FormatUrl(self, name, url):
-    custom_deps_url = self.get_custom_deps(name, url)
-    if url != custom_deps_url:
-      return custom_deps_url
-    if url is None:
-      return None
-    if not isinstance(url, basestring):
-      raise gclient_utils.Error(
-          ('dependency url must be either string or None, '
-           'instead of %s') % self.url.__class__.__name__)
-    # For relative URLs, strip the parent url (self.url) from the last '/'
-    # and append the relative url.
-    if url[0] == '/':
-      if self.url is None:
-        raise gclient_utils.Error(
-            'Trying to set a relative url for %s, but parent has no url.' % name
-        )
-      url = self.url[:self.url.rfind('/')] + url
-    return url
-
   def _deps_to_objects(self, deps, use_relative_paths):
     """Convert a deps dict to a dict of Dependency objects."""
     deps_to_add = []
     for name, dep_value in deps.iteritems():
+      should_process = self.should_process
       if dep_value is None:
         continue
 
       condition = dep_value.get('condition')
-      dep_type = dep_value.get('dep_type', 'git')
+      dep_type = dep_value.get('dep_type')
 
-      should_process = True
-      if condition and not self.get_option('process_all_deps', False):
-        should_process = gclient_eval.EvaluateCondition(
+      if condition and not self._get_option('process_all_deps', False):
+        should_process = should_process and gclient_eval.EvaluateCondition(
             condition, self.get_vars())
 
-      if not should_process:
-        continue
-
       if dep_type == 'cipd':
         cipd_root = self.GetCipdRoot()
         for package in dep_value.get('packages', []):
+          if 'version' in package:
+            # Matches version to vars value.
+            version = package['version']
+            package['version'] = version
           deps_to_add.append(
               CipdDependency(
                   parent=self,
@@ -585,24 +612,25 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
                   dep_value=package,
                   cipd_root=cipd_root,
                   custom_vars=self.custom_vars,
+                  should_process=should_process,
                   relative=use_relative_paths,
                   condition=condition))
       else:
-        url = self.FormatUrl(name, dep_value.get('url'))
-        if url:
-          deps_to_add.append(
-              GitDependency(
-                  parent=self,
-                  name=name,
-                  url=url,
-                  managed=None,
-                  custom_deps=None,
-                  custom_vars=self.custom_vars,
-                  custom_hooks=None,
-                  deps_file=self.recursedeps.get(name, self.deps_file),
-                  should_recurse=name in self.recursedeps,
-                  relative=use_relative_paths,
-                  condition=condition))
+        url = dep_value.get('url')
+        deps_to_add.append(
+            GitDependency(
+                parent=self,
+                name=name,
+                url=url,
+                managed=None,
+                custom_deps=None,
+                custom_vars=self.custom_vars,
+                custom_hooks=None,
+                deps_file=self.recursedeps.get(name, self.deps_file),
+                should_process=should_process,
+                should_recurse=name in self.recursedeps,
+                relative=use_relative_paths,
+                condition=condition))
 
     deps_to_add.sort(key=lambda x: x.name)
     return deps_to_add
@@ -638,7 +666,7 @@ 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, self._get_option('validate_syntax', False),
             filepath, self.get_vars())
       except SyntaxError as e:
         gclient_utils.SyntaxErrorToError(filepath, e)
@@ -742,8 +770,11 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     self.add_dependencies_and_close(deps_to_add, hooks_to_run)
     logging.info('ParseDepsFile(%s) done' % self.name)
 
-  def get_option(self, attr, default=None):
-    return getattr(self.root._options, attr, default)
+  def _get_option(self, attr, default):
+    obj = self
+    while not hasattr(obj, '_options'):
+      obj = obj.parent
+    return getattr(obj._options, attr, default)
 
   def add_dependencies_and_close(self, deps_to_add, hooks):
     """Adds the dependencies, hooks and mark the parsing as done."""
@@ -769,9 +800,10 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       # Don't enforce this for custom_deps.
       if dep.name in self._custom_deps:
         continue
-      parsed_url = urlparse.urlparse(dep.url)
-      if parsed_url.netloc and parsed_url.netloc not in self._allowed_hosts:
-        bad_deps.append(dep)
+      if isinstance(dep.url, basestring):
+        parsed_url = urlparse.urlparse(dep.url)
+        if parsed_url.netloc and parsed_url.netloc not in self._allowed_hosts:
+          bad_deps.append(dep)
     return bad_deps
 
   def FuzzyMatchUrl(self, candidates):
@@ -813,6 +845,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     """Runs |command| then parse the DEPS file."""
     logging.info('Dependency(%s).run()' % self.name)
     assert self._file_list == []
+    if not self.should_process:
+      return
     # When running runhooks, there's no need to consult the SCM.
     # All known hooks are expected to run unconditionally regardless of working
     # copy state, so skip the SCM status check.
@@ -821,7 +855,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     file_list = [] if not options.nohooks else None
     revision_override = revision_overrides.pop(
         self.FuzzyMatchUrl(revision_overrides), None)
-    if run_scm:
+    if run_scm and self.url:
       # Create a shallow copy to mutate revision.
       options = copy.copy(options)
       options.revision = revision_override
@@ -862,7 +896,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
         self.RunPreDepsHooks()
       # Parse the dependencies of this dependency.
       for s in self.dependencies:
-        work_queue.enqueue(s)
+        if s.should_process:
+          work_queue.enqueue(s)
 
     if command == 'recurse':
       # Skip file only checkout.
@@ -872,8 +907,10 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
         # Pass in the SCM type as an env variable.  Make sure we don't put
         # unicode strings in the environment.
         env = os.environ.copy()
-        env['GCLIENT_SCM'] = str(scm)
-        env['GCLIENT_URL'] = str(self.url)
+        if scm:
+          env['GCLIENT_SCM'] = str(scm)
+        if self.url:
+          env['GCLIENT_URL'] = str(self.url)
         env['GCLIENT_DEP_PATH'] = str(self.name)
         if options.prepend_dir and scm == 'git':
           print_stdout = False
@@ -904,7 +941,9 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
           print_stdout = True
           filter_fn = None
 
-        if os.path.isdir(cwd):
+        if self.url is None:
+          print('Skipped omitted dependency %s' % cwd, file=sys.stderr)
+        elif os.path.isdir(cwd):
           try:
             gclient_utils.CheckCallAndFilter(
                 args, cwd=cwd, env=env, print_stdout=print_stdout,
@@ -948,6 +987,9 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     RunOnDeps() must have been called before to load the DEPS.
     """
     result = []
+    if not self.should_process or not self.should_recurse:
+      # Don't run the hook when it is above recursion_limit.
+      return result
     # If "--force" was specified, run all hooks regardless of what files have
     # changed.
     if self.deps_hooks:
@@ -990,13 +1032,14 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       return None
     return self.root.GetCipdRoot()
 
-  def subtree(self):
+  def subtree(self, include_all):
     """Breadth first recursion excluding root node."""
     dependencies = self.dependencies
     for d in dependencies:
-      yield d
+      if d.should_process or include_all:
+        yield d
     for d in dependencies:
-      for i in d.subtree():
+      for i in d.subtree(include_all):
         yield i
 
   @gclient_utils.lockedmethod
@@ -1074,7 +1117,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
   def __str__(self):
     out = []
     for i in ('name', 'url', 'custom_deps',
-              'custom_vars', 'deps_hooks', 'file_list',
+              'custom_vars', 'deps_hooks', 'file_list', 'should_process',
               'processed', 'hooks_ran', 'deps_parsed', 'requirements',
               'allowed_hosts'):
       # First try the native property if it exists.
@@ -1226,12 +1269,13 @@ solutions = %(solution_list)s
     super(GClient, self).__init__(
         parent=None,
         name=None,
-        url='None',
+        url=None,
         managed=True,
         custom_deps=None,
         custom_vars=None,
         custom_hooks=None,
         deps_file='unused',
+        should_process=True,
         should_recurse=True,
         relative=None,
         condition=None,
@@ -1254,7 +1298,7 @@ solutions = %(solution_list)s
     """Verify that the config matches the state of the existing checked-out
     solutions."""
     for dep in self.dependencies:
-      if dep.managed:
+      if dep.managed and dep.url:
         scm = dep.CreateSCM()
         actual_url = scm.GetActualRemoteURL(self._options)
         if actual_url and not scm.DoesRemoteURLMatch(self._options):
@@ -1324,38 +1368,8 @@ it or fix the checkout.
                                 'not specified')
 
     deps_to_add = []
-    solutions = config_dict.get('solutions', [])
-    if len(solutions) == 1 and solutions[0].get('url') is None:
-      s = solutions[0]
-      # If url is not given, or is None, but a DEPS file is specified, we
-      # treat the given DEPS file as a .gclient file and add its dependencies
-      # instead.
-      temp_dep = GitDependency(
-          parent=self,
-          name=s.get('name') or '.',
-          url=None,
-          managed=s.get('managed', True),
-          custom_deps=s.get('custom_deps', {}),
-          custom_vars=s.get('custom_vars', {}),
-          custom_hooks=s.get('custom_hooks', []),
-          deps_file=s.get('deps_file', 'DEPS'),
-          should_recurse=True,
-          relative=None,
-          condition=None,
-          print_outbuf=True)
-      temp_dep.ParseDepsFile()
-      for dep in temp_dep.dependencies:
-        # Note that hooks are not preserved, since they might depend on the
-        # existence of a checkout.
-        dep._custom_vars = temp_dep.get_vars()
-        dep._custom_deps = temp_dep.custom_deps
-        dep._parent = self
-      deps_to_add.extend(temp_dep.dependencies)
-    else:
-      for s in solutions:
-        if not s.get('name') or not s.get('url'):
-          raise gclient_utils.Error('Invalid .gclient file. Solution is '
-                                    'incomplete: %s' % s)
+    for s in config_dict.get('solutions', []):
+      try:
         deps_to_add.append(GitDependency(
             parent=self,
             name=s['name'],
@@ -1365,14 +1379,15 @@ it or fix the checkout.
             custom_vars=s.get('custom_vars', {}),
             custom_hooks=s.get('custom_hooks', []),
             deps_file=s.get('deps_file', 'DEPS'),
+            should_process=True,
             should_recurse=True,
             relative=None,
             condition=None,
             print_outbuf=True))
+      except KeyError:
+        raise gclient_utils.Error('Invalid .gclient file. Solution is '
+                                  'incomplete: %s' % s)
     self.add_dependencies_and_close(deps_to_add, config_dict.get('hooks', []))
-    if not self.dependencies:
-      raise gclient_utils.Error('No solution specified')
-
     logging.info('SetConfig() done')
 
   def SaveConfig(self):
@@ -1432,7 +1447,7 @@ it or fix the checkout.
     # Sometimes pprint.pformat will use {', sometimes it'll use { ' ... It
     # makes testing a bit too fun.
     result = 'entries = {\n'
-    for entry in self.root.subtree():
+    for entry in self.root.subtree(False):
       result += '  %s: %s,\n' % (pprint.pformat(entry.name),
           pprint.pformat(entry.url))
     result += '}\n'
@@ -1500,6 +1515,9 @@ it or fix the checkout.
       command: The command to use (e.g., 'status' or 'diff')
       args: list of str - extra arguments to add to the command line.
     """
+    if not self.dependencies:
+      raise gclient_utils.Error('No solution specified')
+
     revision_overrides = {}
     patch_refs = {}
     # It's unnecessary to check for revision overrides for 'recurse'.
@@ -1524,7 +1542,8 @@ it or fix the checkout.
         self._options.jobs, pm, ignore_requirements=ignore_requirements,
         verbose=self._options.verbose)
     for s in self.dependencies:
-      work_queue.enqueue(s)
+      if s.should_process:
+        work_queue.enqueue(s)
     work_queue.flush(revision_overrides, command, args, options=self._options,
                      patch_refs=patch_refs)
 
@@ -1561,7 +1580,7 @@ it or fix the checkout.
       # Notify the user if there is an orphaned entry in their working copy.
       # Only delete the directory if there are no changes in it, and
       # delete_unversioned_trees is set to true.
-      entries = [i.name for i in self.root.subtree()]
+      entries = [i.name for i in self.root.subtree(False) if i.url]
       full_entries = [os.path.join(self.root_dir, e.replace('/', os.path.sep))
                       for e in entries]
 
@@ -1658,59 +1677,68 @@ it or fix the checkout.
     work_queue = gclient_utils.ExecutionQueue(
         self._options.jobs, None, False, verbose=self._options.verbose)
     for s in self.dependencies:
-      work_queue.enqueue(s)
+      if s.should_process:
+        work_queue.enqueue(s)
     work_queue.flush({}, None, [], options=self._options, patch_refs=None)
 
-    def ShouldPrint(dep):
+    def ShouldPrintRevision(dep):
       return (not self._options.filter
               or dep.FuzzyMatchUrl(self._options.filter))
 
-    for dep in self.subtree():
-      if self._options.snapshot or self._options.actual:
-        dep.PinToActualRevision()
-
     if self._options.snapshot:
-      json_output = [
-          {
-              'name': d.name,
-              'solution_url': d.url,
-              'deps_file': d.deps_file,
-              'managed': d.managed,
-              'custom_deps': {
-                  subdep.name: subdep.url
-                  for subdep in d.subtree()
-                  if ShouldPrint(subdep)
-              },
-          }
-          for d in self.dependencies
-          if ShouldPrint(d)
-      ]
-      output = json.dumps(json_output, indent=2, separators=(',', ': '))
-      if not self._options.output_json:
-        output = self.DEFAULT_SNAPSHOT_FILE_TEXT % {'solution_list': output}
-    elif self._options.output_json:
-      json_output = {
-          d.name: {
-              'url': d.url.split('@')[0],
-              'rev': d.url.split('@')[1] if '@' in d.url else None,
-          }
-          for d in self.subtree()
-          if ShouldPrint(d)
-      }
-      output = json.dumps(json_output, indent=2, separators=(',', ': '))
+      json_output = []
+      # First level at .gclient
+      for d in self.dependencies:
+        entries = {}
+        def GrabDeps(dep):
+          """Recursively grab dependencies."""
+          for d in dep.dependencies:
+            d.PinToActualRevision()
+            if ShouldPrintRevision(d):
+              entries[d.name] = d.url
+            GrabDeps(d)
+        GrabDeps(d)
+        json_output.append({
+            'name': d.name,
+            'solution_url': d.url,
+            'deps_file': d.deps_file,
+            'managed': d.managed,
+            'custom_deps': entries,
+        })
+      if self._options.output_json == '-':
+        print(json.dumps(json_output, indent=2, separators=(',', ': ')))
+      elif self._options.output_json:
+        with open(self._options.output_json, 'w') as f:
+          json.dump(json_output, f)
+      else:
+        # Print the snapshot configuration file
+        print(self.DEFAULT_SNAPSHOT_FILE_TEXT % {
+            'solution_list': pprint.pformat(json_output, indent=2),
+        })
     else:
-      output = '\n'.join(
-          '%s: %s' % (d.name, d.url)
-          for d in self.subtree()
-          if ShouldPrint(d)
-      )
-
-    if self._options.output_json and self._options.output_json != '-':
-      with open(self._options.output_json, 'w') as f:
-        f.write(output)
-    else:
-      print(output)
-
+      entries = {}
+      for d in self.root.subtree(False):
+        if self._options.actual:
+          d.PinToActualRevision()
+        if ShouldPrintRevision(d):
+          entries[d.name] = d.url
+      if self._options.output_json:
+        json_output = {
+            name: {
+                'url': rev.split('@')[0] if rev else None,
+                'rev': rev.split('@')[1] if rev and '@' in rev else None,
+            }
+            for name, rev in entries.iteritems()
+        }
+        if self._options.output_json == '-':
+          print(json.dumps(json_output, indent=2, separators=(',', ': ')))
+        else:
+          with open(self._options.output_json, 'w') as f:
+            json.dump(json_output, f)
+      else:
+        keys = sorted(entries.keys())
+        for x in keys:
+          print('%s: %s' % (x, entries[x]))
     logging.info(str(self))
 
   def ParseDepsFile(self):
@@ -1756,8 +1784,9 @@ it or fix the checkout.
 class CipdDependency(Dependency):
   """A Dependency object that represents a single CIPD package."""
 
-  def __init__(self, parent, name, dep_value, cipd_root, custom_vars, relative,
-               condition):
+  def __init__(
+      self, parent, name, dep_value, cipd_root,
+      custom_vars, should_process, relative, condition):
     package = dep_value['package']
     version = dep_value['version']
     url = urlparse.urljoin(
@@ -1771,6 +1800,7 @@ class CipdDependency(Dependency):
         custom_vars=custom_vars,
         custom_hooks=None,
         deps_file=None,
+        should_process=should_process,
         should_recurse=False,
         relative=relative,
         condition=condition)
@@ -1782,7 +1812,6 @@ class CipdDependency(Dependency):
     self._cipd_root = cipd_root
     self._cipd_subdir = os.path.relpath(
         os.path.join(self.root.root_dir, name), cipd_root.root_dir)
-    self._package_path = name
     self._package_name = package
     self._package_version = version
 
@@ -1791,6 +1820,8 @@ class CipdDependency(Dependency):
           patch_refs):
     """Runs |command| then parse the DEPS file."""
     logging.info('CipdDependency(%s).run()' % self.name)
+    if not self.should_process:
+      return
     self._CreatePackageIfNecessary()
     super(CipdDependency, self).run(revision_overrides, command, args,
                                     work_queue, options, patch_refs)
@@ -1826,10 +1857,6 @@ class CipdDependency(Dependency):
         self.url, self.root.root_dir, self.name, self.outbuf, out_cb,
         root=self._cipd_root, package=self._cipd_package)
 
-  def hierarchy(self, include_url=True):
-    """Returns a human-readable hierarchical reference to a Dependency."""
-    return self.parent.hierarchy(include_url) + ' -> ' + self._package_path
-
   def ToLines(self):
     """Return a list of lines representing this in a DEPS file."""
     s = []
@@ -1957,6 +1984,9 @@ class Flattener(object):
     Arguments:
       dep (Dependency): dependency to process
     """
+    if dep.url is None:
+      return
+
     # Make sure the revision is always fully specified (a hash),
     # as opposed to refs or tags which might change. Similarly,
     # shortened shas might become ambiguous; make sure to always
@@ -1974,8 +2004,7 @@ class Flattener(object):
     """
     for solution in self._client.dependencies:
       self._add_dep(solution)
-      if solution.should_recurse:
-        self._flatten_dep(solution)
+      self._flatten_dep(solution)
 
     if pin_all_deps:
       for dep in self._deps.itervalues():
@@ -1994,6 +2023,7 @@ class Flattener(object):
         deps_path = os.path.join(self._client.root_dir, dep.name, deps_file)
         if not os.path.exists(deps_path):
           return
+      assert dep.url
       self._deps_files.add((dep.url, deps_file, dep.hierarchy_data()))
     for dep in self._deps.itervalues():
       add_deps_file(dep)
@@ -2019,7 +2049,8 @@ class Flattener(object):
     """
     assert dep.name not in self._deps or self._deps.get(dep.name) == dep, (
         dep.name, self._deps.get(dep.name))
-    self._deps[dep.name] = dep
+    if dep.url:
+      self._deps[dep.name] = dep
 
   def _flatten_dep(self, dep):
     """Visits a dependency in order to flatten it (see CMDflatten).
@@ -2530,12 +2561,16 @@ def CMDsync(parser, args):
   ret = client.RunOnDeps('update', args)
   if options.output_json:
     slns = {}
-    for d in client.subtree():
+    for d in client.subtree(True):
       normed = d.name.replace('\\', '/').rstrip('/') + '/'
+      if normed in slns and not d.should_process:
+        # If an unprocessed dependency would override an existing dependency,
+        # ignore it.
+        continue
       slns[normed] = {
           'revision': d.got_revision,
           'scm': d.used_scm.name if d.used_scm else None,
-          'url': str(d.url),
+          'url': str(d.url) if d.url else None,
       }
     with open(options.output_json, 'wb') as f:
       json.dump({'solutions': slns}, f)

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!

tests/gclient_smoketest.py

@@ -246,14 +246,9 @@ class GClientSmoke(GClientSmokeBase):
           ']\n'
           'cache_dir = None\n') % self.git_base)
 
+    test(['config', '--spec', '["blah blah"]'], '["blah blah"]')
+
     os.remove(p)
-    results = self.gclient(['config', '--spec', '["blah blah"]'])
-    self.assertEqual(('', 'Error: No solution specified\n', 1), results)
-
-    results = self.gclient(['config', '--spec',
-                            'solutions=[{"name": "./", "url": None}]'])
-    self.assertEqual(('', 'Error: No solution specified\n', 1), results)
-
     results = self.gclient(['config', 'foo', 'faa', 'fuu'])
     err = ('Usage: gclient.py config [options] [url]\n\n'
            'gclient.py: error: Inconsistent arguments. Use either --spec or one'
@@ -261,6 +256,24 @@ class GClientSmoke(GClientSmokeBase):
     self.check(('', err, 2), results)
     self.assertFalse(os.path.exists(join(self.root_dir, '.gclient')))
 
+  def testSolutionNone(self):
+    results = self.gclient(['config', '--spec',
+                            'solutions=[{"name": "./", "url": None}]'])
+    self.check(('', '', 0), results)
+    results = self.gclient(['sync'])
+    self.check(('', '', 0), results)
+    self.assertTree({})
+    results = self.gclient(['revinfo'])
+    self.check(('./: None\n', '', 0), results)
+    self.check(('', '', 0), self.gclient(['diff']))
+    self.assertTree({})
+    self.check(('', '', 0), self.gclient(['pack']))
+    self.check(('', '', 0), self.gclient(['revert']))
+    self.assertTree({})
+    self.check(('', '', 0), self.gclient(['runhooks']))
+    self.assertTree({})
+    self.check(('', '', 0), self.gclient(['status']))
+
   def testDifferentTopLevelDirectory(self):
     # Check that even if the .gclient file does not mention the directory src
     # itself, but it is included via dependencies, the .gclient file is used.
@@ -796,10 +809,8 @@ class GClientSmokeGIT(GClientSmokeBase):
     with open(output_json) as f:
       output_json = json.load(f)
 
-    self.maxDiff = None
     out = [{
-        'solution_url': '%srepo_1@%s' % (
-            self.git_base, self.githash('repo_1', 2)),
+        'solution_url': self.git_base + 'repo_1',
         'managed': True,
         'name': 'src',
         'deps_file': 'DEPS',
@@ -1228,118 +1239,6 @@ class GClientSmokeGIT(GClientSmokeBase):
             self.githash('repo_8', 1)),
     ], deps_contents.splitlines())
 
-  def testFlattenLocalDepsFile(self):
-    if not self.enabled:
-      return
-
-    output_deps = os.path.join(self.root_dir, 'DEPS.flattened')
-    self.assertFalse(os.path.exists(output_deps))
-
-    output_deps_files = os.path.join(self.root_dir, 'DEPS.files')
-    self.assertFalse(os.path.exists(output_deps_files))
-
-    write(
-        os.path.join(self.root_dir, 'DEPS'),
-        'deps = {\n'
-        '    "src": "' + self.git_base + 'repo_10",\n'
-        '}\n'
-        'recursedeps = ["src"]')
-
-    self.gclient(['config', '--spec', 'solutions=[{"deps_file": "DEPS"}]'])
-    self.gclient(['sync', '--process-all-deps'])
-    self.gclient(['flatten', '-v', '-v', '-v',
-                  '--output-deps', output_deps,
-                  '--output-deps-files', output_deps_files])
-
-    with open(output_deps) as f:
-      deps_contents = f.read()
-
-    self.maxDiff = None
-    self.assertEqual([
-        'gclient_gn_args_file = "src/repo2/gclient.args"',
-        "gclient_gn_args = ['str_var']",
-        'deps = {',
-        '  # src',
-        '  "src": {',
-        '    "url": "' + self.git_base + 'repo_10",',
-        '  },',
-        '',
-        '  # src -> src/repo9 -> src/repo8 -> src/recursed_os_repo',
-        '  "src/recursed_os_repo": {',
-        '    "url": "' + self.git_base + 'repo_5",',
-        '    "condition": \'(checkout_linux) or (checkout_mac)\',',
-        '  },',
-        '',
-        '  # src -> src/repo11',
-        '  "src/repo11": {',
-        '    "url": "' + self.git_base + 'repo_11",',
-        '    "condition": \'(checkout_ios) or (checkout_mac)\',',
-        '  },',
-        '',
-        '  # src -> src/repo11 -> src/repo12',
-        '  "src/repo12": {',
-        '    "url": "' + self.git_base + 'repo_12",',
-        '    "condition": \'(checkout_ios) or (checkout_mac)\',',
-        '  },',
-        '',
-        '  # src -> src/repo9 -> src/repo4',
-        '  "src/repo4": {',
-        '    "url": "' + self.git_base + 'repo_4",',
-        '    "condition": \'checkout_android\',',
-        '  },',
-        '',
-        '  # src -> src/repo6',
-        '  "src/repo6": {',
-        '    "url": "' + self.git_base + 'repo_6",',
-        '  },',
-        '',
-        '  # src -> src/repo9 -> src/repo7',
-        '  "src/repo7": {',
-        '    "url": "' + self.git_base + 'repo_7",',
-        '  },',
-        '',
-        '  # src -> src/repo9 -> src/repo8',
-        '  "src/repo8": {',
-        '    "url": "' + self.git_base + 'repo_8",',
-        '  },',
-        '',
-        '  # src -> src/repo9',
-        '  "src/repo9": {',
-        '    "url": "' + self.git_base + 'repo_9",',
-        '  },',
-        '',
-        '}',
-        '',
-        'vars = {',
-        '  # src -> src/repo9',
-        '  "str_var": \'xyz\',',
-        '',
-        '}',
-        '',
-        '# ' + self.git_base + 'repo_10, DEPS',
-        '# ' + self.git_base + 'repo_11, DEPS',
-        '# ' + self.git_base + 'repo_8, DEPS',
-        '# ' + self.git_base + 'repo_9, DEPS',
-    ], deps_contents.splitlines())
-
-    with open(output_deps_files) as f:
-      deps_files_contents = json.load(f)
-
-    self.assertEqual([
-      {'url': self.git_base + 'repo_10', 'deps_file': 'DEPS',
-       'hierarchy': [['src', self.git_base + 'repo_10']]},
-      {'url': self.git_base + 'repo_11', 'deps_file': 'DEPS',
-       'hierarchy': [['src', self.git_base + 'repo_10'],
-                     ['src/repo11', self.git_base + 'repo_11']]},
-      {'url': self.git_base + 'repo_8', 'deps_file': 'DEPS',
-       'hierarchy': [['src', self.git_base + 'repo_10'],
-                     ['src/repo9', self.git_base + 'repo_9'],
-                     ['src/repo8', self.git_base + 'repo_8']]},
-      {'url': self.git_base + 'repo_9', 'deps_file': 'DEPS',
-       'hierarchy': [['src', self.git_base + 'repo_10'],
-                     ['src/repo9', self.git_base + 'repo_9']]},
-    ], deps_files_contents)
-
   def testFlattenRecursedeps(self):
     if not self.enabled:
       return
@@ -1467,7 +1366,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '    "url": "' + self.git_base + 'repo_14",',
         '  },',
         '',
-        '  # src -> src/another_cipd_dep',
+        '  # src -> src/another_cipd_dep:package1',
         '  "src/another_cipd_dep": {',
         '    "packages": [',
         '      {',
@@ -1482,7 +1381,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '    "dep_type": "cipd",',
         '  },',
         '',
-        '  # src -> src/cipd_dep',
+        '  # src -> src/cipd_dep:package0',
         '  "src/cipd_dep": {',
         '    "packages": [',
         '      {',

tests/gclient_test.py

@@ -77,166 +77,6 @@ class GclientTest(trial_dir.TestCase):
     os.chdir(self.previous_dir)
     super(GclientTest, self).tearDown()
 
-  def testLocalDepsFileRespectsRecursedeps(self):
-    """Test that we only recurse into specified dependencies.
-
-    When parsing a local DEPS file, don't recurse into all of its dependencies,
-    only on the ones specified by the DEPS file.
-    """
-    parser = gclient.OptionParser()
-    options, _ = parser.parse_args([])
-    write(
-        '.gclient',
-        'solutions = [\n'
-        '  {\n'
-        '    "name": None,\n'
-        '    "url": None,\n'
-        '    "deps_file": "DEPS",\n'
-        '  },\n'
-        ']')
-    write(
-        'DEPS',
-        'deps = {\n'
-        '  "foo": "svn://example.com/foo",\n'
-        '  "bar": "svn://example.com/bar",\n'
-        '}\n'
-        'recursedeps=["foo"]')
-    write(
-        os.path.join('foo', 'DEPS'),
-        'deps = {\n'
-        '  "foo/dir1": "svn://example.com/dir1",\n'
-        '}')
-    # We shouldn't process bar/DEPS, since it wasn't specifed as a recursedep.
-    write(
-        os.path.join('bar', 'DEPS'),
-        'ERROR ERROR ERROR')
-
-    obj = gclient.GClient.LoadCurrentConfig(options)
-    obj.RunOnDeps('None', [])
-    self.assertEqual(
-        [('bar', 'svn://example.com/bar'),
-         ('foo', 'svn://example.com/foo'),
-         ('foo/dir1', 'svn://example.com/dir1')],
-        sorted(self._get_processed()))
-
-  def testLocalDepsFilePreservesCustomVars(self):
-    """Test that the processed dependencies respect custom_vars.
-    """
-    parser = gclient.OptionParser()
-    options, _ = parser.parse_args([])
-    write(
-        '.gclient',
-        'solutions = [\n'
-        '  {\n'
-        '    "name": None,\n'
-        '    "url": None,\n'
-        '    "deps_file": "DEPS",\n'
-        '    "custom_vars": {\n'
-        '      "custom_var": "custom_value",\n'
-        '    },\n'
-        '  },\n'
-        ']')
-    write(
-        'DEPS',
-        'deps = {\n'
-        '  "foo": "svn://example.com/foo",\n'
-        '}')
-
-    obj = gclient.GClient.LoadCurrentConfig(options)
-    obj.RunOnDeps('None', [])
-    self.assertEqual(
-        [('foo', 'svn://example.com/foo')],
-        sorted(self._get_processed()))
-
-    # Verify that custom_var is set correctly in foo/dir1 and foo/dir2.
-    foo_vars = obj.dependencies[0].get_vars()
-    self.assertEqual(foo_vars['custom_var'], 'custom_value')
-
-  def testLocalDepsFilePreservesCustomDeps(self):
-    """Test that the processed dependencies respect custom_deps.
-    """
-    parser = gclient.OptionParser()
-    options, _ = parser.parse_args([])
-    write(
-        '.gclient',
-        'solutions = [\n'
-        '  {\n'
-        '    "name": None,\n'
-        '    "url": None,\n'
-        '    "deps_file": "DEPS",\n'
-        '    "custom_deps": {\n'
-        '       "foo/dir1": "svn://example.com/custom",'
-        '    },\n'
-        '  },\n'
-        ']')
-    write(
-        'DEPS',
-        'deps = {\n'
-        '  "foo": "svn://example.com/foo",\n'
-        '}\n'
-        'recursedeps = ["foo"]')
-    write(
-        os.path.join('foo', 'DEPS'),
-        'deps = {\n'
-        '  "foo/dir1": "svn://example.com/dir1",\n'
-        '}')
-
-    obj = gclient.GClient.LoadCurrentConfig(options)
-    obj.RunOnDeps('None', [])
-    self.assertEqual(
-        [('foo', 'svn://example.com/foo'),
-         ('foo/dir1', 'svn://example.com/custom')],
-        sorted(self._get_processed()))
-
-  def testLocalDepsFileInheritsVars(self):
-    """Tests that variables in the local DEPS file are preserved.
-
-    We want to test that the variables in the local DEPS files are preserved
-    when interpreting the DEPS file as a .gclient file.
-
-    In particular, checkout_dir1 should be True in foo/, since it was set in the
-    local DEPS file.
-    """
-    parser = gclient.OptionParser()
-    options, _ = parser.parse_args([])
-    write(
-        '.gclient',
-        'solutions = [\n'
-        '  {\n'
-        '    "name": None,\n'
-        '    "url": None,\n'
-        '    "deps_file": "DEPS",\n'
-        '  },\n'
-        ']')
-    write(
-        'DEPS',
-        'vars = {\n'
-        '  "checkout_dir1": True,\n'
-        '}\n'
-        'deps = {\n'
-        '  "foo": "svn://example.com/foo",\n'
-        '}\n'
-        'recursedeps = ["foo"]\n'
-    )
-    write(
-        os.path.join('foo', 'DEPS'),
-        'vars = {\n'
-        '  "checkout_dir1": False,\n'
-        '}\n'
-        'deps = {\n'
-        '  "foo/dir1": {\n'
-        '    "url": "svn://example.com/dir1",\n'
-        '    "condition": "checkout_dir1",\n'
-        '  },\n'
-        '}')
-
-    obj = gclient.GClient.LoadCurrentConfig(options)
-    obj.RunOnDeps('None', [])
-    self.assertEqual(
-        [('foo', 'svn://example.com/foo'),
-         ('foo/dir1', 'svn://example.com/dir1')],
-        sorted(self._get_processed()))
-
   def testDependencies(self):
     self._dependencies('1')
 
@@ -369,6 +209,7 @@ class GclientTest(trial_dir.TestCase):
         custom_vars=None,
         custom_hooks=None,
         deps_file='',
+        should_process=True,
         should_recurse=False,
         relative=False,
         condition=None,
@@ -390,6 +231,7 @@ class GclientTest(trial_dir.TestCase):
             custom_vars=None,
             custom_hooks=None,
             deps_file='DEPS',
+            should_process=True,
             should_recurse=True,
             relative=False,
             condition=None,
@@ -403,6 +245,7 @@ class GclientTest(trial_dir.TestCase):
             custom_vars=None,
             custom_hooks=None,
             deps_file='DEPS',
+            should_process=True,
             should_recurse=False,
             relative=False,
             condition=None,
@@ -420,6 +263,7 @@ class GclientTest(trial_dir.TestCase):
             custom_vars=None,
             custom_hooks=None,
             deps_file='DEPS',
+            should_process=True,
             should_recurse=False,
             relative=False,
             condition=None,
@@ -431,7 +275,7 @@ class GclientTest(trial_dir.TestCase):
     # pylint: disable=protected-access
     obj.dependencies[0]._file_list.append('foo')
     str_obj = str(obj)
-    self.assertEquals(240, len(str_obj), '%d\n%s' % (len(str_obj), str_obj))
+    self.assertEquals(322, len(str_obj), '%d\n%s' % (len(str_obj), str_obj))
 
   def testHooks(self):
     topdir = self.root_dir
@@ -742,12 +586,14 @@ class GclientTest(trial_dir.TestCase):
         ('foo/rel', 'svn://example.com/rel'),
     ], self._get_processed())
 
-    self.assertEqual(4, len(sol.dependencies))
+    self.assertEqual(6, len(sol.dependencies))
     self.assertEqual([
         ('foo/bar', 'svn://example.com/override'),
         ('foo/baz', 'svn://example.com/baz'),
         ('foo/new', 'svn://example.com/new'),
         ('foo/rel', 'svn://example.com/rel'),
+        ('foo/skip', None),
+        ('foo/skip2', None),
     ], [(dep.name, dep.url) for dep in sol.dependencies])
 
   def testDepsOsOverrideDepsInDepsFile(self):
@@ -1196,6 +1042,7 @@ class GclientTest(trial_dir.TestCase):
             custom_vars=None,
             custom_hooks=None,
             deps_file='DEPS',
+            should_process=True,
             should_recurse=True,
             relative=False,
             condition=None,
@@ -1211,6 +1058,7 @@ class GclientTest(trial_dir.TestCase):
                        'version': 'foo_version'},
             cipd_root=cipd_root,
             custom_vars=None,
+            should_process=True,
             relative=False,
             condition='fake_condition'),
         gclient.CipdDependency(
@@ -1220,6 +1068,7 @@ class GclientTest(trial_dir.TestCase):
                        'version': 'bar_version'},
             cipd_root=cipd_root,
             custom_vars=None,
+            should_process=True,
             relative=False,
             condition='fake_condition'),
       ],