Skip to content

Commit

Permalink
set compute_grit_inputs_for_analyze=True in the analyze step.
Browse files Browse the repository at this point in the history
Normally when grit actions are defined during `gn gen`, we don't
include all of the files that the .grd file depends on as inputs,
because we don't need to; grit generates a .d file for dependencies
so that rebuilds will happen correctly. And, computing the 
dependencies is slow (it's equivalent to calling grit).

However, if we don't include the dependencies in the gn build graph,
then `analyze` doesn't know about them, and hence a change to a file
that grit dependencies on will be ignored.

This CL changes things such that when `gn gen` is being run as part
of the `analyze` step, we do set the extra flag to compute the
dependencies. This'll slow GN down, but that's better than 
misreporting dependencies.

In some cases, the .grd files try to include generated files
(which may not exist at GN time). To work around this, you can
mark the grit targets as `source_is_generated = true`; the
downside of that is that that target will be skipped just as if
compute_grit_inputs_for_analyze=False, however, this is still an
improvement over not checking *any* of the grit targets. A better
fix would be to have grit_info ignore any paths in the generated
directory if they don't exist, and I'll try to do that in a 
follow-up patch.

R=brettw@chromium.org
BUG=639328

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9f6702b06f8a41a159e4599bf192db0abf9e77eb
Reviewed-on: https://chromium-review.googlesource.com/545150
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487135}
  • Loading branch information
dpranke authored and Commit Bot committed Jul 17, 2017
1 parent 3f8a63c commit a3727f9
Show file tree
Hide file tree
Showing 12 changed files with 52 additions and 7 deletions.
10 changes: 10 additions & 0 deletions android_webview/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ grit("generate_aw_strings") {
grit("generate_webui_resources") {
source = "../ui/webui/resources/webui_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

# webui/resources has way too many resources. The whitelist is trim this down
# to a reasonable size
whitelist = rebase_path("ui/grit_resources_whitelist.txt")
Expand All @@ -282,6 +285,9 @@ grit("generate_webui_resources") {
grit("generate_components_resources") {
source = "../components/resources/components_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

# See :generate_webui_resources for an explanation of the whitelist
whitelist = rebase_path("ui/grit_resources_whitelist.txt")
inputs = [
Expand All @@ -295,6 +301,10 @@ grit("generate_components_resources") {
"grit/components_resources.h",
"components_resources.pak",
]

deps = [
"//components/resources:components_resources",
]
}

grit("generate_components_strings") {
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4244,6 +4244,9 @@ proto_library("resource_prefetch_predictor_proto") {
grit("resources") {
source = "browser_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

defines = chrome_grit_defines
if (enable_hangout_services_extension) {
defines += [ "enable_hangout_services_extension" ]
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/resources/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ if (!is_android) {
grit("settings_resources") {
if (use_vulcanize) {
source = "settings/settings_resources_vulcanized.grd"

# The .grd contains references to generated files.
source_is_generated = true

deps = [
"//chrome/browser/resources/settings:build",
]
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/resources/settings/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ polymer_css_build("build") {

grit("flattened_resources") {
source = "settings_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

defines = chrome_grit_defines
outputs = [
"grit/settings_resources.h",
Expand Down
4 changes: 4 additions & 0 deletions content/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ if (is_component_build) {

grit("resources") {
source = "content_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

outputs = [
"grit/content_resources.h",
"content_resources.pak",
Expand Down
4 changes: 4 additions & 0 deletions ios/web/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,10 @@ js_compile_checked("js_resources") {

grit("resources") {
source = "ios_web_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

outputs = [
"grit/ios_web_resources.h",
"ios_web_resources.pak",
Expand Down
4 changes: 4 additions & 0 deletions ios/web/shell/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ service_manifest("shell_browser_manifest_overlay") {
grit("resources") {
visibility = [ ":*" ]
source = "shell_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

grit_flags = [
"-E",
"root_gen_dir=" + rebase_path(root_gen_dir, root_build_dir),
Expand Down
4 changes: 4 additions & 0 deletions ios/web/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ repack("packed_resources") {

grit("resources") {
source = "test_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

inputs = [
"${root_gen_dir}/ios/web/test/mojo_test.mojom.js",
]
Expand Down
1 change: 1 addition & 0 deletions tools/grit/grit_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def DoMain(argv):
# grit build also supports '-E KEY=VALUE', support that to share command
# line flags.
parser.add_option("-E", action="append", dest="build_env", default=[])
parser.add_option("-p", action="store", dest="predetermined_ids_file")
parser.add_option("-w", action="append", dest="whitelist_files", default=[])
parser.add_option("--output-all-resource-defines", action="store_true",
dest="output_all_resource_defines", default=True,
Expand Down
11 changes: 6 additions & 5 deletions tools/grit/grit_rule.gni
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
# Path to .grd file.
#
# source_is_generated (optional, defaults to false)
# Declares that the input 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 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.
# 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
# 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.
#
# inputs (optional)
# List of additional files, required for grit to process source file.
Expand Down
6 changes: 4 additions & 2 deletions tools/mb/mb.py
Original file line number Diff line number Diff line change
Expand Up @@ -806,11 +806,13 @@ def ClobberIfNeeded(self, vals):
self.MaybeMakeDirectory(build_dir)
self.WriteFile(mb_type_path, new_mb_type)

def RunGNGen(self, vals):
def RunGNGen(self, vals, compute_grit_inputs_for_analyze=False):
build_dir = self.args.path[0]

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'

# Since GN hasn't run yet, the build directory may not even exist.
self.MaybeMakeDirectory(self.ToAbsPath(build_dir))
Expand Down Expand Up @@ -1233,7 +1235,7 @@ def GYPCmd(self, output_dir, vals):
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)
ret = self.RunGNGen(vals, compute_grit_inputs_for_analyze=True)
if ret:
return ret

Expand Down
4 changes: 4 additions & 0 deletions ui/resources/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ grit("ui_unscaled_resources_grd") {

grit("webui_resources_grd") {
source = "../webui/resources/webui_resources.grd"

# The .grd contains references to generated files.
source_is_generated = true

outputs = [
"grit/webui_resources.h",
"grit/webui_resources_map.cc",
Expand Down

0 comments on commit a3727f9

Please sign in to comment.