Fix Rust dependency propagation

Rust -L flags are now the transitive closure of their dependencies
but only use --extern on direct dependencies

Change-Id: I76a46dfb5040bec817c7be299e7acc78785f9a7d
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/7500
Commit-Queue: Petr Hosek <phosek@google.com>
Reviewed-by: Petr Hosek <phosek@google.com>
diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc
index 0f3895a..5ca0750 100644
--- a/src/gn/ninja_rust_binary_target_writer.cc
+++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -162,12 +162,20 @@
       const ConfigValues& cur = iter.cur();
       for (const auto& e : cur.externs()) {
         if (e.second.is_source_file()) {
-          deps.push_back(OutputFile(settings_->build_settings(),
-                                    e.second.source_file()));
+          deps.push_back(
+              OutputFile(settings_->build_settings(), e.second.source_file()));
         }
       }
     }
 
+    std::vector<OutputFile> transitive_rustlibs;
+    for (const auto* dep :
+         target_->rust_values().transitive_libs().GetOrdered()) {
+      if (dep->source_types_used().RustSourceUsed()) {
+        transitive_rustlibs.push_back(dep->dependency_output_file());
+      }
+    }
+
     std::vector<OutputFile> tool_outputs;
     SubstitutionWriter::ApplyListToLinkerAsOutputFile(
         target_, tool_, tool_->outputs(), &tool_outputs);
@@ -178,7 +186,7 @@
     std::copy(non_linkable_deps.begin(), non_linkable_deps.end(),
               std::back_inserter(extern_deps));
     WriteExterns(extern_deps);
-    WriteRustdeps(rustdeps, nonrustdeps);
+    WriteRustdeps(transitive_rustlibs, rustdeps, nonrustdeps);
   }
 }
 
