diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index d54e097a5b..17d607a325 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -1,6 +1,6 @@ The Ninja build system ====================== -v1.8.1, Sep 2017 +v1.8.2, Sep 2017 Introduction diff --git a/src/build_log_perftest.cc b/src/build_log_perftest.cc index b4efb1d29e..e471d138cc 100644 --- a/src/build_log_perftest.cc +++ b/src/build_log_perftest.cc @@ -71,7 +71,7 @@ bool WriteTestData(string* err) { long_rule_command += "$in -o $out\n"; State state; - ManifestParser parser(&state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&state, NULL); if (!parser.ParseTest("rule cxx\n command = " + long_rule_command, err)) return false; diff --git a/src/build_test.cc b/src/build_test.cc index a0f898f2ef..46ab33ef86 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 7dd9491e00..ce4ea774f2 100644 --- a/src/graph.cc +++ b/src/graph.cc @@ -170,6 +170,13 @@ bool DependencyScan::VerifyDAG(Node* node, vector* 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 586c588790..a8f0641d5f 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 d4d282499d..422bc9a053 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 2164921d88..27c423bbb7 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -26,9 +26,9 @@ #include "version.h" ManifestParser::ManifestParser(State* state, FileReader* file_reader, - DupeEdgeAction dupe_edge_action) + ManifestParserOptions options) : state_(state), file_reader_(file_reader), - dupe_edge_action_(dupe_edge_action), quiet_(false) { + options_(options), quiet_(false) { env_ = &state->bindings_; } @@ -346,7 +346,7 @@ bool ManifestParser::ParseEdge(string* err) { if (!CanonicalizePath(&path, &slash_bits, &path_err)) return lexer_.Error(path_err, err); if (!state_->AddOut(edge, path, slash_bits)) { - if (dupe_edge_action_ == kDupeEdgeActionError) { + if (options_.dupe_edge_action_ == kDupeEdgeActionError) { lexer_.Error("multiple rules generate " + path + " [-w dupbuild=err]", err); return false; @@ -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::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) { @@ -400,7 +419,7 @@ bool ManifestParser::ParseFileInclude(bool new_scope, string* err) { return false; string path = eval.Evaluate(env_); - ManifestParser subparser(state_, file_reader_, dupe_edge_action_); + ManifestParser subparser(state_, file_reader_, options_); if (new_scope) { subparser.env_ = new BindingEnv(env_); } else { diff --git a/src/manifest_parser.h b/src/manifest_parser.h index 043e4b2a65..2136018a99 100644 --- a/src/manifest_parser.h +++ b/src/manifest_parser.h @@ -31,10 +31,23 @@ enum DupeEdgeAction { kDupeEdgeActionError, }; +enum PhonyCycleAction { + kPhonyCycleActionWarn, + kPhonyCycleActionError, +}; + +struct ManifestParserOptions { + ManifestParserOptions() + : dupe_edge_action_(kDupeEdgeActionWarn), + phony_cycle_action_(kPhonyCycleActionWarn) {} + DupeEdgeAction dupe_edge_action_; + PhonyCycleAction phony_cycle_action_; +}; + /// Parses .ninja files. struct ManifestParser { ManifestParser(State* state, FileReader* file_reader, - DupeEdgeAction dupe_edge_action); + ManifestParserOptions options = ManifestParserOptions()); /// Load and parse a file. bool Load(const string& filename, string* err, Lexer* parent = NULL); @@ -67,7 +80,7 @@ struct ManifestParser { BindingEnv* env_; FileReader* file_reader_; Lexer lexer_; - DupeEdgeAction dupe_edge_action_; + ManifestParserOptions options_; bool quiet_; }; diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc index 60c205441c..67d11f9166 100644 --- a/src/manifest_parser_perftest.cc +++ b/src/manifest_parser_perftest.cc @@ -56,7 +56,7 @@ int LoadManifests(bool measure_command_evaluation) { string err; RealDiskInterface disk_interface; State state; - ManifestParser parser(&state, &disk_interface, kDupeEdgeActionWarn); + ManifestParser parser(&state, &disk_interface); if (!parser.Load("build.ninja", &err)) { fprintf(stderr, "Failed to read test data: %s\n", err.c_str()); exit(1); diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 3c82dc58cf..39ed810658 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -23,7 +23,7 @@ struct ParserTest : public testing::Test { void AssertParse(const char* input) { - ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); + ManifestParser parser(&state, &fs_); string err; EXPECT_TRUE(parser.ParseTest(input, &err)); ASSERT_EQ("", err); @@ -358,7 +358,9 @@ TEST_F(ParserTest, DuplicateEdgeWithMultipleOutputsError) { "build out1 out2: cat in1\n" "build out1: cat in2\n" "build final: cat out1\n"; - ManifestParser parser(&state, &fs_, kDupeEdgeActionError); + ManifestParserOptions parser_opts; + parser_opts.dupe_edge_action_ = kDupeEdgeActionError; + ManifestParser parser(&state, &fs_, parser_opts); string err; EXPECT_FALSE(parser.ParseTest(kInput, &err)); EXPECT_EQ("input:5: multiple rules generate out1 [-w dupbuild=err]\n", err); @@ -373,13 +375,41 @@ TEST_F(ParserTest, DuplicateEdgeInIncludedFile) { "build final: cat out1\n"); const char kInput[] = "subninja sub.ninja\n"; - ManifestParser parser(&state, &fs_, kDupeEdgeActionError); + ManifestParserOptions parser_opts; + parser_opts.dupe_edge_action_ = kDupeEdgeActionError; + ManifestParser parser(&state, &fs_, parser_opts); string err; EXPECT_FALSE(parser.ParseTest(kInput, &err)); EXPECT_EQ("sub.ninja:5: multiple rules generate out1 [-w dupbuild=err]\n", 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" @@ -391,7 +421,7 @@ TEST_F(ParserTest, ReservedWords) { TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest(string("subn", 4), &err)); EXPECT_EQ("input:1: expected '=', got eof\n" @@ -402,7 +432,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("foobar", &err)); EXPECT_EQ("input:1: expected '=', got eof\n" @@ -413,7 +443,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("x 3", &err)); EXPECT_EQ("input:1: expected '=', got identifier\n" @@ -424,7 +454,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("x = 3", &err)); EXPECT_EQ("input:1: unexpected EOF\n" @@ -435,7 +465,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("x = 3\ny 2", &err)); EXPECT_EQ("input:2: expected '=', got identifier\n" @@ -446,7 +476,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("x = $", &err)); EXPECT_EQ("input:1: bad $-escape (literal $ must be written as $$)\n" @@ -457,7 +487,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("x = $\n $[\n", &err)); EXPECT_EQ("input:2: bad $-escape (literal $ must be written as $$)\n" @@ -468,7 +498,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("x = a$\n b$\n $\n", &err)); EXPECT_EQ("input:4: unexpected EOF\n" @@ -477,7 +507,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("build\n", &err)); EXPECT_EQ("input:1: expected path\n" @@ -488,7 +518,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("build x: y z\n", &err)); EXPECT_EQ("input:1: unknown build rule 'y'\n" @@ -499,7 +529,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("build x:: y z\n", &err)); EXPECT_EQ("input:1: expected build command name\n" @@ -510,7 +540,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n command = cat ok\n" "build x: cat $\n :\n", @@ -523,7 +553,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n", &err)); @@ -532,7 +562,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = echo\n" @@ -546,7 +576,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = echo\n" @@ -558,7 +588,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = ${fafsd\n" @@ -573,7 +603,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = cat\n" @@ -588,7 +618,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cat\n" " command = cat\n" @@ -602,7 +632,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule %foo\n", &err)); @@ -611,7 +641,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n" " command = foo\n" @@ -625,7 +655,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n" "build $.: cc bar.cc\n", @@ -638,7 +668,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n && bar", &err)); @@ -647,7 +677,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n" "build $: cc bar.cc\n", @@ -660,7 +690,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("default\n", &err)); @@ -672,7 +702,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("default nonexistent\n", &err)); @@ -684,7 +714,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule r\n command = r\n" "build b: r\n" @@ -698,7 +728,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("default $a\n", &err)); EXPECT_EQ("input:1: empty path\n" @@ -709,7 +739,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule r\n" " command = r\n" @@ -721,7 +751,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; // the indented blank line must terminate the rule // this also verifies that "unexpected (token)" errors are correct @@ -734,7 +764,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("pool\n", &err)); EXPECT_EQ("input:1: expected pool name\n", err); @@ -742,7 +772,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("pool foo\n", &err)); EXPECT_EQ("input:2: expected 'depth =' line\n", err); @@ -750,7 +780,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("pool foo\n" " depth = 4\n" @@ -763,7 +793,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("pool foo\n" " depth = -1\n", &err)); @@ -775,7 +805,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("pool foo\n" " bar = 1\n", &err)); @@ -787,7 +817,7 @@ TEST_F(ParserTest, Errors) { { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; // Pool names are dereferenced at edge parsing time. EXPECT_FALSE(parser.ParseTest("rule run\n" @@ -800,7 +830,7 @@ TEST_F(ParserTest, Errors) { TEST_F(ParserTest, MissingInput) { State local_state; - ManifestParser parser(&local_state, &fs_, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, &fs_); string err; EXPECT_FALSE(parser.Load("build.ninja", &err)); EXPECT_EQ("loading 'build.ninja': No such file or directory", err); @@ -808,7 +838,7 @@ TEST_F(ParserTest, MissingInput) { TEST_F(ParserTest, MultipleOutputs) { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_TRUE(parser.ParseTest("rule cc\n command = foo\n depfile = bar\n" "build a.o b.o: cc c.cc\n", @@ -818,7 +848,7 @@ TEST_F(ParserTest, MultipleOutputs) { TEST_F(ParserTest, MultipleOutputsWithDeps) { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_FALSE(parser.ParseTest("rule cc\n command = foo\n deps = gcc\n" "build a.o b.o: cc c.cc\n", @@ -853,7 +883,7 @@ TEST_F(ParserTest, SubNinja) { } TEST_F(ParserTest, MissingSubNinja) { - ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); + ManifestParser parser(&state, &fs_); string err; EXPECT_FALSE(parser.ParseTest("subninja foo.ninja\n", &err)); EXPECT_EQ("input:1: loading 'foo.ninja': No such file or directory\n" @@ -866,7 +896,7 @@ TEST_F(ParserTest, DuplicateRuleInDifferentSubninjas) { // Test that rules are scoped to subninjas. fs_.Create("test.ninja", "rule cat\n" " command = cat\n"); - ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); + ManifestParser parser(&state, &fs_); string err; EXPECT_TRUE(parser.ParseTest("rule cat\n" " command = cat\n" @@ -879,7 +909,7 @@ TEST_F(ParserTest, DuplicateRuleInDifferentSubninjasWithInclude) { " command = cat\n"); fs_.Create("test.ninja", "include rules.ninja\n" "build x : cat\n"); - ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); + ManifestParser parser(&state, &fs_); string err; EXPECT_TRUE(parser.ParseTest("include rules.ninja\n" "subninja test.ninja\n" @@ -899,7 +929,7 @@ TEST_F(ParserTest, Include) { TEST_F(ParserTest, BrokenInclude) { fs_.Create("include.ninja", "build\n"); - ManifestParser parser(&state, &fs_, kDupeEdgeActionWarn); + ManifestParser parser(&state, &fs_); string err; EXPECT_FALSE(parser.ParseTest("include include.ninja\n", &err)); EXPECT_EQ("include.ninja:1: expected path\n" @@ -974,7 +1004,7 @@ TEST_F(ParserTest, ImplicitOutputDupes) { } TEST_F(ParserTest, NoExplicitOutput) { - ManifestParser parser(&state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&state, NULL); string err; EXPECT_TRUE(parser.ParseTest( "rule cat\n" @@ -1034,7 +1064,7 @@ TEST_F(ParserTest, UTF8) { TEST_F(ParserTest, CRLF) { State local_state; - ManifestParser parser(&local_state, NULL, kDupeEdgeActionWarn); + ManifestParser parser(&local_state, NULL); string err; EXPECT_TRUE(parser.ParseTest("# comment with crlf\r\n", &err)); diff --git a/src/ninja.cc b/src/ninja.cc index 54de7b9f1d..ed004ac8f1 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); @@ -1144,10 +1155,14 @@ int real_main(int argc, char** argv) { for (int cycle = 1; cycle <= kCycleLimit; ++cycle) { NinjaMain ninja(ninja_command, config); - ManifestParser parser(&ninja.state_, &ninja.disk_interface_, - options.dupe_edges_should_err - ? kDupeEdgeActionError - : kDupeEdgeActionWarn); + ManifestParserOptions parser_opts; + 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)) { Error("%s", err.c_str()); diff --git a/src/test.cc b/src/test.cc index 51882f0925..a9816bc232 100644 --- a/src/test.cc +++ b/src/test.cc @@ -95,8 +95,9 @@ Node* StateTestWithBuiltinRules::GetNode(const string& path) { return state_.GetNode(path, 0); } -void AssertParse(State* state, const char* input) { - ManifestParser parser(state, NULL, kDupeEdgeActionWarn); +void AssertParse(State* state, const char* input, + ManifestParserOptions opts) { + ManifestParser parser(state, NULL, opts); string err; EXPECT_TRUE(parser.ParseTest(input, &err)); ASSERT_EQ("", err); diff --git a/src/test.h b/src/test.h index 02ed929943..3bce8f75ae 100644 --- a/src/test.h +++ b/src/test.h @@ -16,6 +16,7 @@ #define NINJA_TEST_H_ #include "disk_interface.h" +#include "manifest_parser.h" #include "state.h" #include "util.h" @@ -122,7 +123,8 @@ struct StateTestWithBuiltinRules : public testing::Test { State state_; }; -void AssertParse(State* state, const char* input); +void AssertParse(State* state, const char* input, + ManifestParserOptions = ManifestParserOptions()); void AssertHash(const char* expected, uint64_t actual); void VerifyGraph(const State& state); diff --git a/src/version.cc b/src/version.cc index e2a9f4905b..3a20205cd8 100644 --- a/src/version.cc +++ b/src/version.cc @@ -18,7 +18,7 @@ #include "util.h" -const char* kNinjaVersion = "1.8.1"; +const char* kNinjaVersion = "1.8.2"; void ParseVersion(const string& version, int* major, int* minor) { size_t end = version.find('.');