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