summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/build_test.cc13
-rw-r--r--src/graph.cc15
-rw-r--r--src/graph.h1
-rw-r--r--src/graph_test.cc12
-rw-r--r--src/manifest_parser.cc19
-rw-r--r--src/manifest_parser.h10
-rw-r--r--src/manifest_parser_test.cc26
-rw-r--r--src/ninja.cc18
8 files changed, 111 insertions, 3 deletions
diff --git a/src/build_test.cc b/src/build_test.cc
index a0f898f..46ab33e 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -1068,6 +1068,19 @@ TEST_F(BuildTest, PhonyNoWork) {
EXPECT_TRUE(builder_.AlreadyUpToDate());
}
+// Test a self-referencing phony. Ideally this should not work, but
+// ninja 1.7 and below tolerated and CMake 2.8.12.x and 3.0.x both
+// incorrectly produce it. We tolerate it for compatibility.
+TEST_F(BuildTest, PhonySelfReference) {
+ string err;
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"build a: phony a\n"));
+
+ EXPECT_TRUE(builder_.AddTarget("a", &err));
+ ASSERT_EQ("", err);
+ EXPECT_TRUE(builder_.AlreadyUpToDate());
+}
+
TEST_F(BuildTest, Fail) {
ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
"rule fail\n"
diff --git a/src/graph.cc b/src/graph.cc
index 7dd9491..ce4ea77 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -170,6 +170,13 @@ bool DependencyScan::VerifyDAG(Node* node, vector<Node*>* stack, string* err) {
err->append(" -> ");
}
err->append((*start)->path());
+
+ if ((start + 1) == stack->end() && edge->maybe_phonycycle_diagnostic()) {
+ // The manifest parser would have filtered out the self-referencing
+ // input if it were not configured to allow the error.
+ err->append(" [-w phonycycle=err]");
+ }
+
return false;
}
@@ -410,6 +417,14 @@ bool Edge::use_console() const {
return pool() == &State::kConsolePool;
}
+bool Edge::maybe_phonycycle_diagnostic() const {
+ // CMake 2.8.12.x and 3.0.x produced self-referencing phony rules
+ // of the form "build a: phony ... a ...". Restrict our
+ // "phonycycle" diagnostic option to the form it used.
+ return is_phony() && outputs_.size() == 1 && implicit_outs_ == 0 &&
+ implicit_deps_ == 0;
+}
+
// static
string Node::PathDecanonicalized(const string& path, uint64_t slash_bits) {
string result = path;
diff --git a/src/graph.h b/src/graph.h
index 586c588..a8f0641 100644
--- a/src/graph.h
+++ b/src/graph.h
@@ -201,6 +201,7 @@ struct Edge {
bool is_phony() const;
bool use_console() const;
+ bool maybe_phonycycle_diagnostic() const;
};
diff --git a/src/graph_test.cc b/src/graph_test.cc
index d4d2824..422bc9a 100644
--- a/src/graph_test.cc
+++ b/src/graph_test.cc
@@ -323,6 +323,18 @@ TEST_F(GraphTest, NestedPhonyPrintsDone) {
ASSERT_FALSE(plan_.more_to_do());
}
+TEST_F(GraphTest, PhonySelfReferenceError) {
+ ManifestParserOptions parser_opts;
+ parser_opts.phony_cycle_action_ = kPhonyCycleActionError;
+ AssertParse(&state_,
+"build a: phony a\n",
+ parser_opts);
+
+ string err;
+ EXPECT_FALSE(scan_.RecomputeDirty(GetNode("a"), &err));
+ ASSERT_EQ("dependency cycle: a -> a [-w phonycycle=err]", err);
+}
+
TEST_F(GraphTest, DependencyCycle) {
AssertParse(&state_,
"build out: cat mid\n"
diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index b4d7fe7..27c423b 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -383,6 +383,25 @@ bool ManifestParser::ParseEdge(string* err) {
edge->implicit_deps_ = implicit;
edge->order_only_deps_ = order_only;
+ if (options_.phony_cycle_action_ == kPhonyCycleActionWarn &&
+ edge->maybe_phonycycle_diagnostic()) {
+ // CMake 2.8.12.x and 3.0.x incorrectly write phony build statements
+ // that reference themselves. Ninja used to tolerate these in the
+ // build graph but that has since been fixed. Filter them out to
+ // support users of those old CMake versions.
+ Node* out = edge->outputs_[0];
+ vector<Node*>::iterator new_end =
+ remove(edge->inputs_.begin(), edge->inputs_.end(), out);
+ if (new_end != edge->inputs_.end()) {
+ edge->inputs_.erase(new_end, edge->inputs_.end());
+ if (!quiet_) {
+ Warning("phony target '%s' names itself as an input; "
+ "ignoring [-w phonycycle=warn]",
+ out->path().c_str());
+ }
+ }
+ }
+
// Multiple outputs aren't (yet?) supported with depslog.
string deps_type = edge->GetBinding("deps");
if (!deps_type.empty() && edge->outputs_.size() > 1) {
diff --git a/src/manifest_parser.h b/src/manifest_parser.h
index 4ae21c4..2136018 100644
--- a/src/manifest_parser.h
+++ b/src/manifest_parser.h
@@ -31,9 +31,17 @@ enum DupeEdgeAction {
kDupeEdgeActionError,
};
+enum PhonyCycleAction {
+ kPhonyCycleActionWarn,
+ kPhonyCycleActionError,
+};
+
struct ManifestParserOptions {
- ManifestParserOptions(): dupe_edge_action_(kDupeEdgeActionWarn) {}
+ ManifestParserOptions()
+ : dupe_edge_action_(kDupeEdgeActionWarn),
+ phony_cycle_action_(kPhonyCycleActionWarn) {}
DupeEdgeAction dupe_edge_action_;
+ PhonyCycleAction phony_cycle_action_;
};
/// Parses .ninja files.
diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc
index 756efdf..39ed810 100644
--- a/src/manifest_parser_test.cc
+++ b/src/manifest_parser_test.cc
@@ -384,6 +384,32 @@ TEST_F(ParserTest, DuplicateEdgeInIncludedFile) {
err);
}
+TEST_F(ParserTest, PhonySelfReferenceIgnored) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(
+"build a: phony a\n"
+));
+
+ Node* node = state.LookupNode("a");
+ Edge* edge = node->in_edge();
+ ASSERT_TRUE(edge->inputs_.empty());
+}
+
+TEST_F(ParserTest, PhonySelfReferenceKept) {
+ const char kInput[] =
+"build a: phony a\n";
+ ManifestParserOptions parser_opts;
+ parser_opts.phony_cycle_action_ = kPhonyCycleActionError;
+ ManifestParser parser(&state, &fs_, parser_opts);
+ string err;
+ EXPECT_TRUE(parser.ParseTest(kInput, &err));
+ EXPECT_EQ("", err);
+
+ Node* node = state.LookupNode("a");
+ Edge* edge = node->in_edge();
+ ASSERT_EQ(edge->inputs_.size(), 1);
+ ASSERT_EQ(edge->inputs_[0], node);
+}
+
TEST_F(ParserTest, ReservedWords) {
ASSERT_NO_FATAL_FAILURE(AssertParse(
"rule build\n"
diff --git a/src/ninja.cc b/src/ninja.cc
index 586e8dc..ed004ac 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -70,6 +70,9 @@ struct Options {
/// Whether duplicate rules for one target should warn or print an error.
bool dupe_edges_should_err;
+
+ /// Whether phony cycles should warn or print an error.
+ bool phony_cycle_should_err;
};
/// The Ninja main() loads up a series of data structures; various tools need
@@ -845,7 +848,8 @@ bool DebugEnable(const string& name) {
bool WarningEnable(const string& name, Options* options) {
if (name == "list") {
printf("warning flags:\n"
-" dupbuild={err,warn} multiple build lines for one target\n");
+" dupbuild={err,warn} multiple build lines for one target\n"
+" phonycycle={err,warn} phony build statement references itself\n");
return false;
} else if (name == "dupbuild=err") {
options->dupe_edges_should_err = true;
@@ -853,9 +857,16 @@ bool WarningEnable(const string& name, Options* options) {
} else if (name == "dupbuild=warn") {
options->dupe_edges_should_err = false;
return true;
+ } else if (name == "phonycycle=err") {
+ options->phony_cycle_should_err = true;
+ return true;
+ } else if (name == "phonycycle=warn") {
+ options->phony_cycle_should_err = false;
+ return true;
} else {
const char* suggestion =
- SpellcheckString(name.c_str(), "dupbuild=err", "dupbuild=warn", NULL);
+ SpellcheckString(name.c_str(), "dupbuild=err", "dupbuild=warn",
+ "phonycycle=err", "phonycycle=warn", NULL);
if (suggestion) {
Error("unknown warning flag '%s', did you mean '%s'?",
name.c_str(), suggestion);
@@ -1148,6 +1159,9 @@ int real_main(int argc, char** argv) {
if (options.dupe_edges_should_err) {
parser_opts.dupe_edge_action_ = kDupeEdgeActionError;
}
+ if (options.phony_cycle_should_err) {
+ parser_opts.phony_cycle_action_ = kPhonyCycleActionError;
+ }
ManifestParser parser(&ninja.state_, &ninja.disk_interface_, parser_opts);
string err;
if (!parser.Load(options.input_file, &err)) {
generated by cgit v0.12 at 2024-07-05 05:29:50 (GMT)