Make "gn analyze" know about all inputs for jinja_template()

* Renames compute_grit_inputs_for_analyze -> compute_inputs_for_analyze
* Computes .py inputs via exec_script (when compute_inputs_for_analyze=true)
* Enforce that sub-included .jinja files are listed in targets.

Bug: 843562
Change-Id: I3698c7846375530a86474bd5960bb002ac45f71a
Reviewed-on: https://chromium-review.googlesource.com/1112867
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570650}

build/android/gyp/jinja_template.py

@@ -106,7 +106,9 @@ def _ParseVariables(variables_arg, error_func):
 def main():
   parser = argparse.ArgumentParser()
   parser.add_argument('--inputs', required=True,
-                      help='The template files to process.')
+                      help='GN-list of template files to process.')
+  parser.add_argument('--includes', default='',
+                      help="GN-list of files that get {% include %}'ed.")
   parser.add_argument('--output', help='The output file to generate. Valid '
                       'only if there is a single input.')
   parser.add_argument('--outputs-zip', help='A zip file for the processed '
@@ -126,6 +128,7 @@ def main():
   options = parser.parse_args()
 
   inputs = build_utils.ParseGnList(options.inputs)
+  includes = build_utils.ParseGnList(options.includes)
 
   if (options.output is None) == (options.outputs_zip is None):
     parser.error('Exactly one of --output and --output-zip must be given')
@@ -145,8 +148,15 @@ def main():
 
   if options.depfile:
     output = options.output or options.outputs_zip
-    deps = processor.GetLoadedTemplates()
-    build_utils.WriteDepfile(options.depfile, output, deps)
+    all_inputs = set(processor.GetLoadedTemplates())
+    all_inputs.difference_update(inputs)
+    all_inputs.difference_update(includes)
+    if all_inputs:
+      # TODO(agrieve): Change this to an exception once downstream violations
+      #     are fixed. https://crbug.com/843562
+      print ('Found files not listed via --includes:\n' +
+             '\n'.join(sorted(all_inputs)))
+    build_utils.WriteDepfile(options.depfile, output)
 
 
 if __name__ == '__main__':

build/config/android/rules.gni

@@ -8,6 +8,7 @@ import("//build/config/android/config.gni")
 import("//build/config/android/internal_rules.gni")
 import("//build/config/clang/clang.gni")
 import("//build/config/compiler/compiler.gni")
+import("//build/config/compute_inputs_for_analyze.gni")
 import("//build/config/dcheck_always_on.gni")
 import("//build/toolchain/toolchain.gni")
 
@@ -594,10 +595,24 @@ if (enable_java_templates) {
     }
   }
 
+  if (compute_inputs_for_analyze) {
+    _jinja_template_pydeps =
+        exec_script("//build/print_python_deps.py",
+                    [
+                      rebase_path("//build/android/gyp/jinja_template.py"),
+                      "--no-header",
+                      "--gn-paths",
+                      "--root",
+                      rebase_path("//", root_build_dir),
+                    ],
+                    "list lines")
+  }
+
   # Declare a target for processing a Jinja template.
   #
   # Variables
   #   input: The template file to be processed.
+  #   includes: List of files {% include %}'ed by input.
   #   output: Where to save the result.
   #   variables: (Optional) A list of variables to make available to the template
   #     processing environment, e.g. ["name=foo", "color=red"].
@@ -620,6 +635,12 @@ if (enable_java_templates) {
       inputs = [
         invoker.input,
       ]
+      if (defined(invoker.includes)) {
+        inputs += invoker.includes
+      }
+      if (compute_inputs_for_analyze) {
+        inputs += _jinja_template_pydeps
+      }
       script = "//build/android/gyp/jinja_template.py"
       depfile = "$target_gen_dir/$target_name.d"
 
@@ -637,6 +658,10 @@ if (enable_java_templates) {
         "--depfile",
         rebase_path(depfile, root_build_dir),
       ]
+      if (defined(invoker.includes)) {
+        _rebased_includes = rebase_path(invoker.includes, root_build_dir)
+        args += [ "--includes=$_rebased_includes" ]
+      }
       if (defined(invoker.variables)) {
         args += [ "--variables=${invoker.variables}" ]
       }

build/config/compute_inputs_for_analyze.gni

@@ -0,0 +1,14 @@
+# Copyright 2018 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+declare_args() {
+  # Enable this flag when running "gn analyze".
+  #
+  # This causes some gn actions to compute inputs immediately (via exec_script)
+  # where they would normally compute them only when executed (and write them to
+  # a depfile).
+  #
+  # This flag will slow down GN, but is required for analyze to work properly.
+  compute_inputs_for_analyze = false
+}

build/print_python_deps.py

@@ -78,6 +78,10 @@ def main():
                       help='Directory to make paths relative to.')
   parser.add_argument('--output',
                       help='Write output to a file rather than stdout.')
