GN: Use implicit dependency for binary input deps

tools/gn/ninja_binary_target_writer.cc

Index: tools/gn/ninja_binary_target_writer.cc
diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc
index b012e42b0088a0106ffaa01ae1d5421dd6a38fef..80ffd95a85e76bc912874b9eb27dfa73f5100fc1 100644
--- a/tools/gn/ninja_binary_target_writer.cc
+++ b/tools/gn/ninja_binary_target_writer.cc
@@ -270,30 +270,15 @@ void NinjaBinaryTargetWriter::Run() {
 
   WriteCompilerVars(used_types);
 
-  // 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.
+  // The input dependencies will be an implicit dependency. This will cause
+  // Ninja to make sure the inputs are up to date before compiling this source;
+  // changes in the inputs deps will cause the file to be recompiled.
   //
-  // This is important to prevent changes in unrelated actions that are
-  // upstream of this target from causing everything to be recompiled.
-  //
-  // Why can we get away with this rather than using implicit deps ("|", which
-  // will force rebuilds when the inputs change)? For source code, the
-  // computed dependencies of all headers will be computed by the compiler,
-  // which will cause source rebuilds if any "real" upstream dependencies
-  // change.
-  //
-  // If a .cc file is generated by an input dependency, Ninja will see the
-  // input to the build rule doesn't exist, and that it is an output from a
-  // previous step, and build the previous step first. This is a "real"
-  // dependency and doesn't need | or || to express.
-  //
-  // The only case where this rule matters is for the first build where no .d
-  // files exist, and Ninja doesn't know what that source file depends on. In
-  // this case it's sufficient to ensure that the upstream dependencies are
-  // built first. This is exactly what Ninja's order-only dependencies
-  // expresses.
-  OutputFile order_only_dep =
+  // This is to make sure that we force rebuild the source when the inputs
+  // change. Why cannot we rely on the dependencies computed by the compiler?
+  // These dependencies only include headers and might miss other resources
+  // that are used by the input file.
+  OutputFile input_dep =
       WriteInputDepsStampAndGetDep(std::vector<const Target*>());
 
   // For GCC builds, the .gch files are not object files, but still need to be
@@ -301,7 +286,7 @@ void NinjaBinaryTargetWriter::Run() {
   // |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,
                    &pch_obj_files, &pch_other_files);
   std::vector<OutputFile>* pch_files = !pch_obj_files.empty() ?
       &pch_obj_files : &pch_other_files;
@@ -320,7 +305,7 @@ void NinjaBinaryTargetWriter::Run() {
   //    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, &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());
@@ -453,7 +438,7 @@ void NinjaBinaryTargetWriter::WriteOneFlag(
 
 void NinjaBinaryTargetWriter::WritePCHCommands(
     const SourceFileTypeSet& used_types,
-    const OutputFile& order_only_dep,
+    const OutputFile& input_dep,
     std::vector<OutputFile>* object_files,
     std::vector<OutputFile>* other_files) {
   if (!target_->config_values().has_precompiled_headers())
@@ -466,7 +451,7 @@ void NinjaBinaryTargetWriter::WritePCHCommands(
     WritePCHCommand(SUBSTITUTION_CFLAGS_C,
                     Toolchain::TYPE_CC,
                     tool_c->precompiled_header_type(),
-                    order_only_dep, object_files, other_files);
+                    input_dep, object_files, other_files);
   }
   const Tool* tool_cxx = target_->toolchain()->GetTool(Toolchain::TYPE_CXX);
   if (tool_cxx &&
@@ -475,7 +460,7 @@ void NinjaBinaryTargetWriter::WritePCHCommands(
     WritePCHCommand(SUBSTITUTION_CFLAGS_CC,
                     Toolchain::TYPE_CXX,
                     tool_cxx->precompiled_header_type(),
-                    order_only_dep, object_files, other_files);
+                    input_dep, object_files, other_files);
   }
 
   const Tool* tool_objc = target_->toolchain()->GetTool(Toolchain::TYPE_OBJC);
@@ -485,7 +470,7 @@ void NinjaBinaryTargetWriter::WritePCHCommands(
     WritePCHCommand(SUBSTITUTION_CFLAGS_OBJC,
                     Toolchain::TYPE_OBJC,
                     tool_objc->precompiled_header_type(),
-                    order_only_dep, object_files, other_files);
+                    input_dep, object_files, other_files);
   }
 
   const Tool* tool_objcxx =
@@ -496,7 +481,7 @@ void NinjaBinaryTargetWriter::WritePCHCommands(
     WritePCHCommand(SUBSTITUTION_CFLAGS_OBJCC,
                     Toolchain::TYPE_OBJCXX,
                     tool_objcxx->precompiled_header_type(),
-                    order_only_dep, object_files, other_files);
+                    input_dep, object_files, other_files);
   }
 }
 
@@ -504,16 +489,16 @@ void NinjaBinaryTargetWriter::WritePCHCommand(
     SubstitutionType flag_type,
     Toolchain::ToolType tool_type,
     Tool::PrecompiledHeaderType header_type,
-    const OutputFile& order_only_dep,
+    const OutputFile& input_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,
                              object_files);
       break;
     case Tool::PCH_GCC:
