Support link_output, depend_output in Rust linked tools. This is a reland of [1], which only adds support for `link_output` and `depend_output` to all Rust link tools, without changing support for `runtime_outputs`. [1] https://gn-review.googlesource.com/c/gn/+/17401 Fixed: 377 Change-Id: Id3ad98e402c50af61899df03f438ee16d16d468e Reviewed-on: https://gn-review.googlesource.com/c/gn/+/17541 Commit-Queue: David Turner <digit@google.com> Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/docs/reference.md b/docs/reference.md index 2c8aa92..c482630 100644 --- a/docs/reference.md +++ b/docs/reference.md
@@ -3890,13 +3890,13 @@ link_output [string with substitutions] depend_output [string with substitutions] - Valid for: "solink" only (optional) + Valid for: "solink", "rust_dylib" or "rust_cdylib" only (optional) - These two files specify which of the outputs from the solink tool - should be used for linking and dependency tracking. These should match - entries in the "outputs". If unspecified, the first item in the - "outputs" array will be used for all. See "Separate linking and - dependencies for shared libraries" below for more. + These two files specify which of the outputs from the tool should + be used for linking and dependency tracking. These should match entries + in the "outputs". If unspecified, the first item in the "outputs" array + will be used for all. See "Separate linking and dependencies for shared + libraries" below for more. On Windows, where the tools produce a .dll shared library and a .lib import library, you will want the first two to be the import library @@ -4142,8 +4142,8 @@ {{solibs}} Extra libraries from shared library dependencies not specified in the {{inputs}}. This is the list of link_output files from shared libraries - (if the solink tool specifies a "link_output" variable separate from - the "depend_output"). + (if the solink, rust_dylib and rust_cdylib tools specify a "link_output" + variable separate from the "depend_output"). These should generally be treated the same as libs by your tool.
diff --git a/src/gn/c_tool.cc b/src/gn/c_tool.cc index 767b324..e0bea41 100644 --- a/src/gn/c_tool.cc +++ b/src/gn/c_tool.cc
@@ -220,8 +220,7 @@ if (!ValidateLinkAndDependOutput(depend_output(), "depend_output", err)) { return false; } - if ((!link_output().empty() && depend_output().empty()) || - (link_output().empty() && !depend_output().empty())) { + if (link_output().empty() != depend_output().empty()) { *err = Err(defined_from(), "Both link_output and depend_output should either " "be specified or they should both be empty.");
diff --git a/src/gn/c_tool.h b/src/gn/c_tool.h index c1b5883..86a60e8 100644 --- a/src/gn/c_tool.h +++ b/src/gn/c_tool.h
@@ -83,14 +83,6 @@ depend_output_ = std::move(dep_out); } - // Other functions ---------------------------------------------------------- - - // Returns true if this tool has separate outputs for dependency tracking - // and linking. - bool has_separate_solink_files() const { - return !link_output_.empty() || !depend_output_.empty(); - } - private: // Initialization functions ------------------------------------------------- // @@ -100,9 +92,8 @@ bool ValidateOutputSubstitution(const Substitution* sub_type) const; bool ValidateRuntimeOutputs(Err* err); // Validates either link_output or depend_output. To generalize to either, - // pass - // the associated pattern, and the variable name that should appear in error - // messages. + // pass the associated pattern, and the variable name that should appear in + // error messages. bool ValidateLinkAndDependOutput(const SubstitutionPattern& pattern, const char* variable_name, Err* err);
diff --git a/src/gn/function_toolchain.cc b/src/gn/function_toolchain.cc index 1df3845..a97a19c 100644 --- a/src/gn/function_toolchain.cc +++ b/src/gn/function_toolchain.cc
@@ -509,13 +509,13 @@ link_output [string with substitutions] depend_output [string with substitutions] - Valid for: "solink" only (optional) + Valid for: "solink", "rust_dylib" or "rust_cdylib" only (optional) - These two files specify which of the outputs from the solink tool - should be used for linking and dependency tracking. These should match - entries in the "outputs". If unspecified, the first item in the - "outputs" array will be used for all. See "Separate linking and - dependencies for shared libraries" below for more. + These two files specify which of the outputs from the tool should + be used for linking and dependency tracking. These should match entries + in the "outputs". If unspecified, the first item in the "outputs" array + will be used for all. See "Separate linking and dependencies for shared + libraries" below for more. On Windows, where the tools produce a .dll shared library and a .lib import library, you will want the first two to be the import library @@ -760,8 +760,8 @@ {{solibs}} Extra libraries from shared library dependencies not specified in the {{inputs}}. This is the list of link_output files from shared libraries - (if the solink tool specifies a "link_output" variable separate from - the "depend_output"). + (if the solink, rust_dylib and rust_cdylib tools specify a "link_output" + variable separate from the "depend_output"). These should generally be treated the same as libs by your tool.
diff --git a/src/gn/function_toolchain_unittest.cc b/src/gn/function_toolchain_unittest.cc index bb9c3be..838eb3e 100644 --- a/src/gn/function_toolchain_unittest.cc +++ b/src/gn/function_toolchain_unittest.cc
@@ -174,6 +174,50 @@ } } +TEST_F(FunctionToolchain, RustLinkDependAndRuntimeOutputs) { + TestWithScope setup; + + // These runtime outputs are a subset of the outputs so are OK. + { + TestParseInput input( + R"(toolchain("good") { + tool("rust_dylib") { + command = "rust_dylib" + outputs = [ "interface", "lib", "unstripped", "stripped" ] + depend_output = "interface" + link_output = "lib" + runtime_outputs = [ "stripped" ] + } + })"); + ASSERT_FALSE(input.has_error()); + + Err err; + input.parsed()->Execute(setup.scope(), &err); + ASSERT_FALSE(err.has_error()) << err.message(); + + // It should have generated a toolchain. + ASSERT_EQ(1u, setup.items().size()); + const Toolchain* toolchain = setup.items()[0]->AsToolchain(); + ASSERT_TRUE(toolchain); + + // The toolchain should have a link tool with the two outputs. + const Tool* link = toolchain->GetTool(RustTool::kRsToolDylib); + ASSERT_TRUE(link); + ASSERT_EQ(4u, link->outputs().list().size()); + EXPECT_EQ("interface", link->outputs().list()[0].AsString()); + EXPECT_EQ("lib", link->outputs().list()[1].AsString()); + EXPECT_EQ("unstripped", link->outputs().list()[2].AsString()); + EXPECT_EQ("stripped", link->outputs().list()[3].AsString()); + ASSERT_EQ(1u, link->runtime_outputs().list().size()); + EXPECT_EQ("stripped", link->runtime_outputs().list()[0].AsString()); + + const RustTool* rust_tool = link->AsRust(); + ASSERT_TRUE(rust_tool); + EXPECT_EQ("interface", rust_tool->depend_output().AsString()); + EXPECT_EQ("lib", rust_tool->link_output().AsString()); + } +} + TEST_F(FunctionToolchain, Command) { TestWithScope setup;
diff --git a/src/gn/rust_tool.cc b/src/gn/rust_tool.cc index 7d35742..2cf2e2d 100644 --- a/src/gn/rust_tool.cc +++ b/src/gn/rust_tool.cc
@@ -54,6 +54,8 @@ void RustTool::SetComplete() { SetToolComplete(); + link_output_.FillRequiredTypes(&substitution_bits_); + depend_output_.FillRequiredTypes(&substitution_bits_); } std::string_view RustTool::GetSysroot() const { @@ -131,6 +133,33 @@ return true; } +// Validates either link_output or depend_output. To generalize to either, pass +// the associated pattern, and the variable name that should appear in error +// messages. +bool RustTool::ValidateLinkAndDependOutput(const SubstitutionPattern& pattern, + const char* variable_name, + Err* err) { + if (pattern.empty()) + return true; // Empty is always OK. + + // It should only be specified for certain tool types. + if (name_ == kRsToolRlib || name_ == kRsToolStaticlib) { + *err = Err(defined_from(), + "This tool specifies a " + std::string(variable_name) + ".", + "This is only valid for linking tools, not rust_rlib or " + "rust_staticlib."); + return false; + } + + if (!IsPatternInOutputList(outputs(), pattern)) { + *err = Err(defined_from(), "This tool's link_output is bad.", + "It must match one of the outputs."); + return false; + } + + return true; +} + bool RustTool::InitTool(Scope* scope, Toolchain* toolchain, Err* err) { // Initialize default vars. if (!Tool::InitTool(scope, toolchain, err)) { @@ -159,6 +188,25 @@ return false; } + if (!ReadPattern(scope, "link_output", &link_output_, err) || + !ReadPattern(scope, "depend_output", &depend_output_, err)) { + return false; + } + + // Validate link_output and depend_output. + if (!ValidateLinkAndDependOutput(link_output(), "link_output", err)) { + return false; + } + if (!ValidateLinkAndDependOutput(depend_output(), "depend_output", err)) { + return false; + } + if (link_output().empty() != depend_output().empty()) { + *err = Err(defined_from(), + "Both link_output and depend_output should either " + "be specified or they should both be empty."); + return false; + } + return true; }
diff --git a/src/gn/rust_tool.h b/src/gn/rust_tool.h index 4ceb265..76e8bf1 100644 --- a/src/gn/rust_tool.h +++ b/src/gn/rust_tool.h
@@ -52,6 +52,19 @@ dynamic_link_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) { + DCHECK(!complete_); + link_output_ = std::move(link_out); + } + + const SubstitutionPattern& depend_output() const { return depend_output_; } + void set_depend_output(SubstitutionPattern dep_out) { + DCHECK(!complete_); + depend_output_ = std::move(dep_out); + } + private: std::string rust_sysroot_; std::string dynamic_link_switch_; @@ -61,7 +74,22 @@ const char* var, SubstitutionList* field, Err* err); + + // Initialization functions ------------------------------------------------- + // + // Initialization methods used by InitTool(). If successful, will set the + // field and return true, otherwise will return false. Must be called before + // SetComplete(). bool ValidateRuntimeOutputs(Err* err); + // Validates either link_output or depend_output. To generalize to either, + // pass the associated pattern, and the variable name that should appear in + // error messages. + bool ValidateLinkAndDependOutput(const SubstitutionPattern& pattern, + const char* variable_name, + Err* err); + + SubstitutionPattern link_output_; + SubstitutionPattern depend_output_; RustTool(const RustTool&) = delete; RustTool& operator=(const RustTool&) = delete;
diff --git a/src/gn/target.cc b/src/gn/target.cc index b6c6ca0..fb8dbef 100644 --- a/src/gn/target.cc +++ b/src/gn/target.cc
@@ -14,6 +14,7 @@ #include "gn/deps_iterator.h" #include "gn/filesystem_utils.h" #include "gn/functions.h" +#include "gn/rust_tool.h" #include "gn/scheduler.h" #include "gn/substitution_writer.h" #include "gn/tool.h" @@ -825,42 +826,56 @@ this, tool, tool->outputs().list()[0]); break; case RUST_PROC_MACRO: - case SHARED_LIBRARY: + case SHARED_LIBRARY: { CHECK(tool->outputs().list().size() >= 1); check_tool_outputs = true; + + const SubstitutionPattern* link_output_ptr = nullptr; + const SubstitutionPattern* depend_output_ptr = nullptr; + const SubstitutionList* runtime_outputs_ptr = nullptr; + if (const CTool* ctool = tool->AsC()) { - if (ctool->link_output().empty() && ctool->depend_output().empty()) { - // Default behavior, use the first output file for both. - link_output_file_ = dependency_output_file_ = - SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( - this, tool, tool->outputs().list()[0]); - } else { - // Use the tool-specified ones. - if (!ctool->link_output().empty()) { - link_output_file_ = - SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( - this, tool, ctool->link_output()); - } - if (!ctool->depend_output().empty()) { - dependency_output_file_ = - SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( - this, tool, ctool->depend_output()); - } - } - if (tool->runtime_outputs().list().empty()) { - // Default to the link output for the runtime output. - runtime_outputs_.push_back(link_output_file_); - } else { - SubstitutionWriter::ApplyListToLinkerAsOutputFile( - this, tool, tool->runtime_outputs(), &runtime_outputs_); - } - } else if (tool->AsRust()) { + link_output_ptr = + ctool->link_output().empty() ? nullptr : &ctool->link_output(); + depend_output_ptr = + ctool->depend_output().empty() ? nullptr : &ctool->depend_output(); + runtime_outputs_ptr = &ctool->runtime_outputs(); + } else if (const RustTool* rust_tool = tool->AsRust()) { + link_output_ptr = rust_tool->link_output().empty() + ? nullptr + : &rust_tool->link_output(); + depend_output_ptr = rust_tool->depend_output().empty() + ? nullptr + : &rust_tool->depend_output(); + runtime_outputs_ptr = &rust_tool->runtime_outputs(); + } + + if (!link_output_ptr && !depend_output_ptr) { // Default behavior, use the first output file for both. link_output_file_ = dependency_output_file_ = SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( this, tool, tool->outputs().list()[0]); + } else { + if (link_output_ptr) { + link_output_file_ = + SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( + this, tool, *link_output_ptr); + } + if (depend_output_ptr) { + dependency_output_file_ = + SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( + this, tool, *depend_output_ptr); + } + } + if (!runtime_outputs_ptr || runtime_outputs_ptr->list().empty()) { + // Default to the link output for the runtime output. + runtime_outputs_.push_back(link_output_file_); + } else { + SubstitutionWriter::ApplyListToLinkerAsOutputFile( + this, tool, *runtime_outputs_ptr, &runtime_outputs_); } break; + } case UNKNOWN: default: NOTREACHED();
diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc index 8af8f6f..5cc2182 100644 --- a/src/gn/target_unittest.cc +++ b/src/gn/target_unittest.cc
@@ -553,6 +553,48 @@ EXPECT_EQ("./liba.so", target.runtime_outputs()[0].value()); } +TEST_F(TargetTest, RustLinkAndDepOutputs) { + TestWithScope setup; + Err err; + + Toolchain toolchain(setup.settings(), Label(SourceDir("//tc/"), "tc")); + + std::unique_ptr<Tool> tool = Tool::CreateTool(RustTool::kRsToolDylib); + RustTool* rust_tool = tool->AsRust(); + rust_tool->set_output_prefix("lib"); + rust_tool->set_default_output_extension(".so"); + + const char kLinkPattern[] = + "{{root_out_dir}}/{{target_output_name}}{{output_extension}}"; + SubstitutionPattern link_output = + SubstitutionPattern::MakeForTest(kLinkPattern); + rust_tool->set_link_output(link_output); + + const char kDependPattern[] = + "{{root_out_dir}}/{{target_output_name}}{{output_extension}}.TOC"; + SubstitutionPattern depend_output = + SubstitutionPattern::MakeForTest(kDependPattern); + rust_tool->set_depend_output(depend_output); + + rust_tool->set_outputs( + SubstitutionList::MakeForTest(kLinkPattern, kDependPattern)); + + toolchain.SetTool(std::move(tool)); + + Target target(setup.settings(), Label(SourceDir("//a/"), "a")); + target.source_types_used().Set(SourceFile::SOURCE_RS); + target.rust_values().set_crate_type(RustValues::CRATE_DYLIB); + target.set_output_type(Target::SHARED_LIBRARY); + target.SetToolchain(&toolchain); + ASSERT_TRUE(target.OnResolved(&err)); + + EXPECT_EQ("./liba.so", target.link_output_file().value()); + EXPECT_EQ("./liba.so.TOC", target.dependency_output_file().value()); + + ASSERT_EQ(1u, target.runtime_outputs().size()); + EXPECT_EQ("./liba.so", target.runtime_outputs()[0].value()); +} + // Tests that runtime_outputs works without an explicit link_output for // solink tools. // @@ -609,6 +651,59 @@ EXPECT_EQ("//out/Debug/a.pdb", computed_outputs[2].value()); } +TEST_F(TargetTest, RustRuntimeOuputs) { + TestWithScope setup; + Err err; + + Toolchain toolchain(setup.settings(), Label(SourceDir("//tc/"), "tc")); + + std::unique_ptr<Tool> tool = Tool::CreateTool(RustTool::kRsToolCDylib); + RustTool* rust_tool = tool->AsRust(); + rust_tool->set_output_prefix(""); + rust_tool->set_default_output_extension(".dll"); + + // Say the linker makes a DLL< an import library, and a symbol file we want + // to treat as a runtime output. + const char kLibPattern[] = + "{{root_out_dir}}/{{target_output_name}}{{output_extension}}.lib"; + const char kDllPattern[] = + "{{root_out_dir}}/{{target_output_name}}{{output_extension}}"; + const char kPdbPattern[] = "{{root_out_dir}}/{{target_output_name}}.pdb"; + SubstitutionPattern pdb_pattern = + SubstitutionPattern::MakeForTest(kPdbPattern); + + rust_tool->set_outputs( + SubstitutionList::MakeForTest(kLibPattern, kDllPattern, kPdbPattern)); + + // Say we only want the DLL and symbol file treaded as runtime outputs. + rust_tool->set_runtime_outputs( + SubstitutionList::MakeForTest(kDllPattern, kPdbPattern)); + + toolchain.SetTool(std::move(tool)); + + Target target(setup.settings(), Label(SourceDir("//a/"), "a")); + target.source_types_used().Set(SourceFile::SOURCE_RS); + target.rust_values().set_crate_type(RustValues::CRATE_CDYLIB); + target.set_output_type(Target::SHARED_LIBRARY); + target.SetToolchain(&toolchain); + ASSERT_TRUE(target.OnResolved(&err)); + + EXPECT_EQ("./a.dll.lib", target.link_output_file().value()); + EXPECT_EQ("./a.dll.lib", target.dependency_output_file().value()); + + ASSERT_EQ(2u, target.runtime_outputs().size()); + EXPECT_EQ("./a.dll", target.runtime_outputs()[0].value()); + EXPECT_EQ("./a.pdb", target.runtime_outputs()[1].value()); + + // Test GetOutputsAsSourceFiles(). + std::vector<SourceFile> computed_outputs; + EXPECT_TRUE(target.GetOutputsAsSourceFiles(LocationRange(), true, + &computed_outputs, &err)); + ASSERT_EQ(3u, computed_outputs.size()); + EXPECT_EQ("//out/Debug/a.dll.lib", computed_outputs[0].value()); + EXPECT_EQ("//out/Debug/a.dll", computed_outputs[1].value()); + EXPECT_EQ("//out/Debug/a.pdb", computed_outputs[2].value()); +} // Tests Target::GetOutputFilesForSource for binary targets (these require a // tool definition). Also tests GetOutputsAsSourceFiles() for source sets. TEST_F(TargetTest, GetOutputFilesForSource_Binary) {