msvs: fix many actions on the same script resulting in too-long command lines

pylib/gyp/generator/msvs.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.
 
 import copy
 import ntpath
 import os
 import posixpath
 import re
 import subprocess
@@ skipping 224 matching lines @@
   else:
     return config_name
 
 
 def _ConfigFullName(config_name, config_data):
   platform_name = _ConfigPlatform(config_data)
   return '%s|%s' % (_ConfigBaseName(config_name, platform_name), platform_name)
 
 
 def _BuildCommandLineForRuleRaw(spec, cmd, cygwin_shell, has_input_path,
-                                quote_cmd):
+                                quote_cmd, do_setup_env=True):

[jeanluc1, 2012/05/16 17:46:26]

I would not provide a default but force caller to specify the value.

[scottmg, 2012/05/16 17:56:37]

Done.

 
   if [x for x in cmd if '$(InputDir)' in x]:
     input_dir_preamble = (
       'set INPUTDIR=$(InputDir)\n'
       'set INPUTDIR=%INPUTDIR:$(ProjectDir)=%\n'
       'set INPUTDIR=%INPUTDIR:~0,-1%\n'
       )
   else:
     input_dir_preamble = ''
 
@@ skipping 10 matching lines @@
                             '`cygpath -m "${INPUTDIR}"`') for i in direct_cmd]
     if has_input_path:
       direct_cmd = [i.replace('$(InputPath)',
                               '`cygpath -m "${INPUTPATH}"`')
                     for i in direct_cmd]
     direct_cmd = ['"%s"' % i for i in direct_cmd]
     direct_cmd = [i.replace('"', '\\"') for i in direct_cmd]
     #direct_cmd = gyp.common.EncodePOSIXShellList(direct_cmd)
     direct_cmd = ' '.join(direct_cmd)
     # TODO(quote):  regularize quoting path names throughout the module
-    cmd = (
-        'call "$(ProjectDir)%(cygwin_dir)s\\setup_env.bat" && '
-        'set CYGWIN=nontsec&& ')
+    cmd = 'call "$(ProjectDir)%(cygwin_dir)s\\setup_env.bat" && ' \
+          if do_setup_env else ''

[jeanluc1, 2012/05/16 17:46:26]

I don't understand what this line does.  do_setup_env is a boolean.  So if it's
True, we have
   if True else ''.
Why the continuation?

[scottmg, 2012/05/16 17:56:37]

It's like ?: in C (in a misguided attempt to terseify initialization).
Simplified.

