Make restarting state machines explicit

If my memory is correct, @edolstra objected to modifying `wantedOutputs`
upon falling back to doing a build (as we did before), because we should
only modify it in response to new requests --- *actual* wants --- and
not because we are "incidentally" building all the outptus beyond what
may have been requested.

That's a fair point, and the alternative is to replace the boolean soup
with proper enums: Instead of modifying `wantedOuputs` som more, we'll
modify `needsRestart` to indicate we are passed the need.

1 files changed, 43 insertions, 7 deletions

diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh
index 3a6f0c2d9..9b5bd1805 100644
--- a/src/libstore/build/derivation-goal.hh
+++ b/src/libstore/build/derivation-goal.hh
@@ -79,21 +79,57 @@ struct DerivationGoal : public Goal
     std::map<std::pair<StorePath, std::string>, StorePath> inputDrvOutputs;
 
     /**
+     * See `needRestart`; just for that field.
+     */
+    enum struct NeedRestartForMoreOutputs {
+        /**
+         * The goal state machine is progressing based on the current value of
+         * `wantedOutputs. No actions are needed.
+         */
+        OutputsUnmodifedDontNeed,
+        /**
+         * `wantedOutputs` has been extended, but the state machine is
+         * proceeding according to its old value, so we need to restart.
+         */
+        OutputsAddedDoNeed,
+        /**
+         * The goal state machine has progressed to the point of doing a build,
+         * in which case all outputs will be produced, so extensions to
+         * `wantedOutputs` no longer require a restart.
+         */
+        BuildInProgressWillNotNeed,
+    };
+
+    /**
      * Whether additional wanted outputs have been added.
      */
-    bool needRestart = false;
+    NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed;
 
     /**
-     * Whether to retry substituting the outputs after building the
-     * inputs. This is done in case of an incomplete closure.
+     * See `retrySubstitution`; just for that field.
      */
-    bool retrySubstitution = false;
+    enum RetrySubstitution {
+        /**
+         * No issues have yet arose, no need to restart.
+         */
+        NoNeed,
+        /**
+         * Something failed and there is an incomplete closure. Let's retry
+         * substituting.
+         */
+        YesNeed,
+        /**
+         * We are current or have already retried substitution, and whether or
+         * not something goes wrong we will not retry again.
+         */
+        AlreadyRetried,
+    };
 
     /**
-     * Whether we've retried substitution, in which case we won't try
-     * again.
+     * Whether to retry substituting the outputs after building the
+     * inputs. This is done in case of an incomplete closure.
      */
-    bool retriedSubstitution = false;
+    RetrySubstitution retrySubstitution = RetrySubstitution::NoNeed;
 
     /**
      * The derivation stored at drvPath.