GN: Use implicit dependency for binary input deps
This is to ensure that the input gets rebuilt when the depencies change
even when they are not included in the depfile generated by the
compiler.
BUG=612623
Review-Url: https://codereview.chromium.org/2071573003
Cr-Original-Commit-Position: refs/heads/master@{#400515}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 57ac2eae0f67ef9310e2fad40258b1d45ac82435
diff --git a/tools/gn/docs/reference.md b/tools/gn/docs/reference.md
index 760bfe8..e1ae36e 100644
--- a/tools/gn/docs/reference.md
+++ b/tools/gn/docs/reference.md
@@ -4788,13 +4788,11 @@
files in a target are compiled. So if you depend on generated headers,
you do not typically need to list them in the inputs section.
- Inputs for binary targets will be treated as order-only dependencies,
- meaning that they will be forced up to date before compiling or
- any files in the target, but changes in the inputs will not
- necessarily force the target to compile. This is because it is
- expected that the compiler will report the precise list of input
- dependencies required to recompile each file once the initial build
- is done.
+ Inputs for binary targets will be treated as implicit dependencies,
+ meaning that changes in any of the inputs will force all sources in
+ the target to be recompiled. If an input only applies to a subset of
+ source files, you may want to split those into a separate target to
+ avoid unnecessary recompiles.
```
diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc
index b012e42..9b0458a 100644
--- a/tools/gn/ninja_binary_target_writer.cc
+++ b/tools/gn/ninja_binary_target_writer.cc
@@ -270,6 +270,8 @@
WriteCompilerVars(used_types);
+ OutputFile input_dep = WriteInputsStampAndGetDep();
+
// The input dependencies will be an order-only dependency. This will cause
// Ninja to make sure the inputs are up to date before compiling this source,
// but changes in the inputs deps won't cause the file to be recompiled.
@@ -301,7 +303,7 @@
// |pch_other_files|. This is to prevent linking against them.
std::vector<OutputFile> pch_obj_files;
std::vector<OutputFile> pch_other_files;
- WritePCHCommands(used_types, order_only_dep,
+ WritePCHCommands(used_types, input_dep, order_only_dep,
&pch_obj_files, &pch_other_files);
std::vector<OutputFile>* pch_files = !pch_obj_files.empty() ?
&pch_obj_files : &pch_other_files;
@@ -320,7 +322,7 @@
// object file list.
std::vector<OutputFile> obj_files;
std::vector<SourceFile> other_files;
- WriteSources(*pch_files, order_only_dep, &obj_files, &other_files);
+ WriteSources(*pch_files, input_dep, order_only_dep, &obj_files, &other_files);
// Link all MSVC pch object files. The vector will be empty on GCC toolchains.
obj_files.insert(obj_files.end(), pch_obj_files.begin(), pch_obj_files.end());
@@ -401,6 +403,43 @@
WriteSharedVars(subst);
}
+OutputFile NinjaBinaryTargetWriter::WriteInputsStampAndGetDep() const {
+ CHECK(target_->toolchain())
+ << "Toolchain not set on target "
+ << target_->label().GetUserVisibleName(true);
+
+ if (target_->inputs().size() == 0)
+ return OutputFile(); // No inputs
+
+ // If we only have one input, return it directly instead of writing a stamp
+ // file for it.
+ if (target_->inputs().size() == 1)
+ return OutputFile(settings_->build_settings(), target_->inputs()[0]);
+
+ // Make a stamp file.
+ OutputFile input_stamp_file(
+ RebasePath(GetTargetOutputDir(target_).value(),
+ settings_->build_settings()->build_dir(),
+ settings_->build_settings()->root_path_utf8()));
+ input_stamp_file.value().append(target_->label().name());
+ input_stamp_file.value().append(".inputs.stamp");
+
+ out_ << "build ";
+ path_output_.WriteFile(out_, input_stamp_file);
+ out_ << ": "
+ << GetNinjaRulePrefixForToolchain(settings_)
+ << Toolchain::ToolTypeToName(Toolchain::TYPE_STAMP);
+
+ // File inputs.
+ for (const auto& input : target_->inputs()) {
+ out_ << " ";
+ path_output_.WriteFile(out_, input);
+ }
+
+ out_ << "\n";
+ return input_stamp_file;
+}
+
void NinjaBinaryTargetWriter::WriteOneFlag(
SubstitutionType subst_enum,
bool has_precompiled_headers,
@@ -453,6 +492,7 @@
void NinjaBinaryTargetWriter::WritePCHCommands(
const SourceFileTypeSet& used_types,
+ const OutputFile& input_dep,
const OutputFile& order_only_dep,
std::vector<OutputFile>* object_files,
std::vector<OutputFile>* other_files) {
@@ -466,7 +506,7 @@
WritePCHCommand(SUBSTITUTION_CFLAGS_C,
Toolchain::TYPE_CC,
tool_c->precompiled_header_type(),
- order_only_dep, object_files, other_files);
+ input_dep, order_only_dep, object_files, other_files);
}
const Tool* tool_cxx = target_->toolchain()->GetTool(Toolchain::TYPE_CXX);
if (tool_cxx &&
@@ -475,7 +515,7 @@
WritePCHCommand(SUBSTITUTION_CFLAGS_CC,
Toolchain::TYPE_CXX,
tool_cxx->precompiled_header_type(),
- order_only_dep, object_files, other_files);
+ input_dep, order_only_dep, object_files, other_files);
}
const Tool* tool_objc = target_->toolchain()->GetTool(Toolchain::TYPE_OBJC);
@@ -485,7 +525,7 @@
WritePCHCommand(SUBSTITUTION_CFLAGS_OBJC,
Toolchain::TYPE_OBJC,
tool_objc->precompiled_header_type(),
- order_only_dep, object_files, other_files);
+ input_dep, order_only_dep, object_files, other_files);
}
const Tool* tool_objcxx =
@@ -496,7 +536,7 @@
WritePCHCommand(SUBSTITUTION_CFLAGS_OBJCC,
Toolchain::TYPE_OBJCXX,
tool_objcxx->precompiled_header_type(),
- order_only_dep, object_files, other_files);
+ input_dep, order_only_dep, object_files, other_files);
}
}
@@ -504,16 +544,17 @@
SubstitutionType flag_type,
Toolchain::ToolType tool_type,
Tool::PrecompiledHeaderType header_type,
+ const OutputFile& input_dep,
const OutputFile& order_only_dep,
std::vector<OutputFile>* object_files,
std::vector<OutputFile>* other_files) {
switch (header_type) {
case Tool::PCH_MSVC:
- WriteWindowsPCHCommand(flag_type, tool_type, order_only_dep,
+ WriteWindowsPCHCommand(flag_type, tool_type, input_dep, order_only_dep,
object_files);
break;
case Tool::PCH_GCC:
- WriteGCCPCHCommand(flag_type, tool_type, order_only_dep,
+ WriteGCCPCHCommand(flag_type, tool_type, input_dep, order_only_dep,
other_files);
break;
case Tool::PCH_NONE:
@@ -525,6 +566,7 @@
void NinjaBinaryTargetWriter::WriteGCCPCHCommand(
SubstitutionType flag_type,
Toolchain::ToolType tool_type,
+ const OutputFile& input_dep,
const OutputFile& order_only_dep,
std::vector<OutputFile>* gch_files) {
// Compute the pch output file (it will be language-specific).
@@ -535,10 +577,13 @@
gch_files->insert(gch_files->end(), outputs.begin(), outputs.end());
+ std::vector<OutputFile> extra_deps;
+ if (!input_dep.value().empty())
+ extra_deps.push_back(input_dep);
+
// Build line to compile the file.
WriteCompilerBuildLine(target_->config_values().precompiled_source(),
- std::vector<OutputFile>(), order_only_dep, tool_type,
- outputs);
+ extra_deps, order_only_dep, tool_type, outputs);
// This build line needs a custom language-specific flags value. Rule-specific
// variables are just indented underneath the rule line.
@@ -573,6 +618,7 @@
void NinjaBinaryTargetWriter::WriteWindowsPCHCommand(
SubstitutionType flag_type,
Toolchain::ToolType tool_type,
+ const OutputFile& input_dep,
const OutputFile& order_only_dep,
std::vector<OutputFile>* object_files) {
// Compute the pch output file (it will be language-specific).
@@ -583,10 +629,13 @@
object_files->insert(object_files->end(), outputs.begin(), outputs.end());
+ std::vector<OutputFile> extra_deps;
+ if (!input_dep.value().empty())
+ extra_deps.push_back(input_dep);
+
// Build line to compile the file.
WriteCompilerBuildLine(target_->config_values().precompiled_source(),
- std::vector<OutputFile>(), order_only_dep, tool_type,
- outputs);
+ extra_deps, order_only_dep, tool_type, outputs);
// This build line needs a custom language-specific flags value. Rule-specific
// variables are just indented underneath the rule line.
@@ -604,6 +653,7 @@
void NinjaBinaryTargetWriter::WriteSources(
const std::vector<OutputFile>& pch_deps,
+ const OutputFile& input_dep,
const OutputFile& order_only_dep,
std::vector<OutputFile>* object_files,
std::vector<SourceFile>* other_files) {
@@ -621,6 +671,9 @@
continue; // No output for this source.
}
+ if (!input_dep.value().empty())
+ deps.push_back(input_dep);
+
if (tool_type != Toolchain::TYPE_NONE) {
// Only include PCH deps that correspond to the tool type, for instance,
// do not specify target_name.precompile.cc.obj (a CXX PCH file) as a dep
diff --git a/tools/gn/ninja_binary_target_writer.h b/tools/gn/ninja_binary_target_writer.h
index 1876486..0188720 100644
--- a/tools/gn/ninja_binary_target_writer.h
+++ b/tools/gn/ninja_binary_target_writer.h
@@ -31,6 +31,12 @@
// Writes all flags for the compiler: includes, defines, cflags, etc.
void WriteCompilerVars(const SourceFileTypeSet& used_types);
+ // Writes to the output stream a stamp rule for inputs, and
+ // returns the file to be appended to source rules that encodes the
+ // implicit dependencies for the current target. The returned OutputFile
+ // will be empty if there are no inputs.
+ OutputFile WriteInputsStampAndGetDep() const;
+
// has_precompiled_headers is set when this substitution matches a tool type
// that supports precompiled headers, and this target supports precompiled
// headers. It doesn't indicate if the tool has precompiled headers (this
@@ -55,6 +61,7 @@
// compiling this target. It will be empty if there are no input deps.
void WritePCHCommands(const SourceFileTypeSet& used_types,
const OutputFile& input_dep,
+ const OutputFile& order_only_dep,
std::vector<OutputFile>* object_files,
std::vector<OutputFile>* other_files);
@@ -63,16 +70,19 @@
Toolchain::ToolType tool_type,
Tool::PrecompiledHeaderType header_type,
const OutputFile& input_dep,
+ const OutputFile& order_only_dep,
std::vector<OutputFile>* object_files,
std::vector<OutputFile>* other_files);
void WriteGCCPCHCommand(SubstitutionType flag_type,
Toolchain::ToolType tool_type,
+ const OutputFile& input_dep,
const OutputFile& order_only_dep,
std::vector<OutputFile>* gch_files);
void WriteWindowsPCHCommand(SubstitutionType flag_type,
Toolchain::ToolType tool_type,
+ const OutputFile& input_dep,
const OutputFile& order_only_dep,
std::vector<OutputFile>* object_files);
@@ -84,6 +94,7 @@
//
// The files produced by the compiler will be added to two output vectors.
void WriteSources(const std::vector<OutputFile>& pch_deps,
+ const OutputFile& input_dep,
const OutputFile& order_only_dep,
std::vector<OutputFile>* object_files,
std::vector<SourceFile>* other_files);
diff --git a/tools/gn/ninja_binary_target_writer_unittest.cc b/tools/gn/ninja_binary_target_writer_unittest.cc
index f2297fb..1cfdb56 100644
--- a/tools/gn/ninja_binary_target_writer_unittest.cc
+++ b/tools/gn/ninja_binary_target_writer_unittest.cc
@@ -854,3 +854,85 @@
// Should have issued an error.
EXPECT_TRUE(scheduler.is_failed());
}
+
+// This tests that output extension and output dir overrides apply, and input
+// dependencies are applied.
+TEST(NinjaBinaryTargetWriter, InputFiles) {
+ TestWithScope setup;
+ Err err;
+
+ setup.build_settings()->SetBuildDir(SourceDir("//out/Debug/"));
+
+ // This target has one input.
+ {
+ Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+ target.set_output_type(Target::SOURCE_SET);
+ target.visibility().SetPublic();
+ target.sources().push_back(SourceFile("//foo/input1.cc"));
+ target.sources().push_back(SourceFile("//foo/input2.cc"));
+ target.inputs().push_back(SourceFile("//foo/input.data"));
+ target.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(target.OnResolved(&err));
+
+ std::ostringstream out;
+ NinjaBinaryTargetWriter writer(&target, out);
+ writer.Run();
+
+ const char expected[] =
+ "defines =\n"
+ "include_dirs =\n"
+ "cflags =\n"
+ "cflags_cc =\n"
+ "root_out_dir = .\n"
+ "target_out_dir = obj/foo\n"
+ "target_output_name = bar\n"
+ "\n"
+ "build obj/foo/bar.input1.o: cxx ../../foo/input1.cc"
+ " | ../../foo/input.data\n"
+ "build obj/foo/bar.input2.o: cxx ../../foo/input2.cc"
+ " | ../../foo/input.data\n"
+ "\n"
+ "build obj/foo/bar.stamp: stamp obj/foo/bar.input1.o "
+ "obj/foo/bar.input2.o\n";
+
+ EXPECT_EQ(expected, out.str());
+ }
+
+ // This target has multiple inputs.
+ {
+ Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+ target.set_output_type(Target::SOURCE_SET);
+ target.visibility().SetPublic();
+ target.sources().push_back(SourceFile("//foo/input1.cc"));
+ target.sources().push_back(SourceFile("//foo/input2.cc"));
+ target.inputs().push_back(SourceFile("//foo/input1.data"));
+ target.inputs().push_back(SourceFile("//foo/input2.data"));
+ target.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(target.OnResolved(&err));
+
+ std::ostringstream out;
+ NinjaBinaryTargetWriter writer(&target, out);
+ writer.Run();
+
+ const char expected[] =
+ "defines =\n"
+ "include_dirs =\n"
+ "cflags =\n"
+ "cflags_cc =\n"
+ "root_out_dir = .\n"
+ "target_out_dir = obj/foo\n"
+ "target_output_name = bar\n"
+ "\n"
+ "build obj/foo/bar.inputs.stamp: stamp"
+ " ../../foo/input1.data ../../foo/input2.data\n"
+ "build obj/foo/bar.input1.o: cxx ../../foo/input1.cc"
+ " | obj/foo/bar.inputs.stamp\n"
+ "build obj/foo/bar.input2.o: cxx ../../foo/input2.cc"
+ " | obj/foo/bar.inputs.stamp\n"
+ "\n"
+ "build obj/foo/bar.stamp: stamp obj/foo/bar.input1.o "
+ "obj/foo/bar.input2.o\n";
+
+ EXPECT_EQ(expected, out.str());
+ }
+}
diff --git a/tools/gn/ninja_target_writer.cc b/tools/gn/ninja_target_writer.cc
index 486930b..7843d75 100644
--- a/tools/gn/ninja_target_writer.cc
+++ b/tools/gn/ninja_target_writer.cc
@@ -165,9 +165,13 @@
target_->output_type() == Target::ACTION_FOREACH)
input_deps_sources.push_back(&target_->action_values().script());
- // Input files.
- for (const auto& input : target_->inputs())
- input_deps_sources.push_back(&input);
+ // Input files are only considered for non-binary targets which use an
+ // implicit dependency instead. The implicit depedency in this case is
+ // handled separately by the binary target writer.
+ if (!target_->IsBinary()) {
+ for (const auto& input : target_->inputs())
+ input_deps_sources.push_back(&input);
+ }
// For an action (where we run a script only once) the sources are the same
// as the inputs. For action_foreach, the sources will be operated on
diff --git a/tools/gn/ninja_target_writer_unittest.cc b/tools/gn/ninja_target_writer_unittest.cc
index ccb9c7a..55bbde0 100644
--- a/tools/gn/ninja_target_writer_unittest.cc
+++ b/tools/gn/ninja_target_writer_unittest.cc
@@ -85,12 +85,9 @@
OutputFile dep =
writer.WriteInputDepsStampAndGetDep(std::vector<const Target*>());
- // Since there is more than one dependency, a stamp file will be returned
- // and the rule for the stamp file will be written to the stream.
- EXPECT_EQ("obj/foo/target.inputdeps.stamp", dep.value());
- EXPECT_EQ("build obj/foo/target.inputdeps.stamp: stamp "
- "../../foo/input.txt obj/foo/base.stamp\n",
- stream.str());
+ // Since there is only one dependency, a stamp file will be returned
+ // directly without writing any additional rules.
+ EXPECT_EQ("obj/foo/base.stamp", dep.value());
}
// Input deps for action which should depend on the base since its a hard dep
diff --git a/tools/gn/variables.cc b/tools/gn/variables.cc
index 3c717d8..7b2c4a0 100644
--- a/tools/gn/variables.cc
+++ b/tools/gn/variables.cc
@@ -1048,13 +1048,11 @@
" files in a target are compiled. So if you depend on generated headers,\n"
" you do not typically need to list them in the inputs section.\n"
"\n"
- " Inputs for binary targets will be treated as order-only dependencies,\n"
- " meaning that they will be forced up to date before compiling or\n"
- " any files in the target, but changes in the inputs will not\n"
- " necessarily force the target to compile. This is because it is\n"
- " expected that the compiler will report the precise list of input\n"
- " dependencies required to recompile each file once the initial build\n"
- " is done.\n"
+ " Inputs for binary targets will be treated as implicit dependencies,\n"
+ " meaning that changes in any of the inputs will force all sources in\n"
+ " the target to be recompiled. If an input only applies to a subset of\n"
+ " source files, you may want to split those into a separate target to\n"
+ " avoid unnecessary recompiles.\n"
"\n"
"Example\n"
"\n"
