ninja windows: handle more cases in program quoting

pylib/gyp/msvs_emulation.py

 # Copyright (c) 2012 Google Inc. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """
 This module helps emulate Visual Studio 2008 behavior on top of other
 build systems, primarily ninja.
 """
 
 import os
@@ skipping 35 matching lines @@
   # These commands are used in rsp files, so no escaping for the shell (via ^)
   # is necessary.
 
   # Finally, wrap the whole thing in quotes so that the above quote rule
   # applies and whitespace isn't a word break.
   return '"' + arg + '"'
 
 
 def EncodeRspFileList(args):
   """Process a list of arguments using QuoteCmdExeArgument."""
-  return ' '.join(QuoteForRspFile(arg) for arg in args)
+  # Note that the first argument is assumed to be the command. We take extra
+  # steps to make sure that calls to .bat files are handled correctly, and
+  # that paths are normalized and quoted as necessary.
+  program = args[0] if args else ''

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

nit: I'd `if not args: return ''` at the top instead of the if here (and then
just `program = args[0]` here)

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

Done.

+  if program.startswith('call '):
+    call, space, batch = program.partition(' ')

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

call, batch = program.split(' ', 1) ? Or at least s/space/_/

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

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

+    program = 'call ' + QuoteForRspFile(os.path.normpath(batch))
+  else:
+    program = os.path.normpath(program)
+    if ' ' in program:
+      program = QuoteForRspFile(program)

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

You do this here only if ' ' in program, but unconditionally in the bat branch.
Is that intentional?

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

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.

+  return program + ' ' + ' '.join(QuoteForRspFile(arg) for arg in args[1:])
 
 
 def _GenericRetrieve(root, default, path):
   """Given a list of dictionary keys |path| and a tree of dicts |root|, find
   value at path, or return |default| if any of the path doesn't exist."""
   if not root:
     return default
   if not path:
     return root
   return _GenericRetrieve(root.get(path[0]), default, path[1:])
@@ skipping 79 matching lines @@
     """Get a dict of variables mapping internal VS macro names to their gyp
     equivalents."""
     replacements = {
         '$(VSInstallDir)': self.vs_version.Path(),
         '$(VCInstallDir)': os.path.join(self.vs_version.Path(), 'VC'),
         '$(OutDir)\\': base_to_build + '\\' if base_to_build else '',
         '$(IntDir)': '$!INTERMEDIATE_DIR',
         '$(InputPath)': '${source}',
         '$(InputName)': '${root}',
         '$(ProjectName)': self.spec['target_name'],
         '$(PlatformName)': 'Win32', # TODO(scottmg): Support for x64 toolchain.

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

That part landed already and needs a rebase, right?

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

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

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

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

     }
     # Chromium uses DXSDK_DIR in include/lib paths, but it may or may not be
     # set. This happens when the SDK is sync'd via src-internal, rather than
     # by typical end-user installation of the SDK. If it's not set, we don't
     # want to leave the unexpanded variable in the path, so simply strip it.
     replacements['$(DXSDK_DIR)'] = self.dxsdk_dir if self.dxsdk_dir else ''
     return replacements
 
   def ConvertVSMacros(self, s, base_to_build=None):
     """Convert from VS macro names to something equivalent."""
@@ skipping 303 matching lines @@
         vs.Path(), r'Common7\Tools\vsvars32.bat'))
 
 def ExpandMacros(string, expansions):
   """Expand $(Variable) per expansions dict. See MsvsSettings.GetVSMacroEnv
   for the canonical way to retrieve a suitable dict."""
   if '$' in string:
     for old, new in expansions.iteritems():
       assert '$(' not in new, new
       string = string.replace(old, new)
   return string