Oh, currently also requires trunk clang, but will work with chromium's clang again soon.

You can see the logic for escaping on Windows is totally separate from the logic for escaping on Posix. Since you're running on Posix, you're getting that escaping, which escapes spaces. The Windows one quotes this string instead and doesn't need to escape the spaces (escaping the spaces is better but it doesn't work on Windows.

You can see in escape.cc there are different nodes. It can escape for Ninja, and it can escape for a shell command embedded in a Ninja file. It seems like rspfile escaping will have slightly different rules than any of those and we probably need to make GN aware of this.

On Windows, .rsp files for mesa contain:

-D"PACKAGE_STRING=\"Mesa 9.0.3\""

On non-Windows, if I hack up the toolchain definition to use rsp files for compiles, .rsp files for mesa contain:

-DPACKAGE_STRING=\"Mesa\ 9.0.3\"



It looks like stuff in rsp files _is_ parsed according to shell rules, just not by the shell itself. See e.g. https://github.com/llvm-mirror/clang/blob/master/tools/driver/driver.cpp#L341 -- llvm::cl::ExpandResponseFiles(Saver, Tokenizer, argv, MarkEOLs); is called with a different Tokenizer (either llvm::cl::TokenizeWindowsCommandLine() or llvm::cl::TokenizeGNUCommandLine() depending on if clang-cl is used or regular clang). Both are implemented here: https://github.com/llvm-mirror/llvm/blob/master/lib/Support/CommandLine.cpp


So what GN is doing looks correct, and I suppose I just need a way to ask gn to write windows-style rsp files on non-Windows (retitling bug) for compilations that target windows. (It's a bit weird since stuff in rsp files is interpreted by the compiler, while stuff on the command line _does_ go through the host shell)

Alternatively, clang-cl could use posix quoting rules for response files when running in a non-windows shell. Hm.

(prior discussion https://llvm.org/bugs/show_bug.cgi?id=23709 -- looks like that wasn't about running clang-cl on non-windows though)

We ended up going back and forth with Sony over response file syntax. I think our conclusion was that the syntax used really just depends on the build system. For example, if you take a Unix makefile system to Windows you'll probably use Unix-style quoting and if you are distributing MSBuild compilation from Windows to Unix you'll use Windows-style quoting. Our best proxy for "what kind of build system are we using" is probably the driver mode, which isn't a very good signal. =/

We could add a clang flag to override the quoting rules, similar to the way we look for --driver-mode=cl before response file expansion.

We could also change gn to do Windows-style quoting on Unix. Windows-style quoting will tokenize more or less the same way on all platforms. For example, -D"PACKAGE_STRING=\"Mesa 9.0.3\"" comes out the same. This is what clang does in its crash reproducer shell script, so that you can always copy-paste the command line regardless of whether you're using cmd or bash.

Actually, that won't work because ninja does execl("/bin/sh", ...), so it will interpolate dollar signs in quoted strings. It's probably too late to use execv in ninja for consistency with CreateProcess.

 bug 603902  is similar to this one

After thinking about this a bit, I think this should be changed in clang-cl. gn writes ninja files that look like

  rule cc
    command = /Users/thakis/src/llvm-build/bin/clang-cl /nologo /showIncludes /FC @${out}.rsp /c ${in} /Fo${out} /Fd"${target_out_dir}/${label_name}_c.pdb"
    description = CC ${out}
    rspfile = ${out}.rsp
    rspfile_content = ${defines} ${include_dirs} ${cflags} ${cflags_c}

and the each target ninja file sets defines to something, and uses cc. Since it's not clear if defines in the target ninja file is passed as a command-line parameter or in an rspfile (and it could be used in both contexts), gn would have to write defines_rsp and defines_cmd variables (if both are needed -- in practice probably never the case, but still ugly).

I think on non-Windows, shells never use cmd-style quoting, and requiring rsp files to match the syntax of the outer shell makes sense to me. Does Sony use clang-cl on non-Windows hosts?

On Windows, it makes sense to me to have an explicit switch that controls rsp quoting for clang-cl (and if we have that switch, it could exist on non-Windows too), but I think by default clang-cl should use unix rsp file parsing rules on non-Windows hosts.

Straw man:

Nicos-MacBook-Pro:clang thakis$ svn diff tools/
Index: tools/driver/driver.cpp
===================================================================
--- tools/driver/driver.cpp	(revision 266548)
+++ tools/driver/driver.cpp	(working copy)
@@ -339,12 +339,14 @@
   // Finally, our -cc1 tools don't care which tokenization mode we use because
   // response files written by clang will tokenize the same way in either mode.
   llvm::cl::TokenizerCallback Tokenizer = &llvm::cl::TokenizeGNUCommandLine;
+#ifdef LLVM_ON_WIN32
   if (TargetAndMode.second == "--driver-mode=cl" ||
       std::find_if(argv.begin(), argv.end(), [](const char *F) {
         return F && strcmp(F, "--driver-mode=cl") == 0;
       }) != argv.end()) {
     Tokenizer = &llvm::cl::TokenizeWindowsCommandLine;
   }
+#endif
 
   // Determines whether we want nullptr markers in argv to indicate response
   // files end-of-lines. We only use this for the /LINK driver argument.

We added an --rsp-quoting flag to clang and clang-cl a while ago, and https://codereview.chromium.org/1947223003/ hooks it up.

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

commit 6bcb18bfa6f9a7bbc3f9fa4cc3953d0a215eaac5
Author: thakis <thakis@chromium.org>
Date: Thu May 05 20:23:52 2016

Tell clang-cl to use POSIX quoting for response files on non-Windows hosts.

gn always quotes arguments using the host rules.

No behavior change except when targeting Windows from non-Windows
hosts, which isn't allowed yet in build/config/BUILDCONFIG.gn

BUG= 600223 

Review-Url: https://codereview.chromium.org/1947223003
Cr-Commit-Position: refs/heads/master@{#391892}

[modify] https://crrev.com/6bcb18bfa6f9a7bbc3f9fa4cc3953d0a215eaac5/build/toolchain/win/BUILD.gn