From ce9f17f55a5452e3b458c350ebf5f5eeeefa7c32 Mon Sep 17 00:00:00 2001 From: Michael Moss Date: Wed, 31 Jan 2018 13:16:35 -0800 Subject: [PATCH] Allow custom_vars to override vars in flattened output. Flattening currently ignores custom_vars, but it looks like that was just an attempt to avoid including all the runtime-calculated values (e.g. the host OS) in the flattened output, and not because there's any real issue with supporting explicit overrides. This is useful for changing default values in the flattened buildspecs (e.g. to enable internal checkouts by default). R=dpranke@google.com, iannucci@google.com Bug: 570091,807318 Change-Id: I6d743f33cf7e3e202d5e68d21657f74f0e4c001b Reviewed-on: https://chromium-review.googlesource.com/895927 Reviewed-by: Dirk Pranke Commit-Queue: Michael Moss --- gclient.py | 23 +++++++++++++++++++---- tests/gclient_smoketest.py | 11 ++++++++--- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/gclient.py b/gclient.py index 814bf5992..ed61ff8bf 100755 --- a/gclient.py +++ b/gclient.py @@ -2088,12 +2088,27 @@ class Flattener(object): self._allowed_hosts.update(dep.allowed_hosts) - # Only include vars listed in the DEPS files, not possible local overrides. + # Only include vars explicitly listed in the DEPS files or gclient solution, + # not automatic, local overrides (i.e. not all of dep.get_vars()). + hierarchy = dep.hierarchy(include_url=False) for key, value in dep._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. assert key not in self._vars or self._vars[key][1] == value - self._vars[key] = (dep, value) + self._vars[key] = (hierarchy, value) + # Override explicit custom variables. + for key, value in dep.custom_vars.iteritems(): + # Do custom_vars that don't correspond to DEPS vars ever make sense? DEPS + # conditionals shouldn't be using vars that aren't also defined in the + # DEPS (presubmit actually disallows this), so any new custom_var must be + # unused in the DEPS, so no need to add it to the flattened output either. + if key not in self._vars: + continue + # Don't "override" existing vars if it's actually the same value. + elif self._vars[key][1] == value: + continue + # Anything else is overriding a default value from the DEPS. + self._vars[key] = (hierarchy + ' [custom_var override]', value) self._pre_deps_hooks.extend([(dep, hook) for hook in dep.pre_deps_hooks]) @@ -2292,9 +2307,9 @@ def _VarsToLines(variables): return [] s = ['vars = {'] for key, tup in sorted(variables.iteritems()): - dep, value = tup + hierarchy, value = tup s.extend([ - ' # %s' % dep.hierarchy(include_url=False), + ' # %s' % hierarchy, ' "%s": %r,' % (key, value), '', ]) diff --git a/tests/gclient_smoketest.py b/tests/gclient_smoketest.py index ebfbb4f68..65dbfa86a 100755 --- a/tests/gclient_smoketest.py +++ b/tests/gclient_smoketest.py @@ -653,7 +653,12 @@ class GClientSmokeGIT(GClientSmokeBase): output_deps = os.path.join(self.root_dir, 'DEPS.flattened') self.assertFalse(os.path.exists(output_deps)) - self.gclient(['config', self.git_base + 'repo_6', '--name', 'src']) + self.gclient(['config', self.git_base + 'repo_6', '--name', 'src', + # This should be ignored because 'custom_true_var' isn't + # defined in the DEPS. + '--custom-var', 'custom_true_var=True', + # This should override 'true_var=True' from the DEPS. + '--custom-var', 'true_var="0"']) self.gclient(['sync']) self.gclient(['flatten', '-v', '-v', '-v', '--output-deps', output_deps]) @@ -810,8 +815,8 @@ class GClientSmokeGIT(GClientSmokeBase): ' # src', ' "true_str_var": \'True\',', '', - ' # src', - ' "true_var": True,', + ' # src [custom_var override]', + ' "true_var": \'0\',', '', '}', '',