Revert "Rust: link_output, depend_output and runtime_outputs for dylibs"
This reverts commit 0ee833e823f2e11be136728169906d0710bee910.
Reason for revert: breaks Chromium build: https://crrev.com/c/5762050
Original change's description:
> Rust: link_output, depend_output and runtime_outputs for dylibs
>
> Ensure that the rust_dylib and rust_cdylib tool() definition
> support the `link_output`, `depend_output` and `runtime_outputs`
> argument, when generating shared libraries from Rust sources,
> just like the `solink` tool used for C++ sources.
>
> Bug: 377
> Change-Id: I2286f42661b9ebbff5d5b8455c2be49aaf45afa9
> Reviewed-on: https://gn-review.googlesource.com/c/gn/+/17401
> Reviewed-by: Takuto Ikuta <[email protected]>
> Commit-Queue: David Turner <[email protected]>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 377
Change-Id: I3926c0d744da32fd14397148caf042793755ad25
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/17480
Reviewed-by: Dirk Pranke <[email protected]>
Commit-Queue: Dirk Pranke <[email protected]>
Reviewed-by: Nico Weber <[email protected]>
Reviewed-by: Sylvain Defresne <[email protected]>
diff --git a/docs/reference.md b/docs/reference.md
index b99c339..a9fad5b 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", "rust_dylib" or "rust_cdylib" only (optional)
+ Valid for: "solink" only (optional)
- 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.
+ 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.
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, rust_dylib and rust_cdylib tools specify a "link_output"
- variable separate from the "depend_output").
+ (if the solink tool specifies 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 e0bea41..767b324 100644
--- a/src/gn/c_tool.cc
+++ b/src/gn/c_tool.cc
@@ -220,7 +220,8 @@
if (!ValidateLinkAndDependOutput(depend_output(), "depend_output", err)) {
return false;
}
- if (link_output().empty() != depend_output().empty()) {
+ if ((!link_output().empty() && depend_output().empty()) ||
+ (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 86a60e8..c1b5883 100644
--- a/src/gn/c_tool.h
+++ b/src/gn/c_tool.h
@@ -83,6 +83,14 @@
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 -------------------------------------------------
//
@@ -92,8 +100,9 @@
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 a97a19c..1df3845 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", "rust_dylib" or "rust_cdylib" only (optional)
+ Valid for: "solink" only (optional)
- 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.
+ 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.
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, rust_dylib and rust_cdylib tools specify a "link_output"
- variable separate from the "depend_output").
+ (if the solink tool specifies 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 91d2235..74cbef9 100644
--- a/src/gn/function_toolchain_unittest.cc
+++ b/src/gn/function_toolchain_unittest.cc
@@ -117,100 +117,6 @@
}
}
-TEST_F(FunctionToolchain, RustRuntimeOutputs) {
- 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 = [ "foo" ]
- runtime_outputs = [ "foo" ]
- }
- })");
- 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(1u, link->outputs().list().size());
- EXPECT_EQ("foo", link->outputs().list()[0].AsString());
- ASSERT_EQ(1u, link->runtime_outputs().list().size());
- EXPECT_EQ("foo", link->runtime_outputs().list()[0].AsString());
- }
-
- // This one is not a subset so should throw an error.
- {
- TestParseInput input(
- R"(toolchain("bad") {
- tool("rust_dylib") {
- outputs = [ "foo" ]
- runtime_outputs = [ "bar" ]
- }
- })");
- ASSERT_FALSE(input.has_error());
-
- Err err;
- input.parsed()->Execute(setup.scope(), &err);
- ASSERT_TRUE(err.has_error()) << err.message();
- }
-}
-
-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 1b0898c..f799d73 100644
--- a/src/gn/rust_tool.cc
+++ b/src/gn/rust_tool.cc
@@ -54,56 +54,6 @@
void RustTool::SetComplete() {
SetToolComplete();
- link_output_.FillRequiredTypes(&substitution_bits_);
- depend_output_.FillRequiredTypes(&substitution_bits_);
-}
-
-bool RustTool::ValidateRuntimeOutputs(Err* err) {
- if (runtime_outputs().list().empty())
- return true; // Empty is always OK.
-
- if (name_ != kRsToolDylib && name_ != kRsToolCDylib) {
- *err = Err(
- defined_from(), "This tool specifies runtime_outputs.",
- "This is only valid for linker tools (rust_staticlib doesn't count).");
- return false;
- }
-
- for (const SubstitutionPattern& pattern : runtime_outputs().list()) {
- if (!IsPatternInOutputList(outputs(), pattern)) {
- *err = Err(defined_from(), "This tool's runtime_outputs is bad.",
- "It must be a subset of the outputs. The bad one is:\n " +
- pattern.AsString());
- return false;
- }
- }
- 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_ != kRsToolDylib && name_ != kRsToolCDylib) {
- *err = Err(defined_from(),
- "This tool specifies a " + std::string(variable_name) + ".",
- "This is only valid for solink and solink_module tools.");
- 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;
}
std::string_view RustTool::GetSysroot() const {
@@ -181,31 +131,6 @@
!ReadString(scope, "dynamic_link_switch", &dynamic_link_switch_, err)) {
return false;
}
-
- if (name_ == kRsToolDylib || name_ == kRsToolCDylib) {
- 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;
- }
- }
-
- if (!ValidateRuntimeOutputs(err)) {
- return false;
- }
}
return true;
diff --git a/src/gn/rust_tool.h b/src/gn/rust_tool.h
index 76e8bf1..857e205 100644
--- a/src/gn/rust_tool.h
+++ b/src/gn/rust_tool.h
@@ -52,19 +52,6 @@
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_;
@@ -75,22 +62,6 @@
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 fb8dbef..b6c6ca0 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -14,7 +14,6 @@
#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"
@@ -826,56 +825,42 @@
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()) {
- 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) {
+ 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()) {
// 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 5cc2182..8af8f6f 100644
--- a/src/gn/target_unittest.cc
+++ b/src/gn/target_unittest.cc
@@ -553,48 +553,6 @@
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.
//
@@ -651,59 +609,6 @@
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) {
