Fix postponed re-examination logic

Now postponed takes precedence over changed.

6 files changed, 29 insertions, 25 deletions

diff --git a/build/algorithm.cxx b/build/algorithm.cxx
index c9dcbfe..a19db36 100644
--- a/build/algorithm.cxx
+++ b/build/algorithm.cxx
@@ -321,19 +321,21 @@ namespace build
     case target_state::unknown:
     case target_state::postponed:
       {
-        t.raw_state = target_state::failed; // So the rule can just throw.
-
         auto g (
           make_exception_guard (
-            [](action a, target& t){info << "while " << diag_doing (a, t);},
+            [](action a, target& t)
+            {
+              t.raw_state = target_state::failed;
+              info << "while " << diag_doing (a, t);
+            },
             a, t));
 
         target_state ts (t.recipe (a) (a, t));
         assert (ts != target_state::unknown && ts != target_state::failed);
 
-        // The recipe may have set the target's state manually.
+        // Set the target's state unless it should be the group's state.
         //
-        if (t.raw_state == target_state::failed)
+        if (t.raw_state != target_state::group)
           t.raw_state = ts;
 
         return ts;
@@ -360,8 +362,8 @@ namespace build
         continue;
 
       target_state ts (execute (a, *pt));
-      if (ts == target_state::changed ||
-          (ts == target_state::postponed && r == target_state::unchanged))
+      if (ts == target_state::postponed ||
+          (ts == target_state::changed && r == target_state::unchanged))
         r = ts;
     }
 
@@ -379,8 +381,8 @@ namespace build
         continue;
 
       target_state ts (execute (a, *pt));
-      if (ts == target_state::changed ||
-          (ts == target_state::postponed && r == target_state::unchanged))
+      if (ts == target_state::postponed ||
+          (ts == target_state::changed && r == target_state::unchanged))
         r = ts;
     }
 
@@ -441,12 +443,8 @@ namespace build
   {
     target_state r (execute (a, *t.group));
 
-    // The standard execute() logic sets the state to failed just
-    // before calling the recipe (so that the recipe can just throw
-    // to indicate a failure). After the recipe is successfully
-    // executed and unless the recipe has updated the state manually,
-    // the recipe's return value is set as the new state. But we
-    // don't want that. So we are going to set it manually.
+    // Indicate to the standard execute() logic that this target's
+    // state comes from the group.
     //
     t.raw_state = target_state::group;
 
@@ -480,6 +478,6 @@ namespace build
     //
     target_state ts (reverse_execute_prerequisites (a, t));
 
-    return r ? target_state::changed : ts;
+    return r && ts != target_state::postponed ? target_state::changed : ts;
   }
 }
diff --git a/build/algorithm.ixx b/build/algorithm.ixx
index 4581d5b..7849713 100644
--- a/build/algorithm.ixx
+++ b/build/algorithm.ixx
@@ -108,7 +108,10 @@ namespace build
   inline target_state
   execute (action a, target& t)
   {
-    t.dependents--;
+    // This can happen when we re-examine the state after being postponed.
+    //
+    if (t.dependents != 0)
+      t.dependents--;
 
     switch (target_state ts = t.state ())
     {
diff --git a/build/bin/rule.cxx b/build/bin/rule.cxx
index 0eb8363..f02029b 100644
--- a/build/bin/rule.cxx
+++ b/build/bin/rule.cxx
@@ -119,16 +119,16 @@ namespace build
       if (m1 != nullptr)
       {
         ts = execute (a, *m1);
-        if (ts == target_state::changed ||
-            (ts == target_state::postponed && r == target_state::unchanged))
+        if (ts == target_state::postponed ||
+            (ts == target_state::changed && r == target_state::unchanged))
           r = ts;
       }
 
       if (m2 != nullptr)
       {
         ts = execute (a, *m2);
-        if (ts == target_state::changed ||
-            (ts == target_state::postponed && r == target_state::unchanged))
+        if (ts == target_state::postponed ||
+            (ts == target_state::changed && r == target_state::unchanged))
           r = ts;
       }
 
diff --git a/build/cli/rule.cxx b/build/cli/rule.cxx
index 9abe665..dff13f0 100644
--- a/build/cli/rule.cxx
+++ b/build/cli/rule.cxx
@@ -294,7 +294,7 @@ namespace build
       //
       target_state ts (reverse_execute_prerequisites (a, t));
 
-      return r ? target_state::changed : ts;
+      return r && ts != target_state::postponed ? target_state::changed : ts;
     }
   }
 }
diff --git a/build/operation.cxx b/build/operation.cxx
index cff0802..61111fe 100644
--- a/build/operation.cxx
+++ b/build/operation.cxx
@@ -131,7 +131,6 @@ namespace build
         {
         case target_state::postponed:
           {
-            info << diag_doing (a, t) << " is postponed";
             psp.push_back (t);
             break;
           }
@@ -163,7 +162,7 @@ namespace build
     for (target& t: psp)
     {
       if (t.state () == target_state::postponed)
-        execute_direct (a, t); // Try again, now ignoring the execution mode.
+        execute (a, t); // Re-examine the state.
 
       switch (t.state ())
       {
diff --git a/build/rule.cxx b/build/rule.cxx
index f53d9d9..f9b9a3c 100644
--- a/build/rule.cxx
+++ b/build/rule.cxx
@@ -228,10 +228,14 @@ namespace build
     rmdir_status rs (rmdir (t.dir, t));
 
     target_state ts (target_state::unchanged);
-
     if (t.has_prerequisites ())
+    {
       ts = reverse_execute_prerequisites (a, t);
 
+      if (ts == target_state::postponed)
+        return ts;
+    }
+
     // If we couldn't remove the directory, return postponed meaning
     // that the operation could not be performed at this time.
     //