Fixing msvs handling of actions which have overlapping inputs....

pylib/gyp/generator/msvs.py

 #!/usr/bin/python
 
 # Copyright (c) 2011 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 ntpath
 import posixpath
 import os
 import re
@@ skipping 270 matching lines @@
   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 _PrepareActionRaw(spec, rule['action'], mcs,
                            has_input_path, quote_cmd)
 
 
-def _PickPrimaryInput(inputs):
-  # Pick second input as the primary one, unless there's only one.
-  # TODO(bradnelson): this is a bit of a hack,
-  # find something more general.
-  if len(inputs) > 1:
-    return inputs[1]
-  else:
-    return inputs[0]
+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
+      dicts describing the actions attached to that input file.
+    inputs: list of inputs
+    outputs: list of outputs
+    description: description of the action
+    command: command line to execute
+  """
+  # Require there to be at least one input (call sites will ensure this).
+  assert inputs
+
+  action = {
+      'inputs': inputs,
+      'outputs': outputs,
+      'description': description,
+      'command': command,
+  }
+
+  # Pick where to stick this action. Pick the slot with the least actions
+  # currently.
+  chosen_input = None
+  for i in inputs:
+    if (chosen_input is None or
+        len(actions_dict.get(i, [])) < len(actions_dict.get(chosen_input, []))):
+      chosen_input = i
+  assert chosen_input is not None
+

[jeanluc1, 2011/02/01 02:00:08]

You could have done:

chosen_input = reduce(lambda x,y:len(actions_dict[x]) < len(actions_dict[y]) and
x or y, actions_dict)

+  # Add it there.
+  if chosen_input not in actions_dict:
+    actions_dict[chosen_input] = []
+  actions_dict[chosen_input].append(action)
 
 
-def _AddCustomBuildToolForMSVS(p, spec, inputs, outputs, description, cmd):
+def _AddCustomBuildToolForMSVS(p, spec, primary_input,
+                               inputs, outputs, description, cmd):
   """Add a custom build tool to execute something.
 
   Arguments:
     p: the target project
     spec: the target project dict
+    primary_input: input file to attach the build tool to
     inputs: list of inputs
     outputs: list of outputs
     description: description of the action
     cmd: command line to execute
   """
   inputs = [_FixPath(i) for i in inputs]
   outputs = [_FixPath(i) for i in outputs]
   tool = MSVSProject.Tool(
       'VCCustomBuildTool', {
       'Description': description,
       'AdditionalDependencies': ';'.join(inputs),
       'Outputs': ';'.join(outputs),
       'CommandLine': cmd,
       })
-  primary_input = _PickPrimaryInput(inputs)
   # Add to the properties of primary input for each config.
   for config_name, c_data in spec['configurations'].iteritems():
-    p.AddFileConfig(primary_input,
+    p.AddFileConfig(_FixPath(primary_input),
                     _ConfigFullName(config_name, c_data), tools=[tool])
 
 
+def _AddAccumulatedActions(p, spec, actions_dict):
+  """Add actions accumulated into an actions_dict, merging as needed.
+
+  Arguments:
+    p: the target project
+    spec: the target project dict
+    actions_dict: dictionary keyed on input name, which maps to a list of
+      dicts describing the actions attached to that input file.

[jeanluc1, 2011/02/01 02:00:08]

two more spaces?

