GN: depend on data deps of source sets.

Previously if a target depended on a source set and that source set had
data deps, building the target would not force the data dep target to be
built.

This patch hooks up the dependencies such that this happens as expected.

BUG=477104

Review URL: https://codereview.chromium.org/1095673002

Cr-Commit-Position: refs/heads/master@{#325544}

tools/gn/ninja_binary_target_writer.cc

@@ -428,6 +428,13 @@ void NinjaBinaryTargetWriter::ClassifyDependency(
         }
       }
     }
+
+    // Add the source set itself as a non-linkable dependency on the current
+    // target. This will make sure that anything the source set's stamp file
+    // depends on (like data deps) are also built before the current target
+    // can be complete. Otherwise, these will be skipped since this target
+    // will depend only on the source set's object files.
+    non_linkable_deps->push_back(dep);
   } else if (can_link_libs && dep->IsLinkable()) {
     linkable_deps->push_back(dep);
   } else {
@@ -437,8 +444,7 @@ void NinjaBinaryTargetWriter::ClassifyDependency(
 
 void NinjaBinaryTargetWriter::WriteOrderOnlyDependencies(
     const UniqueVector<const Target*>& non_linkable_deps) {
-  const std::vector<SourceFile>& data = target_->data();
-  if (!non_linkable_deps.empty() || !data.empty()) {
+  if (!non_linkable_deps.empty()) {
     out_ << " ||";
 
     // Non-linkable targets.

tools/gn/ninja_binary_target_writer_unittest.cc

@@ -84,7 +84,8 @@ TEST(NinjaBinaryTargetWriter, SourceSet) {
         // specified, with the target's first, followed by the source set's, in
         // order.
         "build ./libshlib.so: solink obj/foo/bar.input1.o "
-            "obj/foo/bar.input2.o ../../foo/input3.o ../../foo/input4.obj\n"
+            "obj/foo/bar.input2.o ../../foo/input3.o ../../foo/input4.obj "
+            "|| obj/foo/bar.stamp\n"
         "  ldflags =\n"
         "  libs =\n"
         "  output_extension = .so\n";
@@ -119,7 +120,7 @@ TEST(NinjaBinaryTargetWriter, SourceSet) {
         "\n"
         // There are no sources so there are no params to alink. (In practice
         // this will probably fail in the archive tool.)
-        "build obj/foo/libstlib.a: alink\n"
+        "build obj/foo/libstlib.a: alink || obj/foo/bar.stamp\n"
         "  ldflags =\n"
         "  libs =\n"
         "  output_extension = \n";
@@ -151,7 +152,8 @@ TEST(NinjaBinaryTargetWriter, SourceSet) {
         // specified, with the target's first, followed by the source set's, in
         // order.
         "build obj/foo/libstlib.a: alink obj/foo/bar.input1.o "
-            "obj/foo/bar.input2.o ../../foo/input3.o ../../foo/input4.obj\n"
+            "obj/foo/bar.input2.o ../../foo/input3.o ../../foo/input4.obj "
+            "|| obj/foo/bar.stamp\n"
         "  ldflags =\n"
         "  libs =\n"
         "  output_extension = \n";
@@ -267,3 +269,91 @@ TEST(NinjaBinaryTargetWriter, EmptyProductExtension) {
   std::string out_str = out.str();
   EXPECT_EQ(expected, out_str);
 }
+
+TEST(NinjaBinaryTargetWriter, SourceSetDataDeps) {
+  TestWithScope setup;
+  setup.build_settings()->SetBuildDir(SourceDir("//out/Debug/"));
+  setup.settings()->set_target_os(Settings::LINUX);
+
+  Err err;
+
+  // This target is a data (runtime) dependency of the intermediate target.
+  Target data(setup.settings(), Label(SourceDir("//foo/"), "data_target"));
+  data.set_output_type(Target::EXECUTABLE);
+  data.visibility().SetPublic();
+  data.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(data.OnResolved(&err));
+
+  // Intermediate source set target.
+  Target inter(setup.settings(), Label(SourceDir("//foo/"), "inter"));
+  inter.set_output_type(Target::SOURCE_SET);
+  inter.visibility().SetPublic();
+  inter.data_deps().push_back(LabelTargetPair(&data));
+  inter.SetToolchain(setup.toolchain());
+  inter.sources().push_back(SourceFile("//foo/inter.cc"));
+  ASSERT_TRUE(inter.OnResolved(&err)) << err.message();
+
+  // Write out the intermediate target.
+  std::ostringstream inter_out;
+  NinjaBinaryTargetWriter inter_writer(&inter, inter_out);
+  inter_writer.Run();
+
+  // The intermediate source set will be a stamp file that depends on the
+  // object files, and will have an order-only dependency on its data dep and
+  // data file.
+  const char inter_expected[] =
+      "defines =\n"
+      "include_dirs =\n"
+      "cflags =\n"
+      "cflags_c =\n"
+      "cflags_cc =\n"
+      "cflags_objc =\n"
+      "cflags_objcc =\n"
+      "root_out_dir = .\n"
+      "target_out_dir = obj/foo\n"
+      "target_output_name = inter\n"
+      "\n"
+      "build obj/foo/inter.inter.o: cxx ../../foo/inter.cc\n"
+      "\n"
+      "build obj/foo/inter.stamp: stamp obj/foo/inter.inter.o || "
+          "./data_target\n";
+  EXPECT_EQ(inter_expected, inter_out.str());
+
+  // Final target.
+  Target exe(setup.settings(), Label(SourceDir("//foo/"), "exe"));
+  exe.set_output_type(Target::EXECUTABLE);
+  exe.public_deps().push_back(LabelTargetPair(&inter));
+  exe.SetToolchain(setup.toolchain());
+  exe.sources().push_back(SourceFile("//foo/final.cc"));
+  ASSERT_TRUE(exe.OnResolved(&err));
+
+  std::ostringstream final_out;
+  NinjaBinaryTargetWriter final_writer(&exe, final_out);
+  final_writer.Run();
+
+  // The final output depends on both object files (one from the final target,
+  // one from the source set) and has an order-only dependency on the source
+  // set's stamp file and the final target's data file. The source set stamp
+  // dependency will create an implicit order-only dependency on the data
+  // target.
+  const char final_expected[] =
+      "defines =\n"
+      "include_dirs =\n"
+      "cflags =\n"
+      "cflags_c =\n"
+      "cflags_cc =\n"
+      "cflags_objc =\n"
+      "cflags_objcc =\n"
+      "root_out_dir = .\n"
+      "target_out_dir = obj/foo\n"
+      "target_output_name = exe\n"
+      "\n"
+      "build obj/foo/exe.final.o: cxx ../../foo/final.cc\n"
+      "\n"
+      "build ./exe: link obj/foo/exe.final.o obj/foo/inter.inter.o || "
+          "obj/foo/inter.stamp\n"
+      "  ldflags =\n"
+      "  libs =\n"
+      "  output_extension = \n";
+  EXPECT_EQ(final_expected, final_out.str());
+}