Properly verify runtime_outputs in rust tool definitions.
Ensure that GN will complain when runtime_outputs is set
in a Rust tool type that does not support it. Before this
CL the value was read by the tool, and just ignored (without
any warning about unused variables). This applied to rust_rlib
and rust_staticlib which should not support this argument.
The CL at [1] was reverted, because it broke the Chromium build.
The root cause for this is that it removed runtime_outputs support
from the rust_bin tool by mistake. Adding proper tests here
should prevent similar errors in the future.
[1] https://gn-review.googlesource.com/c/gn/+/17401
+ Improve the FunctionToolchain.RuntimeOutputs test case
to verify that all C++ and Rust link tools support
runtime_outputs properly, and that other tools will
error when it is specified.
Bug: 377
Change-Id: If282807a325bf9ecaa57821a5563b8829687d6c0
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/17540
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Commit-Queue: David Turner <digit@google.com>
diff --git a/src/gn/function_toolchain_unittest.cc b/src/gn/function_toolchain_unittest.cc
index 74cbef9..bb9c3be 100644
--- a/src/gn/function_toolchain_unittest.cc
+++ b/src/gn/function_toolchain_unittest.cc
@@ -2,9 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "base/strings/string_util.h"
+#include "gn/c_tool.h"
#include "gn/functions.h"
#include "gn/rust_tool.h"
-#include "gn/scheduler.h"
#include "gn/test_with_scheduler.h"
#include "gn/test_with_scope.h"
#include "util/test/test.h"
@@ -35,53 +36,109 @@
}
}
-TEST_F(FunctionToolchain, RuntimeOutputs) {
- TestWithScope setup;
+// Substitute all instances of |src| in |input| with |dst|.
+static std::string StrSubstitute(std::string_view input,
+ std::string_view src,
+ std::string_view dst) {
+ std::string result(input);
+ base::ReplaceSubstringsAfterOffset(&result, 0, src, dst);
+ return result;
+}
- // These runtime outputs are a subset of the outputs so are OK.
- {
- TestParseInput input(
- R"(toolchain("good") {
- tool("link") {
- command = "link"
+TEST_F(FunctionToolchain, RuntimeOutputs) {
+ // A list of tool types which support runtime_outputs.
+ const char* kGoodToolTypes[] = {
+ CTool::kCToolLink, CTool::kCToolSolink, CTool::kCToolSolinkModule,
+ RustTool::kRsToolBin, RustTool::kRsToolDylib, RustTool::kRsToolCDylib,
+ RustTool::kRsToolMacro,
+ };
+ for (const auto& tool_type : kGoodToolTypes) {
+ TestWithScope setup;
+
+ {
+ // These runtime outputs are a subset of the outputs so are OK.
+ const char kInput[] = R"(
+ toolchain("good") {
+ tool("{{name}}") {
+ command = "{{name}}"
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();
+ std::string input_str = StrSubstitute(kInput, "{{name}}", tool_type);
+ TestParseInput input(input_str);
+ ASSERT_FALSE(input.has_error()) << "INPUT [" << input_str << "]";
- // It should have generated a toolchain.
- ASSERT_EQ(1u, setup.items().size());
- const Toolchain* toolchain = setup.items()[0]->AsToolchain();
- ASSERT_TRUE(toolchain);
+ Err err;
+ input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_FALSE(err.has_error()) << err.message();
- // The toolchain should have a link tool with the two outputs.
- const Tool* link = toolchain->GetTool(CTool::kCToolLink);
- 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());
+ // 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(tool_type);
+ ASSERT_TRUE(link) << tool_type;
+ 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.
+ {
+ const char kInput[] = R"(
+ toolchain("bad") {
+ tool("{{name}}") {
+ command = "{{name}}"
+ outputs = [ "foo" ]
+ runtime_outputs = [ "bar" ]
+ }
+ })";
+ std::string input_str = StrSubstitute(kInput, "{{name}}", tool_type);
+ TestParseInput input(input_str);
+ ASSERT_FALSE(input.has_error());
+
+ Err err;
+ input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_TRUE(err.has_error()) << "INPUT [" << input_str << "]";
+ }
}
- // This one is not a subset so should throw an error.
- {
- TestParseInput input(
- R"(toolchain("bad") {
- tool("link") {
+ // A list of tool types which do not support runtime_outputs.
+ const char* const kBadToolTypes[] = {
+ CTool::kCToolCc, CTool::kCToolCxx, CTool::kCToolCxxModule,
+ CTool::kCToolObjC, CTool::kCToolObjCxx, CTool::kCToolRc,
+ CTool::kCToolAsm, CTool::kCToolSwift, CTool::kCToolAlink,
+ RustTool::kRsToolRlib, RustTool::kRsToolStaticlib,
+ };
+ for (const auto& tool_type : kBadToolTypes) {
+ TestWithScope setup;
+ {
+ // These runtime outputs are a subset of the outputs so are OK.
+ // But these tool names do not support runtime outputs!
+ const char kInput[] = R"(
+ toolchain("unsupported") {
+ tool("{{name}}") {
+ command = "{{name}}"
outputs = [ "foo" ]
- runtime_outputs = [ "bar" ]
- }
- })");
- ASSERT_FALSE(input.has_error());
+ runtime_outputs = [ "foo" ]
+ }
+ })";
- Err err;
- input.parsed()->Execute(setup.scope(), &err);
- ASSERT_TRUE(err.has_error()) << err.message();
+ std::string input_str = StrSubstitute(kInput, "{{name}}", tool_type);
+ TestParseInput input(input_str);
+ ASSERT_FALSE(input.has_error()) << "INPUT [" << input_str << "]";
+
+ Err err;
+ input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_TRUE(err.has_error()) << "INPUT [" << input_str << "]";
+ ASSERT_EQ(err.message(), "This tool specifies runtime_outputs.")
+ << tool_type;
+ }
}
}
diff --git a/src/gn/rust_tool.cc b/src/gn/rust_tool.cc
index f799d73..7d35742 100644
--- a/src/gn/rust_tool.cc
+++ b/src/gn/rust_tool.cc
@@ -109,6 +109,28 @@
return true;
}
+bool RustTool::ValidateRuntimeOutputs(Err* err) {
+ if (runtime_outputs().list().empty())
+ return true; // Empty is always OK.
+
+ if (name_ == kRsToolRlib || name_ == kRsToolStaticlib) {
+ *err =
+ Err(defined_from(), "This tool specifies runtime_outputs.",
+ "This is only valid for linker tools (rust_rlib 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;
+}
+
bool RustTool::InitTool(Scope* scope, Toolchain* toolchain, Err* err) {
// Initialize default vars.
if (!Tool::InitTool(scope, toolchain, err)) {
@@ -133,6 +155,10 @@
}
}
+ if (!ValidateRuntimeOutputs(err)) {
+ return false;
+ }
+
return true;
}
diff --git a/src/gn/rust_tool.h b/src/gn/rust_tool.h
index 857e205..4ceb265 100644
--- a/src/gn/rust_tool.h
+++ b/src/gn/rust_tool.h
@@ -61,6 +61,7 @@
const char* var,
SubstitutionList* field,
Err* err);
+ bool ValidateRuntimeOutputs(Err* err);
RustTool(const RustTool&) = delete;
RustTool& operator=(const RustTool&) = delete;
