Revert "If a substitute closure is incomplete, build dependencies, then retry the substituter"

As summarized in
https://github.com/NixOS/nix/issues/77#issuecomment-2843228280 the
motivation is that the complicated retry logic this introduced was
making the cleanup task #12628 harder to accomplish. It was not easy to
ascertain just what policy / semantics the extra control-flow was
implementing, in order to figure out a different way to implementing it
either.

After talking to Eelco about it, he decided we could just....get rid of
the feature entirely! It's a bit scary removing a decade+ old feature,
but I think he is right. See the release notes for more explanation.

This reverts commit 299141ecbd.

Co-authored-by: Eelco Dolstra <edolstra@gmail.com>

src/libstore/build/derivation-goal.cc

@@ -285,40 +285,13 @@ Goal::Co DerivationGoal::haveDerivation()
 
     assert(!drv->type().isImpure());
 
-    if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) {
+    if (nrFailed > 0 && nrFailed > nrNoSubstituters && !settings.tryFallback) {
         co_return done(BuildResult::TransientFailure, {},
             Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ",
                 worker.store.printStorePath(drvPath)));
     }
 
-    /*  If the substitutes form an incomplete closure, then we should
-        build the dependencies of this derivation, but after that, we
-        can still use the substitutes for this derivation itself.
-
-        If the nrIncompleteClosure != nrFailed, we have another issue as well.
-        In particular, it may be the case that the hole in the closure is
-        an output of the current derivation, which causes a loop if retried.
-     */
-    {
-        bool substitutionFailed =
-            nrIncompleteClosure > 0 &&
-            nrIncompleteClosure == nrFailed;
-        switch (retrySubstitution) {
-        case RetrySubstitution::NoNeed:
-            if (substitutionFailed)
-                retrySubstitution = RetrySubstitution::YesNeed;
-            break;
-        case RetrySubstitution::YesNeed:
-            // Should not be able to reach this state from here.
-            assert(false);
-            break;
-        case RetrySubstitution::AlreadyRetried:
-            debug("substitution failed again, but we already retried once. Not retrying again.");
-            break;
-        }
-    }
-
-    nrFailed = nrNoSubstituters = nrIncompleteClosure = 0;
+    nrFailed = nrNoSubstituters = 0;
 
     if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) {
         needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed;
@@ -456,11 +429,6 @@ Goal::Co DerivationGoal::gaveUpOnSubstitution()
         co_return done(BuildResult::DependencyFailed, {}, Error(msg));
     }
 
-    if (retrySubstitution == RetrySubstitution::YesNeed) {
-        retrySubstitution = RetrySubstitution::AlreadyRetried;
-        co_return haveDerivation();
-    }
-
     /* Gather information necessary for computing the closure and/or
        running the build hook. */
 

src/libstore/build/drv-output-substitution-goal.cc

@@ -139,7 +139,7 @@ Goal::Co DrvOutputSubstitutionGoal::realisationFetched(Goals waitees, std::share
 
     if (nrFailed > 0) {
         debug("The output path of the derivation output '%s' could not be substituted", id.to_string());
-        co_return amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed);
+        co_return amDone(nrNoSubstituters > 0 ? ecNoSubstituters : ecFailed);
     }
 
     worker.store.registerDrvOutput(*outputInfo);

src/libstore/build/goal.cc

@@ -151,7 +151,7 @@ Goal::Done Goal::amDone(ExitCode result, std::optional<Error> ex)
     trace("done");
     assert(top_co);
     assert(exitCode == ecBusy);
-    assert(result == ecSuccess || result == ecFailed || result == ecNoSubstituters || result == ecIncompleteClosure);
+    assert(result == ecSuccess || result == ecFailed || result == ecNoSubstituters);
     exitCode = result;
 
     if (ex) {
@@ -170,12 +170,10 @@ Goal::Done Goal::amDone(ExitCode result, std::optional<Error> ex)
 
             goal->trace(fmt("waitee '%s' done; %d left", name, goal->waitees.size()));
 
-            if (result == ecFailed || result == ecNoSubstituters || result == ecIncompleteClosure) ++goal->nrFailed;
+            if (result == ecFailed || result == ecNoSubstituters) ++goal->nrFailed;
 
             if (result == ecNoSubstituters) ++goal->nrNoSubstituters;
 
-            if (result == ecIncompleteClosure) ++goal->nrIncompleteClosure;
-
             if (goal->waitees.empty()) {
                 worker.wakeUp(goal);
             } else if (result == ecFailed && !settings.keepGoing) {

src/libstore/build/substitution-goal.cc

@@ -169,7 +169,7 @@ Goal::Co PathSubstitutionGoal::tryToRun(StorePath subPath, nix::ref<Store> sub,
 
     if (nrFailed > 0) {
         co_return done(
-            nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed,
+            nrNoSubstituters > 0 ? ecNoSubstituters : ecFailed,
             BuildResult::DependencyFailed,
             fmt("some references of path '%s' could not be realised", worker.store.printStorePath(storePath)));
     }

src/libstore/include/nix/store/build/derivation-goal.hh

@@ -73,32 +73,6 @@ struct DerivationGoal : public Goal
      */
     NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed;
 
-    /**
-     * See `retrySubstitution`; just for that field.
-     */
-    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 to retry substituting the outputs after building the
-     * inputs. This is done in case of an incomplete closure.
-     */
-    RetrySubstitution retrySubstitution = RetrySubstitution::NoNeed;
-
     /**
      * The derivation stored at drvPath.
      */

src/libstore/include/nix/store/build/goal.hh

@@ -61,7 +61,7 @@ private:
     Goals waitees;
 
 public:
-    typedef enum {ecBusy, ecSuccess, ecFailed, ecNoSubstituters, ecIncompleteClosure} ExitCode;
+    typedef enum {ecBusy, ecSuccess, ecFailed, ecNoSubstituters} ExitCode;
 
     /**
      * Backlink to the worker.
@@ -85,12 +85,6 @@ public:
      */
     size_t nrNoSubstituters = 0;
 
-    /**
-     * Number of substitution goals we are/were waiting for that
-     * failed because they had unsubstitutable references.
-     */
-    size_t nrIncompleteClosure = 0;
-
     /**
      * Name of this goal for debugging purposes.
      */