Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(42)

Unified Diff: tools/gn/ninja_build_writer.cc

Issue 1123283003: GN: Don't generate phony rules called "all". (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « tools/gn/ninja_build_writer.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/gn/ninja_build_writer.cc
diff --git a/tools/gn/ninja_build_writer.cc b/tools/gn/ninja_build_writer.cc
index 248a45a2a674f1b58bf340790639ad18e896179c..591947d38cae9ca29918c11fc7697625fd56a229 100644
--- a/tools/gn/ninja_build_writer.cc
+++ b/tools/gn/ninja_build_writer.cc
@@ -186,6 +186,12 @@ void NinjaBuildWriter::WriteSubninjas() {
bool NinjaBuildWriter::WritePhonyAndAllRules(Err* err) {
std::string all_rules;
+ // Track rules as we generate them so we don't accidentally write a phony
+ // rule that collides with something else.
+ // GN internally generates an "all" target, so don't duplicate it.
+ std::set<std::string> written_rules;
+ written_rules.insert("all");
+
// Write phony rules for all uniquely-named targets in the default toolchain.
// Don't do other toolchains or we'll get naming conflicts, and if the name
// isn't unique, also skip it. The exception is for the toplevel targets
@@ -214,6 +220,12 @@ bool NinjaBuildWriter::WritePhonyAndAllRules(Err* err) {
// even if there are non-executable targets with the same name.
if (target->output_type() == Target::EXECUTABLE)
exe_count[label.name()]++;
+
+ // Add the files to the list of generated targets so we don't write phony
+ // rules that collide.
+ std::string target_file(target->dependency_output_file().value());
+ NormalizePath(&target_file);
+ written_rules.insert(target_file);
}
for (const auto& target : default_toolchain_targets_) {
@@ -229,7 +241,7 @@ bool NinjaBuildWriter::WritePhonyAndAllRules(Err* err) {
// Write the long name "foo/bar:baz" for the target "//foo/bar:baz".
std::string long_name = label.GetUserVisibleName(false);
base::TrimString(long_name, "/", &long_name);
- WritePhonyRule(target, target_file, long_name);
+ WritePhonyRule(target, target_file, long_name, &written_rules);
// Write the directory name with no target name if they match
// (e.g. "//foo/bar:bar" -> "foo/bar").
@@ -239,7 +251,7 @@ bool NinjaBuildWriter::WritePhonyAndAllRules(Err* err) {
// That may have generated a name the same as the short name of the
// target which we already wrote.
if (medium_name != label.name())
- WritePhonyRule(target, target_file, medium_name);
+ WritePhonyRule(target, target_file, medium_name, &written_rules);
}
// Write short names for ones which are either completely unique or there
@@ -247,7 +259,7 @@ bool NinjaBuildWriter::WritePhonyAndAllRules(Err* err) {
if (small_name_count[label.name()] == 1 ||
(target->output_type() == Target::EXECUTABLE &&
exe_count[label.name()] == 1)) {
- WritePhonyRule(target, target_file, label.name());
+ WritePhonyRule(target, target_file, label.name(), &written_rules);
}
if (!all_rules.empty())
@@ -260,7 +272,7 @@ bool NinjaBuildWriter::WritePhonyAndAllRules(Err* err) {
for (const auto& toplevel_target : toplevel_targets) {
if (small_name_count[toplevel_target->label().name()] > 1) {
WritePhonyRule(toplevel_target, toplevel_target->dependency_output_file(),
- toplevel_target->label().name());
+ toplevel_target->label().name(), &written_rules);
}
}
@@ -289,10 +301,15 @@ bool NinjaBuildWriter::WritePhonyAndAllRules(Err* err) {
void NinjaBuildWriter::WritePhonyRule(const Target* target,
const OutputFile& target_file,
- const std::string& phony_name) {
+ const std::string& phony_name,
+ std::set<std::string>* written_rules) {
if (target_file.value() == phony_name)
return; // No need for a phony rule.
+ if (written_rules->find(phony_name) != written_rules->end())
+ return; // Already exists.
+ written_rules->insert(phony_name);
+
EscapeOptions ninja_escape;
ninja_escape.mode = ESCAPE_NINJA;
« no previous file with comments | « tools/gn/ninja_build_writer.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698