[rust] Fix and test dylib support
Rust dylibs are handled pretty much exactly like rlibs. Like rlibs,
they embed Rust metadata which is used by the compiler when building
dependencies. This is a bit awkward to express in GN given that they
have a shared_library target type, whereas rlibs have their own target
type.
Bug: fuchsia:59286
Change-Id: I300163bae8327bc9328c785a4b9033511bfbdebc
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/10800
Commit-Queue: Tyler Mandry <tmandry@google.com>
Reviewed-by: Petr Hosek <phosek@google.com>
Reviewed-by: Benjamin Brittain <bwb@google.com>
diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc
index afdf5d9..140fe09 100644
--- a/src/gn/ninja_rust_binary_target_writer.cc
+++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -14,6 +14,7 @@
#include "gn/ninja_utils.h"
#include "gn/rust_substitution_type.h"
#include "gn/substitution_writer.h"
+#include "gn/rust_values.h"
#include "gn/target.h"
namespace {
@@ -252,7 +253,10 @@
for (const Target* target : deps) {
if (target->output_type() == Target::RUST_LIBRARY ||
- target->output_type() == Target::RUST_PROC_MACRO) {
+ target->output_type() == Target::RUST_PROC_MACRO ||
+ (target->source_types_used().RustSourceUsed() &&
+ target->rust_values().InferredCrateType(target) ==
+ RustValues::CRATE_DYLIB)) {
out_ << " --extern ";
const auto& renamed_dep =
target_->rust_values().aliased_deps().find(target->label());
diff --git a/src/gn/ninja_rust_binary_target_writer_unittest.cc b/src/gn/ninja_rust_binary_target_writer_unittest.cc
index b03b2cb..6bcb204 100644
--- a/src/gn/ninja_rust_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -168,6 +168,103 @@
}
}
+TEST_F(NinjaRustBinaryTargetWriterTest, DylibDeps) {
+ Err err;
+ TestWithScope setup;
+
+ Target dylib(setup.settings(), Label(SourceDir("//bar/"), "mylib"));
+ dylib.set_output_type(Target::SHARED_LIBRARY);
+ dylib.visibility().SetPublic();
+ SourceFile barlib("//bar/lib.rs");
+ dylib.sources().push_back(SourceFile("//bar/mylib.rs"));
+ dylib.sources().push_back(barlib);
+ dylib.source_types_used().Set(SourceFile::SOURCE_RS);
+ dylib.rust_values().set_crate_type(RustValues::CRATE_DYLIB); // TODO
+ dylib.rust_values().set_crate_root(barlib);
+ dylib.rust_values().crate_name() = "mylib";
+ dylib.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(dylib.OnResolved(&err));
+
+ {
+ std::ostringstream out;
+ NinjaRustBinaryTargetWriter writer(&dylib, out);
+ writer.Run();
+
+ const char expected[] =
+ "crate_name = mylib\n"
+ "crate_type = dylib\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 = libmylib\n"
+ "\n"
+ "build obj/bar/libmylib.so: rust_dylib ../../bar/lib.rs | "
+ "../../bar/mylib.rs ../../bar/lib.rs\n"
+ " externs =\n"
+ " rustdeps =\n"
+ " ldflags =\n"
+ " sources = ../../bar/mylib.rs ../../bar/lib.rs\n";
+ std::string out_str = out.str();
+ EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
+ }
+
+ Target another_dylib(setup.settings(), Label(SourceDir("//foo/"), "direct"));
+ another_dylib.set_output_type(Target::SHARED_LIBRARY);
+ another_dylib.visibility().SetPublic();
+ SourceFile lib("//foo/main.rs");
+ another_dylib.sources().push_back(SourceFile("//foo/direct.rs"));
+ another_dylib.sources().push_back(lib);
+ another_dylib.source_types_used().Set(SourceFile::SOURCE_RS);
+ another_dylib.rust_values().set_crate_type(RustValues::CRATE_DYLIB);
+ another_dylib.rust_values().set_crate_root(lib);
+ another_dylib.rust_values().crate_name() = "direct";
+ another_dylib.SetToolchain(setup.toolchain());
+ another_dylib.private_deps().push_back(LabelTargetPair(&dylib));
+ ASSERT_TRUE(another_dylib.OnResolved(&err));
+
+ 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(&another_dylib));
+ 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/foo/libdirect.so\n"
+ " externs = --extern direct=obj/foo/libdirect.so\n"
+ " rustdeps = -Ldependency=obj/foo -Ldependency=obj/bar\n"
+ " ldflags =\n"
+ " sources = ../../foo/source.rs ../../foo/main.rs\n";
+ std::string out_str = out.str();
+ EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
+ }
+}
+
TEST_F(NinjaRustBinaryTargetWriterTest, RlibDepsAcrossGroups) {
Err err;
TestWithScope setup;
diff --git a/src/gn/rust_values.cc b/src/gn/rust_values.cc
index ed314eb..6531291 100644
--- a/src/gn/rust_values.cc
+++ b/src/gn/rust_values.cc
@@ -2,8 +2,51 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "gn/rust_tool.h"
#include "gn/rust_values.h"
+#include "gn/target.h"
RustValues::RustValues() : crate_type_(RustValues::CRATE_AUTO) {}
-RustValues::~RustValues() = default;
\ No newline at end of file
+RustValues::~RustValues() = default;
+
+// static
+RustValues::CrateType RustValues::InferredCrateType(const Target* target) {
+ // TODO: Consider removing crate_type. It allows for things like
+ //
+ // executable("foo") {
+ // crate_type = "rlib"
+ // }
+ //
+ // Which doesn't make sense.
+ if (!target->source_types_used().RustSourceUsed()) {
+ return CRATE_AUTO;
+ }
+
+ CrateType crate_type = target->rust_values().crate_type();
+ if (crate_type != CRATE_AUTO) {
+ return crate_type;
+ }
+
+ switch (target->output_type()) {
+ case Target::EXECUTABLE:
+ return CRATE_BIN;
+ case Target::SHARED_LIBRARY:
+ return CRATE_DYLIB;
+ case Target::STATIC_LIBRARY:
+ return CRATE_STATICLIB;
+ case Target::RUST_LIBRARY:
+ return CRATE_RLIB;
+ case Target::RUST_PROC_MACRO:
+ return CRATE_PROC_MACRO;
+ default:
+ return CRATE_AUTO;
+ }
+}
+
+// static
+bool RustValues::IsRustLibrary(const Target* target) {
+ return target->output_type() == Target::RUST_LIBRARY ||
+ InferredCrateType(target) == CRATE_DYLIB ||
+ InferredCrateType(target) == CRATE_PROC_MACRO;
+}
diff --git a/src/gn/rust_values.h b/src/gn/rust_values.h
index 8f7fff6..ee97717 100644
--- a/src/gn/rust_values.h
+++ b/src/gn/rust_values.h
@@ -19,10 +19,13 @@
RustValues();
~RustValues();
- // Library crate types are specified here. Shared library crate types must be
- // specified, all other crate types can be automatically deduced from the
- // target type (e.g. executables use crate_type = "bin", static_libraries use
- // crate_type = "staticlib") unless explicitly set.
+ // Library crate types.
+ //
+ // The default value CRATE_AUTO means the type should be deduced from the
+ // target type (see InferredCrateType() below).
+ //
+ // Shared library crate types must be specified explicitly, all other target
+ // types can be deduced.
enum CrateType {
CRATE_AUTO = 0,
CRATE_BIN,
@@ -45,6 +48,26 @@
CrateType crate_type() const { return crate_type_; }
void set_crate_type(CrateType s) { crate_type_ = s; }
+ // Same as crate_type(), except attempt to resolve CRATE_AUTO based on the
+ // target type.
+ //
+ // Dylib and cdylib targets should call set_crate_type(CRATE_[C]DYLIB)
+ // explicitly to resolve ambiguity. For shared libraries, this assumes
+ // CRATE_DYLIB by default.
+ //
+ // For unsupported target types and targets without Rust sources,
+ // returns CRATE_AUTO.
+ static CrateType InferredCrateType(const Target* target);
+
+ // Returns whether this target is a Rust rlib, dylib, or proc macro.
+ //
+ // Notably, this does not include staticlib or cdylib targets that have Rust
+ // source, because they look like native libraries to the Rust compiler.
+ //
+ // It does include proc_macro targets, which are sometimes a special case.
+ // (TODO: Should it?)
+ static bool IsRustLibrary(const Target* target);
+
// Any renamed dependencies for the `extern` flags.
const std::map<Label, std::string>& aliased_deps() const {
return aliased_deps_;
diff --git a/src/gn/substitution_list.cc b/src/gn/substitution_list.cc
index ecef4f5..21311c6 100644
--- a/src/gn/substitution_list.cc
+++ b/src/gn/substitution_list.cc
@@ -59,7 +59,8 @@
Err err;
SubstitutionList result;
- result.Parse(input_strings, nullptr, &err);
+ CHECK(result.Parse(input_strings, nullptr, &err))
+ << err.message() << "\n" << err.help_text();
return result;
}
diff --git a/src/gn/substitution_pattern.cc b/src/gn/substitution_pattern.cc
index 8465903..35c14fa 100644
--- a/src/gn/substitution_pattern.cc
+++ b/src/gn/substitution_pattern.cc
@@ -93,7 +93,8 @@
SubstitutionPattern SubstitutionPattern::MakeForTest(const char* str) {
Err err;
SubstitutionPattern pattern;
- CHECK(pattern.Parse(str, nullptr, &err)) << err.message();
+ CHECK(pattern.Parse(str, nullptr, &err))
+ << err.message() << "\n" << err.help_text();
return pattern;
}
diff --git a/src/gn/target.cc b/src/gn/target.cc
index f9e063b..f0af195 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -643,12 +643,8 @@
void Target::PullDependentTargetLibsFrom(const Target* dep, bool is_public) {
// Direct dependent libraries.
if (dep->output_type() == STATIC_LIBRARY ||
- dep->output_type() == SHARED_LIBRARY) {
- inherited_libraries_.Append(dep, is_public);
- rust_values().transitive_libs().Append(dep, is_public);
- }
-
- if (dep->output_type() == RUST_LIBRARY ||
+ dep->output_type() == SHARED_LIBRARY ||
+ dep->output_type() == RUST_LIBRARY ||
dep->output_type() == RUST_PROC_MACRO ||
dep->output_type() == SOURCE_SET ||
(dep->output_type() == CREATE_BUNDLE &&
@@ -656,18 +652,24 @@
inherited_libraries_.Append(dep, is_public);
}
- // Propagate public dependent libraries.
- for (const auto& transitive :
- dep->rust_values().transitive_libs().GetOrderedAndPublicFlag()) {
- if (transitive.second &&
- (dep->output_type() == STATIC_LIBRARY ||
- dep->output_type() == SHARED_LIBRARY)) {
- rust_values().transitive_libs().Append(transitive.first, is_public);
+ if (dep->output_type() == STATIC_LIBRARY ||
+ dep->output_type() == SHARED_LIBRARY ||
+ dep->output_type() == RUST_LIBRARY) {
+ rust_values().transitive_libs().Append(dep, is_public);
+
+ // Propagate public dependent libraries.
+ for (const auto& transitive :
+ dep->rust_values().transitive_libs().GetOrderedAndPublicFlag()) {
+ if (transitive.second) {
+ rust_values().transitive_libs().Append(transitive.first, is_public);
+ }
}
}
- if (dep->output_type() == RUST_LIBRARY) {
- rust_values().transitive_libs().Append(dep, is_public);
+ // Rust libraries (those meant for consumption by another Rust target) are
+ // handled the same way, whether static or dynamic.
+ if (dep->output_type() == RUST_LIBRARY ||
+ RustValues::InferredCrateType(dep) == RustValues::CRATE_DYLIB) {
rust_values().transitive_libs().AppendInherited(
dep->rust_values().transitive_libs(), is_public);
@@ -675,8 +677,7 @@
// 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)) {
+ if (!RustValues::IsRustLibrary(inherited.first)) {
inherited_libraries_.Append(inherited.first, inherited.second);
}
}
@@ -704,7 +705,8 @@
//
// Static libraries and source sets aren't inherited across shared
// library boundaries because they will be linked into the shared
- // library.
+ // library. Rust dylib deps are handled above and transitive deps are
+ // resolved by the compiler.
inherited_libraries_.AppendPublicSharedLibraries(dep->inherited_libraries(),
is_public);
} else if (!dep->IsFinal()) {
diff --git a/src/gn/tool.cc b/src/gn/tool.cc
index 18f2036..bf3aba6 100644
--- a/src/gn/tool.cc
+++ b/src/gn/tool.cc
@@ -346,24 +346,7 @@
// rules). See the header for why.
// TODO(crbug.com/gn/39): Don't emit stamp files for single-output targets.
if (target->source_types_used().RustSourceUsed()) {
- switch (target->rust_values().crate_type()) {
- case RustValues::CRATE_AUTO: {
- switch (target->output_type()) {
- case Target::EXECUTABLE:
- return RustTool::kRsToolBin;
- case Target::SHARED_LIBRARY:
- return RustTool::kRsToolDylib;
- case Target::STATIC_LIBRARY:
- return RustTool::kRsToolStaticlib;
- case Target::RUST_LIBRARY:
- return RustTool::kRsToolRlib;
- case Target::RUST_PROC_MACRO:
- return RustTool::kRsToolMacro;
- default:
- break;
- }
- break;
- }
+ switch (target->rust_values().InferredCrateType(target)) {
case RustValues::CRATE_BIN:
return RustTool::kRsToolBin;
case RustValues::CRATE_CDYLIB:
@@ -376,6 +359,8 @@
return RustTool::kRsToolRlib;
case RustValues::CRATE_STATICLIB:
return RustTool::kRsToolStaticlib;
+ case RustValues::CRATE_AUTO:
+ break;
default:
NOTREACHED();
}