-      WriteGCCPCHCommand(flag_type, tool_type, order_only_dep,
+      WriteGCCPCHCommand(flag_type, tool_type, input_dep,
                          other_files);
       break;
     case Tool::PCH_NONE:
@@ -525,7 +510,7 @@ void NinjaBinaryTargetWriter::WritePCHCommand(
 void NinjaBinaryTargetWriter::WriteGCCPCHCommand(
     SubstitutionType flag_type,
     Toolchain::ToolType tool_type,
-    const OutputFile& order_only_dep,
+    const OutputFile& input_dep,
     std::vector<OutputFile>* gch_files) {
   // Compute the pch output file (it will be language-specific).
   std::vector<OutputFile> outputs;
@@ -535,10 +520,14 @@ void NinjaBinaryTargetWriter::WriteGCCPCHCommand(
 
   gch_files->insert(gch_files->end(), outputs.begin(), outputs.end());
 
+  std::vector<OutputFile> deps;
+  if (!input_dep.value().empty()) {

[brettw, 2016/06/16 19:41:34]

No {}, same for the additions below.

[Petr Hosek, 2016/06/16 23:11:47]

Done.

+    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);
+                         deps, tool_type, outputs);
 
   // This build line needs a custom language-specific flags value. Rule-specific
   // variables are just indented underneath the rule line.
@@ -573,7 +562,7 @@ void NinjaBinaryTargetWriter::WriteGCCPCHCommand(
 void NinjaBinaryTargetWriter::WriteWindowsPCHCommand(
     SubstitutionType flag_type,
     Toolchain::ToolType tool_type,
-    const OutputFile& order_only_dep,
+    const OutputFile& input_dep,
     std::vector<OutputFile>* object_files) {
   // Compute the pch output file (it will be language-specific).
   std::vector<OutputFile> outputs;
@@ -583,10 +572,14 @@ void NinjaBinaryTargetWriter::WriteWindowsPCHCommand(
 
   object_files->insert(object_files->end(), outputs.begin(), outputs.end());
 
+  std::vector<OutputFile> deps;
+  if (!input_dep.value().empty()) {
+    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);
+                         deps, tool_type, outputs);
 
   // This build line needs a custom language-specific flags value. Rule-specific
   // variables are just indented underneath the rule line.
@@ -604,13 +597,14 @@ void NinjaBinaryTargetWriter::WriteWindowsPCHCommand(
 
 void NinjaBinaryTargetWriter::WriteSources(
     const std::vector<OutputFile>& pch_deps,
-    const OutputFile& order_only_dep,
+    const OutputFile& input_dep,
     std::vector<OutputFile>* object_files,
     std::vector<SourceFile>* other_files) {
   object_files->reserve(object_files->size() + target_->sources().size());
 
   std::vector<OutputFile> tool_outputs;  // Prevent reallocation in loop.
   std::vector<OutputFile> deps;
+
   for (const auto& source : target_->sources()) {
     // Clear the vector but maintain the max capacity to prevent reallocations.
     deps.resize(0);
@@ -621,6 +615,10 @@ void NinjaBinaryTargetWriter::WriteSources(
       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
@@ -650,8 +648,7 @@ void NinjaBinaryTargetWriter::WriteSources(
           }
         }
       }
-      WriteCompilerBuildLine(source, deps, order_only_dep, tool_type,
-                             tool_outputs);
+      WriteCompilerBuildLine(source, deps, tool_type, tool_outputs);
     }
 
     // It's theoretically possible for a compiler to produce more than one
@@ -664,7 +661,6 @@ void NinjaBinaryTargetWriter::WriteSources(
 void NinjaBinaryTargetWriter::WriteCompilerBuildLine(
     const SourceFile& source,
     const std::vector<OutputFile>& extra_deps,
-    const OutputFile& order_only_dep,
     Toolchain::ToolType tool_type,
     const std::vector<OutputFile>& outputs) {
   out_ << "build";
@@ -681,11 +677,6 @@ void NinjaBinaryTargetWriter::WriteCompilerBuildLine(
       path_output_.WriteFile(out_, dep);
     }
   }
-
-  if (!order_only_dep.value().empty()) {
-    out_ << " || ";
-    path_output_.WriteFile(out_, order_only_dep);
-  }
   out_ << std::endl;
 }
 
@@ -713,7 +704,7 @@ void NinjaBinaryTargetWriter::WriteLinkerStuff(
   path_output_.WriteFiles(out_, extra_object_files);
 
   // Dependencies.
-  std::vector<OutputFile> implicit_deps;
+  std::vector<OutputFile> input_deps;
   std::vector<OutputFile> solibs;
   for (const Target* cur : linkable_deps) {
     // All linkable deps should have a link output file.
@@ -725,7 +716,7 @@ void NinjaBinaryTargetWriter::WriteLinkerStuff(
         cur->link_output_file().value()) {
       // This is a shared library with separate link and deps files. Save for
       // later.
-      implicit_deps.push_back(cur->dependency_output_file());
+      input_deps.push_back(cur->dependency_output_file());
       solibs.push_back(cur->link_output_file());
     } else {
       // Normal case, just link to this target.
@@ -739,7 +730,7 @@ void NinjaBinaryTargetWriter::WriteLinkerStuff(
     for (const SourceFile& src_file : other_files) {
       if (GetSourceFileType(src_file) == SOURCE_DEF) {
         optional_def_file = &src_file;
-        implicit_deps.push_back(
+        input_deps.push_back(
             OutputFile(settings_->build_settings(), src_file));
         break;  // Only one def file is allowed.
       }
@@ -750,15 +741,15 @@ void NinjaBinaryTargetWriter::WriteLinkerStuff(
   const OrderedSet<LibFile>& libs = target_->all_libs();
   for (size_t i = 0; i < libs.size(); i++) {
     if (libs[i].is_source_file()) {
-      implicit_deps.push_back(
+      input_deps.push_back(
           OutputFile(settings_->build_settings(), libs[i].source_file()));
     }
   }
 
   // Append implicit dependencies collected above.
-  if (!implicit_deps.empty()) {
+  if (!input_deps.empty()) {
     out_ << " |";
-    path_output_.WriteFiles(out_, implicit_deps);
+    path_output_.WriteFiles(out_, input_deps);
   }
 
   // Append data dependencies as order-only dependencies.