From 11eb715369bbda0e1d873614e8b0dcbf0b5ae80b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Hajdan=2C=20Jr?= <phajdan.jr@chromium.org>
Date: Thu, 10 Aug 2017 12:50:11 +0200
Subject: [PATCH] gclient flatten: do not add deps_os entries to deps, even
 with recursedeps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Bug: 570091
Change-Id: I8e67d617c2738459634aabbffd22135ee5d27cc4
Reviewed-on: https://chromium-review.googlesource.com/609065
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
---
 gclient.py                    | 20 ++++++++++++--------
 testing_support/fake_repos.py |  8 ++++++++
 tests/gclient_smoketest.py    |  8 ++++++++
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/gclient.py b/gclient.py
index 1c81dcbef7..99f50a027f 100755
--- a/gclient.py
+++ b/gclient.py
@@ -1727,6 +1727,7 @@ class Flattener(object):
           in DEPS
     """
     for solution in self._client.dependencies:
+      self._add_dep(solution)
       self._flatten_dep(solution)
 
     if pin_all_deps:
@@ -1754,6 +1755,16 @@ class Flattener(object):
         _VarsToLines(self._vars) +
         [''])  # Ensure newline at end of file.
 
+  def _add_dep(self, dep):
+    """Helper to add a dependency to flattened DEPS.
+
+    Arguments:
+      dep (Dependency): dependency to add
+    """
+    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
+
   def _flatten_dep(self, dep):
     """Visits a dependency in order to flatten it (see CMDflatten).
 
@@ -1762,10 +1773,6 @@ class Flattener(object):
     """
     self._allowed_hosts.update(dep.allowed_hosts)
 
-    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():
       # Make sure there are no conflicting variables. It is fine however
       # to use same variable name, as long as the value is consistent.
@@ -1776,10 +1783,7 @@ 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 or
-              self._deps[sub_dep.name] == sub_dep), (
-                 dep, sub_dep, self._deps.get(sub_dep.name))
-      self._deps[sub_dep.name] = sub_dep
+      self._add_dep(sub_dep)
 
     for hook_os, os_hooks in dep.os_deps_hooks.iteritems():
       self._hooks_os.setdefault(hook_os, []).extend(
diff --git a/testing_support/fake_repos.py b/testing_support/fake_repos.py
index 6d386d1d61..10d3e9ffb4 100755
--- a/testing_support/fake_repos.py
+++ b/testing_support/fake_repos.py
@@ -568,7 +568,15 @@ deps = {
   # in recursedeps.
   'src/repo7': '/repo_7',
 }
+deps_os = {
+  'android': {
+    # This entry should only appear in flattened |deps_os|,
+    # not |deps|, even when used with |recursedeps|.
+    'src/repo4': '/repo_4',
+  }
+}
 recursedeps = [
+  'src/repo4',
   'src/repo8',
 ]""",
       'origin': 'git/repo_9@1\n',
diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py
index f6446b0464..fae17ed0f4 100755
--- a/tests/gclient_smoketest.py
+++ b/tests/gclient_smoketest.py
@@ -911,6 +911,14 @@ class GClientSmokeGIT(GClientSmokeBase):
         '}',
         '',
         'deps_os = {',
+        '  "android": {',
+        '    # src -> src/repo9 -> src/repo4',
+        '    "src/repo4": {',
+        '      "url": "/repo_4",',
+        '    },',
+        '',
+        '  },',
+        '',
         '  "mac": {',
         '    # src -> src/repo9 -> src/repo8 -> src/recursed_os_repo',
         '    "src/recursed_os_repo": {',