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())