Changes ninja generator to only output empty names if not already output
This builds on https://codereview.chromium.org/543743003/ .
BUG=410410
TEST=covered by test now
R=thakis@chromium.org
Review URL: https://codereview.chromium.org/547393003
git-svn-id: http://gyp.googlecode.com/svn/trunk@1974 78cadc50-ecff-11dd-a971-7dbc132099af
diff --git a/pylib/gyp/generator/ninja.py b/pylib/gyp/generator/ninja.py
index f5e1be9..4484f93 100644
--- a/pylib/gyp/generator/ninja.py
+++ b/pylib/gyp/generator/ninja.py
@@ -2152,7 +2152,12 @@
# short name of targets that were skipped because they didn't contain anything
# interesting.
- empty_target_names = []
+ # NOTE: there may be overlap between this an non_empty_target_names.
+ empty_target_names = set()
+
+ # Set of non-empty short target names.
+ # NOTE: there may be overlap between this an empty_target_names.
+ non_empty_target_names = set()
for qualified_target in target_list:
# qualified_target is like: third_party/icu/icu.gyp:icui18n#target
@@ -2197,8 +2202,9 @@
target_outputs[qualified_target] = target
if qualified_target in all_targets:
all_outputs.add(target.FinalOutput())
+ non_empty_target_names.add(name)
else:
- empty_target_names.append(name)
+ empty_target_names.add(name)
if target_short_names:
# Write a short name to build this target. This benefits both the
@@ -2210,10 +2216,11 @@
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 = empty_target_names - non_empty_target_names
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):
diff --git a/test/ninja/empty-and-non-empty-duplicate-name/gyptest-empty-and-non-empty-duplicate-name.py b/test/ninja/empty-and-non-empty-duplicate-name/gyptest-empty-and-non-empty-duplicate-name.py
new file mode 100644
index 0000000..1279ead
--- /dev/null
+++ b/test/ninja/empty-and-non-empty-duplicate-name/gyptest-empty-and-non-empty-duplicate-name.py
@@ -0,0 +1,22 @@
+#!/usr/bin/env python
+
+# Copyright (c) 2014 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.
+
+"""
+Verifies a phony target isn't output if a target exists with the same name that
+was output.
+"""
+
+import TestGyp
+
+test = TestGyp.TestGyp(formats=['ninja'])
+
+test.run_gyp('test.gyp')
+
+# Check for both \r and \n to cover both windows and linux.
+test.must_not_contain('out/Default/build.ninja', 'build empty_target: phony\r')
+test.must_not_contain('out/Default/build.ninja', 'build empty_target: phony\n')
+
+test.pass_test()
diff --git a/test/ninja/empty-and-non-empty-duplicate-name/subdir/included.gyp b/test/ninja/empty-and-non-empty-duplicate-name/subdir/included.gyp
new file mode 100644
index 0000000..1b9fc42
--- /dev/null
+++ b/test/ninja/empty-and-non-empty-duplicate-name/subdir/included.gyp
@@ -0,0 +1,19 @@
+# Copyright (c) 2014 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.
+
+{
+ 'targets': [
+ {
+ 'target_name': 'empty_target',
+ 'type': 'executable',
+ 'sources': [
+ 'test.cc',
+ ],
+ },
+ {
+ 'target_name': 'included_empty_target',
+ 'type': 'none',
+ },
+ ],
+}
diff --git a/test/ninja/empty-and-non-empty-duplicate-name/test.gyp b/test/ninja/empty-and-non-empty-duplicate-name/test.gyp
new file mode 100644
index 0000000..9aa6287
--- /dev/null
+++ b/test/ninja/empty-and-non-empty-duplicate-name/test.gyp
@@ -0,0 +1,19 @@
+# Copyright (c) 2014 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.
+
+{
+ 'targets': [
+ {
+ 'target_name': 'All',
+ 'type': 'none',
+ 'dependencies': [
+ 'subdir/included.gyp:included_empty_target'
+ ]
+ },
+ {
+ 'target_name': 'empty_target',
+ 'type': 'none',
+ },
+ ],
+}
