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.

diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index 638d751..e8c0436 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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());