New issue
Advanced search Search tips

Issue 835319 link

Starred by 1 user
Status: Fixed
Owner:
thakis@chromium.org
Closed: Apr 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

gn writes invalid ninja files if a pool called "console" in the toplevel BUILD.gn file is used.

Project Member Reported by thakis@chromium.org, Apr 20 2018 
If you do

diff --git a/BUILD.gn b/BUILD.gn
index 06c117e4e79a..fdade06c53e5 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -8,6 +8,7 @@
 # you add a new build file, there must be some path of dependencies from this
 # file to your new one or GN won't know about it.
 
+if (current_toolchain == default_toolchain) {
 import("//build/config/compiler/compiler.gni")
 import("//build/config/features.gni")
 import("//build/config/sanitizers/sanitizers.gni")
@@ -1161,3 +1162,9 @@ if (closure_compile) {
 assert_valid_out_dir("_unused") {
   actual_sources = [ "$root_build_dir/foo" ]
 }
+}
+if (current_toolchain == default_toolchain) {
+  pool("console") {
+    depth = 1
+  }
+}
diff --git a/build/toolchain/gcc_toolchain.gni b/build/toolchain/gcc_toolchain.gni
index 48fbf6996e5c..a17076551b7b 100644
--- a/build/toolchain/gcc_toolchain.gni
+++ b/build/toolchain/gcc_toolchain.gni
@@ -366,7 +366,7 @@ template("gcc_toolchain") {
       soname = "{{target_output_name}}{{output_extension}}"  # e.g. "libfoo.so".
       sofile = "{{output_dir}}/$soname"  # Possibly including toolchain dir.
       rspfile = sofile + ".rsp"
-      pool = "//build/toolchain:link_pool($default_toolchain)"
+      pool = "//:console($default_toolchain)"
       whitelist_flag = " "
       if (enable_resource_whitelist_generation) {
         whitelist_file = "$sofile.whitelist"


then gn will generate

pool console
  depth = 1

in its toplevel build.ninja, and ninja will then complain that the console pool is being redefined.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 20 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9867a58c30f15c66d0b08b5f78050ff127155f9

commit e9867a58c30f15c66d0b08b5f78050ff127155f9
Author: Nico Weber <thakis@chromium.org>
Date: Fri Apr 20 16:25:14 2018

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-Commit-Position: refs/heads/master@{#552353}
[modify] https://crrev.com/e9867a58c30f15c66d0b08b5f78050ff127155f9/tools/gn/functions.cc
[modify] https://crrev.com/e9867a58c30f15c66d0b08b5f78050ff127155f9/tools/gn/ninja_action_target_writer_unittest.cc
[modify] https://crrev.com/e9867a58c30f15c66d0b08b5f78050ff127155f9/tools/gn/ninja_build_writer.cc
[modify] https://crrev.com/e9867a58c30f15c66d0b08b5f78050ff127155f9/tools/gn/ninja_build_writer_unittest.cc

Comment 2 by thakis@chromium.org, Apr 20 2018

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 20 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a8fe0c1bdb12ce92c841d4527b54ae3e023cb590

commit a8fe0c1bdb12ce92c841d4527b54ae3e023cb590
Author: Nico Weber <thakis@chromium.org>
Date: Fri Apr 20 18:00:17 2018

Re-run `gn help --markdown all > tools/gn/docs/reference.md`.

This should've been part of https://chromium-review.googlesource.com/c/chromium/src/+/1020010

Bug:  835319 
Change-Id: Ibad6a9b3f686ed8305ffc77c866917124fef9ba5
Reviewed-on: https://chromium-review.googlesource.com/1021597
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552387}
[modify] https://crrev.com/a8fe0c1bdb12ce92c841d4527b54ae3e023cb590/tools/gn/docs/reference.md
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 20 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4e9ebc09180660b31579eaabca9ab257ef03613

commit b4e9ebc09180660b31579eaabca9ab257ef03613
Author: Nico Weber <thakis@chromium.org>
Date: Fri Apr 20 18:30:41 2018

Roll buildtools 8febfea9bc..ab7b6a7b35

  In order to roll GN b709e226c5..76b9b6c759 (r549249:r552354) and pick up
  the following changes:

  e9867a58c30f gn: Don't write invalid output when pool //:console exists; treat all other pools named 'console' as invalid.
  cd02e3d2d629 Update GN reference page.
  95fe44b3c31c GN: Fix bootstrap call to setup_toolchain
  8ff145a7c498 Fix gn bootstrap
  5208b8e8bb6b GN: Allow XCode projects to use ICECC environment variables.
  e20ec0676c3a Fix gn bootstrap

TBR=dpranke@chromium.org

Bug:  835319 
Change-Id: I28d18644949fccf058cef6e428cea4fa502a0f39
Reviewed-on: https://chromium-review.googlesource.com/1021837
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552397}
[modify] https://crrev.com/b4e9ebc09180660b31579eaabca9ab257ef03613/DEPS
►

Sign in to add a comment