ninja: split out the logic for linking from target writing

pylib/gyp/generator/ninja.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 gyp
 import gyp.common
 import gyp.system_test
 import gyp.xcode_emulation
 import os.path
 import re
@@ skipping 263 matching lines @@
               lambda path, lang: self.GypPathToUniqueOutput(path + '-' + lang)))
       # Some actions/rules output 'sources' that are already object files.
       link_deps += [self.GypPathToNinja(f) for f in sources if f.endswith('.o')]
 
     # The final output of our target depends on the last output of the
     # above steps.
     output = output_binary = None
     if link_deps or sources_depends or actions_depends:
       output, output_binary = self.WriteTarget(
           spec, config_name, config, link_deps,
-          sources_depends or actions_depends, mac_bundle_depends,
-          order_only=actions_depends)
+          sources_depends or actions_depends, mac_bundle_depends)
     if self.name != output and self.toolset == 'target':
       # Write a short name to build this target.  This benefits both the
       # "build chrome" case as well as the gyp tests, which expect to be
       # able to run actions and build libraries by their short name.
       self.ninja.build(self.name, 'phony', output)
 
     return output, output_binary, compile_depends
 
   def WriteActionsRulesCopies(self, spec, extra_sources, prebuild,
                               mac_bundle_depends):
@@ skipping 269 matching lines @@
       var_name = {
         'c': 'cflags_pch_c',
         'cc': 'cflags_pch_cc',
         'm': 'cflags_pch_objc',
         'mm': 'cflags_pch_objcc',
       }[lang]
 
       cmd = { 'c': 'cc', 'cc': 'cxx', 'm': 'objc', 'mm': 'objcxx', }.get(lang)
       self.ninja.build(gch, cmd, input, variables=[(var_name, lang_flag)])
 
-  def WriteTarget(self, spec, config_name, config, link_deps, final_deps,
-                  mac_bundle_depends, order_only):
-    if spec['type'] == 'none':
-      # This target doesn't have any explicit final output, but is instead
-      # used for its effects before the final output (e.g. copies steps).
-      # Reuse the existing output if it's easy.
-      if len(final_deps) == 1:
-        return final_deps[0], final_deps[0]
-      # Otherwise, fall through to writing out a stamp file.
 
-    output_uses_linker = spec['type'] in ('executable', 'loadable_module',
-                                          'shared_library')
+  def WriteLink(self, spec, config_name, config, link_deps):
+    """Write out a link step.  Returns the path to the output."""
+
+    command = {
+      'executable':      'link',
+      'loadable_module': 'solink_module',
+      'shared_library':  'solink',
+    }[spec['type']]
 
     implicit_deps = set()
+
     if 'dependencies' in spec:
       # Two kinds of dependencies:
       # - Linkable dependencies (like a .a or a .so): add them to the link line.
       # - Non-linkable dependencies (like a rule that generates a file
       #   and writes a stamp file): add them to implicit_deps
-      if output_uses_linker:
-        extra_link_deps = set()
-        for dep in spec['dependencies']:
-          _, binary, _, linkable = self.target_outputs.get(
-              dep, (None, None, [], False))
-          if not binary:
-            continue
-          if linkable:
-            extra_link_deps.add(binary)
-          else:
-            # TODO: Chrome-specific HACK.  Chrome runs this lastchange rule on
-            # every build, but we don't want to rebuild when it runs.
-            if 'lastchange' in binary:
-              continue
-            # TODO(evan): it's confusing that the variable here is called
-            # "binary", despite being a stamp file.  Fix this.
-            implicit_deps.add(binary)
-        link_deps.extend(list(extra_link_deps))
+      extra_link_deps = set()
+      for dep in spec['dependencies']:
+        output, binary, _, linkable = self.target_outputs.get(
+          dep, (None, None, [], False))
+        if not binary:
+          continue
+        if linkable:
+          extra_link_deps.add(binary)
+
+        # TODO: Chrome-specific HACK.  Chrome runs this lastchange rule on
+        # every build, but we don't want to rebuild when it runs.
+        if 'lastchange' in binary:

[Nico, 2012/01/12 06:24:33]

'lastchange' is probably a stamp file, so i think it would be clearer if you
said "in output" here (binary = output in that case; but you're adding output to
implicit_deps 2 lines below)

+          continue
+        implicit_deps.add(output)

[Evan Martin, 2012/01/12 02:53:16]

From here to "if linkable" above is different than the left side.  Otherwise
it's intended to be all the same, with "output_uses_linker" no longer necessary
because all code in this function is for the linker codepath.

