From 92b74c340ab6397cb728391995f86dfdb39bb186 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 24 Mar 2015 14:16:07 -0700 Subject: [PATCH] Add an opt-in flag to make duplicate edges an error (`-w dupbuild=err`). This is step 1 on #931. Duplicated edges will become an error by default in the future. --- src/manifest_parser.cc | 14 ++++++++++---- src/manifest_parser.h | 6 ++++-- src/manifest_parser_test.cc | 13 +++++++++++++ src/ninja.cc | 41 ++++++++++++++++++++++++++++++++++++++--- 4 files changed, 65 insertions(+), 9 deletions(-) diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc index 638d751..e8c0436 100644 --- a/src/manifest_parser.cc +++ b/src/manifest_parser.cc @@ -24,8 +24,10 @@ #include "util.h" #include "version.h" -ManifestParser::ManifestParser(State* state, FileReader* file_reader) - : state_(state), file_reader_(file_reader), quiet_(false) { +ManifestParser::ManifestParser(State* state, FileReader* file_reader, + bool dupe_edge_should_err) + : state_(state), file_reader_(file_reader), + dupe_edge_should_err_(dupe_edge_should_err), quiet_(false) { env_ = &state->bindings_; } @@ -329,10 +331,14 @@ 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 (!quiet_) { + if (dupe_edge_should_err_) { + lexer_.Error("multiple rules generate " + path + " [-w dupbuild=err]", + err); + return false; + } else if (!quiet_) { Warning("multiple rules generate %s. " "builds involving this target will not be correct; " - "continuing anyway", + "continuing anyway [-w dupbuild=warn]", path.c_str()); } } diff --git a/src/manifest_parser.h b/src/manifest_parser.h index 00a711d..f72cd6f 100644 --- a/src/manifest_parser.h +++ b/src/manifest_parser.h @@ -32,10 +32,11 @@ struct ManifestParser { virtual bool ReadFile(const string& path, string* content, string* err) = 0; }; - ManifestParser(State* state, FileReader* file_reader); + ManifestParser(State* state, FileReader* file_reader, + bool dupe_edge_should_err = false); /// Load and parse a file. - bool Load(const string& filename, string* err, Lexer* parent=NULL); + bool Load(const string& filename, string* err, Lexer* parent = NULL); /// Parse a text string of input. Used by tests. bool ParseTest(const string& input, string* err) { @@ -65,6 +66,7 @@ private: BindingEnv* env_; FileReader* file_reader_; Lexer lexer_; + bool dupe_edge_should_err_; bool quiet_; }; diff --git a/src/manifest_parser_test.cc b/src/manifest_parser_test.cc index 7e72b34..8f7b575 100644 --- a/src/manifest_parser_test.cc +++ b/src/manifest_parser_test.cc @@ -364,6 +364,19 @@ TEST_F(ParserTest, NoDeadPointerFromDuplicateEdge) { // That's all the checking that this test needs. } +TEST_F(ParserTest, DuplicateEdgeWithMultipleOutputsError) { + const char kInput[] = +"rule cat\n" +" command = cat $in > $out\n" +"build out1 out2: cat in1\n" +"build out1: cat in2\n" +"build final: cat out1\n"; + ManifestParser parser(&state, this, /*dupe_edges_should_err=*/true); + string err; + EXPECT_FALSE(parser.ParseTest(kInput, &err)); + EXPECT_EQ("input:5: multiple rules generate out1 [-w dupbuild=err]\n", err); +} + TEST_F(ParserTest, ReservedWords) { ASSERT_NO_FATAL_FAILURE(AssertParse( "rule build\n" diff --git a/src/ninja.cc b/src/ninja.cc index 4987bb2..1e43137 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -64,6 +64,9 @@ 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; }; /// The Ninja main() loads up a series of data structures; various tools need @@ -199,7 +202,8 @@ void Usage(const BuildConfig& config) { "\n" " -d MODE enable debugging (use -d list to list modes)\n" " -t TOOL run a subtool (use -t list to list subtools)\n" -" terminates toplevel options; further flags are passed to the tool\n", +" terminates toplevel options; further flags are passed to the tool\n" +" -w FLAG adjust warnings (use -w list to list warnings)\n", kNinjaVersion, config.parallelism); } @@ -792,6 +796,32 @@ bool DebugEnable(const string& name) { } } +/// Set a warning flag. Returns false if Ninja should exit instead of +/// continuing. +bool WarningEnable(const string& name, Options* options) { + if (name == "list") { + printf("warning flags:\n" +" dupbuild={err,warn} multiple build lines for one target\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 { + const char* suggestion = + SpellcheckString(name.c_str(), "dupbuild=err", "dupbuild=warn", NULL); + if (suggestion) { + Error("unknown warning flag '%s', did you mean '%s'?", + name.c_str(), suggestion); + } else { + Error("unknown warning flag '%s'", name.c_str()); + } + return false; + } +} + bool NinjaMain::OpenBuildLog(bool recompact_only) { string log_path = ".ninja_log"; if (!build_dir_.empty()) @@ -962,7 +992,7 @@ int ReadFlags(int* argc, char*** argv, int opt; while (!options->tool && - (opt = getopt_long(*argc, *argv, "d:f:j:k:l:nt:vC:h", kLongOptions, + (opt = getopt_long(*argc, *argv, "d:f:j:k:l:nt:vw:C:h", kLongOptions, NULL)) != -1) { switch (opt) { case 'd': @@ -1011,6 +1041,10 @@ int ReadFlags(int* argc, char*** argv, case 'v': config->verbosity = BuildConfig::VERBOSE; break; + case 'w': + if (!WarningEnable(optarg, options)) + return 1; + break; case 'C': options->working_dir = optarg; break; @@ -1067,7 +1101,8 @@ int real_main(int argc, char** argv) { NinjaMain ninja(ninja_command, config); RealFileReader file_reader; - ManifestParser parser(&ninja.state_, &file_reader); + ManifestParser parser(&ninja.state_, &file_reader, + options.dupe_edges_should_err); string err; if (!parser.Load(options.input_file, &err)) { Error("%s", err.c_str()); -- 2.7.4