[rust] Fix command arguments for non-rust native dependencies.
The ninja Rust binary target writer did the following to generate
the command arguments set to 'rustc' for non-rust native library
dependencies (e.g. any native object file or shared library):
1- Collect the ordered set of each dependency's directory,
then write it as a series of `-Lnative=<dir>` arguments.
2- For any dependency whose name begins with 'lib', write it
as `-l<name>`, where <name> is the dependency's file name
without a "lib" prefix and without a file extension.
2- For non-"lib" dependencies, just use `-Clink-arg=<path>`
instead.
However, while working on the Fuchsia build system, I found that rules 1
and 2 are both problematic:
- Rule 1 means there is a possibility of linking the wrong library
in the final binary. E.g. consider a dependency list like:
dir1/libfoo.so
dir2/libbar.so
This gets translated into:
-Lnative=dir1 -Lnative=dir2 -lfoo -lbar
And if dir1/libbar.so happens to exist, it will be wrongly selected
by the linker.
- Rule 2 assumes the file extension is something like ".so", ".dylib"
or ".a" , i.e. something the linker will look for automatically,
however, in the Fuchsia build, we have to link against files named
"libc.so.debug" and "libzircon.so.debug" (where the ".debug" prefix
means these are the unstripped versions of the libraries). This
ends up translated into:
-Lnative=<dir2> -Lnative=<dir1> -lc.so -lzircon.so
And the linker will look for libc.so.so and libzircon.so.so instead
of the real files (and of course print an error message). Our
current work-around is to create link scripts with this name that
redirect to the real file, but this is ugly.
This CL simply fixes the issue by using `-Clink-arg=<path>` in all
cases. The `-Lnative=<dir>` arguments are still required to make
`#[link(...)]` directives work correctly.
Bug: 217
Change-Id: I5f7210300eea0effe3ed99b4f6aff5f07fb05518
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/10721
Commit-Queue: David Turner <digit@google.com>
Reviewed-by: Petr Hosek <phosek@google.com>
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc
index 59960a1..38f4686 100644
--- a/src/gn/ninja_rust_binary_target_writer.cc
+++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -299,7 +299,6 @@
EscapeOptions lib_escape_opts;
lib_escape_opts.mode = ESCAPE_NINJA_COMMAND;
- const std::string_view lib_prefix("lib");
// Non-Rust native dependencies.
UniqueVector<SourceDir> nonrustdep_dirs;
@@ -307,22 +306,15 @@
nonrustdep_dirs.push_back(
nonrustdep.AsSourceFile(settings_->build_settings()).GetDir());
}
- // First -Lnative to specify search directories
+ // First -Lnative to specify the search directories.
+ // This is necessary for #[link(...)] directives to work properly.
for (const auto& nonrustdep_dir : nonrustdep_dirs) {
out_ << " -Lnative=";
path_output_.WriteDir(out_, nonrustdep_dir, PathOutput::DIR_NO_LAST_SLASH);
}
- // Now the dependencies themselves.
for (const auto& nonrustdep : nonrustdeps) {
- std::string_view file = FindFilenameNoExtension(&nonrustdep.value());
- if (!file.compare(0, lib_prefix.size(), lib_prefix)) {
- out_ << " -l";
- EscapeStringToStream(out_, file.substr(lib_prefix.size()),
- lib_escape_opts);
- } else {
- out_ << " -Clink-arg=";
- path_output_.WriteFile(out_, nonrustdep);
- }
+ out_ << " -Clink-arg=";
+ path_output_.WriteFile(out_, nonrustdep);
}
WriteLinkerFlags(out_, tool_, nullptr);
diff --git a/src/gn/ninja_rust_binary_target_writer_unittest.cc b/src/gn/ninja_rust_binary_target_writer_unittest.cc
index e58572b..6840b72 100644
--- a/src/gn/ninja_rust_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -438,9 +438,11 @@
"obj/foo/libstatic.a ./libshared.so ./libshared_with_toc.so.TOC "
"|| obj/baz/sourceset.stamp\n"
" externs = --extern mylib=obj/bar/libmylib.rlib\n"
- " rustdeps = -Ldependency=obj/bar -Lnative=obj/baz -Lnative=obj/foo "
- "-Lnative=. -Clink-arg=obj/baz/sourceset.csourceset.o -lstatic "
- "-lshared -lshared_with_toc\n"
+ " rustdeps = -Ldependency=obj/bar "
+ "-Lnative=obj/baz -Lnative=obj/foo -Lnative=. "
+ "-Clink-arg=obj/baz/sourceset.csourceset.o "
+ "-Clink-arg=obj/foo/libstatic.a -Clink-arg=./libshared.so "
+ "-Clink-arg=./libshared_with_toc.so\n"
" sources = ../../foo/source.rs ../../foo/main.rs\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
@@ -477,7 +479,7 @@
"build ./foo_bar: rust_bin ../../foo/main.rs | ../../foo/source.rs "
"../../foo/main.rs obj/foo/libstatic.a\n"
" externs =\n"
- " rustdeps = -Lnative=obj/foo -lstatic\n"
+ " rustdeps = -Lnative=obj/foo -Clink-arg=obj/foo/libstatic.a\n"
" sources = ../../foo/source.rs ../../foo/main.rs\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
@@ -511,10 +513,11 @@
"target_out_dir = obj/baz\n"
"target_output_name = libbaz\n"
"\n"
- "build obj/baz/libbaz.a: rust_staticlib ../../baz/lib.rs | ../../baz/lib.rs "
+ "build obj/baz/libbaz.a: rust_staticlib ../../baz/lib.rs | "
+ "../../baz/lib.rs "
"obj/foo/libstatic.a\n"
" externs =\n"
- " rustdeps = -Lnative=obj/foo -lstatic\n"
+ " rustdeps = -Lnative=obj/foo -Clink-arg=obj/foo/libstatic.a\n"
" sources = ../../baz/lib.rs\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