+      link_deps.extend(list(extra_link_deps))
+
+    if self.is_mac_bundle:
+      output = self.ComputeMacBundleBinaryOutput(spec)
+    else:
+      output = self.ComputeOutput(spec)
+
+    if self.flavor == 'mac':
+      ldflags = self.xcode_settings.GetLdflags(config_name,
+          self.ExpandSpecial(generator_default_variables['PRODUCT_DIR']),
+          self.GypPathToNinja)
+    else:
+      ldflags = config.get('ldflags', [])
+    self.WriteVariableList('ldflags',
+                           gyp.common.uniquer(map(self.ExpandSpecial,
+                                                  ldflags)))
+
+    libraries = gyp.common.uniquer(map(self.ExpandSpecial,
+                                       spec.get('libraries', [])))
+    if self.flavor == 'mac':
+      libraries = self.xcode_settings.AdjustFrameworkLibraries(libraries)
+    self.WriteVariableList('libs', libraries)
+
+    extra_bindings = []
+    if command in ('solink', 'solink_module'):
+      extra_bindings.append(('soname', os.path.split(output)[1]))
+
+    self.ninja.build(output, command, link_deps,
+                     implicit=list(implicit_deps),
+                     variables=extra_bindings)
+    return output
+
+  def WriteTarget(self, spec, config_name, config, link_deps, final_deps,
+                  mac_bundle_depends):
+    if spec['type'] == 'none':
+      # This target doesn't have any explicit final output, but is instead
+      # used for its effects before the final output (e.g. copies steps).
+      output = self.WriteCollapsedDependencies('foobar', final_deps)[0]

[Nico, 2012/01/12 06:24:33]

nit: Maybe there's a better name than 'foobar'.

+      return output, output
+    elif spec['type'] == 'static_library':
+      output_binary = self.ComputeOutput(spec)
+      self.ninja.build(output_binary, 'alink', link_deps,
+                       order_only=final_deps)
+    else:
+      output_binary = self.WriteLink(spec, config_name, config, link_deps)
 
     if self.is_mac_bundle:
       output = self.ComputeMacBundleOutput(spec)
-      output_binary = self.ComputeMacBundleBinaryOutput(spec)
-      if not link_deps:
-        output_binary = self.ComputeOutput(spec, type='none')
       mac_bundle_depends.append(output_binary)
     else:
-      output = output_binary = self.ComputeOutput(spec)
-
-    command_map = {
-      'executable':      'link',
-      'static_library':  'alink',
-      'loadable_module': 'solink_module',
-      'shared_library':  'solink',
-      'none':            'stamp',
-    }
-    command = command_map[spec['type']]
-
-    if link_deps:
-      final_deps = link_deps
-    else:
-      command = 'stamp'
-      order_only += final_deps
-      final_deps = []
-
-    if output_uses_linker:
-      if self.flavor == 'mac':
-        ldflags = self.xcode_settings.GetLdflags(config_name,
-            self.ExpandSpecial(generator_default_variables['PRODUCT_DIR']),
-            self.GypPathToNinja)
-      else:
-        ldflags = config.get('ldflags', [])
-      self.WriteVariableList('ldflags',
-                             gyp.common.uniquer(map(self.ExpandSpecial,
-                                                    ldflags)))
-
-      libraries = gyp.common.uniquer(map(self.ExpandSpecial,
-                                         spec.get('libraries', [])))
-      if self.flavor == 'mac':
-        libraries = self.xcode_settings.AdjustFrameworkLibraries(libraries)
-      self.WriteVariableList('libs', libraries)
-
-    extra_bindings = []
-    if command in ('solink', 'solink_module'):
-      extra_bindings.append(('soname', os.path.split(output_binary)[1]))
-
-    self.ninja.build(output_binary, command, final_deps,
-                     implicit=list(implicit_deps),
-                     order_only=order_only,
-                     variables=extra_bindings)
+      output = output_binary
 
     if self.is_mac_bundle:
       if spec['type'] in ('shared_library', 'loadable_module'):
         variables = [('version', self.xcode_settings.GetFrameworkVersion())]
         self.ninja.build(output, 'package_framework', mac_bundle_depends,
                          variables=variables)
       else:
         self.ninja.build(output, 'stamp', mac_bundle_depends)
 
     return output, output_binary
@@ skipping 343 matching lines @@
     if output:
       linkable = spec['type'] in ('static_library', 'shared_library')
       target_outputs[qualified_target] = (
           output, output_binary, compile_depends, linkable)
 
       if qualified_target in all_targets:
         all_outputs.add(output)
 
   if all_outputs:
     master_ninja.build('all', 'phony', list(all_outputs))