gn: Don't write invalid output when pool //:console exists; treat all other pools named 'console' as invalid.
Before, defining and referencing pool "//:console" would make gn write a
ninja file that tries to redeclare ninja's built-in console pool, causing
ninja to reject the generated manifest file.
Don't write pools definitions that cause this. This means //:console now represents
ninja's built-in console pool.
Disallow pools in other scopes or in the non-default toolchain to be called "console".
If this was allowed but these pools behaved like regular pools instead of like the
console pool, that'd be confusing. If they'd all behave like the console pool,
that would be inconsistent with how regular pools work in gn -- the pool depth
there is per-toolchain, while it'd always be globally 1 shared among all console pools
in all toolchains.
While here, also improve the source location for the "pool depths must not be negative"
diagnostic.
Bug: 835319
Change-Id: Idbec9c08f42a0de7b316d8e5e8819c6c8c3c3c45
Reviewed-on: https://chromium-review.googlesource.com/1020010
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552353}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: e9867a58c30f15c66d0b08b5f78050ff127155f9diff --git a/tools/gn/functions.cc b/tools/gn/functions.cc
index 36b6207..105dadf 100644
--- a/tools/gn/functions.cc
+++ b/tools/gn/functions.cc
@@ -852,6 +852,13 @@
context of more than one toolchain it is recommended to specify an
explicit toolchain when defining and referencing a pool.
+ A pool named "console" defined in the root build file represents Ninja's
+ console pool. Targets using this pool will have access to the console's
+ stdin and stdout, and output will not be buffered. This special pool must
+ have a depth of 1. Pools not defined in the root must not be named "console".
+ The console pool can only be defined for the default toolchain.
+ Refer to the Ninja documentation on the console pool for more info.
+
A pool is referenced by its label just like a target.
Variables
@@ -906,13 +913,31 @@
return Value();
if (depth->int_value() < 0) {
- *err = Err(function, "depth must be positive or nul.");
+ *err = Err(*depth, "depth must be positive or 0.");
return Value();
}
// Create the new pool.
std::unique_ptr<Pool> pool = std::make_unique<Pool>(
scope->settings(), label, scope->build_dependency_files());
+
+ if (label.name() == "console") {
+ const Settings* settings = scope->settings();
+ if (!settings->is_default()) {
+ *err = Err(
+ function,
+ "\"console\" pool must be defined only in the default toolchain.");
+ return Value();
+ }
+ if (label.dir() != settings->build_settings()->root_target_label().dir()) {
+ *err = Err(function, "\"console\" pool must be defined in the root //.");
+ return Value();
+ }
+ if (depth->int_value() != 1) {
+ *err = Err(*depth, "\"console\" pool must have depth 1.");
+ return Value();
+ }
+ }
pool->set_depth(depth->int_value());
// Save the generated item.
diff --git a/tools/gn/ninja_action_target_writer_unittest.cc b/tools/gn/ninja_action_target_writer_unittest.cc
index 53f96c3..1b2ddc8 100644
--- a/tools/gn/ninja_action_target_writer_unittest.cc
+++ b/tools/gn/ninja_action_target_writer_unittest.cc
@@ -75,7 +75,7 @@
// Tests an action with no sources and pool
-TEST(NinjaActionTargetWriter, ActionNoSourcesPool) {
+TEST(NinjaActionTargetWriter, ActionNoSourcesConsole) {
Err err;
TestWithScope setup;
@@ -89,9 +89,9 @@
SubstitutionList::MakeForTest("//out/Debug/foo.out");
Pool pool(setup.settings(),
- Label(SourceDir("//foo/"), "pool", setup.toolchain()->label().dir(),
+ Label(SourceDir("//"), "console", setup.toolchain()->label().dir(),
setup.toolchain()->label().name()));
- pool.set_depth(5);
+ pool.set_depth(1);
target.action_values().set_pool(LabelPtrPair<Pool>(&pool));
target.SetToolchain(setup.toolchain());
@@ -104,6 +104,8 @@
NinjaActionTargetWriter writer(&target, out);
writer.Run();
+ // The console pool's name must be mapped exactly to the string "console"
+ // which is a special pre-defined pool name in ninja.
const char* expected = 1 /* skip initial newline */ + R"(
rule __foo_bar___rule
command = /usr/bin/python ../../foo/script.py
@@ -111,7 +113,7 @@
restat = 1
build foo.out: __foo_bar___rule | ../../foo/script.py ../../foo/included.txt
- pool = foo_pool
+ pool = console
build obj/foo/bar.stamp: stamp foo.out
)";
diff --git a/tools/gn/ninja_build_writer.cc b/tools/gn/ninja_build_writer.cc
index 334071a..2f782f0 100644
--- a/tools/gn/ninja_build_writer.cc
+++ b/tools/gn/ninja_build_writer.cc
@@ -317,7 +317,10 @@
return pool_name(a) < pool_name(b);
});
for (const Pool* pool : sorted_pools) {
- out_ << "pool " << pool_name(pool) << std::endl
+ std::string name = pool_name(pool);
+ if (name == "console")
+ continue;
+ out_ << "pool " << name << std::endl
<< " depth = " << pool->depth() << std::endl
<< std::endl;
}
diff --git a/tools/gn/ninja_build_writer_unittest.cc b/tools/gn/ninja_build_writer_unittest.cc
index 1937135..c27100c 100644
--- a/tools/gn/ninja_build_writer_unittest.cc
+++ b/tools/gn/ninja_build_writer_unittest.cc
@@ -34,16 +34,26 @@
target_bar.SetToolchain(setup.toolchain());
ASSERT_TRUE(target_bar.OnResolved(&err));
- // Make a secondary toolchain that references a pool.
+ // Make a secondary toolchain that references two pools.
Label other_toolchain_label(SourceDir("//other/"), "toolchain");
- Pool other_pool(setup.settings(), Label(SourceDir("//other/"), "pool",
- other_toolchain_label.dir(),
- other_toolchain_label.name()));
- other_pool.set_depth(42);
Toolchain other_toolchain(setup.settings(), other_toolchain_label);
TestWithScope::SetupToolchain(&other_toolchain);
- other_toolchain.GetTool(Toolchain::TYPE_LINK)->set_pool(
- LabelPtrPair<Pool>(&other_pool));
+
+ Pool other_regular_pool(
+ setup.settings(),
+ Label(SourceDir("//other/"), "depth_pool", other_toolchain_label.dir(),
+ other_toolchain_label.name()));
+ other_regular_pool.set_depth(42);
+ other_toolchain.GetTool(Toolchain::TYPE_LINK)
+ ->set_pool(LabelPtrPair<Pool>(&other_regular_pool));
+
+ // The console pool must be in the default toolchain.
+ Pool console_pool(setup.settings(), Label(SourceDir("//"), "console",
+ setup.toolchain()->label().dir(),
+ setup.toolchain()->label().name()));
+ console_pool.set_depth(1);
+ other_toolchain.GetTool(Toolchain::TYPE_STAMP)
+ ->set_pool(LabelPtrPair<Pool>(&console_pool));
// Settings to go with the other toolchain.
Settings other_settings(setup.build_settings(), "toolchain/");
@@ -68,7 +78,7 @@
" generator = 1\n"
" depfile = build.ninja.d\n";
const char expected_other_pool[] =
- "pool other_toolchain_other_pool\n"
+ "pool other_toolchain_other_depth_pool\n"
" depth = 42\n";
const char expected_toolchain[] =
"subninja toolchain.ninja\n";
@@ -95,6 +105,9 @@
EXPECT_SNIPPET(expected_root_target);
EXPECT_SNIPPET(expected_default);
#undef EXPECT_SNIPPET
+
+ // A pool definition for ninja's built-in console pool must not be written.
+ EXPECT_EQ(std::string::npos, out_str.find("pool console"));
}
TEST_F(NinjaBuildWriterTest, DuplicateOutputs) {
