ninja/mac: Don't choke on bundles that have no 'sources'.

pylib/gyp/generator/ninja.py

Index: pylib/gyp/generator/ninja.py
===================================================================
--- pylib/gyp/generator/ninja.py	(revision 1133)
+++ pylib/gyp/generator/ninja.py	(working copy)
@@ -279,16 +279,16 @@
     # The final output of our target depends on the last output of the
     # above steps.
     output = output_binary = None
-    final_deps = link_deps or sources_depends or actions_depends
-    if final_deps:
+    if link_deps or sources_depends or actions_depends:
       output, output_binary = self.WriteTarget(
-          spec, config_name, config, final_deps, mac_bundle_depends,
+          spec, config_name, config, link_deps,
+          sources_depends or actions_depends, mac_bundle_depends,
           order_only=actions_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)
+    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)

[Evan Martin, 2012/01/07 21:29:00]

Hm, now I'm confused.  Won't this write out this phony rule when output=None? 
Do we want that?

[Nico, 2012/01/07 21:32:33]

Yes, that's the point of this change :-) output will be none if "link_deps or
sources_depends or actions_depends" is false, which happens for empty bundles
(first target in the test case). In that case, we want to write a phony target,
so that building that empty bundle target doesn't error out with "no target
named empty_bundle".

As said in mail 1, ""The completely empty bundles aren't needed by the chromium
build, but I found that ninja was inconsistent with make/xcode while writing the
test, so I fixed that too.""". So if you don't like this change, I can revert it
and instead (later) make gyp's input.py error out on completely empty bundles.

I do want all generators to have the same behavior, but I don't care much how
that's done.

     return output, output_binary, compile_depends
 
   def WriteActionsRulesCopies(self, spec, extra_sources, prebuild,
@@ -572,7 +572,7 @@
       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, final_deps,
+  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
@@ -582,13 +582,6 @@
         return final_deps[0], final_deps[0]
       # Otherwise, fall through to writing out a stamp file.
 
-    if self.is_mac_bundle:
-      output = self.ComputeMacBundleOutput(spec)
-      output_binary = self.ComputeMacBundleBinaryOutput(spec)
-      mac_bundle_depends.append(output_binary)
-    else:
-      output = output_binary = self.ComputeOutput(spec)
-
     output_uses_linker = spec['type'] in ('executable', 'loadable_module',
                                           'shared_library')
 
@@ -611,7 +604,17 @@
             # every build, but we don't want to rebuild when it runs.
             if 'lastchange' not in input:
               implicit_deps.add(input)
-        final_deps.extend(list(extra_deps))
+        link_deps.extend(list(extra_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',
@@ -621,6 +624,13 @@
     }
     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,
@@ -669,8 +679,10 @@
     path = self.ExpandSpecial(generator_default_variables['PRODUCT_DIR'])
     return os.path.join(path, self.xcode_settings.GetExecutablePath())
 
-  def ComputeOutputFileName(self, spec):
+  def ComputeOutputFileName(self, spec, type=None):
     """Compute the filename of the final output for the current target."""
+    if not type:
+      type = spec['type']
 
     # Compute filename prefix: the product prefix, or a default for
     # the product type.
@@ -679,7 +691,7 @@
       'shared_library': 'lib',
       'static_library': 'lib',
       }
-    prefix = spec.get('product_prefix', DEFAULT_PREFIX.get(spec['type'], ''))
+    prefix = spec.get('product_prefix', DEFAULT_PREFIX.get(type, ''))
 
     # Compute filename extension: the product extension, or a default
     # for the product type.
@@ -689,7 +701,7 @@
       'shared_library': 'so',
       }
     extension = spec.get('product_extension',
-                         DEFAULT_EXTENSION.get(spec['type'], ''))
+                         DEFAULT_EXTENSION.get(type, ''))
     if extension:
       extension = '.' + extension
 
@@ -703,24 +715,26 @@
         # Snip out an extra 'lib' from libs if appropriate.
         target = StripPrefix(target, 'lib')
 
-    if spec['type'] in ('static_library', 'loadable_module', 'shared_library',
+    if type in ('static_library', 'loadable_module', 'shared_library',
                         'executable'):
       return '%s%s%s' % (prefix, target, extension)
-    elif spec['type'] == 'none':
+    elif type == 'none':
       return '%s.stamp' % target
     else:
-      raise 'Unhandled output type', spec['type']
+      raise 'Unhandled output type', type
 
-  def ComputeOutput(self, spec):
+  def ComputeOutput(self, spec, type=None):
     """Compute the path for the final output of the spec."""
+    assert not self.is_mac_bundle or type
 
-    assert not self.is_mac_bundle
+    if not type:
+      type = spec['type']
 
-    if self.flavor == 'mac' and spec['type'] in (
+    if self.flavor == 'mac' and type in (
         'static_library', 'executable', 'shared_library', 'loadable_module'):
       filename = self.xcode_settings.GetExecutablePath()
     else:
-      filename = self.ComputeOutputFileName(spec)
+      filename = self.ComputeOutputFileName(spec, type)
 
     if 'product_dir' in spec:
       path = os.path.join(spec['product_dir'], filename)
@@ -732,9 +746,9 @@
     if self.flavor == 'mac' and self.toolset == 'target':
       type_in_output_root += ['shared_library', 'static_library']
 
-    if spec['type'] in type_in_output_root:
+    if type in type_in_output_root:
       return filename
-    elif spec['type'] == 'shared_library':
+    elif type == 'shared_library':
       libdir = 'lib'
       if self.toolset != 'target':
         libdir = 'lib/%s' % self.toolset