+    cmd += 'set CYGWIN=nontsec&& '
     if direct_cmd.find('NUMBER_OF_PROCESSORS') >= 0:
       cmd += 'set /a NUMBER_OF_PROCESSORS_PLUS_1=%%NUMBER_OF_PROCESSORS%%+1&& '
     if direct_cmd.find('INTDIR') >= 0:
       cmd += 'set INTDIR=$(IntDir)&& '
     if direct_cmd.find('OUTDIR') >= 0:
       cmd += 'set OUTDIR=$(OutDir)&& '
     if has_input_path and direct_cmd.find('INPUTPATH') >= 0:
       cmd += 'set INPUTPATH=$(InputPath) && '
     cmd += 'bash -c "%(cmd)s"'
     cmd = cmd % {'cygwin_dir': cygwin_dir,
@@ skipping 11 matching lines @@
     arguments = [i.replace('$(InputDir)','%INPUTDIR%') for i in arguments]
     if quote_cmd:
       # Support a mode for using cmd directly.
       # Convert any paths to native form (first element is used directly).
       # TODO(quote):  regularize quoting path names throughout the module
       arguments = ['"%s"' % i for i in arguments]
     # Collapse into a single command.
     return input_dir_preamble + ' '.join(command + arguments)
 
 
-def _BuildCommandLineForRule(spec, rule, has_input_path):
+def _BuildCommandLineForRule(spec, rule, has_input_path, do_setup_env=True):
   # Find path to cygwin.
   cygwin_dir = _FixPath(spec.get('msvs_cygwin_dirs', ['.'])[0])
 
   # Currently this weird argument munging is used to duplicate the way a
   # python script would need to be run as part of the chrome tree.
   # Eventually we should add some sort of rule_default option to set this
   # per project. For now the behavior chrome needs is the default.
   mcs = rule.get('msvs_cygwin_shell')
   if mcs is None:
     mcs = int(spec.get('msvs_cygwin_shell', 1))
   elif isinstance(mcs, str):
     mcs = int(mcs)
   quote_cmd = int(rule.get('msvs_quote_cmd', 1))
   return _BuildCommandLineForRuleRaw(spec, rule['action'], mcs, has_input_path,
-                                     quote_cmd)
+                                     quote_cmd, do_setup_env=do_setup_env)
 
 
 def _AddActionStep(actions_dict, inputs, outputs, description, command):
   """Merge action into an existing list of actions.
 
   Care must be taken so that actions which have overlapping inputs either don't
   get assigned to the same input, or get collapsed into one.
 
   Arguments:
     actions_dict: dictionary keyed on input name, which maps to a list of
@@ skipping 1101 matching lines @@
                             _ConfigFullName(config_name, config),
                             {}, tools=[tool])
   # Do nothing if there was no precompiled source.
   if extensions_excluded_from_precompile:
     DisableForSourceTree(sources)
 
 
 def _AddActions(actions_to_add, spec, relative_path_of_gyp_file):
   # Add actions.
   actions = spec.get('actions', [])
+  # Don't setup_env every time, or when all the actions are run together in

[jeanluc1, 2012/05/16 17:46:26]

-->  Don't setup_env every time.  When

[scottmg, 2012/05/16 17:56:37]

Done.

+  # one batch file in VS, the PATH will grow too long.
+  first_action = True
   for a in actions:
-    cmd = _BuildCommandLineForRule(spec, a, has_input_path=False)
+    cmd = _BuildCommandLineForRule(spec, a, has_input_path=False,
+                                   do_setup_env=first_action)
     # Attach actions to the gyp file if nothing else is there.
     inputs = a.get('inputs') or [relative_path_of_gyp_file]
     # Add the action.
     _AddActionStep(actions_to_add,
                    inputs=inputs,
                    outputs=a.get('outputs', []),
                    description=a.get('message', a['action_name']),
                    command=cmd)
+    first_action = False
 
 
 def _WriteMSVSUserFile(project_path, version, spec):
   # Add run_as and test targets.
   if 'run_as' in spec:
     run_as = spec['run_as']
     action = run_as.get('action', [])
     environment = run_as.get('environment', [])
     working_directory = run_as.get('working_directory', '.')
   elif int(spec.get('test', 0)):
@@ skipping 1547 matching lines @@
       cmd = action['command']
       # For most actions, add 'call' so that actions that invoke batch files
       # return and continue executing.  msbuild_use_call provides a way to
       # disable this but I have not seen any adverse effect from doing that
       # for everything.
       if action.get('msbuild_use_call', True):
         cmd = 'call ' + cmd
       commands.append(cmd)
     # Add the custom build action for one input file.
     description = ', and also '.join(descriptions)
-    command = ' && '.join(commands)
+
+    # We can't join the commands simply with && because the command line will
+    # get too long. See also _AddActions: cygwin's setup_env mustn't be called
+    # for every invocation or the command that sets the PATH will grow too
+    # long.
+    command = (
+        '\r\nif %errorlevel% neq 0 exit /b %errorlevel%\r\n'.join(commands))
     _AddMSBuildAction(spec,
                       primary_input,
                       inputs,
                       outputs,
                       command,
                       description,
                       sources_handled_by_action,
                       actions_spec)
   return actions_spec, sources_handled_by_action
 
@@ skipping 12 matching lines @@
   action_spec.extend(
       # TODO(jeanluc) 'Document' for all or just if as_sources?
       [['FileType', 'Document'],
        ['Command', command],
        ['Message', description],
        ['Outputs', outputs]
       ])
   if additional_inputs:
     action_spec.append(['AdditionalInputs', additional_inputs])
   actions_spec.append(action_spec)