Implement Make-style order-only dependencies

Previously, the implementation of order-only dependencies differed
between Make and Ninja in two important ways:

 1) If the order-only dependency existed but was out of date, it
    would never be rebuilt, whereas Make would always rebuild out of
    date order-only dependencies.

 2) If the order-only dependency did not exist, it would cause
    its reverse dependencies to always build, whereas Make would only
    rebuild a file if a non-order-only dependency was out of date.

A key distinction between Ninja and Make as seen through the above
two points was that in Ninja, order-only dependencies cared about
whether the target as a file exists (so perhaps a better name for
the old semantics would have been "missing-only dependencies").

These differences made it impossible to introduce an order-only
dependency on an always out-of-date (i.e. missing) target without
also causing the depender and its reverse dependencies to rebuild
unnecessarily on every build.  Build systems which must perform some
action (such as logging the build start time, or printing a message)
at the start of every build typically implement this by adding to
every target an order-only dependency which performs this action,
which would have forced an entire rebuild on every invocation of
Ninja under the old semantics.

This commit causes Ninja to conform to the Make-style behaviour.

diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc
index ff197b1..7fd2715 100644 (file)
--- a/doc/manual.asciidoc
+++ b/doc/manual.asciidoc
@@ -502,9 +502,9 @@ Note that dependencies as loaded through depfiles have slightly different
 semantics, as described in the <<ref_rule,rule reference>>.
 
 3. _Order-only dependencies_, expressed with the syntax +|| _dep1_
-   _dep2_+ on the end of a build line.  When these are missing, the
-   output is not rebuilt until they are built, but once they are
-   available further changes to the files do not affect the output.
+   _dep2_+ on the end of a build line.  When these are out of date, the
+   output is not rebuilt until they are built, but changes in order-only
+   dependencies alone do not cause the output to be rebuilt.
 +
 Order-only dependencies can be useful for bootstrapping dependencies
 that are only discovered during build time: for example, to generate a

diff --git a/src/build_test.cc b/src/build_test.cc
index a4bf256..4544e2c 100644 (file)
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -532,6 +532,53 @@ TEST_F(BuildTest, OrderOnlyDeps) {
   ASSERT_EQ(1u, commands_ran_.size());
 }
 
+TEST_F(BuildTest, RebuildOrderOnlyDeps) {
+  string err;
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cc\n  command = cc $in\n"
+"rule true\n  command = true\n"
+"build oo.h: cc oo.h.in\n"
+"build foo.o: cc foo.c || oo.h\n"));
+
+  fs_.Create("foo.c", now_, "");
+  fs_.Create("oo.h.in", now_, "");
+
+  // foo.o and order-only dep dirty, build both.
+  EXPECT_TRUE(builder_.AddTarget("foo.o", &err));
+  EXPECT_TRUE(builder_.Build(&err));
+  ASSERT_EQ("", err);
+  ASSERT_EQ(2u, commands_ran_.size());
+
+  // all clean, no rebuild.
+  commands_ran_.clear();
+  state_.Reset();
+  EXPECT_TRUE(builder_.AddTarget("foo.o", &err));
+  EXPECT_EQ("", err);
+  EXPECT_TRUE(builder_.AlreadyUpToDate());
+
+  // order-only dep missing, build it only.
+  fs_.RemoveFile("oo.h");
+  commands_ran_.clear();
+  state_.Reset();
+  EXPECT_TRUE(builder_.AddTarget("foo.o", &err));
+  EXPECT_TRUE(builder_.Build(&err));
+  ASSERT_EQ("", err);
+  ASSERT_EQ(1u, commands_ran_.size());
+  ASSERT_EQ("cc oo.h.in", commands_ran_[0]);
+
+  now_++;
+
+  // order-only dep dirty, build it only.
+  fs_.Create("oo.h.in", now_, "");
+  commands_ran_.clear();
+  state_.Reset();
+  EXPECT_TRUE(builder_.AddTarget("foo.o", &err));
+  EXPECT_TRUE(builder_.Build(&err));
+  ASSERT_EQ("", err);
+  ASSERT_EQ(1u, commands_ran_.size());
+  ASSERT_EQ("cc oo.h.in", commands_ran_[0]);
+}
+
 TEST_F(BuildTest, Phony) {
   string err;
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,

diff --git a/src/graph.cc b/src/graph.cc
index 65fed2e..9df6ecb 100644 (file)
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -51,27 +51,20 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface,
       }
     }
 
-    if (is_order_only(i - inputs_.begin())) {
-      // Order-only deps only make us dirty if they're missing.
-      if (!(*i)->file_->exists()) {
-        dirty = true;
-        outputs_ready_ = false;
-      }
-      continue;
-    }
-
     // If an input is not ready, neither are our outputs.
     if (Edge* edge = (*i)->in_edge_)
       if (!edge->outputs_ready_)
         outputs_ready_ = false;
 
-    // If a regular input is dirty (or missing), we're dirty.
-    // Otherwise consider mtime.
-    if ((*i)->dirty_) {
-      dirty = true;
-    } else {
-      if ((*i)->file_->mtime_ > most_recent_input)
-        most_recent_input = (*i)->file_->mtime_;
+    if (!is_order_only(i - inputs_.begin())) {
+       // If a regular input is dirty (or missing), we're dirty.
+       // Otherwise consider mtime.
+       if ((*i)->dirty_) {
+         dirty = true;
+       } else {
+         if ((*i)->file_->mtime_ > most_recent_input)
+           most_recent_input = (*i)->file_->mtime_;
+       }
     }
   }