Count data deps even if it was already a regular dep

tools/gn/runtime_deps.cc

Index: tools/gn/runtime_deps.cc
diff --git a/tools/gn/runtime_deps.cc b/tools/gn/runtime_deps.cc
index 097044f7328cccd7dd09d7da9c66dd5a1751e15c..8664b86592912f6fdc5858ed2f703d3d6851ce87 100644
--- a/tools/gn/runtime_deps.cc
+++ b/tools/gn/runtime_deps.cc
@@ -4,6 +4,7 @@
 
 #include "tools/gn/runtime_deps.h"
 
+#include <map>
 #include <set>
 #include <sstream>
 
@@ -58,15 +59,27 @@ const OutputFile& GetMainOutput(const Target* target) {
 
 // To avoid duplicate traversals of targets, or duplicating output files that
 // might be listed by more than one target, the set of targets and output files
-// that have been found so far is passed.
+// that have been found so far is passed. The "value" of the seen_targets map
+// is a boolean indicating if the seen dep was a data dep (true = data_dep).
+// data deps add more stuff, so we will want to revisit a target if it's a
+// data dependency and we've previously only seen it as a regular dep.
 void RecursiveCollectRuntimeDeps(const Target* target,
                                  bool is_target_data_dep,
                                  RuntimeDepsVector* deps,
-                                 std::set<const Target*>* seen_targets,
+                                 std::map<const Target*, bool>* seen_targets,
                                  std::set<OutputFile>* found_files) {
-  if (seen_targets->find(target) != seen_targets->end())
-    return;  // Already checked.
-  seen_targets->insert(target);
+  const auto& found_seen_target = seen_targets->find(target);
+  if (found_seen_target != seen_targets->end()) {
+    // Already visited.
+    if (found_seen_target->second || !is_target_data_dep) {
+      // Already visited as a data dep, or the current dep is not a data
+      // dep so visiting again will be a no-op.
+      return;
+    }
+    // In the else case, the previously seen target was a regular dependency
+    // and we'll now process it as a data dependency.
+  }
+  (*seen_targets)[target] = is_target_data_dep;
 
   // Add the main output file for executables and shared libraries.
   if (target->output_type() == Target::EXECUTABLE ||
@@ -139,13 +152,15 @@ const char kRuntimeDeps_Help[] =
     "  from all transitive dependencies. Executables and shared libraries are\n"
     "  considered runtime dependencies of themselves.\n"
     "\n"
-    "Details\n"
+    "Executables\n"
     "\n"
     "  Executable targets and those executable targets' transitive\n"
     "  dependencies are not considered unless that executable is listed in\n"
     "  \"data_deps\". Otherwise, GN assumes that the executable (and\n"
     "  everything it requires) is a build-time dependency only.\n"
     "\n"
+    "Actions and copies\n"
+    "\n"
     "  Action and copy targets that are listed as \"data_deps\" will have all\n"
     "  of their outputs and data files considered as runtime dependencies.\n"
     "  Action and copy targets that are \"deps\" or \"public_deps\" will have\n"
@@ -153,6 +168,32 @@ const char kRuntimeDeps_Help[] =
     "  targets can list an output file in both the \"outputs\" and \"data\"\n"
     "  lists to force an output file as a runtime dependency in all cases.\n"
     "\n"
+    "  The different rules for deps and data_deps are to express build-time\n"
+    "  (deps) vs. run-time (data_deps) outputs. If GN counted all build-time\n"
+    "  copy steps as data dependencies, there would be a lot of extra stuff,\n"
+    "  and if GN counted all run-time dependencies as regular deps, the\n"
+    "  build's paralellism would be unnecessarily constrained.\n"

[Dirk Pranke, 2015/07/21 23:00:51]

typo: parallelism

+    "\n"
+    "  This rule can sometimes lead to unintuitive results. For example,\n"
+    "  given the three targets:\n"
+    "    A  --[data_deps]-->  B  --[deps]-->  ACTION\n"
+    "  GN would say that A does not have runtime deps on the result of the\n"
+    "  ACTION, which is often correct. But the purpose of the B target might\n"
+    "  be to collect many actions into one logic unit, and the \"data\"-ness\n"
+    "  of A's dependency is lost. Solutions:\n"
+    "\n"
+    "   - List the outputs of the action in it's data section (if the\n"
+    "     results of that action are always runtime files).\n"
+    "   - Have B list the action in data_deps (if the outputs of the actions\n"
+    "     are always runtime files).\n"
+    "   - Have B list the action in both deps and data deps (if the outputs\n"
+    "     might be used in both contexts and you don't care about unnecessary\n"
+    "     entries in the list of files required at runtime).\n"
+    "   - Split B into run-time and build-time versions with the appropriate\n"
+    "     \"deps\" for each.\n"
+    "\n"
+    "Static libraries and source sets\n"
+    "\n"
     "  The results of static_library or source_set targets are not considered\n"
     "  runtime dependencies since these are assumed to be intermediate\n"
     "  targets only. If you need to list a static library as a runtime\n"
@@ -160,14 +201,17 @@ const char kRuntimeDeps_Help[] =
     "  current platform and list it in the \"data\" list of a target\n"
     "  (possibly on the static library target itself).\n"
     "\n"
+    "Multiple outputs\n"
+    "\n"
     "  When a tool produces more than one output, only the first output\n"
     "  is considered. For example, a shared library target may produce a\n"
     "  .dll and a .lib file on Windows. Only the .dll file will be considered\n"
-    "  a runtime dependency.\n";
+    "  a runtime dependency. This applies only to linker tools, scripts and\n"
+    "  copy steps with multiple outputs will also get all outputs listed.\n";
 
 RuntimeDepsVector ComputeRuntimeDeps(const Target* target) {
   RuntimeDepsVector result;
-  std::set<const Target*> seen_targets;
+  std::map<const Target*, bool> seen_targets;
   std::set<OutputFile> found_files;
 
   // The initial target is not considered a data dependency so that actions's