Avoid unused and incorrect linker args in {{rustdeps}}
For non-final Rust targets, libraries and library search paths do not
need to appear in the {{rustdeps}} at all, since there is no linker
step that will take place to consume them.
For framework search paths, pass `-Lframework=$dir` to `rustc` instead
of just `$dir` which then is incorrectly seen as being an input source
file.
The -Lframework flag is listed here:
https://doc.rust-lang.org/rustc/command-line-arguments.html#-l-add-a-directory-to-the-library-search-path
Bug: 340
Change-Id: Idb1e4e7cc72f465084f3cf9464bdc605b1535c13
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/15580
Reviewed-by: Tyler Mandry <tmandry@google.com>
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Commit-Queue: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/json_project_writer_unittest.cc b/src/gn/json_project_writer_unittest.cc
index 1915c30..cab6dd7 100644
--- a/src/gn/json_project_writer_unittest.cc
+++ b/src/gn/json_project_writer_unittest.cc
@@ -158,6 +158,7 @@
},
"rust_bin": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -167,6 +168,7 @@
"rust_cdylib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -177,6 +179,7 @@
"rust_dylib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -187,6 +190,7 @@
"rust_macro": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -197,6 +201,7 @@
"rust_rlib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".rlib",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -207,6 +212,7 @@
"rust_staticlib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".a",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -377,6 +383,7 @@
},
"rust_bin": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -386,6 +393,7 @@
"rust_cdylib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -396,6 +404,7 @@
"rust_dylib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -406,6 +415,7 @@
"rust_macro": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -416,6 +426,7 @@
"rust_rlib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".rlib",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -426,6 +437,7 @@
"rust_staticlib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".a",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -620,6 +632,7 @@
},
"rust_bin": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -629,6 +642,7 @@
"rust_cdylib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -639,6 +653,7 @@
"rust_dylib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -649,6 +664,7 @@
"rust_macro": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".so",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -659,6 +675,7 @@
"rust_rlib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".rlib",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
@@ -669,6 +686,7 @@
"rust_staticlib": {
"command": "{{rustenv}} rustc --crate-name {{crate_name}} {{source}} --crate-type {{crate_type}} {{rustflags}} -o {{output}} {{rustdeps}} {{externs}}",
"default_output_extension": ".a",
+ "framework_dir_switch": "-Lframework=",
"framework_switch": "-lframework=",
"lib_dir_switch": "-Lnative=",
"lib_switch": "-l",
diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc
index 694f332..554f88c 100644
--- a/src/gn/ninja_rust_binary_target_writer.cc
+++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -209,7 +209,8 @@
std::copy(classified_deps.non_linkable_deps.begin(),
classified_deps.non_linkable_deps.end(),
std::back_inserter(extern_deps));
- WriteExternsAndDeps(extern_deps, transitive_crates, rustdeps, nonrustdeps);
+ WriteExternsAndDeps(target_->IsFinal(), extern_deps, transitive_crates,
+ rustdeps, nonrustdeps);
WriteSourcesAndInputs();
WritePool(out_);
}
@@ -255,6 +256,7 @@
}
void NinjaRustBinaryTargetWriter::WriteExternsAndDeps(
+ bool target_is_final,
const std::vector<const Target*>& deps,
const std::vector<ExternCrate>& transitive_rust_deps,
const std::vector<OutputFile>& rustdeps,
@@ -347,30 +349,33 @@
path_output_.WriteDir(out_, dir, PathOutput::DIR_NO_LAST_SLASH);
}
- // Non-Rust native dependencies.
- UniqueVector<SourceDir> nonrustdep_dirs;
- for (const auto& nonrustdep : nonrustdeps) {
- nonrustdep_dirs.push_back(
- nonrustdep.AsSourceFile(settings_->build_settings()).GetDir());
+ if (target_is_final) {
+ // Non-Rust native dependencies.
+ UniqueVector<SourceDir> nonrustdep_dirs;
+ for (const auto& nonrustdep : nonrustdeps) {
+ nonrustdep_dirs.push_back(
+ nonrustdep.AsSourceFile(settings_->build_settings()).GetDir());
+ }
+ // 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);
+ }
+ // Before outputting any libraries to link, ensure the linker is in a mode
+ // that allows dynamic linking, as rustc may have previously put it into
+ // static-only mode.
+ if (nonrustdeps.size() > 0) {
+ out_ << " " << tool_->dynamic_link_switch();
+ }
+ for (const auto& nonrustdep : nonrustdeps) {
+ out_ << " -Clink-arg=";
+ path_output_.WriteFile(out_, nonrustdep);
+ }
+ WriteLibrarySearchPath(out_, tool_);
+ WriteLibs(out_, tool_);
}
- // 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);
- }
- // Before outputting any libraries to link, ensure the linker is in a mode
- // that allows dynamic linking, as rustc may have previously put it into
- // static-only mode.
- if (nonrustdeps.size() > 0) {
- out_ << " " << tool_->dynamic_link_switch();
- }
- for (const auto& nonrustdep : nonrustdeps) {
- out_ << " -Clink-arg=";
- path_output_.WriteFile(out_, nonrustdep);
- }
- WriteLibrarySearchPath(out_, tool_);
- WriteLibs(out_, tool_);
out_ << std::endl;
out_ << " ldflags =";
WriteCustomLinkerFlags(out_, tool_);
diff --git a/src/gn/ninja_rust_binary_target_writer.h b/src/gn/ninja_rust_binary_target_writer.h
index 69ec916..470a899 100644
--- a/src/gn/ninja_rust_binary_target_writer.h
+++ b/src/gn/ninja_rust_binary_target_writer.h
@@ -28,7 +28,8 @@
void WriteCompilerVars();
void WriteSources(const OutputFile& input_dep,
const std::vector<OutputFile>& order_only_deps);
- void WriteExternsAndDeps(const std::vector<const Target*>& deps,
+ void WriteExternsAndDeps(bool target_is_final,
+ const std::vector<const Target*>& deps,
const std::vector<ExternCrate>& transitive_rust_deps,
const std::vector<OutputFile>& rustdeps,
const std::vector<OutputFile>& nonrustdeps);
diff --git a/src/gn/ninja_rust_binary_target_writer_unittest.cc b/src/gn/ninja_rust_binary_target_writer_unittest.cc
index 21b104f..5eda4a7 100644
--- a/src/gn/ninja_rust_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -896,8 +896,7 @@
" source_file_part = lib.rs\n"
" source_name_part = lib\n"
" externs =\n"
- " rustdeps = -Lnative=obj/foo -Clink-arg=-Balternative-dynamic "
- "-Clink-arg=obj/foo/libstatic.a\n"
+ " rustdeps =\n"
" ldflags =\n"
" sources = ../../baz/lib.rs\n";
std::string out_str = out.str();
@@ -1146,6 +1145,21 @@
Err err;
TestWithScope setup;
+ Target rlib(setup.settings(), Label(SourceDir("//bar/"), "rlib"));
+ rlib.set_output_type(Target::RUST_LIBRARY);
+ rlib.visibility().SetPublic();
+ SourceFile barlib("//bar/lib.rs");
+ rlib.sources().push_back(SourceFile("//bar/rlib.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() = "rlibcrate";
+ rlib.config_values().libs().push_back(LibFile("rliblib"));
+ rlib.config_values().lib_dirs().push_back(SourceDir("//rliblibdir/"));
+ rlib.config_values().framework_dirs().push_back(SourceDir("//rlibfwdir/"));
+ rlib.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(rlib.OnResolved(&err));
+
Target target(setup.settings(), Label(SourceDir("//foo/"), "bar"));
target.set_output_type(Target::EXECUTABLE);
target.visibility().SetPublic();
@@ -1154,8 +1168,12 @@
target.sources().push_back(main);
target.source_types_used().Set(SourceFile::SOURCE_RS);
target.set_output_dir(SourceDir("//out/Debug/foo/"));
- target.config_values().libs().push_back(LibFile("quux"));
- target.config_values().lib_dirs().push_back(SourceDir("//baz/"));
+ target.config_values().libs().push_back(
+ LibFile(SourceFile("//dir1/ar.a")));
+ target.config_values().libs().push_back(LibFile("binlib"));
+ target.config_values().lib_dirs().push_back(SourceDir("//binlibdir/"));
+ target.config_values().framework_dirs().push_back(SourceDir("//binfwdir/"));
+ target.public_deps().push_back(LabelTargetPair(&rlib));
target.rust_values().set_crate_root(main);
target.rust_values().crate_name() = "foo_bar";
target.SetToolchain(setup.toolchain());
@@ -1178,11 +1196,14 @@
"target_output_name = bar\n"
"\n"
"build ./foo_bar: rust_bin ../../foo/main.rs | ../../foo/input.rs "
- "../../foo/main.rs\n"
+ "../../foo/main.rs obj/bar/librlib.rlib\n"
" source_file_part = main.rs\n"
" source_name_part = main\n"
- " externs =\n"
- " rustdeps = -Lnative=../../baz -lquux\n"
+ " externs = --extern rlibcrate=obj/bar/librlib.rlib\n"
+ " rustdeps = -Ldependency=obj/bar -Lnative=../../binlibdir "
+ "-Lnative=../../rliblibdir -Lframework=../../binfwdir "
+ "-Lframework=../../rlibfwdir -Clink-arg=../../dir1/ar.a -lbinlib "
+ "-lrliblib\n"
" ldflags =\n"
" sources = ../../foo/input.rs ../../foo/main.rs\n";
std::string out_str = out.str();
@@ -1190,6 +1211,81 @@
}
}
+TEST_F(NinjaRustBinaryTargetWriterTest, RlibWithLibDeps) {
+ Err err;
+ TestWithScope setup;
+
+ Target public_rlib(setup.settings(), Label(SourceDir("//bar/"), "publiclib"));
+ public_rlib.set_output_type(Target::RUST_LIBRARY);
+ public_rlib.visibility().SetPublic();
+ SourceFile barlib("//bar/lib.rs");
+ public_rlib.sources().push_back(SourceFile("//bar/publiclib.rs"));
+ public_rlib.sources().push_back(barlib);
+ public_rlib.source_types_used().Set(SourceFile::SOURCE_RS);
+ public_rlib.rust_values().set_crate_root(barlib);
+ public_rlib.rust_values().crate_name() = "publiccrate";
+ public_rlib.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(public_rlib.OnResolved(&err));
+
+ Target rlib(setup.settings(), Label(SourceDir("//foo/"), "rlibcrate"));
+ rlib.set_output_type(Target::RUST_LIBRARY);
+ rlib.visibility().SetPublic();
+ SourceFile lib("//foo/input.rs");
+ rlib.sources().push_back(lib);
+ rlib.source_types_used().Set(SourceFile::SOURCE_RS);
+ rlib.rust_values().set_crate_root(lib);
+ rlib.rust_values().crate_name() = "rlibcrate";
+ rlib.SetToolchain(setup.toolchain());
+ rlib.public_deps().push_back(LabelTargetPair(&public_rlib));
+
+ // This adds dependencies on static libraries, which are not consumed by an
+ // rlib since it does not get linked. None of these should appear in
+ // `rustdeps` for a `rust_rlib`, though they would for a `rust_bin` (as tested
+ // for above).
+ //
+ // 1. A dependency on an archive file directly as happens with a `deps` rule
+ // pointing to a `static_library` target.
+ rlib.config_values().libs().push_back(
+ LibFile(SourceFile("//dir1/ar.a")));
+ // 2. A dependency on a library name as happens with a `libs` rule.
+ rlib.config_values().libs().push_back(LibFile("quux"));
+ // 3. A dependency on a library path which will be used for linking, which is
+ // separate from the dependency paths for finding Rust crates.
+ rlib.config_values().lib_dirs().push_back(SourceDir("//baz/"));
+ // 4. A framework search directory will be used for linking frameworks, which
+ // is also separate from finding Rust crates.
+ rlib.config_values().framework_dirs().push_back(SourceDir("//fwdir/"));
+
+ ASSERT_TRUE(rlib.OnResolved(&err));
+
+ {
+ std::ostringstream out;
+ NinjaRustBinaryTargetWriter writer(&rlib, out);
+ writer.Run();
+
+ const char expected[] =
+ "crate_name = rlibcrate\n"
+ "crate_type = rlib\n"
+ "output_extension = .rlib\n"
+ "output_dir = \n"
+ "rustflags =\n"
+ "rustenv =\n"
+ "root_out_dir = .\n"
+ "target_out_dir = obj/foo\n"
+ "target_output_name = librlibcrate\n"
+ "\n"
+ "build obj/foo/librlibcrate.rlib: rust_rlib ../../foo/input.rs | ../../foo/input.rs obj/bar/libpubliclib.rlib\n"
+ " source_file_part = input.rs\n"
+ " source_name_part = input\n"
+ " externs = --extern publiccrate=obj/bar/libpubliclib.rlib\n"
+ " rustdeps = -Ldependency=obj/bar\n"
+ " ldflags =\n"
+ " sources = ../../foo/input.rs\n";
+ std::string out_str = out.str();
+ EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
+ }
+}
+
// Test that neither public nor private rust dependencies of a proc-macro are
// transitively acquired as accessible dependencies by users of the macro. But
// the macro itself is listed as an accessible dependency (via --extern).
diff --git a/src/gn/rust_tool.cc b/src/gn/rust_tool.cc
index 5f2c2de..65ed27a 100644
--- a/src/gn/rust_tool.cc
+++ b/src/gn/rust_tool.cc
@@ -18,6 +18,7 @@
CHECK(ValidateName(n));
// TODO: should these be settable in toolchain definition?
set_framework_switch("-lframework=");
+ set_framework_dir_switch("-Lframework=");
set_lib_dir_switch("-Lnative=");
set_lib_switch("-l");
set_linker_arg("-Clink-arg=");