+  parser.add_argument('--no-header', action='store_true',
+                      help='Do not write the "# Generated by" header.')
+  parser.add_argument('--gn-paths', action='store_true',
+                      help='Write paths as //foo/bar/baz.py')
   parser.add_argument('--whitelist', default=[], action='append',
                       dest='whitelists',
                       help='Recursively include all non-test python files '
@@ -97,10 +101,12 @@ def main():
   normalized_cmdline = _NormalizeCommandLine(options)
   out = open(options.output, 'w') if options.output else sys.stdout
   with out:
-    out.write('# Generated by running:\n')
-    out.write('#   %s\n' % normalized_cmdline)
+    if not options.no_header:
+      out.write('# Generated by running:\n')
+      out.write('#   %s\n' % normalized_cmdline)
+    prefix = '//' if options.gn_paths else ''
     for path in sorted(paths):
-      out.write(path + '\n')
+      out.write(prefix + path + '\n')
 
 
 if __name__ == '__main__':

chrome/android/BUILD.gn

@@ -74,6 +74,10 @@ jinja_template("chrome_modern_public_android_manifest") {
 
 jinja_template("monochrome_public_android_manifest") {
   input = "java/AndroidManifest_monochrome.xml"
+  includes = [
+    "//android_webview/apk/java/AndroidManifest.xml",
+    "java/AndroidManifest.xml",
+  ]
   output = monochrome_public_android_manifest
   variables = chrome_public_jinja_variables +
               monochrome_android_manifest_jinja_variables +

tools/grit/grit_rule.gni

@@ -13,7 +13,7 @@
 #   source_is_generated (optional, defaults to false)
 #       Declares that the input (or one of its dependencies) is generated so
 #       that grit_info is not run on the file when
-#       compute_grit_inputs_for_analyze is set (because the file will not
+#       compute_inputs_for_analyze is set (because the file will not
 #       exist at analyze time). For "analyze" to be correct in this case, the
 #       inputs to the grit file must either be listed in the inputs variable
 #       or are listed as inputs to dependencies of this target.
@@ -86,6 +86,7 @@
 #   }
 import("//build/config/android/config.gni")
 import("//build/config/chrome_build.gni")
+import("//build/config/compute_inputs_for_analyze.gni")
 import("//build/config/crypto.gni")
 import("//build/config/features.gni")
 import("//build/config/ui.gni")
@@ -94,18 +95,6 @@ import("//third_party/closure_compiler/closure_args.gni")
 assert(!enable_resource_whitelist_generation || is_android,
        "resource whitelist generation only works on android")
 
-declare_args() {
-  # When set, this will fork out to grit at GN-time to get all inputs
-  # referenced by the .grd file.
-  #
-  # Normal builds don't need this since proper incremental builds are handled
-  # by grit writing out the inputs in .d files at build-time. But for analyze
-  # to work on the bots, it needs to know about all input files at GN-time so
-  # it knows which tests to trigger when something changes. This option will
-  # slow down the GN run.
-  compute_grit_inputs_for_analyze = false
-}
-
 grit_defines = []
 
 # Mac and iOS want Title Case strings.
@@ -415,7 +404,7 @@ template("grit") {
     # Must be after the args are computed since they are re-used.
     # See the comments for the two variables used in this condition for
     # why this works this way.
-    if (compute_grit_inputs_for_analyze && !source_is_generated) {
+    if (compute_inputs_for_analyze && !source_is_generated) {
       grit_info_script = "//tools/grit/grit_info.py"
       grit_info_args = [
                          "--inputs",

tools/mb/mb.py

@@ -739,13 +739,13 @@ def FlattenMixins(self, mixins, vals, visited):
         self.FlattenMixins(mixin_vals['mixins'], vals, visited)
     return vals
 
-  def RunGNGen(self, vals, compute_grit_inputs_for_analyze=False):
+  def RunGNGen(self, vals, compute_inputs_for_analyze=False):
     build_dir = self.args.path
 
     cmd = self.GNCmd('gen', build_dir, '--check')
     gn_args = self.GNArgs(vals)
-    if compute_grit_inputs_for_analyze:
-      gn_args += ' compute_grit_inputs_for_analyze=true'
+    if compute_inputs_for_analyze:
+      gn_args += ' compute_inputs_for_analyze=true'
 
     # Since GN hasn't run yet, the build directory may not even exist.
     self.MaybeMakeDirectory(self.ToAbsPath(build_dir))
@@ -1078,7 +1078,7 @@ def ToSrcRelPath(self, path):
   def RunGNAnalyze(self, vals):
     # Analyze runs before 'gn gen' now, so we need to run gn gen
     # in order to ensure that we have a build directory.
-    ret = self.RunGNGen(vals, compute_grit_inputs_for_analyze=True)
+    ret = self.RunGNGen(vals, compute_inputs_for_analyze=True)
     if ret:
       return ret