Allow rlib linking into C++ targets.
Previously, we assumed that the only way to link Rust rlibs into C++ was
by having an intermediate Rust "static library" target, which is in
effect a complete static library containing all the dependencies of that
particular Rust code (including transitive dependencies and the standard
library).
[rlibs] -> [Rust staticlib] -> [solink/link etc.]
But this doesn't work in cases where multiple Rust libraries may
sometimes be linked in different permutations. In Chrome we have
just that situation. In a debug build, Chrome exists as multiple
.so files, whilst in a release build, it's linked into a single
executable.
Debug build:
[rlib A] -> [Rust staticlib A] -> [solink A]
[rlib B] -> [Rust staticlib B] -> [solink B]
Release build:
[rlib A] -> [Rust staticlib A] -\
|
[rlib B] -> [Rust staticlib B] -+--> [exe - violates ODR]
The ODR violations in producing the final linked product assume
there are bits of Rust stuff repeated across both staticlibs.
e.g. bits of the Rust standard library. This would also happen
if rlibs A and B both depended upon rlib C.
Furthermore, these rlibs are so deeply nested in our dependency
tree that they stand no realistic chance of determining whether
the final product is a single binary or several binaries. gn does
not encourage this kind of global knowledge anyway, and it feels wrong
for a distant rlib to make decisions based upon the final output.
Therefore, we have switched to an alternative strategy where Rust rlibs
are linked directly into the final C++ executables and shared objects,
together with the Rust standard library and other dependencies which
are normally inserted by rustc as it calls the linker.
In order to achieve this with gn, we need rlibs to be linked into C++
targets.
Instead of including them directly into the {{inputs}} for a C++
link target, they're provided as a separate
rlibs = ...
substitution. This is because some toolchains may wish to treat them
differently (for instance, not including them inside a
-Wl,--whole-archive directive).
This should have no effect on projects using the former strategy
of linking rlibs together into a static library, because that static
library should "absorb" its dependencies rather than passing them on
to downstream C++ targets, and thus such dependencies should not be
visible to the Ninja C target writer.
An alternative would have been to place such rlibs within the existing
libs =
subtitution; however, such entries are typically of the form "-lfoo"
and are assembled from config directives rather than OutputFile paths.
This change doesn't deliberately change any behavior relating to
Rust procedural macros. It seems questionable to have any C++ targets
ever depend directly on procedural macros; further work is required
here.
Change-Id: I5fd81a872339e7334889fc3b73d346586163f0dc
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/8260
Commit-Queue: Brett Wilson <brettw@chromium.org>
Reviewed-by: Petr Hosek <phosek@google.com>
diff --git a/docs/reference.md b/docs/reference.md
index 2cdff65..afd928c 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -3596,7 +3596,7 @@
tool("link") {
command = "link -o {{output}} {{ldflags}} @{{output}}.rsp"
rspfile = "{{output}}.rsp"
- rspfile_content = "{{inputs}} {{solibs}} {{libs}}"
+ rspfile_content = "{{inputs}} {{solibs}} {{libs}} {{rlibs}}"
}
runtime_outputs [string list with substitutions]
@@ -3746,6 +3746,13 @@
Example: "libfoo.so libbar.so"
+ {{rlibs}}
+ Any Rust .rlibs which need to be linked into a final C++ target.
+ These should be treated as {{inputs}} except that sometimes
+ they might have different linker directives applied.
+
+ Example: "obj/foo/libfoo.rlib"
+
{{frameworks}}
Shared libraries packaged as framework bundle. This is principally
used on Apple's platforms (macOS and iOS). All name must be ending
diff --git a/src/gn/c_substitution_type.cc b/src/gn/c_substitution_type.cc
index b21fea4..b9b7eba 100644
--- a/src/gn/c_substitution_type.cc
+++ b/src/gn/c_substitution_type.cc
@@ -19,6 +19,7 @@
&CSubstitutionLinkerInputs, &CSubstitutionLinkerInputsNewline,
&CSubstitutionLdFlags, &CSubstitutionLibs,
&CSubstitutionSoLibs, &CSubstitutionFrameworks,
+ &CSubstitutionRlibs,
&CSubstitutionArFlags,
};
@@ -44,6 +45,7 @@
const Substitution CSubstitutionLdFlags = {"{{ldflags}}", "ldflags"};
const Substitution CSubstitutionLibs = {"{{libs}}", "libs"};
const Substitution CSubstitutionSoLibs = {"{{solibs}}", "solibs"};
+const Substitution CSubstitutionRlibs = {"{{rlibs}}", "rlibs"};
const Substitution CSubstitutionFrameworks = {"{{frameworks}}", "frameworks"};
// Valid for alink only.
@@ -71,7 +73,8 @@
type == &CSubstitutionLinkerInputs ||
type == &CSubstitutionLinkerInputsNewline ||
type == &CSubstitutionLdFlags || type == &CSubstitutionLibs ||
- type == &CSubstitutionSoLibs || type == &CSubstitutionFrameworks;
+ type == &CSubstitutionSoLibs || type == &CSubstitutionFrameworks ||
+ type == &CSubstitutionRlibs;
}
bool IsValidLinkerOutputsSubstitution(const Substitution* type) {
diff --git a/src/gn/c_substitution_type.h b/src/gn/c_substitution_type.h
index e955312..d4ba606 100644
--- a/src/gn/c_substitution_type.h
+++ b/src/gn/c_substitution_type.h
@@ -31,6 +31,7 @@
extern const Substitution CSubstitutionLibs;
extern const Substitution CSubstitutionSoLibs;
extern const Substitution CSubstitutionFrameworks;
+extern const Substitution CSubstitutionRlibs;
// Valid for alink only.
extern const Substitution CSubstitutionArFlags;
diff --git a/src/gn/function_toolchain.cc b/src/gn/function_toolchain.cc
index e5ddc59..8175c8d 100644
--- a/src/gn/function_toolchain.cc
+++ b/src/gn/function_toolchain.cc
@@ -542,7 +542,7 @@
tool("link") {
command = "link -o {{output}} {{ldflags}} @{{output}}.rsp"
rspfile = "{{output}}.rsp"
- rspfile_content = "{{inputs}} {{solibs}} {{libs}}"
+ rspfile_content = "{{inputs}} {{solibs}} {{libs}} {{rlibs}}"
}
runtime_outputs [string list with substitutions]
@@ -691,6 +691,13 @@
Example: "libfoo.so libbar.so"
+ {{rlibs}}
+ Any Rust .rlibs which need to be linked into a final C++ target.
+ These should be treated as {{inputs}} except that sometimes
+ they might have different linker directives applied.
+
+ Example: "obj/foo/libfoo.rlib"
+
{{frameworks}}
Shared libraries packaged as framework bundle. This is principally
used on Apple's platforms (macOS and iOS). All name must be ending
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index 5fa646f..aba4b68 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -530,6 +530,19 @@
if (!input_dep.value().empty() && object_files.empty())
implicit_deps.push_back(input_dep);
+ // Any C++ target which depends on a Rust .rlib has to depend on its
+ // entire tree of transitive rlibs.
+ std::vector<OutputFile> transitive_rustlibs;
+ if (target_->IsFinal()) {
+ for (const auto* dep :
+ target_->rust_values().transitive_libs().GetOrdered()) {
+ if (dep->output_type() == Target::RUST_LIBRARY) {
+ transitive_rustlibs.push_back(dep->dependency_output_file());
+ implicit_deps.push_back(dep->dependency_output_file());
+ }
+ }
+ }
+
// Append implicit dependencies collected above.
if (!implicit_deps.empty()) {
out_ << " |";
@@ -574,7 +587,8 @@
out_ << std::endl;
}
WriteOutputSubstitutions();
- WriteSolibs(solibs);
+ WriteLibsList("solibs", solibs);
+ WriteLibsList("rlibs", transitive_rustlibs);
}
void NinjaCBinaryTargetWriter::WriteOutputSubstitutions() {
@@ -588,13 +602,14 @@
out_ << std::endl;
}
-void NinjaCBinaryTargetWriter::WriteSolibs(
- const std::vector<OutputFile>& solibs) {
- if (solibs.empty())
+void NinjaCBinaryTargetWriter::WriteLibsList(
+ const std::string& label,
+ const std::vector<OutputFile>& libs) {
+ if (libs.empty())
return;
- out_ << " solibs =";
- path_output_.WriteFiles(out_, solibs);
+ out_ << " " << label << " =";
+ path_output_.WriteFiles(out_, libs);
out_ << std::endl;
}
diff --git a/src/gn/ninja_c_binary_target_writer.h b/src/gn/ninja_c_binary_target_writer.h
index ee6b6ac..d69baf2 100644
--- a/src/gn/ninja_c_binary_target_writer.h
+++ b/src/gn/ninja_c_binary_target_writer.h
@@ -78,7 +78,8 @@
const std::vector<SourceFile>& other_files,
const OutputFile& input_dep);
void WriteOutputSubstitutions();
- void WriteSolibs(const std::vector<OutputFile>& solibs);
+ void WriteLibsList(const std::string& label,
+ const std::vector<OutputFile>& libs);
// Writes the implicit dependencies for the link or stamp line. This is
// the "||" and everything following it on the ninja line.
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc
index 6b0fd64..315232b 100644
--- a/src/gn/ninja_c_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -1326,12 +1326,64 @@
library_target.SetToolchain(setup.toolchain());
ASSERT_TRUE(library_target.OnResolved(&err));
+ Target rlib_target2(setup.settings(), Label(SourceDir("//qux/"), "lib2"));
+ rlib_target2.set_output_type(Target::RUST_LIBRARY);
+ rlib_target2.visibility().SetPublic();
+ SourceFile quxlib("//qux/lib.rs");
+ rlib_target2.sources().push_back(quxlib);
+ rlib_target2.source_types_used().Set(SourceFile::SOURCE_RS);
+ rlib_target2.rust_values().set_crate_root(quxlib);
+ rlib_target2.rust_values().crate_name() = "lib2";
+ rlib_target2.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(rlib_target2.OnResolved(&err));
+
+ Target rlib_target3(setup.settings(), Label(SourceDir("//quxqux/"), "lib3"));
+ rlib_target3.set_output_type(Target::RUST_LIBRARY);
+ rlib_target3.visibility().SetPublic();
+ SourceFile quxquxlib("//quxqux/lib.rs");
+ rlib_target3.sources().push_back(quxquxlib);
+ rlib_target3.source_types_used().Set(SourceFile::SOURCE_RS);
+ rlib_target3.rust_values().set_crate_root(quxlib);
+ rlib_target3.rust_values().crate_name() = "lib3";
+ rlib_target3.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(rlib_target3.OnResolved(&err));
+
+ Target procmacro(setup.settings(),
+ Label(SourceDir("//quuxmacro/"), "procmacro"));
+ procmacro.set_output_type(Target::RUST_PROC_MACRO);
+ procmacro.visibility().SetPublic();
+ SourceFile procmacrolib("//procmacro/lib.rs");
+ procmacro.sources().push_back(procmacrolib);
+ procmacro.source_types_used().Set(SourceFile::SOURCE_RS);
+ procmacro.public_deps().push_back(LabelTargetPair(&rlib_target2));
+ procmacro.public_deps().push_back(LabelTargetPair(&rlib_target3));
+ procmacro.rust_values().set_crate_root(procmacrolib);
+ procmacro.rust_values().crate_name() = "procmacro";
+ procmacro.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(procmacro.OnResolved(&err));
+
+ Target rlib_target4(setup.settings(), Label(SourceDir("//quux/"), "lib4"));
+ rlib_target4.set_output_type(Target::RUST_LIBRARY);
+ rlib_target4.visibility().SetPublic();
+ SourceFile quuxlib("//quux/lib.rs");
+ rlib_target4.sources().push_back(quuxlib);
+ rlib_target4.source_types_used().Set(SourceFile::SOURCE_RS);
+ rlib_target4.public_deps().push_back(LabelTargetPair(&rlib_target2));
+ // Transitive proc macros should not impact C++ targets; we're
+ // adding one to ensure the ninja instructions below are unaffected.
+ rlib_target4.public_deps().push_back(LabelTargetPair(&procmacro));
+ rlib_target4.rust_values().set_crate_root(quuxlib);
+ rlib_target4.rust_values().crate_name() = "lib4";
+ rlib_target4.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(rlib_target4.OnResolved(&err));
+
Target target(setup.settings(), Label(SourceDir("//bar/"), "bar"));
target.set_output_type(Target::EXECUTABLE);
target.visibility().SetPublic();
target.sources().push_back(SourceFile("//bar/bar.cc"));
target.source_types_used().Set(SourceFile::SOURCE_CPP);
target.private_deps().push_back(LabelTargetPair(&library_target));
+ target.private_deps().push_back(LabelTargetPair(&rlib_target4));
target.SetToolchain(setup.toolchain());
ASSERT_TRUE(target.OnResolved(&err));
@@ -1350,12 +1402,14 @@
"\n"
"build obj/bar/bar.bar.o: cxx ../../bar/bar.cc\n"
"\n"
- "build ./bar: link obj/bar/bar.bar.o obj/foo/libfoo.a\n"
+ "build ./bar: link obj/bar/bar.bar.o obj/foo/libfoo.a | "
+ "obj/quux/lib4.rlib obj/qux/lib2.rlib\n"
" ldflags =\n"
" libs =\n"
" frameworks =\n"
" output_extension = \n"
- " output_dir = \n";
+ " output_dir = \n"
+ " rlibs = obj/quux/lib4.rlib obj/qux/lib2.rlib\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;