+  """
+  for input in actions_dict:
+    inputs = set()
+    outputs = set()
+    description = ''
+    command = ''
+    for action in actions_dict[input]:
+      inputs.update(set(action['inputs']))
+      outputs.update(set(action['outputs']))
+      if description:
+        description += ', and also '
+      description += action['description']

[jeanluc1, 2011/02/01 02:00:08]

Another option is to pass a list for description and command, and inside of
_addCustom do a ', and also'.join().

[bradn, 2011/02/01 02:55:52]

Done.

+      if command:
+        command += ' && '
+      command += action['command']
+    # Add the custom build step for one input file.
+    _AddCustomBuildToolForMSVS(p, spec,
+                               primary_input=input,
+                               inputs=inputs,
+                               outputs=outputs,
+                               description=description,
+                               cmd=command)
+
+
 def _RuleExpandPath(path, input_file):
   """Given the input file to which a rule applied, string substitute a path.
 
   Arguments:
     path: a path to string expand
     input_file: the file to which the rule applied.
   Returns:
     The string substituted path.
   """
   path = path.replace('$(InputName)',
@@ skipping 143 matching lines @@
   cmd = ['make',
          'OutDir=$(OutDir)',
          'IntDir=$(IntDir)',
          '-j', '${NUMBER_OF_PROCESSORS_PLUS_1}',
          '-f', filename]
   cmd = _PrepareActionRaw(spec, cmd, True, False, True)
   # TODO(bradnelson): this won't be needed if we have a better way to pick
   #                   the primary input.
   all_inputs = list(all_inputs)
   all_inputs.insert(1, filename)
-  actions_to_add.append({
-      'inputs': [_FixPath(i) for i in all_inputs],
-      'outputs': [_FixPath(i) for i in all_outputs],
-      'description': 'Running %s' % cmd,
-      'cmd': cmd,
-      })
+  _AddActionStep(inputs=[_FixPath(i) for i in all_inputs],

[jeanluc1, 2011/02/01 02:00:08]

I would expect the first argument to be actions_to_add or something similar.

[bradn, 2011/02/01 02:55:52]

Done.

+                 outputs=[_FixPath(i) for i in all_outputs],
+                 description='Running %s' % cmd,
+                 command=cmd)
 
 
 def _EscapeEnvironmentVariableExpansion(s):
   """Escapes any % characters so that Windows-style environment variable
      expansions will leave them alone.
      See http://connect.microsoft.com/VisualStudio/feedback/details/106127/cl-d-name-text-containing-percentage-characters-doesnt-compile
      to understand why we have to do this."""
   s = s.replace('%', '%%')
   return s
 
@@ skipping 82 matching lines @@
 
   # Handle rules that use a native rules file.
   if rules_native:
    _GenerateNativeRulesForMSVS(p, rules_native, output_dir, spec, options)
 
   # Handle external rules (non-native rules).
   if rules_external:
     _GenerateExternalRules(rules_external, output_dir, spec,
                            sources, options, actions_to_add)
   _AdjustSourcesForRules(rules, sources, excluded_sources)
-  
-  
+
+
 def _AdjustSourcesForRules(rules, sources, excluded_sources):
   # Add outputs generated by each rule (if applicable).
   for rule in rules:
     # Done if not processing outputs as sources.
     if int(rule.get('process_outputs_as_sources', False)):
       # Add in the outputs from this rule.
       trigger_files = _FindRuleTriggerFiles(rule, sources)
       for tf in trigger_files:
         inputs, outputs = _RuleInputsAndOutputs(rule, tf)
         inputs.remove(tf)
@@ skipping 64 matching lines @@
   gyp_dir = os.path.split(project.path)[0]
 
   config_type = _GetMSVSConfigurationType(spec, project.build_file)
   for config_name, config in spec['configurations'].iteritems():
     _AddConfigurationToMSVSProject(p, spec, config_type, config_name, config)
 
   # Prepare list of sources and excluded sources.
   sources, excluded_sources = _PrepareListOfSources(spec, project.build_file)
 
   # Add rules.
-  actions_to_add = []
+  actions_to_add = {}
   _GenerateRulesForMSVS(p, gyp_dir, options, spec,
                         sources, excluded_sources,
                         actions_to_add)
-  sources, excluded_sources, excluded_idl = _AdjustSourcesAndConverToFilterHierarchy(spec, options,
-      gyp_dir, sources, excluded_sources)
+  sources, excluded_sources, excluded_idl = (
+      _AdjustSourcesAndConverToFilterHierarchy(
+          spec, options, gyp_dir, sources, excluded_sources))
 
   # Add in files.
   p.AddFiles(sources)
 
-  # Add deferred actions to add.
-  for a in actions_to_add:
-    _AddCustomBuildToolForMSVS(p, spec,
-                               inputs=a['inputs'],
-                               outputs=a['outputs'],
-                               description=a['description'],
-                               cmd=a['cmd'])
-
   _ExcludeFilesFromBeingBuilt(p, spec, excluded_sources, excluded_idl)
   _AddToolFilesToMSVS(p, spec)
   _HandlePreCompileHeaderStubs(p, spec)
-  _AddActionsToMSVS(p, spec)
+  _AddActionsToMSVS(actions_to_add, spec, project.build_file)
+  _AddCopiesForMSVS(actions_to_add, spec)
   _WriteMSVSUserFile(project.path, version, spec)
-  _AddCopiesForMSVS(p, spec)
+
+  # Do this after all actions have been decided.
+  _AddAccumulatedActions(p, spec, actions_to_add)
 
   # Write it out.
   p.Write()
 
 
 def _GetUniquePlatforms(spec):
   """Return the list of unique platforms for this spec, e.g ['win32', ...]
 
   Arguments:
     spec: The target dictionary containing the properties of the target.
