gclient flatten: do not recurse into deps that should not be recursed

This finally makes the code structure seem right as well: there's just
one method (_flatten_dep), with a simpler control flow.

Also added a regression test.

Bug: 570091
Change-Id: I22ac7a3af0429a7ffd874b4b1715c0f6c72e0006
Reviewed-on: https://chromium-review.googlesource.com/608241
Reviewed-by: Michael Moss <mmoss@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>

gclient.py

@@ -1762,7 +1762,8 @@ class Flattener(object):
     """
     self._allowed_hosts.update(dep.allowed_hosts)
 
-    assert dep.name not in self._deps
+    assert dep.name not in self._deps or self._deps[dep.name] == dep, (
+        dep.name, self._deps.get(dep.name))
     self._deps[dep.name] = dep
 
     for key, value in dep.get_vars().iteritems():
@@ -1775,14 +1776,18 @@ class Flattener(object):
     self._pre_deps_hooks.extend([(dep, hook) for hook in dep.pre_deps_hooks])
 
     for sub_dep in dep.dependencies:
-      assert sub_dep.name not in self._deps
+      assert (sub_dep.name not in self._deps or
+              self._deps[sub_dep.name] == sub_dep), (
+                 dep, sub_dep, self._deps.get(sub_dep.name))
       self._deps[sub_dep.name] = sub_dep
 
     for hook_os, os_hooks in dep.os_deps_hooks.iteritems():
       self._hooks_os.setdefault(hook_os, []).extend(
           [(dep, hook) for hook in os_hooks])
 
-    self._add_deps_os(dep)
+    for dep_os, os_deps in dep.os_dependencies.iteritems():
+      for os_dep in os_deps:
+        self._deps_os.setdefault(dep_os, {})[os_dep.name] = os_dep
 
     # Process recursedeps.
     deps_by_name = dict((d.name, d) for d in dep.dependencies)
@@ -1794,20 +1799,7 @@ class Flattener(object):
         if os_dep.name not in deps_by_name:
           deps_by_name[os_dep.name] = os_dep
     for recurse_dep_name in (dep.recursedeps or []):
-      for sub_dep in deps_by_name[recurse_dep_name].dependencies:
+      self._flatten_dep(deps_by_name[recurse_dep_name])
-        self._flatten_dep(sub_dep)
-
-      self._add_deps_os(deps_by_name[recurse_dep_name])
-
-  def _add_deps_os(self, dep):
-    """Helper for flatten that collects deps_os from |dep|.
-
-    Arguments:
-      dep (Dependency): dependency to process
-    """
-    for dep_os, os_deps in dep.os_dependencies.iteritems():
-      for os_dep in os_deps:
-        self._deps_os.setdefault(dep_os, {})[os_dep.name] = os_dep
 
 
 def CMDflatten(parser, args):

testing_support/fake_repos.py

@@ -562,6 +562,11 @@ deps_os ={
       'DEPS': """
 deps = {
   'src/repo8': '/repo_8',
+
+  # This entry should appear in flattened file,
+  # but not recursed into, since it's not
+  # in recursedeps.
+  'src/repo7': '/repo_7',
 }
 recursedeps = [
   'src/repo8',

tests/gclient_smoketest.py

@@ -893,6 +893,11 @@ class GClientSmokeGIT(GClientSmokeBase):
         '    "url": "/repo_6",',
         '  },',
         '',
+        '  # src -> src/repo9 -> src/repo7',
+        '  "src/repo7": {',
+        '    "url": "/repo_7",',
+        '  },',
+        '',
         '  # src -> src/repo9 -> src/repo8',
         '  "src/repo8": {',
         '    "url": "/repo_8",',