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

pylib/gyp/generator/msvs.py

Index: pylib/gyp/generator/msvs.py
===================================================================
--- pylib/gyp/generator/msvs.py	(revision 879)
+++ pylib/gyp/generator/msvs.py	(working copy)
@@ -288,22 +288,50 @@
                            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.
 
-def _AddCustomBuildToolForMSVS(p, spec, inputs, outputs, description, cmd):
+  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 = reduce(lambda x,y:len(actions_dict.get(x, []))
+                                   < len(actions_dict.get(y, []))
+                        and x or y, inputs)
+
+  # Add it there.
+  if chosen_input not in actions_dict:
+    actions_dict[chosen_input] = []
+  actions_dict[chosen_input].append(action)
+
+
+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
@@ -318,13 +346,44 @@
       '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.
+  """
+  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']
+      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.
 
@@ -488,12 +547,11 @@
   #                   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(actions_to_add,
+                 inputs=[_FixPath(i) for i in all_inputs],
+                 outputs=[_FixPath(i) for i in all_outputs],
+                 description='Running %s' % cmd,
+                 command=cmd)
 
 
 def _EscapeEnvironmentVariableExpansion(s):
@@ -596,8 +654,8 @@
     _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:
@@ -682,31 +740,27 @@
   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()
 
@@ -1036,23 +1090,14 @@
   _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]
   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

[bradn, 2011/02/01 03:02:51]

Ah, its VERY subtle.
This line MODIFIES the spec (python references).
That's why this worked before.

+    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.
@@ -1182,16 +1227,19 @@
                       {}, 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.
+    inputs = a.get('inputs') or [gyp_file]
+    # 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):
@@ -1215,11 +1263,11 @@
   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):
@@ -1436,7 +1484,7 @@
   # 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