Check for inputs not generated by deps

tools/gn/target.cc

Index: tools/gn/target.cc
diff --git a/tools/gn/target.cc b/tools/gn/target.cc
index 40c8a21e2adf743d0b0de24edcfee564f6559b3e..6502eef65d19b1c75a4e498be6aaf35b8479df6a 100644
--- a/tools/gn/target.cc
+++ b/tools/gn/target.cc
@@ -60,6 +60,47 @@ Err MakeStaticLibDepsError(const Target* from, const Target* to) {
                  "Use source sets for intermediate targets instead.");
 }
 
+// Set check_private_deps to true for the first invocation since a target
+// can see all of its dependencies. For recursive invocations this will be set
+// to false to follow only public dependency paths.
+//
+// Pass a pointer to an empty set for the first invocation. This will be used
+// to avoid duplicate checking.
+bool EnsureFileIsGeneratedByDependency(const Target* target,
+                                       const OutputFile& file,
+                                       bool check_private_deps,
+                                       std::set<const Target*>* seen_targets) {
+  if (seen_targets->find(target) != seen_targets->end())
+    return false;  // Already checked this one and it's not found.
+  seen_targets->insert(target);
+
+  // Assume that we have relatively few generated inputs so brute-force
+  // searching here is OK. If this becomes a bottleneck, consider storing
+  // computed_outputs as a hash set.
+  for (const OutputFile& cur : target->computed_outputs()) {

[scottmg, 2015/05/14 23:04:04]

(this could be std::find, but whatever you prefer)

[brettw, 2015/05/14 23:21:03]

In this case the find code is silly looking because of hte long name of the
container.

+    if (file == cur)
+      return true;
+  }
+
+  // Check all public dependencies (don't do data ones since those are
+  // runtime-only).
+  for (const auto& pair : target->public_deps()) {
+    if (EnsureFileIsGeneratedByDependency(pair.ptr, file, false,
+                                          seen_targets))
+      return true;  // Found a path.
+  }
+
+  // Only check private deps if requested.
+  if (check_private_deps) {
+    for (const auto& pair : target->private_deps()) {
+      if (EnsureFileIsGeneratedByDependency(pair.ptr, file, false,
+                                            seen_targets))
+        return true;  // Found a path.
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 Target::Target(const Settings* settings, const Label& label)
@@ -132,12 +173,16 @@ bool Target::OnResolved(Err* err) {
 
   FillOutputFiles();
 
-  if (!CheckVisibility(err))
-    return false;
-  if (!CheckTestonly(err))
-    return false;
-  if (!CheckNoNestedStaticLibs(err))
-    return false;
+  if (settings()->build_settings()->check_for_bad_items()) {
+    if (!CheckVisibility(err))
+      return false;
+    if (!CheckTestonly(err))
+      return false;
+    if (!CheckNoNestedStaticLibs(err))
+      return false;
+    if (!CheckSourcesGenerated(err))
+      return false;
+  }
 
   return true;
 }
@@ -304,6 +349,7 @@ void Target::PullRecursiveHardDeps() {
 
 void Target::FillOutputFiles() {
   const Tool* tool = toolchain_->GetToolForTargetFinalOutput(this);
+  bool check_tool_outputs = false;
   switch (output_type_) {
     case GROUP:
     case SOURCE_SET:
@@ -322,6 +368,7 @@ void Target::FillOutputFiles() {
       // Executables don't get linked to, but the first output is used for
       // dependency management.
       CHECK_GE(tool->outputs().list().size(), 1u);
+      check_tool_outputs = true;
       dependency_output_file_ =
           SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
               this, tool, tool->outputs().list()[0]);
@@ -330,12 +377,14 @@ void Target::FillOutputFiles() {
       // Static libraries both have dependencies and linking going off of the
       // first output.
       CHECK(tool->outputs().list().size() >= 1);
+      check_tool_outputs = true;
       link_output_file_ = dependency_output_file_ =
           SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
               this, tool, tool->outputs().list()[0]);
       break;
     case SHARED_LIBRARY:
       CHECK(tool->outputs().list().size() >= 1);
+      check_tool_outputs = true;
       if (tool->link_output().empty() && tool->depend_output().empty()) {
         // Default behavior, use the first output file for both.
         link_output_file_ = dependency_output_file_ =
@@ -359,6 +408,18 @@ void Target::FillOutputFiles() {
     default:
       NOTREACHED();
   }
+
+  // Count all outputs from this tool as something generated by this target.
+  if (check_tool_outputs) {
+    SubstitutionWriter::ApplyListToLinkerAsOutputFile(
+        this, tool, tool->outputs(), &computed_outputs_);
+  }
+
+  // Also count anything the target has declared to be an output.
+  std::vector<SourceFile> outputs_as_sources;
+  action_values_.GetOutputsAsSourceFiles(this, &outputs_as_sources);
+  for (const SourceFile& out : outputs_as_sources)
+    computed_outputs_.push_back(OutputFile(settings()->build_settings(), out));
 }
 
 bool Target::CheckVisibility(Err* err) const {
@@ -409,3 +470,51 @@ bool Target::CheckNoNestedStaticLibs(Err* err) const {
   }
   return true;
 }
+
+bool Target::CheckSourcesGenerated(Err* err) const {
+  // Checks that any inputs or sources to this target that are in the build
+  // directory are generated by a target that this one transitively depends on
+  // in some way. We already guarantee that all generated files are written
+  // to the build dir.
+  for (const SourceFile& file : sources_) {
+    if (!CheckSourceGenerated(file, err))
+      return false;
+  }
+  for (const SourceFile& file : inputs_) {
+    if (!CheckSourceGenerated(file, err))
+      return false;
+  }
+  return true;
+}
+
+bool Target::CheckSourceGenerated(const SourceFile& source, Err* err) const {
+  if (!IsStringInOutputDir(settings()->build_settings()->build_dir(),
+                           source.value()))
+    return true;  // Not in output dir, this is OK.
+
+  OutputFile out_file(settings()->build_settings(), source);
+  std::set<const Target*> seen_targets;
+  if (!EnsureFileIsGeneratedByDependency(this, out_file, true, &seen_targets)) {
+    *err = Err(defined_from(),
+        "Target has an input not generated by a dependency.",
+        "The source or input file\n  " + source.value() + "\n"
+        "on the target\n  " + label().GetUserVisibleName(false) + "\n"
+        "is in the build directory but was not specified as an output by\n"
+        "any of this target's dependencies or transitive public dependencies.\n"
+        "\n"
+        "If you have generated inputs, there needs to be a dependency path\n"
+        "between the two targets in addition to just listing the files.\n"
+        "For indirect dependencies, the intermediate ones must be\n"
+        "public_deps. data_deps don't count since they're only runtime\n"
+        "dependencies.\n"
+        "\n"
+        "Start with:\n"
+        "  gn refs " +
+        DirectoryWithNoLastSlash(settings()->build_settings()->build_dir()) +
+        " " + source.value() + "\n"
+        "which will print all targets that reference the file EITHER as an\n"
+        "input or an output, and go from there.");
+    return false;
+  }
+  return true;
+}