@@ skipping 310 matching lines @@
   Arguments:
     spec: The target dictionary containing the properties of the target.
     build_file: Filename of the .gyp file that the vcproj file comes from.
   Returns:
     A pair of (list of sources, list of excluded sources)
   """
   sources = set()
   _AddNormalizedSources(sources, spec.get('sources', []))
   excluded_sources = set()
   # Add in the gyp file.
-  gyp_file = os.path.split(build_file)[1]
+  gyp_file = posixpath.split(build_file)[1]

[jeanluc1, 2011/02/01 02:00:08]

Why posixpath?

[bradn, 2011/02/01 02:55:52]

Previously the case where there are no inputs below didn't actually create an
action. So this was wrong, and never caught. At this point the build file is
still in /s.

   sources.add(_NormalizedSource(gyp_file))
   # Add in 'action' inputs and outputs.
   for a in spec.get('actions', []):
-    inputs = a.get('inputs')
-    if not inputs:
-      # This is an action with no inputs.  Make the primary input
-      # be the .gyp file itself so Visual Studio has a place to
-      # hang the custom build rule.
-      inputs = [gyp_file]
-      a['inputs'] = inputs
+    inputs = a.get('inputs', [])
     inputs = [_NormalizedSource(i) for i in inputs]
-    primary_input = _PickPrimaryInput(inputs)
     inputs = set(inputs)
     sources.update(inputs)
-    inputs.remove(primary_input)
-    excluded_sources.update(inputs)
     if int(a.get('process_outputs_as_sources', False)):
       _AddNormalizedSources(sources, a.get('outputs', []))
   # Add in 'copies' inputs and outputs.
   for cpy in spec.get('copies', []):
     _AddNormalizedSources(sources, cpy.get('files', []))
   return (sources, excluded_sources)
 
 
 def _AdjustSourcesAndConverToFilterHierarchy(spec, options, gyp_dir, sources, excluded_sources):
   """Adjusts the list of sources and excluded sources.
@@ skipping 109 matching lines @@
     source = config.get('msvs_precompiled_source')
     if source:
       source = _FixPath(source)
       # UsePrecompiledHeader=1 for if using precompiled headers.
       tool = MSVSProject.Tool('VCCLCompilerTool',
                               {'UsePrecompiledHeader': '1'})
       p.AddFileConfig(source, _ConfigFullName(config_name, config),
                       {}, tools=[tool])
 
 
-def _AddActionsToMSVS(p, spec):
+def _AddActionsToMSVS(actions_to_add, spec, gyp_file):
   # Add actions.
   actions = spec.get('actions', [])
   for a in actions:
     cmd = _PrepareAction(spec, a, has_input_path=False)
