From adae2a62f752852dde2054c23b205a5dc0cd994f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Hajdan=2C=20Jr?= Date: Fri, 18 Aug 2017 16:49:57 +0200 Subject: [PATCH] gclient flatten: correctness fixes for OS-specific recursedeps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to pass OS info to recursively called _flatten_dep. Regular deps entries in OS-specific DEPS file recursed into need to be marked as OS-specific in the flattened file. Bug: 570091 Change-Id: If3055b84143d8a52d10d8753113893b5054b4d07 Reviewed-on: https://chromium-review.googlesource.com/621046 Reviewed-by: Michael Moss Commit-Queue: Paweł Hajdan Jr. --- gclient.py | 58 ++++++++++++++++++++++++++--------- testing_support/fake_repos.py | 5 ++- tests/gclient_smoketest.py | 20 +++++++++--- 3 files changed, 62 insertions(+), 21 deletions(-) diff --git a/gclient.py b/gclient.py index bc1df6e5b..84010b33a 100755 --- a/gclient.py +++ b/gclient.py @@ -1794,12 +1794,29 @@ class Flattener(object): if dep.url: self._deps[dep.name] = dep - def _flatten_dep(self, dep): + def _add_os_dep(self, os_dep, dep_os): + """Helper to add an OS-specific dependency to flattened DEPS. + + Arguments: + os_dep (Dependency): dependency to add + dep_os (str): name of the OS + """ + assert ( + os_dep.name not in self._deps_os.get(dep_os, {}) or + self._deps_os.get(dep_os, {}).get(os_dep.name) == os_dep), ( + os_dep.name, self._deps_os.get(dep_os, {}).get(os_dep.name)) + if os_dep.url: + self._deps_os.setdefault(dep_os, {})[os_dep.name] = os_dep + + def _flatten_dep(self, dep, dep_os=None): """Visits a dependency in order to flatten it (see CMDflatten). Arguments: dep (Dependency): dependency to process + dep_os (str or None): name of the OS |dep| is specific to """ + logging.debug('_flatten_dep(%s, %s)', dep.name, dep_os) + if not dep.deps_parsed: dep.ParseDepsFile() @@ -1811,31 +1828,42 @@ class Flattener(object): assert key not in self._vars or self._vars[key][1] == value self._vars[key] = (dep, value) - self._hooks.extend([(dep, hook) for hook in dep.deps_hooks]) self._pre_deps_hooks.extend([(dep, hook) for hook in dep.pre_deps_hooks]) + if dep_os: + if dep.deps_hooks: + self._hooks_os.setdefault(dep_os, []).extend( + [(dep, hook) for hook in dep.deps_hooks]) + else: + self._hooks.extend([(dep, hook) for hook in dep.deps_hooks]) + for sub_dep in dep.dependencies: - self._add_dep(sub_dep) + if dep_os: + self._add_os_dep(sub_dep, dep_os) + else: + self._add_dep(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]) - for dep_os, os_deps in dep.os_dependencies.iteritems(): + for sub_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) - # Allow recursedeps entries that refer to deps_os entries. - # In case there are multiple entries with the same name, - # we have to pick something - e.g. the first one. - for os_deps in dep.os_dependencies.itervalues(): + self._add_os_dep(os_dep, sub_dep_os) + + # Process recursedeps. |deps_by_name| is a map where keys are dependency + # names, and values are maps of OS names to |Dependency| instances. + # |None| in place of OS name means the dependency is not OS-specific. + deps_by_name = dict((d.name, {None: d}) for d in dep.dependencies) + for sub_dep_os, os_deps in dep.os_dependencies.iteritems(): for os_dep in os_deps: - if os_dep.name not in deps_by_name: - deps_by_name[os_dep.name] = os_dep + assert sub_dep_os not in deps_by_name.get(os_dep.name, {}), ( + os_dep.name, sub_dep_os) + deps_by_name.setdefault(os_dep.name, {})[sub_dep_os] = os_dep for recurse_dep_name in (dep.recursedeps or []): - self._flatten_dep(deps_by_name[recurse_dep_name]) + dep_info = deps_by_name[recurse_dep_name] + for sub_dep_os, os_dep in dep_info.iteritems(): + self._flatten_dep(os_dep, dep_os=(sub_dep_os or dep_os)) def CMDflatten(parser, args): diff --git a/testing_support/fake_repos.py b/testing_support/fake_repos.py index ea74fccc9..06d181247 100755 --- a/testing_support/fake_repos.py +++ b/testing_support/fake_repos.py @@ -594,6 +594,9 @@ deps = { 'src/repo6': '/repo_6', } deps_os = { + 'mac': { + 'src/repo11': '/repo_11', + }, 'ios': { 'src/repo11': '/repo_11', } @@ -608,7 +611,7 @@ recursedeps = [ self._commit_git('repo_11', { 'DEPS': """ deps = { - 'src/repo12': '/repo12', + 'src/repo12': '/repo_12', }""", 'origin': 'git/repo_11@1\n', }) diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index 3a6dce207..e6b514820 100755 --- a/tests/gclient_smoketest.py +++ b/tests/gclient_smoketest.py @@ -892,11 +892,6 @@ class GClientSmokeGIT(GClientSmokeBase): ' "url": "git://127.0.0.1:20000/git/repo_10",', ' },', '', - ' # src -> src/repo11 -> src/repo12', - ' "src/repo12": {', - ' "url": "/repo12",', - ' },', - '', ' # src -> src/repo6', ' "src/repo6": {', ' "url": "/repo_6",', @@ -934,6 +929,11 @@ class GClientSmokeGIT(GClientSmokeBase): ' "url": "/repo_11",', ' },', '', + ' # src -> src/repo11 -> src/repo12', + ' "src/repo12": {', + ' "url": "/repo_12",', + ' },', + '', ' },', '', ' "mac": {', @@ -942,6 +942,16 @@ class GClientSmokeGIT(GClientSmokeBase): ' "url": "/repo_5",', ' },', '', + ' # src -> src/repo11', + ' "src/repo11": {', + ' "url": "/repo_11",', + ' },', + '', + ' # src -> src/repo11 -> src/repo12', + ' "src/repo12": {', + ' "url": "/repo_12",', + ' },', + '', ' },', '', ' "unix": {',