Changes ninja generator to only output empty names if not already output

pylib/gyp/generator/ninja.py

 # Copyright (c) 2013 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 collections
 import copy
 import hashlib
 import json
 import multiprocessing
 import os.path
@@ skipping 1677 matching lines @@
               (sys.executable, use_separate_mspdbsrv))
   exe_cmd = FullLinkCommand(exe_cmd, '$binary', 'exe')
   master_ninja.rule('link' + rule_name_suffix,
                     description='LINK%s $binary' % rule_name_suffix.upper(),
                     command=exe_cmd,
                     rspfile='$binary.rsp',
                     rspfile_content='$in_newline $libs $ldflags',
                     pool='link_pool')
 
 
+class VisitedTarget:
+  """Used to track the set of qualified targets with the same short name as
+  well as if the target is empty."""
+  def __init__(self):
+    # True if the only targets with this name are empty (not interesting).
+    self.has_empty_target = True
+    # List of qualified targets with the same name.
+    self.qualified_targets = []
+
+
 def GenerateOutputForConfig(target_list, target_dicts, data, params,
                             config_name):
   options = params['options']
   flavor = gyp.common.GetFlavor(params)
   generator_flags = params.get('generator_flags', {})
 
   # build_dir: relative path from source root to our output files.
   # e.g. "out/Debug"
   build_dir = os.path.normpath(
       os.path.join(ComputeOutputDir(params), config_name))
@@ skipping 435 matching lines @@
                                         os.path.normpath(build_file)):
       all_targets.add(target)
   all_outputs = set()
 
   # target_outputs is a map from qualified target name to a Target object.
   target_outputs = {}
   # target_short_names is a map from target short name to a list of Target
   # objects.
   target_short_names = {}
 
-  # short name of targets that were skipped because they didn't contain anything
-  # interesting.
-  empty_target_names = []
+  # Maps from short name to a VisitedTarget.
+  name_to_visited = {}
 
   for qualified_target in target_list:
     # qualified_target is like: third_party/icu/icu.gyp:icui18n#target
     build_file, name, toolset = \
         gyp.common.ParseQualifiedTarget(qualified_target)
 
     this_make_global_settings = data[build_file].get('make_global_settings', [])
     assert make_global_settings == this_make_global_settings, (
         "make_global_settings needs to be the same for all targets. %s vs. %s" %
         (this_make_global_settings, make_global_settings))
@@ skipping 24 matching lines @@
         ninja_file.write(ninja_output.getvalue())
       ninja_output.close()
       master_ninja.subninja(output_file)
 
     if target:
       if name != target.FinalOutput() and spec['toolset'] == 'target':
         target_short_names.setdefault(name, []).append(target)
       target_outputs[qualified_target] = target
       if qualified_target in all_targets:
         all_outputs.add(target.FinalOutput())
-    else:
-      empty_target_names.append(name)
+
+    if name not in name_to_visited:
+      name_to_visited[name] = VisitedTarget()
+      name_to_visited[name].has_empty_target = (target == None)

[Nico, 2014/09/08 21:29:02]

nit: `target is None`

+    elif target != None:

[Nico, 2014/09/08 21:29:02]

nit: `target is not None`

+      name_to_visited[name].has_empty_target = False
+    name_to_visited[name].qualified_targets.append(qualified_target)
 
   if target_short_names:
     # 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.
     master_ninja.newline()
     master_ninja.comment('Short names for targets.')
     for short_name in target_short_names:
       master_ninja.build(short_name, 'phony', [x.FinalOutput() for x in
                                                target_short_names[short_name]])
 
+  # Write phony targets for any empty targets that weren't written yet. As
+  # short names are  not necessarily unique only do this for short names that
+  # haven't already been output for another target.
+  empty_target_names = [x for x in name_to_visited
+                          if name_to_visited[x].has_empty_target]
   if empty_target_names:
-    # Write out any targets that were skipped because they didn't contain
-    # anything interesting. This way the targets can still be built without
-    # causing build errors.
     master_ninja.newline()
     master_ninja.comment('Empty targets (output for completeness).')
     for name in sorted(empty_target_names):
       master_ninja.build(name, 'phony')
 
   if all_outputs:
     master_ninja.newline()
     master_ninja.build('all', 'phony', list(all_outputs))
     master_ninja.default(generator_flags.get('default_target', 'all'))
 
   master_ninja_file.close()
 
+  for short_name in \
+        [x for x in name_to_visited
+           if len(name_to_visited[x].qualified_targets) > 1]:
+    print 'WARNING: %s is defined in multiple places: %s' % \
+        (short_name, name_to_visited[short_name].qualified_targets)

[Nico, 2014/09/08 21:29:02]

That just moves the warning from build time to gyp time; this would still add
warnings to the chromium build.

gyp's semantics so far have been that it's ok to have multiple empty targets
with the same name from what I can tell, so I'm not sure if the ninja generator
should warn about this. (If we think that gyp should warn about this, input.py
should probably do it.)

Without this, you'd need less bookkeeping above too.

[sky, 2014/09/08 21:50:21]

Done.

+
 
 def PerformBuild(data, configurations, params):
   options = params['options']
   for config in configurations:
     builddir = os.path.join(options.toplevel_dir, 'out', config)
     arguments = ['ninja', '-C', builddir]
     print 'Building [%s]: %s' % (config, arguments)
     subprocess.check_call(arguments)
 
 
@@ skipping 30 matching lines @@
           arglists.append(
               (target_list, target_dicts, data, params, config_name))
         pool.map(CallGenerateOutputForConfig, arglists)
       except KeyboardInterrupt, e:
         pool.terminate()
         raise e
     else:
       for config_name in config_names:
         GenerateOutputForConfig(target_list, target_dicts, data, params,
                                 config_name)