-    _AddCustomBuildToolForMSVS(p, spec,
-                               inputs=a.get('inputs', []),
-                               outputs=a.get('outputs', []),
-                               description=a.get('message', a['action_name']),
-                               cmd=cmd)
+    # Attach actions to the gyp file if nothing else is there.

[jeanluc1, 2011/02/01 02:00:08]

When would this happen?  What does it mean?

[bradn, 2011/02/01 02:55:52]

We have a few test cases where actions have no inputs (though technically they
should be on the script that's running).
It's not ideal that this is even supported, but it works currently, and one of
the actions tests fails without it.
This is the complement of the bit that adds the gyp file in around 1099.

Actually the more interesting question is why the null_input test in subdir3
passed before on windows.
I'm looking into that now.

+    inputs = a.get('inputs')
+    if not inputs:
+      inputs = [gyp_file]

[jeanluc1, 2011/02/01 02:00:08]

Could have been:
inputs = a.get('inputs') or [gyp_file]

[bradn, 2011/02/01 02:55:52]

Duh...
Done.

+    # Add the action.
+    _AddActionStep(actions_to_add,
+                   inputs=inputs,
+                   outputs=a.get('outputs', []),
+                   description=a.get('message', a['action_name']),
+                   command=cmd)
 
 
 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)):
     action = ['$(TargetPath)', '--gtest_print_time']
     environment = []
     working_directory = '.'
   else:
     return  # Nothing to add
   # Write out the user file.
   user_file = _CreateMSVSUserFile(project_path, version, spec)
   for config_name, c_data in spec['configurations'].iteritems():
     user_file.AddDebugSettings(_ConfigFullName(config_name, c_data),
                                action, environment, working_directory)
   user_file.Write()
 
 
-def _AddCopiesForMSVS(p, spec):
+def _AddCopiesForMSVS(actions_to_add, spec):
   copies = _GetCopies(spec)
   for inputs, outputs, cmd, description in copies:
-    _AddCustomBuildToolForMSVS(p, spec, inputs=inputs, outputs=outputs,
-                               description=description, cmd=cmd)
+    _AddActionStep(actions_to_add, inputs=inputs, outputs=outputs,
+                   description=description, command=cmd)
 
 
 def _GetCopies(spec):
   copies = []
   # Add copies.
   for cpy in spec.get('copies', []):
     for src in cpy.get('files', []):
       dst = os.path.join(cpy['destination'], os.path.basename(src))
       # _AddCustomBuildToolForMSVS() will call _FixPath() on the inputs and
       # outputs, so do the same for our generated command line.
@@ skipping 196 matching lines @@
   configs = set()
   for qualified_target in target_list:
     spec = target_dicts[qualified_target]
     for config_name, config in spec['configurations'].iteritems():
       configs.add(_ConfigFullName(config_name, config))
   configs = list(configs)
 
   # Figure out all the projects that will be generated and their guids
   project_objects = _CreateProjectObjects(target_list, target_dicts, options,
                                           msvs_version)
-  
+
   # Generate each project.
   for project in project_objects.values():
     fixpath_prefix = project.fixpath_prefix
     _GenerateProject(project, options, msvs_version)
   fixpath_prefix = None
 
   for build_file in data.keys():
     # Validate build_file extension
     if build_file[-4:] != '.gyp':
       continue
     sln_path = build_file[:-4] + options.suffix + '.sln'
     if options.generator_output:
       sln_path = os.path.join(options.generator_output, sln_path)
     # Get projects in the solution, and their dependents.
     sln_projects = gyp.common.BuildFileTargets(target_list, build_file)
     sln_projects += gyp.common.DeepDependencyTargets(target_dicts, sln_projects)
     # Create folder hierarchy.
     root_entries = _GatherSolutionFolders(
         sln_projects, project_objects, flat=msvs_version.FlatSolution())
     # Create solution.
     sln = MSVSNew.MSVSSolution(sln_path,
                                entries=root_entries,
                                variants=configs,
                                websiteProperties=False,
                                version=msvs_version)
     sln.Write()