No GN phony rules when it matches outputs.
Skip generating a short name phony rule for a target when the short name
matches the target name. It's reasonable that an action named "foo" would
generate a file called foo in the root build directory.
R=agrieve@chromium.org
Review URL: https://codereview.chromium.org/1506343003
Cr-Original-Commit-Position: refs/heads/master@{#364395}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 936b61a117e5a35ccb996f586f5ac5f0c698ce61
diff --git a/tools/gn/ninja_build_writer.cc b/tools/gn/ninja_build_writer.cc
index 50638fb..07d42e1 100644
--- a/tools/gn/ninja_build_writer.cc
+++ b/tools/gn/ninja_build_writer.cc
@@ -85,6 +85,27 @@
return result;
}
+bool HasOutputIdenticalToLabel(const Target* target,
+ const std::string& short_name) {
+ if (target->output_type() != Target::ACTION &&
+ target->output_type() != Target::ACTION_FOREACH)
+ return false;
+
+ // Rather than convert all outputs to be relative to the build directory
+ // and then compare to the short name, convert the short name to look like a
+ // file in the output directory since this is only one conversion.
+ SourceFile short_name_as_source_file(
+ target->settings()->build_settings()->build_dir().value() + short_name);
+
+ std::vector<SourceFile> outputs_as_source;
+ target->action_values().GetOutputsAsSourceFiles(target, &outputs_as_source);
+ for (const SourceFile& output_as_source : outputs_as_source) {
+ if (output_as_source == short_name_as_source_file)
+ return true;
+ }
+ return false;
+}
+
// Given an output that appears more than once, generates an error message
// that describes the problem and which targets generate it.
Err GetDuplicateOutputError(const std::vector<const Target*>& all_targets,
@@ -294,7 +315,24 @@
if (small_name_count[label.name()] == 1 ||
(target->output_type() == Target::EXECUTABLE &&
exe_count[label.name()] == 1)) {
- WritePhonyRule(target, target_file, label.name(), &written_rules);
+ // It's reasonable to generate output files in the root build directory
+ // with the same name as the target. Don't generate phony rules for
+ // these cases.
+ //
+ // All of this does not do the general checking of all target's outputs
+ // which may theoretically collide. But it's not very reasonable for
+ // a script target named "foo" to generate a file named "bar" with no
+ // extension in the root build directory while another target is named
+ // "bar". If this does occur, the user is likely to be confused when
+ // building "bar" that is builds foo anyway, so you probably just
+ // shouldn't do that.
+ //
+ // We should fix this however, and build up all generated script outputs
+ // and check everything against that. There are other edge cases that the
+ // current phony rule generator doesn't check. We may need to make a big
+ // set of every possible generated file in the build for this purpose.
+ if (!HasOutputIdenticalToLabel(target, label.name()))
+ WritePhonyRule(target, target_file, label.name(), &written_rules);
}
if (!all_rules.empty())
