Bypass group targets for order-only deps in source set stamp files

Currently, source sets generated by WriteSourceSetStamp manually collect
their order-only dependencies from their non-linkable dependencies
without bypassing group targets. This causes the source sets to depend
on the group target itself, which recursively includes unnecessary
non-object outputs (like .dwo files).

To avoid these unnecessary dependencies, update WriteSourceSetStamp to
use GetOrderOnlyDepsFromNonLinkableDeps, which recursively expands group
dependencies and returns only the required .linkdeps target (containing
only object files) instead of the group itself.

Bug: 502431091
Change-Id: Ic13a3e76cfbe4683efeea962b837ba1a834047a8
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/22640
Reviewed-by: Junji Watanabe <jwata@google.com>
Commit-Queue: Takuto Ikuta <tikuta@google.com>

src/gn/ninja_c_binary_target_writer.cc

diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index 5346e06..4f19645 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -549,16 +549,8 @@
   // instead.
   DCHECK(classified_deps.extra_object_files.empty());
 
-  std::vector<OutputFile> order_only_deps;
-  for (auto* dep : classified_deps.non_linkable_deps) {
-    if (dep->has_dependency_output()) {
-      OutputFile dep_output = dep->dependency_output();
-      if (dep->output_type() == Target::SOURCE_SET) {
-        dep_output.value().append(".linkdeps");
-      }
-      order_only_deps.push_back(dep_output);
-    }
-  }
+  std::vector<OutputFile> order_only_deps =
+      GetOrderOnlyDepsFromNonLinkableDeps(classified_deps.non_linkable_deps);
 
   // 1. Link-only phony target (.linkdeps) containing only object files.
   std::vector<OutputFile> link_files;

src/gn/ninja_c_binary_target_writer_unittest.cc

diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc
index 01df8b5..73ffbeb 100644
--- a/src/gn/ninja_c_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -2899,7 +2899,7 @@
         "../../bar/bar.swift || phony/bar/group phony/foo/foo\n"
         "  restat = 1\n"
         "\n"
-        "build phony/bar/bar.linkdeps: phony obj/bar/bar.o || phony/bar/group "
+        "build phony/bar/bar.linkdeps: phony obj/bar/bar.o || "
         "phony/foo/foo.linkdeps\n"
         "build phony/bar/bar: phony phony/bar/bar.linkdeps gen/bar/bar.h "
         "obj/bar/Bar.swiftmodule\n";
@@ -3591,3 +3591,79 @@
 
   EXPECT_EQ(expected_root_ninja, root_ninja_str);
 }
+
+TEST_F(NinjaCBinaryTargetWriterTest,
+       SourceSetToGroupToSourceSetWithAdditionalOutputs) {
+  Err err;
+  TestWithScope setup;
+
+  Value outputs_value(nullptr, Value::LIST);
+  outputs_value.list_value().push_back(
+      Value(nullptr, "{{target_out_dir}}/{{source_name_part}}.dwo"));
+
+  Config config(setup.settings(), Label(SourceDir("//foo/"), "split_dwarf"));
+  config.visibility().SetPublic();
+
+  std::vector<SubstitutionPattern> patterns;
+  for (const auto& v : outputs_value.list_value()) {
+    SubstitutionPattern pattern;
+    ASSERT_TRUE(pattern.Parse(v, &err));
+    patterns.push_back(std::move(pattern));
+  }
+  config.own_values().c_additional_outputs() = std::move(patterns);
+  ASSERT_TRUE(config.OnResolved(&err));
+
+  // Source set B (dependee)
+  Target target_b(setup.settings(), Label(SourceDir("//foo/"), "b"));
+  target_b.set_output_type(Target::SOURCE_SET);
+  target_b.sources().push_back(SourceFile("//foo/b.cc"));
+  target_b.source_types_used().Set(SourceFile::SOURCE_CPP);
+  target_b.visibility().SetPublic();
+  target_b.configs().push_back(LabelConfigPair(&config));
+  target_b.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(target_b.OnResolved(&err)) << err.message();
+
+  // Group G (middle)
+  Target target_g(setup.settings(), Label(SourceDir("//foo/"), "g"));
+  target_g.set_output_type(Target::GROUP);
+  target_g.visibility().SetPublic();
+  target_g.private_deps().push_back(LabelTargetPair(&target_b));
+  target_g.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(target_g.OnResolved(&err)) << err.message();
+
+  // Source set A (depender on G)
+  Target target_a(setup.settings(), Label(SourceDir("//foo/"), "a"));
+  target_a.set_output_type(Target::SOURCE_SET);
+  target_a.sources().push_back(SourceFile("//foo/a.cc"));
+  target_a.source_types_used().Set(SourceFile::SOURCE_CPP);
+  target_a.private_deps().push_back(LabelTargetPair(&target_g));
+  target_a.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(target_a.OnResolved(&err)) << err.message();
+
+  // Verify A's output
+  {
+    std::ostringstream out;
+    NinjaCBinaryTargetWriter writer(&target_a, out);
+    writer.Run();
+
+    const char expected[] =
+        "defines =\n"
+        "include_dirs =\n"
+        "cflags =\n"
+        "cflags_cc =\n"
+        "root_out_dir = .\n"
+        "target_gen_dir = gen/foo\n"
+        "target_out_dir = obj/foo\n"
+        "target_output_name = a\n"
+        "\n"
+        "build obj/foo/a.a.o: cxx ../../foo/a.cc\n"
+        "  source_file_part = a.cc\n"
+        "  source_name_part = a\n"
+        "\n"
+        "build phony/foo/a.linkdeps: phony obj/foo/a.a.o"
+        " || phony/foo/b.linkdeps\n"
+        "build phony/foo/a: phony phony/foo/a.linkdeps\n";
+
+    EXPECT_EQ(expected, out.str());
+  }
+}