From 8f47d5aa33c6c303a71093be2eac02672dfb2966 Mon Sep 17 00:00:00 2001 From: Jan Niklas Hasse Date: Wed, 29 Nov 2023 21:16:06 +0100 Subject: Remove `-w dupbuild` completely, always error on duplicate edges Step 5, fixes #931. --- src/manifest_parser.cc | 15 ++--------- src/manifest_parser.h | 6 +---- src/manifest_parser_test.cc | 61 ++++++++++++--------------------------------- src/ninja.cc | 18 ++----------- 4 files changed, 21 insertions(+), 79 deletions(-) diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 103c365..aa52989 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -337,19 +337,8 @@ bool ManifestParser::ParseEdge(string* err) { uint64_t slash_bits; CanonicalizePath(&path, &slash_bits); if (!state_->AddOut(edge, path, slash_bits)) { - if (options_.dupe_edge_action_ == kDupeEdgeActionError) { - lexer_.Error("multiple rules generate " + path, err); - return false; - } else { - if (!quiet_) { - Warning( - "multiple rules generate %s. builds involving this target will " - "not be correct; continuing anyway", - path.c_str()); - } - if (e - i <= static_cast(implicit_outs)) - --implicit_outs; - } + lexer_.Error("multiple rules generate " + path, err); + return false; } } diff --git a/src/manifest_parser.h b/src/manifest_parser.h index 954cf46..db6812d 100644 --- a/src/manifest_parser.h +++ b/src/manifest_parser.h @@ -31,11 +31,7 @@ enum PhonyCycleAction { }; struct ManifestParserOptions { - ManifestParserOptions() - : dupe_edge_action_(kDupeEdgeActionWarn), - phony_cycle_action_(kPhonyCycleActionWarn) {} - DupeEdgeAction dupe_edge_action_; - PhonyCycleAction phony_cycle_action_; + PhonyCycleAction phony_cycle_action_ = kPhonyCycleActionWarn; }; /// Parses .ninja files. diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 66b72e2..8a7b135 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -330,29 +330,6 @@ TEST_F(ParserTest, CanonicalizePathsBackslashes) { } #endif -TEST_F(ParserTest, DuplicateEdgeWithMultipleOutputs) { - ASSERT_NO_FATAL_FAILURE(AssertParse( -"rule cat\n" -" command = cat $in > $out\n" -"build out1 out2: cat in1\n" -"build out1: cat in2\n" -"build final: cat out1\n" -)); - // AssertParse() checks that the generated build graph is self-consistent. - // That's all the checking that this test needs. -} - -TEST_F(ParserTest, NoDeadPointerFromDuplicateEdge) { - ASSERT_NO_FATAL_FAILURE(AssertParse( -"rule cat\n" -" command = cat $in > $out\n" -"build out: cat in\n" -"build out: cat in\n" -)); - // AssertParse() checks that the generated build graph is self-consistent. - // That's all the checking that this test needs. -} - TEST_F(ParserTest, DuplicateEdgeWithMultipleOutputsError) { const char kInput[] = "rule cat\n" @@ -360,9 +337,7 @@ TEST_F(ParserTest, DuplicateEdgeWithMultipleOutputsError) { "build out1 out2: cat in1\n" "build out1: cat in2\n" "build final: cat out1\n"; - ManifestParserOptions parser_opts; - parser_opts.dupe_edge_action_ = kDupeEdgeActionError; - ManifestParser parser(&state, &fs_, parser_opts); + ManifestParser parser(&state, &fs_); string err; EXPECT_FALSE(parser.ParseTest(kInput, &err)); EXPECT_EQ("input:5: multiple rules generate out1\n", err); @@ -377,9 +352,7 @@ TEST_F(ParserTest, DuplicateEdgeInIncludedFile) { "build final: cat out1\n"); const char kInput[] = "subninja sub.ninja\n"; - ManifestParserOptions parser_opts; - parser_opts.dupe_edge_action_ = kDupeEdgeActionError; - ManifestParser parser(&state, &fs_, parser_opts); + ManifestParser parser(&state, &fs_); string err; EXPECT_FALSE(parser.ParseTest(kInput, &err)); EXPECT_EQ("sub.ninja:5: multiple rules generate out1\n", err); @@ -997,28 +970,26 @@ TEST_F(ParserTest, ImplicitOutputEmpty) { EXPECT_FALSE(edge->is_implicit_out(0)); } -TEST_F(ParserTest, ImplicitOutputDupe) { - ASSERT_NO_FATAL_FAILURE(AssertParse( +TEST_F(ParserTest, ImplicitOutputDupeError) { + const char kInput[] = "rule cat\n" " command = cat $in > $out\n" -"build foo baz | foo baq foo: cat bar\n")); - - Edge* edge = state.LookupNode("foo")->in_edge(); - ASSERT_EQ(edge->outputs_.size(), 3); - EXPECT_FALSE(edge->is_implicit_out(0)); - EXPECT_FALSE(edge->is_implicit_out(1)); - EXPECT_TRUE(edge->is_implicit_out(2)); +"build foo baz | foo baq foo: cat bar\n"; + ManifestParser parser(&state, &fs_); + string err; + EXPECT_FALSE(parser.ParseTest(kInput, &err)); + EXPECT_EQ("input:4: multiple rules generate foo\n", err); } -TEST_F(ParserTest, ImplicitOutputDupes) { - ASSERT_NO_FATAL_FAILURE(AssertParse( +TEST_F(ParserTest, ImplicitOutputDupesError) { + const char kInput[] = "rule cat\n" " command = cat $in > $out\n" -"build foo foo foo | foo foo foo foo: cat bar\n")); - - Edge* edge = state.LookupNode("foo")->in_edge(); - ASSERT_EQ(edge->outputs_.size(), 1); - EXPECT_FALSE(edge->is_implicit_out(0)); +"build foo foo foo | foo foo foo foo: cat bar\n"; + ManifestParser parser(&state, &fs_); + string err; + EXPECT_FALSE(parser.ParseTest(kInput, &err)); + EXPECT_EQ("input:4: multiple rules generate foo\n", err); } TEST_F(ParserTest, NoExplicitOutput) { diff --git a/src/ninja.cc b/src/ninja.cc index c011be1..839e9e6 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -77,9 +77,6 @@ struct Options { /// Tool to run rather than building. const Tool* tool; - /// 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; }; @@ -1210,12 +1207,6 @@ bool WarningEnable(const string& name, Options* options) { " phonycycle={err,warn} phony build statement references itself\n" ); return false; - } else if (name == "dupbuild=err") { - options->dupe_edges_should_err = true; - return true; - } 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; @@ -1227,9 +1218,8 @@ bool WarningEnable(const string& name, Options* options) { Warning("deprecated warning 'depfilemulti'"); return true; } else { - const char* suggestion = - SpellcheckString(name.c_str(), "dupbuild=err", "dupbuild=warn", - "phonycycle=err", "phonycycle=warn", NULL); + const char* suggestion = SpellcheckString(name.c_str(), "phonycycle=err", + "phonycycle=warn", nullptr); if (suggestion) { Error("unknown warning flag '%s', did you mean '%s'?", name.c_str(), suggestion); @@ -1525,7 +1515,6 @@ NORETURN void real_main(int argc, char** argv) { BuildConfig config; Options options = {}; options.input_file = "build.ninja"; - options.dupe_edges_should_err = true; setvbuf(stdout, NULL, _IOLBF, BUFSIZ); const char* ninja_command = argv[0]; @@ -1562,9 +1551,6 @@ NORETURN void real_main(int argc, char** argv) { NinjaMain ninja(ninja_command, config); 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; } -- cgit v0.12