summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJan Niklas Hasse <jhasse@bixense.com>2023-11-29 20:16:06 (GMT)
committerJan Niklas Hasse <jhasse@bixense.com>2023-11-29 20:16:06 (GMT)
commit8f47d5aa33c6c303a71093be2eac02672dfb2966 (patch)
tree9c526df6e53bc979a395e46e194ccd2e0f11e17e
parent82a7cb3f263a6af7cc4c24408cf0a4ca5b648b55 (diff)
downloadNinja-8f47d5aa33c6c303a71093be2eac02672dfb2966.zip
Ninja-8f47d5aa33c6c303a71093be2eac02672dfb2966.tar.gz
Ninja-8f47d5aa33c6c303a71093be2eac02672dfb2966.tar.bz2
Remove `-w dupbuild` completely, always error on duplicate edges
Step 5, fixes #931.
-rw-r--r--src/manifest_parser.cc15
-rw-r--r--src/manifest_parser.h6
-rw-r--r--src/manifest_parser_test.cc61
-rw-r--r--src/ninja.cc18
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<size_t>(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;
}