Make 'gclient_gn_args*' handling consistent between sync and flatten.

Previously, 'gclient sync' would process "gn_args" settings in any
recursed DEPS files, potentially producing multiple output files or
conflicting output files, but 'gclient flatten' would only ever include
one set of "gn_args" settings in the flattened output, and only if they
occurred in the top-level DEPS file.

This makes 'gclient sync' and 'gclient flatten' more consistent by
restricting them both to a single instance of those setting, and
requiring those setting to be defined either in the top-level DEPS file,
or in a recursedeps file specificed by the new 'gclient_gn_args_from'
setting.

R=dpranke@google.com, ehmaldonado@google.com

Bug: 825063
Change-Id: If90d952e47367c50b36daade16a26b29aec0c9db
Reviewed-on: https://chromium-review.googlesource.com/1039870
Reviewed-by: Michael Moss <mmoss@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Michael Moss <mmoss@chromium.org>

gclient.py

@@ -397,6 +397,7 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
     # hosts will be allowed. Non-empty set means whitelist of hosts.
     # allowed_hosts var is scoped to its DEPS file, and so it isn't recursive.
     self._allowed_hosts = frozenset()
+    self._gn_args_from = None
     # Spec for .gni output to write (if any).
     self._gn_args_file = None
     self._gn_args = []
@@ -785,8 +786,14 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
             'ParseDepsFile(%s): allowed_hosts must be absent '
             'or a non-empty iterable' % self.name)
 
+    self._gn_args_from = local_scope.get('gclient_gn_args_from')
     self._gn_args_file = local_scope.get('gclient_gn_args_file')
     self._gn_args = local_scope.get('gclient_gn_args', [])
+    # It doesn't make sense to set all of these, since setting gn_args_from to
+    # another DEPS will make gclient ignore any other local gn_args* settings.
+    assert not (self._gn_args_from and self._gn_args_file), \
+        'Only specify one of "gclient_gn_args_from" or ' \
+        '"gclient_gn_args_file + gclient_gn_args".'
 
     self._vars = local_scope.get('vars', {})
     if self.parent:
@@ -837,6 +844,10 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
           rel_deps[
               os.path.normpath(os.path.join(rel_prefix, depname))] = options
         self.recursedeps = rel_deps
+    # To get gn_args from another DEPS, that DEPS must be recursed into.
+    if self._gn_args_from:
+      assert self.recursedeps and self._gn_args_from in self.recursedeps, \
+              'The "gclient_gn_args_from" value must be in recursedeps.'
 
     # If present, save 'target_os' in the local_target_os property.
     if 'target_os' in local_scope:
@@ -1167,12 +1178,6 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       result.extend(s.GetHooks(options))
     return result
 
-  def WriteGNArgsFilesRecursively(self, dependencies):
-    for dep in dependencies:
-      if dep.HasGNArgsFile():
-        dep.WriteGNArgsFile()
-      self.WriteGNArgsFilesRecursively(dep.dependencies)
-
   def RunHooksRecursively(self, options, progress):
     assert self.hooks_ran == False
     self._hooks_ran = True
@@ -1718,9 +1723,14 @@ it or fix the checkout.
       self._cipd_root.run(command)
 
     # Once all the dependencies have been processed, it's now safe to write
-    # out any gn_args_files and run the hooks.
+    # out the gn_args_file and run the hooks.
     if command == 'update':
-      self.WriteGNArgsFilesRecursively(self.dependencies)
+      gn_args_dep = self.dependencies[0]
+      if gn_args_dep._gn_args_from:
+        deps_map = dict([(dep.name, dep) for dep in gn_args_dep.dependencies])
+        gn_args_dep = deps_map.get(gn_args_dep._gn_args_from)
+      if gn_args_dep and gn_args_dep.HasGNArgsFile():
+        gn_args_dep.WriteGNArgsFile()
 
     if not self._options.nohooks:
       if should_show_progress:
@@ -2229,10 +2239,10 @@ class Flattener(object):
       for dep in os_deps.itervalues():
         add_deps_file(dep)
 
+    gn_args_dep = self._deps.get(self._client.dependencies[0]._gn_args_from,
+                                 self._client.dependencies[0])
     self._deps_string = '\n'.join(
-        _GNSettingsToLines(
+        _GNSettingsToLines(gn_args_dep._gn_args_file, gn_args_dep._gn_args) +
-            self._client.dependencies[0]._gn_args_file,
-            self._client.dependencies[0]._gn_args) +
         _AllowedHostsToLines(self._allowed_hosts) +
         _DepsToLines(self._deps) +
         _DepsOsToLines(self._deps_os) +

gclient_eval.py

@@ -139,6 +139,11 @@ _GCLIENT_SCHEMA = schema.Schema(_NodeDictSchema({
         schema.Optional(basestring): _GCLIENT_DEPS_SCHEMA,
     }),
 
+    # Dependency to get gclient_gn_args* settings from. This allows these values
+    # to be set in a recursedeps file, rather than requiring that they exist in
+    # the top-level solution.
+    schema.Optional('gclient_gn_args_from'): basestring,
+
     # Path to GN args file to write selected variables.
     schema.Optional('gclient_gn_args_file'): basestring,
 

recipes/trigger_recipe_roller.txt

@@ -6,3 +6,5 @@ All they had was one poor machine under a desk, and its name was Batty,
 the Build and Test Yeti.
 
 😒
+
+The End

testing_support/fake_repos.py

@@ -620,6 +620,13 @@ deps_os ={
 
     self._commit_git('repo_9', {
       'DEPS': """
+vars = {
+  'str_var': 'xyz',
+}
+gclient_gn_args_file = 'src/repo2/gclient.args'
+gclient_gn_args = [
+  'str_var',
+]
 deps = {
   'src/repo8': '/repo_8',
 
@@ -644,6 +651,7 @@ recursedeps = [
 
     self._commit_git('repo_10', {
       'DEPS': """
+gclient_gn_args_from = 'src/repo9'
 deps = {
   'src/repo9': '/repo_9',
 

tests/gclient_smoketest.py

@@ -1219,6 +1219,8 @@ class GClientSmokeGIT(GClientSmokeBase):
       deps_contents = f.read()
 
     self.assertEqual([
+        'gclient_gn_args_file = "src/repo2/gclient.args"',
+        "gclient_gn_args = ['str_var']",
         'deps = {',
         '  # src',
         '  "src": {',
@@ -1297,6 +1299,12 @@ class GClientSmokeGIT(GClientSmokeBase):
         '',
         '}',
         '',
+        'vars = {',
+        '  # src -> src/repo9',
+        '  "str_var": \'xyz\',',
+        '',
+        '}',
+        '',
         '# git://127.0.0.1:20000/git/repo_11, DEPS',
         '# git://127.0.0.1:20000/git/repo_8, DEPS',
         '# git://127.0.0.1:20000/git/repo_9, DEPS',