Make data_deps in copy() targets work.

Looks like copy targets were never updated for data_deps.

Without this change, having a copy() target have data_deps on
another target and building the copy() target didn't build the
data_dep. For all other target types, it does build the dep,
so this looks like it's unintentional.

See https://gn-review.googlesource.com/c/gn/+/8861/ for a
larger, standalone repro.

Change-Id: I866bbb162619c57636907af13f34213949a54882
Bug: none
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/8860
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>

src/gn/ninja_copy_target_writer.cc

diff --git a/src/gn/ninja_copy_target_writer.cc b/src/gn/ninja_copy_target_writer.cc
index f350086..9299223 100644
--- a/src/gn/ninja_copy_target_writer.cc
+++ b/src/gn/ninja_copy_target_writer.cc
@@ -73,13 +73,17 @@
   std::vector<OutputFile> input_deps = WriteInputDepsStampAndGetDep(
       std::vector<const Target*>(), num_stamp_uses);
 
+  std::vector<OutputFile> data_outs;
+  for (const auto& dep : target_->data_deps())
+    data_outs.push_back(dep.ptr->dependency_output_file());
+
   // Note that we don't write implicit deps for copy steps. "copy" only
   // depends on the output files themselves, rather than having includes
   // (the possibility of generated #includes is the main reason for implicit
   // dependencies).
   //
   // It would seem that specifying implicit dependencies on the deps of the
-  // copy command would still be harmeless. But Chrome implements copy tools
+  // copy command would still be harmless. But Chrome implements copy tools
   // as hard links (much faster) which don't change the timestamp. If the
   // ninja rule looks like this:
   //   output: copy input | foo.stamp
@@ -97,8 +101,9 @@
   //
   // Note that there is the need in some cases for order-only dependencies
   // where a command might need to make sure something else runs before it runs
-  // to avoid conflicts. Such cases should be avoided where possible, but
-  // sometimes that's not possible.
+  // to avoid conflicts. This is also needed for data_deps on a copy target.
+  // Such cases should be avoided where possible, but sometimes that's not
+  // possible.
   for (const auto& input_file : target_->sources()) {
     OutputFile output_file =
         SubstitutionWriter::ApplyPatternToSourceAsOutputFile(
@@ -109,9 +114,10 @@
     path_output_.WriteFile(out_, output_file);
     out_ << ": " << tool_name << " ";
     path_output_.WriteFile(out_, input_file);
-    if (!input_deps.empty()) {
+    if (!input_deps.empty() || !data_outs.empty()) {
       out_ << " ||";
       path_output_.WriteFiles(out_, input_deps);
+      path_output_.WriteFiles(out_, data_outs);
     }
     out_ << std::endl;
   }

src/gn/ninja_copy_target_writer_unittest.cc

diff --git a/src/gn/ninja_copy_target_writer_unittest.cc b/src/gn/ninja_copy_target_writer_unittest.cc
index 0b4caea..f641ffa 100644
--- a/src/gn/ninja_copy_target_writer_unittest.cc
+++ b/src/gn/ninja_copy_target_writer_unittest.cc
@@ -92,3 +92,35 @@
   std::string out_str = out.str();
   EXPECT_EQ(expected_linux, out_str);
 }
+
+TEST(NinjaCopyTargetWriter, DataDeps) {
+  Err err;
+  TestWithScope setup;
+
+  Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+  target.set_output_type(Target::COPY_FILES);
+  target.sources().push_back(SourceFile("//foo/input1.txt"));
+  target.action_values().outputs() =
+      SubstitutionList::MakeForTest("//out/Debug/{{source_name_part}}.out");
+
+  Target data_dep(setup.settings(), Label(SourceDir("//foo/"), "datadep"));
+  data_dep.set_output_type(Target::ACTION);
+  data_dep.visibility().SetPublic();
+  data_dep.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(data_dep.OnResolved(&err));
+
+  target.data_deps().push_back(LabelTargetPair(&data_dep));
+  target.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(target.OnResolved(&err));
+
+  std::ostringstream out;
+  NinjaCopyTargetWriter writer(&target, out);
+  writer.Run();
+
+  const char expected_linux[] =
+      "build input1.out: copy ../../foo/input1.txt || obj/foo/datadep.stamp\n"
+      "\n"
+      "build obj/foo/bar.stamp: stamp input1.out\n";
+  std::string out_str = out.str();
+  EXPECT_EQ(expected_linux, out_str);
+}