ninja windows: handle more cases in program quoting

ninja windows: handle more cases in program quoting

The first argument of a command (the program) can't always be quoted on Windows. In particular, the 'call x.bat' form has to be handled specially.

R=thakis@chromium.org
BUG=125606
Committed: https://code.google.com/p/gyp/source/detail?r=1349

[scottmg, 2012-05-01 00:35:30]

On 2012/05/01 00:08:58, scottmg wrote:

Hmm, it would seem this does not work in all of our fabulous edge cases. Please
ignore.

[scottmg, 2012-05-01 16:50:48]

Handles all existing tests + new one now.

[Nico, 2012-05-01 17:17:26]

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
File pylib/gyp/msvs_emulation.py (right):

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:59: program = args[0] if args else ''
nit: I'd `if not args: return ''` at the top instead of the if here (and then
just `program = args[0]` here)

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:61: call, space, batch = program.partition(' ')
call, batch = program.split(' ', 1) ? Or at least s/space/_/

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:66: program = QuoteForRspFile(program)
You do this here only if ' ' in program, but unconditionally in the bat branch.
Is that intentional?

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:167: '$(PlatformName)': 'Win32', # TODO(scottmg):
Support for x64 toolchain.
That part landed already and needs a rebase, right?

http://chromiumcodereview.appspot.com/10269018/diff/5001/test/win/command-quo...
File test/win/command-quote/command-quote.gyp (right):

http://chromiumcodereview.appspot.com/10269018/diff/5001/test/win/command-quo...
test/win/command-quote/command-quote.gyp:18: },],
Is there already a test case for the non-bat branch (with space)?

[scottmg, 2012-05-01 18:40:14]

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
File pylib/gyp/msvs_emulation.py (right):

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:59: program = args[0] if args else ''
On 2012/05/01 17:17:26, Nico wrote:
> nit: I'd `if not args: return ''` at the top instead of the if here (and then
> just `program = args[0]` here)

Done.

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:61: call, space, batch = program.partition(' ')
On 2012/05/01 17:17:26, Nico wrote:
> call, batch = program.split(' ', 1) ? Or at least s/space/_/

I always forget about [, maxsplit] on split for some reason. Done.

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:66: program = QuoteForRspFile(program)
On 2012/05/01 17:17:26, Nico wrote:
> You do this here only if ' ' in program, but unconditionally in the bat
branch.
> Is that intentional?

Yes, unfortunately. Not adding the quotes here handles the opposite case. If
args[0] is a shell builtin then it can't be quoted. (call, echo, etc.) Added
second test and comment to that effect.

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:167: '$(PlatformName)': 'Win32', # TODO(scottmg):
Support for x64 toolchain.
On 2012/05/01 17:17:26, Nico wrote:
> That part landed already and needs a rebase, right?

You lost me. There's nothing here that's related to this change that I know of.

http://chromiumcodereview.appspot.com/10269018/diff/5001/test/win/command-quo...
File test/win/command-quote/command-quote.gyp (right):

http://chromiumcodereview.appspot.com/10269018/diff/5001/test/win/command-quo...
test/win/command-quote/command-quote.gyp:18: },],
On 2012/05/01 17:17:26, Nico wrote:
> Is there already a test case for the non-bat branch (with space)?

Done.

[Nico, 2012-05-01 20:10:27]

lgtm

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
File pylib/gyp/msvs_emulation.py (right):

http://chromiumcodereview.appspot.com/10269018/diff/5001/pylib/gyp/msvs_emula...
pylib/gyp/msvs_emulation.py:167: '$(PlatformName)': 'Win32', # TODO(scottmg):
Support for x64 toolchain.
On 2012/05/01 18:40:14, scottmg wrote:
> On 2012/05/01 17:17:26, Nico wrote:
> > That part landed already and needs a rebase, right?
> 
> You lost me. There's nothing here that's related to this change that I know
of.

Doh, looked at the delta from a previous patch set here :-)