gn writes invalid ninja files if an action has a newline ("$0x0A") in its command |
||||
Issue description
$ git diff
diff --git a/base/BUILD.gn b/base/BUILD.gn
index 463fa3e30aae..bf58e7a6e3e4 100644
--- a/base/BUILD.gn
+++ b/base/BUILD.gn
@@ -110,6 +110,12 @@ if (is_android) {
}
}
+action("foo") {
+ script = "//build/gn_helpers.py"
+ args = [ "-D", "values=a=0$0x0Ab=1" ]
+ outputs = [ "$root_out_dir/output" ]
+}
+
# Base and everything it depends on should be a static library rather than
# a source set. Base is more of a "library" in the classic sense in that many
# small parts of it are used in many different contexts. This combined with a
Nicos-MacBook-Pro:src thakis$ gn gen out/gn
Done. Made 8922 targets from 1524 files in 5922ms
$ ~/src/ninja/ninja -C out/gn base_unittests
ninja: Entering directory `out/gn'
ninja: error: toolchain.ninja:184: unexpected indent
$ sed -n 182,186p out/gn/toolchain.ninja
command = python ../../build/gn_helpers.py -D values=a=0\
b=1
description = ACTION //base:foo(//build/toolchain/mac:clang_x64)
restat = 1
pool = build_toolchain_action_pool
Newlines must be escaped by '$' right in front of the newline in ninja. Looks like gn did write a '\' there, so maybe it attempted to do the right thing and failed, or maybe that's just the escaping for bash.
My guess is that the $0xFF notation is newer than gn's escaping code and since newlines are the only thing needing escaping on the rhs of a command line, no ninja-level escaping code was added at that point.
,
Apr 16 2018
,
Apr 16 2018
I guess it's OS-All, but we no longer have that label. I hope if I add multiple OSs, it won't show up in the mac triage queue again.
,
Apr 16 2018
A bug that is os-independent should have no boxes checked, not all of them.
,
May 10 2018
I took a short look at this today for a bit. It's not fully clear to me what to do about this. Ninja doesn't have a way to include a \n char on a command ($ followed by a newline is replace with nothing, like the backslash line continuation of C's preprocessor), which is imho a reasonable position to take. So gn would have to write something that can go on a single line but functions as a newilne. In bash, $'\n' does that, but it's bash-specific and on Windows, ninja doesn't run commands in a shell so it's not clear if anything could be done there. So maybe the thing to do is make gn produce an error at gn time instead. |
||||
►
Sign in to add a comment |
||||
Comment 1 by thakis@chromium.org
, Apr 11 2018Components: Build
Labels: Build-Tools-GN
Status: Untriaged (was: Unconfirmed)