@@ -235,12 +243,14 @@
 }
 
 void NinjaRustBinaryTargetWriter::WriteRustdeps(
+    const std::vector<OutputFile>& transitive_rustdeps,
     const std::vector<OutputFile>& rustdeps,
     const std::vector<OutputFile>& nonrustdeps) {
   out_ << "  rustdeps =";
 
   // Rust dependencies.
-  for (const auto& rustdep : rustdeps) {
+  for (const auto& rustdep : transitive_rustdeps) {
+    // TODO switch to using --extern priv: after stabilization
     out_ << " -Ldependency=";
     path_output_.WriteDir(
         out_, rustdep.AsSourceFile(settings_->build_settings()).GetDir(),
diff --git a/src/gn/ninja_rust_binary_target_writer.h b/src/gn/ninja_rust_binary_target_writer.h
index 180bd1f..cd845d5 100644
--- a/src/gn/ninja_rust_binary_target_writer.h
+++ b/src/gn/ninja_rust_binary_target_writer.h
@@ -25,8 +25,9 @@
   void WriteSources(const OutputFile& input_dep,
                     const std::vector<OutputFile>& order_only_deps);
   void WriteExterns(const std::vector<const Target*>& deps);
-  void WriteRustdeps(const std::vector<OutputFile>& rustdeps,
-                     const std::vector<OutputFile>& nonrustdeps);
+  void WriteRustdeps(const std::vector<OutputFile>& transitive_rustdeps,
+      const std::vector<OutputFile>& rustdeps,
+      const std::vector<OutputFile>& nonrustdeps);
   void WriteEdition();
 
   const RustTool* tool_;
diff --git a/src/gn/ninja_rust_binary_target_writer_unittest.cc b/src/gn/ninja_rust_binary_target_writer_unittest.cc
index 44ff61c..b9c2215 100644
--- a/src/gn/ninja_rust_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -181,15 +181,138 @@
         "target_output_name = bar\n"
         "\n"
         "build ./foo_bar: rust_bin ../../foo/main.rs | ../../foo/source.rs "
-        "../../foo/main.rs obj/foo/libdirect.rlib obj/bar/libmylib.rlib\n"
-        "  externs = --extern direct=obj/foo/libdirect.rlib --extern "
-        "mylib=obj/bar/libmylib.rlib\n"
+        "../../foo/main.rs obj/foo/libdirect.rlib\n"
+        "  externs = --extern direct=obj/foo/libdirect.rlib\n"
         "  rustdeps = -Ldependency=obj/foo -Ldependency=obj/bar\n";
     std::string out_str = out.str();
     EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
   }
 }
 
+TEST_F(NinjaRustBinaryTargetWriterTest, RlibDepsAcrossGroups) {
+  Err err;
+  TestWithScope setup;
+
+  Target procmacro(setup.settings(), Label(SourceDir("//bar/"), "mymacro"));
+  procmacro.set_output_type(Target::RUST_PROC_MACRO);
+  procmacro.visibility().SetPublic();
+  SourceFile barproc("//bar/lib.rs");
+  procmacro.sources().push_back(SourceFile("//bar/mylib.rs"));
+  procmacro.sources().push_back(barproc);
+  procmacro.source_types_used().Set(SourceFile::SOURCE_RS);
+  procmacro.rust_values().set_crate_root(barproc);
+  procmacro.rust_values().crate_name() = "mymacro";
+  procmacro.rust_values().set_crate_type(RustValues::CRATE_PROC_MACRO);
+  procmacro.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(procmacro.OnResolved(&err));
+
+  {
+    std::ostringstream out;
+    NinjaRustBinaryTargetWriter writer(&procmacro, out);
+    writer.Run();
+
+    const char expected[] =
+        "crate_name = mymacro\n"
+        "crate_type = proc-macro\n"
+        "output_extension = .so\n"
+        "output_dir = \n"
+        "rustflags =\n"
+        "rustenv =\n"
+        "root_out_dir = .\n"
+        "target_out_dir = obj/bar\n"
+        "target_output_name = libmymacro\n"
+        "\n"
+        "build obj/bar/libmymacro.so: rust_macro ../../bar/lib.rs | "
+        "../../bar/mylib.rs ../../bar/lib.rs\n"
+        "  externs =\n"
+        "  rustdeps =\n";
+    std::string out_str = out.str();
+    EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
+  }
+
+  Target group(setup.settings(), Label(SourceDir("//baz/"), "group"));
+  group.set_output_type(Target::GROUP);
+  group.visibility().SetPublic();
+  group.public_deps().push_back(LabelTargetPair(&procmacro));
+  group.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(group.OnResolved(&err));
+
+  Target rlib(setup.settings(), Label(SourceDir("//bar/"), "mylib"));
+  rlib.set_output_type(Target::RUST_LIBRARY);
+  rlib.visibility().SetPublic();
+  SourceFile barlib("//bar/lib.rs");
+  rlib.sources().push_back(SourceFile("//bar/mylib.rs"));
+  rlib.sources().push_back(barlib);
+  rlib.source_types_used().Set(SourceFile::SOURCE_RS);
+  rlib.rust_values().set_crate_root(barlib);
+  rlib.rust_values().crate_name() = "mylib";
+  rlib.SetToolchain(setup.toolchain());
+  rlib.public_deps().push_back(LabelTargetPair(&group));
+  ASSERT_TRUE(rlib.OnResolved(&err));
+
+  {
+    std::ostringstream out;
+    NinjaRustBinaryTargetWriter writer(&rlib, out);
+    writer.Run();
+
+    const char expected[] =
+        "crate_name = mylib\n"
+        "crate_type = rlib\n"
+        "output_extension = .rlib\n"
+        "output_dir = \n"
+        "rustflags =\n"
+        "rustenv =\n"
+        "root_out_dir = .\n"
+        "target_out_dir = obj/bar\n"
+        "target_output_name = libmylib\n"
+        "\n"
+        "build obj/bar/libmylib.rlib: rust_rlib ../../bar/lib.rs | "
+        "../../bar/mylib.rs ../../bar/lib.rs obj/bar/libmymacro.so || "
+        "obj/baz/group.stamp\n"
+        "  externs = --extern mymacro=obj/bar/libmymacro.so\n"
+        "  rustdeps = -Ldependency=obj/bar\n";
+    std::string out_str = out.str();
+    EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
+  }
+
+  Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
+  target.set_output_type(Target::EXECUTABLE);
+  target.visibility().SetPublic();
+  SourceFile main("//foo/main.rs");
+  target.sources().push_back(SourceFile("//foo/source.rs"));
+  target.sources().push_back(main);
+  target.source_types_used().Set(SourceFile::SOURCE_RS);
+  target.rust_values().set_crate_root(main);
+  target.rust_values().crate_name() = "foo_bar";
+  target.private_deps().push_back(LabelTargetPair(&rlib));
+  target.SetToolchain(setup.toolchain());
+  ASSERT_TRUE(target.OnResolved(&err));
+
+  {
+    std::ostringstream out;
+    NinjaRustBinaryTargetWriter writer(&target, out);
+    writer.Run();
+
+    const char expected[] =
+        "crate_name = foo_bar\n"
+        "crate_type = bin\n"
+        "output_extension = \n"
+        "output_dir = \n"
+        "rustflags =\n"
+        "rustenv =\n"
+        "root_out_dir = .\n"
+        "target_out_dir = obj/foo\n"
+        "target_output_name = bar\n"
+        "\n"
+        "build ./foo_bar: rust_bin ../../foo/main.rs | "
+        "../../foo/source.rs ../../foo/main.rs obj/bar/libmylib.rlib\n"
+        "  externs = --extern mylib=obj/bar/libmylib.rlib\n"
+        "  rustdeps = -Ldependency=obj/bar -Ldependency=obj/bar\n";
+    std::string out_str = out.str();
+    EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
+  }
+}
+
 TEST_F(NinjaRustBinaryTargetWriterTest, RenamedDeps) {
   Err err;
   TestWithScope setup;
@@ -644,9 +767,9 @@
   target.rust_values().set_crate_root(main);
   target.rust_values().crate_name() = "foo_bar";
   target.config_values().externs().push_back(
-    std::pair("lib1", LibFile(SourceFile("//foo/lib1.rlib"))));
+      std::pair("lib1", LibFile(SourceFile("//foo/lib1.rlib"))));
   target.config_values().externs().push_back(
-    std::pair("lib2", LibFile("lib2.rlib")));
+      std::pair("lib2", LibFile("lib2.rlib")));
   target.SetToolchain(setup.toolchain());
   ASSERT_TRUE(target.OnResolved(&err));
 
@@ -668,7 +791,8 @@
         "\n"
         "build ./foo_bar: rust_bin ../../foo/main.rs | ../../foo/source.rs "
         "../../foo/main.rs ../../foo/lib1.rlib\n"
-        "  externs = --extern lib1=../../foo/lib1.rlib --extern lib2=lib2.rlib\n"
+        "  externs = --extern lib1=../../foo/lib1.rlib --extern "
+        "lib2=lib2.rlib\n"
         "  rustdeps =\n";
     std::string out_str = out.str();
     EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
diff --git a/src/gn/rust_tool.cc b/src/gn/rust_tool.cc
index cd57428..6fcb697 100644
--- a/src/gn/rust_tool.cc
+++ b/src/gn/rust_tool.cc
@@ -33,9 +33,9 @@
 }
 
 bool RustTool::ValidateName(const char* name) const {
-  return name == kRsToolBin || name == kRsToolCDylib ||
-         name == kRsToolDylib || name == kRsToolMacro ||
-         name == kRsToolRlib || name == kRsToolStaticlib;
+  return name == kRsToolBin || name == kRsToolCDylib || name == kRsToolDylib ||
+         name == kRsToolMacro || name == kRsToolRlib ||
+         name == kRsToolStaticlib;
 }
 
 void RustTool::SetComplete() {
@@ -105,9 +105,9 @@
 }
 
 bool RustTool::ValidateSubstitution(const Substitution* sub_type) const {
-  if (name_ == kRsToolBin || name_ == kRsToolCDylib ||
-      name_ == kRsToolDylib || name_ == kRsToolMacro ||
-      name_ == kRsToolRlib || name_ == kRsToolStaticlib)
+  if (name_ == kRsToolBin || name_ == kRsToolCDylib || name_ == kRsToolDylib ||
+      name_ == kRsToolMacro || name_ == kRsToolRlib ||
+      name_ == kRsToolStaticlib)
     return IsValidRustSubstitution(sub_type);
   NOTREACHED();
   return false;
diff --git a/src/gn/rust_values.h b/src/gn/rust_values.h
index 9770e34..8f7fff6 100644
--- a/src/gn/rust_values.h
+++ b/src/gn/rust_values.h
@@ -8,6 +8,7 @@
 #include <map>
 
 #include "base/containers/flat_map.h"
+#include "gn/inherited_libraries.h"
 #include "gn/label.h"
 #include "gn/source_file.h"
 
@@ -50,11 +51,16 @@
   }
   std::map<Label, std::string>& aliased_deps() { return aliased_deps_; }
 
