Support non-Rust libs and lib_dirs for Rust targets This is necessary for mixed C/C++ and Rust projects. Change-Id: Iadf9e8d6f2a90ca43fb304b54f4049ed18fdd252 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/5941 Commit-Queue: Petr Hosek <phosek@google.com> Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/tools/gn/c_tool.cc b/tools/gn/c_tool.cc index 59452fb..29d7850 100644 --- a/tools/gn/c_tool.cc +++ b/tools/gn/c_tool.cc
@@ -20,6 +20,10 @@ CTool::CTool(const char* n) : Tool(n), depsformat_(DEPS_GCC), precompiled_header_type_(PCH_NONE) { CHECK(ValidateName(n)); + set_framework_switch("-framework "); + set_lib_dir_switch("-L"); + set_lib_switch("-l"); + set_linker_arg(""); } CTool::~CTool() = default;
diff --git a/tools/gn/c_tool.h b/tools/gn/c_tool.h index 07dfcbe..f9b2653 100644 --- a/tools/gn/c_tool.h +++ b/tools/gn/c_tool.h
@@ -69,18 +69,6 @@ precompiled_header_type_ = pch_type; } - const std::string& lib_switch() const { return lib_switch_; } - void set_lib_switch(std::string s) { - DCHECK(!complete_); - lib_switch_ = std::move(s); - } - - const std::string& lib_dir_switch() const { return lib_dir_switch_; } - void set_lib_dir_switch(std::string s) { - DCHECK(!complete_); - lib_dir_switch_ = std::move(s); - } - // Should match files in the outputs() if nonempty. const SubstitutionPattern& link_output() const { return link_output_; } void set_link_output(SubstitutionPattern link_out) { @@ -126,8 +114,6 @@ DepsFormat depsformat_; PrecompiledHeaderType precompiled_header_type_; - std::string lib_switch_; - std::string lib_dir_switch_; SubstitutionPattern link_output_; SubstitutionPattern depend_output_;
diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc index 3cf7c48..2bbb676 100644 --- a/tools/gn/ninja_binary_target_writer.cc +++ b/tools/gn/ninja_binary_target_writer.cc
@@ -19,6 +19,17 @@ #include "tools/gn/substitution_writer.h" #include "tools/gn/target.h" +namespace { + +// Returns the proper escape options for writing compiler and linker flags. +EscapeOptions GetFlagOptions() { + EscapeOptions opts; + opts.mode = ESCAPE_NINJA_COMMAND; + return opts; +} + +} // namespace + NinjaBinaryTargetWriter::NinjaBinaryTargetWriter(const Target* target, std::ostream& out) : NinjaTargetWriter(target, out), @@ -205,4 +216,63 @@ path_output_.WriteFiles(out_, order_only_deps); } out_ << std::endl; -} \ No newline at end of file +} + +void NinjaBinaryTargetWriter::WriteLinkerFlags( + std::ostream& out, + const Tool* tool, + const SourceFile* optional_def_file) { + if (tool->AsC()) { + // First the ldflags from the target and its config. + RecursiveTargetConfigStringsToStream(target_, &ConfigValues::ldflags, + GetFlagOptions(), out); + } + + // Followed by library search paths that have been recursively pushed + // through the dependency tree. + const OrderedSet<SourceDir> all_lib_dirs = target_->all_lib_dirs(); + if (!all_lib_dirs.empty()) { + // Since we're passing these on the command line to the linker and not + // to Ninja, we need to do shell escaping. + PathOutput lib_path_output(path_output_.current_dir(), + settings_->build_settings()->root_path_utf8(), + ESCAPE_NINJA_COMMAND); + for (size_t i = 0; i < all_lib_dirs.size(); i++) { + out << " " << tool->lib_dir_switch(); + lib_path_output.WriteDir(out, all_lib_dirs[i], + PathOutput::DIR_NO_LAST_SLASH); + } + } + + if (optional_def_file) { + out_ << " /DEF:"; + path_output_.WriteFile(out, *optional_def_file); + } +} + +void NinjaBinaryTargetWriter::WriteLibs(std::ostream& out, const Tool* tool) { + // Libraries that have been recursively pushed through the dependency tree. + EscapeOptions lib_escape_opts; + lib_escape_opts.mode = ESCAPE_NINJA_COMMAND; + const OrderedSet<LibFile> all_libs = target_->all_libs(); + const std::string framework_ending(".framework"); + for (size_t i = 0; i < all_libs.size(); i++) { + const LibFile& lib_file = all_libs[i]; + const std::string& lib_value = lib_file.value(); + if (lib_file.is_source_file()) { + out << " " << tool->linker_arg(); + path_output_.WriteFile(out, lib_file.source_file()); + } else if (base::EndsWith(lib_value, framework_ending, + base::CompareCase::INSENSITIVE_ASCII)) { + // Special-case libraries ending in ".framework" to support Mac: Add the + // -framework switch and don't add the extension to the output. + out << " " << tool->framework_switch(); + EscapeStringToStream( + out, lib_value.substr(0, lib_value.size() - framework_ending.size()), + lib_escape_opts); + } else { + out << " " << tool->lib_switch(); + EscapeStringToStream(out, lib_value, lib_escape_opts); + } + } +}
diff --git a/tools/gn/ninja_binary_target_writer.h b/tools/gn/ninja_binary_target_writer.h index 3274867..c38ee4c 100644 --- a/tools/gn/ninja_binary_target_writer.h +++ b/tools/gn/ninja_binary_target_writer.h
@@ -57,6 +57,11 @@ const char* tool_name, const std::vector<OutputFile>& outputs); + void WriteLinkerFlags(std::ostream& out, + const Tool* tool, + const SourceFile* optional_def_file); + void WriteLibs(std::ostream& out, const Tool* tool); + virtual void AddSourceSetFiles(const Target* source_set, UniqueVector<OutputFile>* obj_files) const;
diff --git a/tools/gn/ninja_c_binary_target_writer.cc b/tools/gn/ninja_c_binary_target_writer.cc index 04698b5..594fec5 100644 --- a/tools/gn/ninja_c_binary_target_writer.cc +++ b/tools/gn/ninja_c_binary_target_writer.cc
@@ -525,8 +525,12 @@ if (target_->output_type() == Target::EXECUTABLE || target_->output_type() == Target::SHARED_LIBRARY || target_->output_type() == Target::LOADABLE_MODULE) { - WriteLinkerFlags(optional_def_file); - WriteLibs(); + out_ << " ldflags ="; + WriteLinkerFlags(out_, tool_, optional_def_file); + out_ << std::endl; + out_ << " libs ="; + WriteLibs(out_, tool_); + out_ << std::endl; } else if (target_->output_type() == Target::STATIC_LIBRARY) { out_ << " arflags ="; RecursiveTargetConfigStringsToStream(target_, &ConfigValues::arflags, @@ -537,68 +541,6 @@ WriteSolibs(solibs); } -void NinjaCBinaryTargetWriter::WriteLinkerFlags( - const SourceFile* optional_def_file) { - out_ << " ldflags ="; - - // First the ldflags from the target and its config. - RecursiveTargetConfigStringsToStream(target_, &ConfigValues::ldflags, - GetFlagOptions(), out_); - - // Followed by library search paths that have been recursively pushed - // through the dependency tree. - const OrderedSet<SourceDir> all_lib_dirs = target_->all_lib_dirs(); - if (!all_lib_dirs.empty()) { - // Since we're passing these on the command line to the linker and not - // to Ninja, we need to do shell escaping. - PathOutput lib_path_output(path_output_.current_dir(), - settings_->build_settings()->root_path_utf8(), - ESCAPE_NINJA_COMMAND); - for (size_t i = 0; i < all_lib_dirs.size(); i++) { - out_ << " " << tool_->lib_dir_switch(); - lib_path_output.WriteDir(out_, all_lib_dirs[i], - PathOutput::DIR_NO_LAST_SLASH); - } - } - - if (optional_def_file) { - out_ << " /DEF:"; - path_output_.WriteFile(out_, *optional_def_file); - } - - out_ << std::endl; -} - -void NinjaCBinaryTargetWriter::WriteLibs() { - out_ << " libs ="; - - // Libraries that have been recursively pushed through the dependency tree. - EscapeOptions lib_escape_opts; - lib_escape_opts.mode = ESCAPE_NINJA_COMMAND; - const OrderedSet<LibFile> all_libs = target_->all_libs(); - const std::string framework_ending(".framework"); - for (size_t i = 0; i < all_libs.size(); i++) { - const LibFile& lib_file = all_libs[i]; - const std::string& lib_value = lib_file.value(); - if (lib_file.is_source_file()) { - out_ << " "; - path_output_.WriteFile(out_, lib_file.source_file()); - } else if (base::EndsWith(lib_value, framework_ending, - base::CompareCase::INSENSITIVE_ASCII)) { - // Special-case libraries ending in ".framework" to support Mac: Add the - // -framework switch and don't add the extension to the output. - out_ << " -framework "; - EscapeStringToStream( - out_, lib_value.substr(0, lib_value.size() - framework_ending.size()), - lib_escape_opts); - } else { - out_ << " " << tool_->lib_switch(); - EscapeStringToStream(out_, lib_value, lib_escape_opts); - } - } - out_ << std::endl; -} - void NinjaCBinaryTargetWriter::WriteOutputSubstitutions() { out_ << " output_extension = " << SubstitutionWriter::GetLinkerSubstitution(
diff --git a/tools/gn/ninja_c_binary_target_writer.h b/tools/gn/ninja_c_binary_target_writer.h index 445198f..a9a612a 100644 --- a/tools/gn/ninja_c_binary_target_writer.h +++ b/tools/gn/ninja_c_binary_target_writer.h
@@ -82,8 +82,6 @@ void WriteLinkerStuff(const std::vector<OutputFile>& object_files, const std::vector<SourceFile>& other_files, const OutputFile& input_dep); - void WriteLinkerFlags(const SourceFile* optional_def_file); - void WriteLibs(); void WriteOutputSubstitutions(); void WriteSolibs(const std::vector<OutputFile>& solibs);
diff --git a/tools/gn/ninja_rust_binary_target_writer.cc b/tools/gn/ninja_rust_binary_target_writer.cc index 47b3fb2..cd81b43 100644 --- a/tools/gn/ninja_rust_binary_target_writer.cc +++ b/tools/gn/ninja_rust_binary_target_writer.cc
@@ -6,7 +6,9 @@ #include <sstream> +#include "base/strings/string_util.h" #include "tools/gn/deps_iterator.h" +#include "tools/gn/filesystem_utils.h" #include "tools/gn/general_tool.h" #include "tools/gn/ninja_target_command_util.h" #include "tools/gn/ninja_utils.h" @@ -145,9 +147,12 @@ std::vector<OutputFile> rustdeps; std::vector<OutputFile> nonrustdeps; for (const auto* non_linkable_dep : non_linkable_deps) { + if (non_linkable_dep->source_types_used().RustSourceUsed() && + non_linkable_dep->output_type() != Target::SOURCE_SET) { + rustdeps.push_back(non_linkable_dep->dependency_output_file()); + } order_only_deps.push_back(non_linkable_dep->dependency_output_file()); } - for (const auto* linkable_dep : linkable_deps) { if (linkable_dep->source_types_used().RustSourceUsed()) { rustdeps.push_back(linkable_dep->dependency_output_file()); @@ -167,7 +172,6 @@ std::copy(non_linkable_deps.begin(), non_linkable_deps.end(), std::back_inserter(extern_deps)); WriteExterns(extern_deps); - WriteRustdeps(rustdeps, nonrustdeps); } } @@ -220,10 +224,9 @@ void NinjaRustBinaryTargetWriter::WriteRustdeps( const std::vector<OutputFile>& rustdeps, const std::vector<OutputFile>& nonrustdeps) { - if (rustdeps.empty() && nonrustdeps.empty()) - return; - out_ << " rustdeps ="; + + // Rust dependencies. for (const auto& rustdep : rustdeps) { out_ << " -Ldependency="; path_output_.WriteDir( @@ -231,11 +234,27 @@ PathOutput::DIR_NO_LAST_SLASH); } + EscapeOptions lib_escape_opts; + lib_escape_opts.mode = ESCAPE_NINJA_COMMAND; + const std::string_view lib_prefix("lib"); + + // Non-Rust native dependencies. for (const auto& rustdep : nonrustdeps) { out_ << " -Lnative="; path_output_.WriteDir( out_, rustdep.AsSourceFile(settings_->build_settings()).GetDir(), PathOutput::DIR_NO_LAST_SLASH); + std::string_view file = FindFilenameNoExtension(&rustdep.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_, rustdep); + } } + + WriteLinkerFlags(out_, tool_, nullptr); + WriteLibs(out_, tool_); out_ << std::endl; }
diff --git a/tools/gn/ninja_rust_binary_target_writer_unittest.cc b/tools/gn/ninja_rust_binary_target_writer_unittest.cc index f9027f5..ed2e547 100644 --- a/tools/gn/ninja_rust_binary_target_writer_unittest.cc +++ b/tools/gn/ninja_rust_binary_target_writer_unittest.cc
@@ -90,9 +90,10 @@ "\n" "build obj/foo/foo_bar: rustc ../../foo/main.rs | ../../foo/input3.rs " "../../foo/main.rs ../../foo/input1.rs ../../foo/input2.rs || " - "obj/foo/sources.stamp\n"; + "obj/foo/sources.stamp\n" + " rustdeps =\n"; std::string out_str = out.str(); - EXPECT_EQ(expected, out_str) << out_str; + EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } } @@ -130,7 +131,8 @@ "target_output_name = mylib\n" "\n" "build obj/bar/libmylib.rlib: rustc ../../bar/lib.rs | " - "../../bar/mylib.rs ../../bar/lib.rs\n"; + "../../bar/mylib.rs ../../bar/lib.rs\n" + " rustdeps =\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << out_str; } @@ -299,7 +301,7 @@ "build obj/foo/foo_bar: rustc ../../foo/main.rs | ../../foo/source.rs " "../../foo/main.rs obj/bar/libmylib.rlib obj/foo/libstatic.a\n" " externs = --extern mylib=obj/bar/libmylib.rlib\n" - " rustdeps = -Ldependency=obj/bar -Lnative=obj/foo\n"; + " rustdeps = -Ldependency=obj/bar -Lnative=obj/foo -lstatic\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } @@ -334,7 +336,7 @@ "\n" "build obj/foo/foo_bar: rustc ../../foo/main.rs | ../../foo/source.rs " "../../foo/main.rs obj/foo/libstatic.a\n" - " rustdeps = -Lnative=obj/foo\n"; + " rustdeps = -Lnative=obj/foo -lstatic\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } @@ -386,9 +388,53 @@ "\n" "build obj/foo/foo_bar.exe: rustc ../../foo/main.rs | ../../foo/input3.rs " "../../foo/main.rs ../../foo/input1.rs ../../foo/input2.rs || " - "obj/foo/sources.stamp\n"; + "obj/foo/sources.stamp\n" + " rustdeps =\n"; std::string out_str = out.str(); - EXPECT_EQ(expected, out_str) << out_str; + EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; + } +} + +TEST_F(NinjaRustBinaryTargetWriterTest, LibsAndLibDirs) { + Err err; + TestWithScope setup; + + Target target(setup.settings(), Label(SourceDir("//foo/"), "bar")); + target.set_output_type(Target::EXECUTABLE); + target.visibility().SetPublic(); + SourceFile main("//foo/main.rs"); + target.sources().push_back(SourceFile("//foo/input.rs")); + 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.rust_values().set_crate_root(main); + target.rust_values().crate_name() = "foo_bar"; + target.SetToolchain(setup.toolchain()); + ASSERT_TRUE(target.OnResolved(&err)); + + { + std::ostringstream out; + NinjaRustBinaryTargetWriter writer(&target, out); + writer.Run(); + + const char expected[] = + "crate_name = foo_bar\n" + "crate_type = bin\n" + "output_dir = foo\n" + "rustc_output_extension = \n" + "rustflags =\n" + "rustenv =\n" + "root_out_dir = .\n" + "target_out_dir = obj/foo\n" + "target_output_name = bar\n" + "\n" + "build obj/foo/foo_bar: rustc ../../foo/main.rs | ../../foo/input.rs " + "../../foo/main.rs\n" + " rustdeps = -Lnative=../../baz -lquux\n"; + std::string out_str = out.str(); + EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } } @@ -426,7 +472,8 @@ "target_output_name = mymacro\n" "\n" "build obj/bar/libmymacro.so: rustc ../../bar/lib.rs | " - "../../bar/mylib.rs ../../bar/lib.rs\n"; + "../../bar/mylib.rs ../../bar/lib.rs\n" + " rustdeps =\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << out_str; } @@ -462,7 +509,8 @@ "\n" "build obj/foo/foo_bar: rustc ../../foo/main.rs | ../../foo/source.rs " "../../foo/main.rs || obj/bar/libmymacro.so\n" - " externs = --extern mymacro=obj/bar/libmymacro.so\n"; + " externs = --extern mymacro=obj/bar/libmymacro.so\n" + " rustdeps = -Ldependency=obj/bar\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << expected << "\n" << out_str; } @@ -502,7 +550,8 @@ "target_output_name = mylib\n" "\n" "build obj/bar/libmylib.rlib: rustc ../../bar/lib.rs | " - "../../bar/mylib.rs ../../bar/lib.rs\n"; + "../../bar/mylib.rs ../../bar/lib.rs\n" + " rustdeps =\n"; std::string out_str = out.str(); EXPECT_EQ(expected, out_str) << out_str; }
diff --git a/tools/gn/rust_tool.cc b/tools/gn/rust_tool.cc index e4e127d..563b20b 100644 --- a/tools/gn/rust_tool.cc +++ b/tools/gn/rust_tool.cc
@@ -11,6 +11,11 @@ RustTool::RustTool(const char* n) : Tool(n), rlib_output_extension_(".rlib") { CHECK(ValidateName(n)); + // TODO: should these be settable in toolchain definition? + set_framework_switch("-lframework="); + set_lib_dir_switch("-Lnative="); + set_lib_switch("-l"); + set_linker_arg("-Clink-arg="); } RustTool::~RustTool() = default; @@ -115,7 +120,6 @@ if (!ReadOutputsPatternList(scope, "outputs", &outputs_, err)) { return false; } - return true; }
diff --git a/tools/gn/tool.h b/tools/gn/tool.h index b963ebf..ae9dd61 100644 --- a/tools/gn/tool.h +++ b/tools/gn/tool.h
@@ -123,6 +123,30 @@ description_ = std::move(desc); } + const std::string& framework_switch() const { return framework_switch_; } + void set_framework_switch(std::string s) { + DCHECK(!complete_); + framework_switch_ = std::move(s); + } + + const std::string& lib_switch() const { return lib_switch_; } + void set_lib_switch(std::string s) { + DCHECK(!complete_); + lib_switch_ = std::move(s); + } + + const std::string& lib_dir_switch() const { return lib_dir_switch_; } + void set_lib_dir_switch(std::string s) { + DCHECK(!complete_); + lib_dir_switch_ = std::move(s); + } + + const std::string& linker_arg() const { return linker_arg_; } + void set_linker_arg(std::string s) { + DCHECK(!complete_); + linker_arg_ = std::move(s); + } + const SubstitutionList& outputs() const { return outputs_; } void set_outputs(SubstitutionList out) { DCHECK(!complete_); @@ -226,6 +250,10 @@ SubstitutionPattern default_output_dir_; SubstitutionPattern depfile_; SubstitutionPattern description_; + std::string framework_switch_; + std::string lib_switch_; + std::string lib_dir_switch_; + std::string linker_arg_; SubstitutionList outputs_; SubstitutionList runtime_outputs_; std::string output_prefix_;