Fix Rust linking -lfoo.so not -lfoo due to TOC
Some toolchains generate a .so.TOC as well as a .so file when creating
shared libraries; they did not previously link correctly into downstream
Rust targets because the generated Ninja rules asked to link to -lfoo.so
rather than -lfoo.
The unit test code here is a bit verbose. It seemed desirable to test
with a toolchain that generates TOC files and one that does not.
However, that may be overkill.
Bug: crbug.com/gn/137
Change-Id: I30918957f8da187c5a1fc3e8f76d91fe5fda494f
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/7020
Reviewed-by: Adrian Taylor <adetaylor@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: 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 729bfc7..2ac1978 100644
--- a/src/gn/ninja_rust_binary_target_writer.cc
+++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -87,8 +87,8 @@
WriteVar(kRustSubstitutionCrateType.ninja_name, crate_type, opts, out);
WriteVar(SubstitutionOutputExtension.ninja_name,
- SubstitutionWriter::GetLinkerSubstitution(target, tool,
- &SubstitutionOutputExtension),
+ SubstitutionWriter::GetLinkerSubstitution(
+ target, tool, &SubstitutionOutputExtension),
opts, out);
WriteVar(SubstitutionOutputDir.ninja_name,
SubstitutionWriter::GetLinkerSubstitution(target, tool,
@@ -146,9 +146,9 @@
}
for (const auto* linkable_dep : linkable_deps) {
if (linkable_dep->source_types_used().RustSourceUsed()) {
- rustdeps.push_back(linkable_dep->dependency_output_file());
+ rustdeps.push_back(linkable_dep->link_output_file());
} else {
- nonrustdeps.push_back(linkable_dep->dependency_output_file());
+ nonrustdeps.push_back(linkable_dep->link_output_file());
}
deps.push_back(linkable_dep->dependency_output_file());
}
@@ -184,13 +184,11 @@
EscapeOptions opts = GetFlagOptions();
WriteCrateVars(target_, tool_, opts, out_);
- WriteOneFlag(target_, &kRustSubstitutionRustFlags, false,
- Tool::kToolNone, &ConfigValues::rustflags, opts,
- path_output_, out_);
+ WriteOneFlag(target_, &kRustSubstitutionRustFlags, false, Tool::kToolNone,
+ &ConfigValues::rustflags, opts, path_output_, out_);
- WriteOneFlag(target_, &kRustSubstitutionRustEnv, false,
- Tool::kToolNone, &ConfigValues::rustenv, opts,
- path_output_, out_);
+ WriteOneFlag(target_, &kRustSubstitutionRustEnv, false, Tool::kToolNone,
+ &ConfigValues::rustenv, opts, path_output_, out_);
WriteSharedVars(subst);
}
@@ -250,18 +248,19 @@
const std::string_view lib_prefix("lib");
// Non-Rust native dependencies.
- for (const auto& rustdep : nonrustdeps) {
+ for (const auto& nonrustdep : nonrustdeps) {
out_ << " -Lnative=";
path_output_.WriteDir(
- out_, rustdep.AsSourceFile(settings_->build_settings()).GetDir(),
+ out_, nonrustdep.AsSourceFile(settings_->build_settings()).GetDir(),
PathOutput::DIR_NO_LAST_SLASH);
- std::string_view file = FindFilenameNoExtension(&rustdep.value());
+ 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);
+ EscapeStringToStream(out_, file.substr(lib_prefix.size()),
+ lib_escape_opts);
} else {
out_ << " -Clink-arg=";
- path_output_.WriteFile(out_, rustdep);
+ path_output_.WriteFile(out_, nonrustdep);
}
}
diff --git a/src/gn/ninja_rust_binary_target_writer_unittest.cc b/src/gn/ninja_rust_binary_target_writer_unittest.cc
index d581b21..44ff61c 100644
--- a/src/gn/ninja_rust_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -269,6 +269,26 @@
staticlib.SetToolchain(setup.toolchain());
ASSERT_TRUE(staticlib.OnResolved(&err));
+ Target sharedlib(setup.settings(), Label(SourceDir("//foo/"), "shared"));
+ sharedlib.set_output_type(Target::SHARED_LIBRARY);
+ sharedlib.visibility().SetPublic();
+ sharedlib.sources().push_back(SourceFile("//foo/static.cpp"));
+ sharedlib.source_types_used().Set(SourceFile::SOURCE_CPP);
+ sharedlib.SetToolchain(setup.toolchain());
+ ASSERT_TRUE(sharedlib.OnResolved(&err));
+
+ Toolchain toolchain_with_toc(
+ setup.settings(), Label(SourceDir("//toolchain_with_toc/"), "with_toc"));
+ TestWithScope::SetupToolchain(&toolchain_with_toc, true);
+ Target sharedlib_with_toc(setup.settings(),
+ Label(SourceDir("//foo/"), "shared_with_toc"));
+ sharedlib_with_toc.set_output_type(Target::SHARED_LIBRARY);
+ sharedlib_with_toc.visibility().SetPublic();
+ sharedlib_with_toc.sources().push_back(SourceFile("//foo/static.cpp"));
+ sharedlib_with_toc.source_types_used().Set(SourceFile::SOURCE_CPP);
+ sharedlib_with_toc.SetToolchain(&toolchain_with_toc);
+ ASSERT_TRUE(sharedlib_with_toc.OnResolved(&err));
+
Target nonrust(setup.settings(), Label(SourceDir("//foo/"), "bar"));
nonrust.set_output_type(Target::EXECUTABLE);
nonrust.visibility().SetPublic();
@@ -280,6 +300,8 @@
nonrust.rust_values().crate_name() = "foo_bar";
nonrust.private_deps().push_back(LabelTargetPair(&rlib));
nonrust.private_deps().push_back(LabelTargetPair(&staticlib));
+ nonrust.private_deps().push_back(LabelTargetPair(&sharedlib));
+ nonrust.private_deps().push_back(LabelTargetPair(&sharedlib_with_toc));
nonrust.SetToolchain(setup.toolchain());
ASSERT_TRUE(nonrust.OnResolved(&err));
@@ -300,9 +322,11 @@
"target_output_name = bar\n"
"\n"
"build ./foo_bar: rust_bin ../../foo/main.rs | ../../foo/source.rs "
- "../../foo/main.rs obj/bar/libmylib.rlib obj/foo/libstatic.a\n"
+ "../../foo/main.rs obj/bar/libmylib.rlib obj/foo/libstatic.a "
+ "./libshared.so ./libshared_with_toc.so.TOC\n"
" externs = --extern mylib=obj/bar/libmylib.rlib\n"
- " rustdeps = -Ldependency=obj/bar -Lnative=obj/foo -lstatic\n";
+ " rustdeps = -Ldependency=obj/bar -Lnative=obj/foo -lstatic "
+ "-Lnative=. -lshared -Lnative=. -lshared_with_toc\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str) << expected << "\n" << out_str;
}
diff --git a/src/gn/test_with_scope.cc b/src/gn/test_with_scope.cc
index 06a8772..dda4b2a 100644
--- a/src/gn/test_with_scope.cc
+++ b/src/gn/test_with_scope.cc
@@ -69,7 +69,7 @@
}
// static
-void TestWithScope::SetupToolchain(Toolchain* toolchain) {
+void TestWithScope::SetupToolchain(Toolchain* toolchain, bool use_toc) {
Err err;
// CC
@@ -137,8 +137,18 @@
solink_tool->set_lib_dir_switch("-L");
solink_tool->set_output_prefix("lib");
solink_tool->set_default_output_extension(".so");
- solink_tool->set_outputs(SubstitutionList::MakeForTest(
- "{{root_out_dir}}/{{target_output_name}}{{output_extension}}"));
+ if (use_toc) {
+ solink_tool->set_outputs(SubstitutionList::MakeForTest(
+ "{{root_out_dir}}/{{target_output_name}}{{output_extension}}.TOC",
+ "{{root_out_dir}}/{{target_output_name}}{{output_extension}}"));
+ solink_tool->set_link_output(SubstitutionPattern::MakeForTest(
+ "{{root_out_dir}}/{{target_output_name}}{{output_extension}}"));
+ solink_tool->set_depend_output(SubstitutionPattern::MakeForTest(
+ "{{root_out_dir}}/{{target_output_name}}{{output_extension}}.TOC"));
+ } else {
+ solink_tool->set_outputs(SubstitutionList::MakeForTest(
+ "{{root_out_dir}}/{{target_output_name}}{{output_extension}}"));
+ }
toolchain->SetTool(std::move(solink));
// SOLINK_MODULE
@@ -232,7 +242,8 @@
toolchain->SetTool(std::move(dylib_tool));
// RUST_PROC_MACRO
- std::unique_ptr<Tool> rust_proc_macro_tool = Tool::CreateTool(RustTool::kRsToolMacro);
+ std::unique_ptr<Tool> rust_proc_macro_tool =
+ Tool::CreateTool(RustTool::kRsToolMacro);
SetCommandForTool(
"{{rustenv}} rustc --crate-name {{crate_name}} {{source}} "
"--crate-type {{crate_type}} {{rustflags}} -o {{output}} "
@@ -258,7 +269,8 @@
toolchain->SetTool(std::move(rlib_tool));
// STATICLIB
- std::unique_ptr<Tool> staticlib_tool = Tool::CreateTool(RustTool::kRsToolStaticlib);
+ std::unique_ptr<Tool> staticlib_tool =
+ Tool::CreateTool(RustTool::kRsToolStaticlib);
SetCommandForTool(
"{{rustenv}} rustc --crate-name {{crate_name}} {{source}} "
"--crate-type {{crate_type}} {{rustflags}} -o {{output}} "
diff --git a/src/gn/test_with_scope.h b/src/gn/test_with_scope.h
index de1f967..6292b6c 100644
--- a/src/gn/test_with_scope.h
+++ b/src/gn/test_with_scope.h
@@ -57,7 +57,10 @@
// The toolchain in this object will be automatically set up with this
// function, it is exposed to allow tests to get the same functionality for
// other toolchains they make.
- static void SetupToolchain(Toolchain* toolchain);
+ // Two slightly different toolchains can be generated by this function,
+ // based on the use_toc argument. This is only currently required by
+ // one test.
+ static void SetupToolchain(Toolchain* toolchain, bool use_toc = false);
// Sets the given text command on the given tool, parsing it as a
// substitution pattern. This will assert if the input is malformed. This is