+  // Transitive closure of libraries that are depended on by this target
+  InheritedLibraries& transitive_libs() { return rust_libs_; }
+  const InheritedLibraries& transitive_libs() const { return rust_libs_; }
+
  private:
   std::string crate_name_;
   SourceFile crate_root_;
   CrateType crate_type_ = CRATE_AUTO;
   std::map<Label, std::string> aliased_deps_;
+  InheritedLibraries rust_libs_;
 
   DISALLOW_COPY_AND_ASSIGN(RustValues);
 };
diff --git a/src/gn/target.cc b/src/gn/target.cc
index fdf473e..2469fba 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -536,17 +536,36 @@
 void Target::PullDependentTargetLibsFrom(const Target* dep, bool is_public) {
   // Direct dependent libraries.
   if (dep->output_type() == STATIC_LIBRARY ||
-      dep->output_type() == SHARED_LIBRARY ||
-      dep->output_type() == SOURCE_SET || dep->output_type() == RUST_LIBRARY ||
-      dep->output_type() == RUST_PROC_MACRO)
+      dep->output_type() == SHARED_LIBRARY) {
     inherited_libraries_.Append(dep, is_public);
+    rust_values().transitive_libs().Append(dep, is_public);
+  }
 
-  if (dep->output_type() == CREATE_BUNDLE &&
-      dep->bundle_data().is_framework()) {
+  if (dep->output_type() == RUST_LIBRARY ||
+      dep->output_type() == RUST_PROC_MACRO ||
+      dep->output_type() == SOURCE_SET ||
+      (dep->output_type() == CREATE_BUNDLE &&
+       dep->bundle_data().is_framework())) {
     inherited_libraries_.Append(dep, is_public);
   }
 
-  if (dep->output_type() == SHARED_LIBRARY) {
+  if (dep->output_type() == RUST_LIBRARY ||
+      dep->output_type() == RUST_PROC_MACRO) {
+    rust_values().transitive_libs().Append(dep, is_public);
+    rust_values().transitive_libs().AppendInherited(
+        dep->rust_values().transitive_libs(), is_public);
+
+    // If there is a transitive dependency that is not a rust library, place it
+    // in the normal location
+    for (const auto& inherited :
+         rust_values().transitive_libs().GetOrderedAndPublicFlag()) {
+      if (!(inherited.first->output_type() == RUST_LIBRARY ||
+            inherited.first->output_type() == RUST_PROC_MACRO)) {
+        inherited_libraries_.Append(inherited.first,
+                                    is_public && inherited.second);
+      }
+    }
+  } else if (dep->output_type() == SHARED_LIBRARY) {
     // Shared library dependendencies are inherited across public shared
     // library boundaries.
     //
@@ -572,6 +591,8 @@
     // The current target isn't linked, so propogate linked deps and
     // libraries up the dependency tree.
     inherited_libraries_.AppendInherited(dep->inherited_libraries(), is_public);
+    rust_values().transitive_libs().AppendInherited(
+        dep->rust_values().transitive_libs(), is_public);
   } else if (dep->complete_static_lib()) {
     // Inherit only final targets through _complete_ static libraries.
     //