v1.8.2

Author: nico

doc/manual.asciidoc

@@ -1,6 +1,6 @@
 The Ninja build system
 ======================
-v1.8.1, Sep 2017
+v1.8.2, Sep 2017
 
 
 Introduction

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;
 

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"

src/graph.cc

@@ -170,6 +170,13 @@ bool DependencyScan::VerifyDAG(Node* node, vector<Node*>* 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;

src/graph.h

@@ -201,6 +201,7 @@ struct Edge {
 
   bool is_phony() const;
   bool use_console() const;
+  bool maybe_phonycycle_diagnostic() const;
 };
 
 

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"

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<Node*>::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 {

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_;
 };
 

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);