when an edge is dirty, mark all outputs dirty

The logic before was like:
  for each output:
    if edge_dirty or output.dirty:
      output.dirty = true
      edge_dirty = true

This was wrong in the case where the second output would case the edge
to be dirty; we needed to go back and mark the first output dirty as
well.  Fixed by taking two passes: one to compute the dirty state,
then a latter sweep to mark all outputs.

Fixes issue 148.

diff --git a/src/build_test.cc b/src/build_test.cc
index eb71864..6beb976 100644 (file)
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -345,9 +345,8 @@ TEST_F(BuildTest, TwoOutputs) {
 }
 
 // Test case from
-// https://github.com/martine/ninja/issues/148
-// disabled right now because the build gets stuck (which is the bug).
-TEST_F(BuildTest, DISABLED_MultiOutIn) {
+//   https://github.com/martine/ninja/issues/148
+TEST_F(BuildTest, MultiOutIn) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
 "rule touch\n"
 "  command = touch $out\n"

diff --git a/src/graph.cc b/src/graph.cc
index 341cfd5..90791f9 100644 (file)
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -39,6 +39,7 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface,
 
   outputs_ready_ = true;
 
+  // Visit all inputs; we're dirty if any of the inputs are dirty.
   time_t most_recent_input = 1;
   for (vector<Node*>::iterator i = inputs_.begin(); i != inputs_.end(); ++i) {
     if ((*i)->file_->StatIfNecessary(disk_interface)) {
@@ -68,21 +69,36 @@ bool Edge::RecomputeDirty(State* state, DiskInterface* disk_interface,
     }
   }
 
-  BuildLog* build_log = state ? state->build_log_ : 0;
-  string command = EvaluateCommand();
+  // We may also be dirty due to output state: missing outputs, out of
+  // date outputs, etc.  Visit all outputs and determine whether they're dirty.
+  if (!dirty) {
+    BuildLog* build_log = state ? state->build_log_ : 0;
+    string command = EvaluateCommand();
+
+    for (vector<Node*>::iterator i = outputs_.begin();
+         i != outputs_.end(); ++i) {
+      (*i)->file_->StatIfNecessary(disk_interface);
+      if (RecomputeOutputDirty(build_log, most_recent_input, command, *i)) {
+        dirty = true;
+        break;
+      }
+    }
+  }
 
-  assert(!outputs_.empty());
+  // Finally, visit each output to mark off that we've visited it, and update
+  // their dirty state if necessary.
   for (vector<Node*>::iterator i = outputs_.begin(); i != outputs_.end(); ++i) {
-    // We may have other outputs that our input-recursive traversal hasn't hit
-    // yet (or never will).  Stat them if we haven't already to mark that we've
-    // visited their dependents.
     (*i)->file_->StatIfNecessary(disk_interface);
-    if (dirty || RecomputeOutputDirty(build_log, most_recent_input, command, *i)) {
+    if (dirty)
       (*i)->dirty_ = true;
-      outputs_ready_ = false;
-    }
   }
 
+  // If we're dirty, our outputs are not ready.  (It's possible to be
+  // clean but still have not be ready in the presence of order-only
+  // inputs.)
+  if (dirty)
+    outputs_ready_ = false;
+
   return true;
 }