Simplify retry handling

src/hydra-queue-runner/hydra-queue-runner.cc

@@ -1100,11 +1100,22 @@ bool State::doBuildStep(std::shared_ptr<StoreAPI> store, Step::ptr step,
 
     if (!result.stopTime) result.stopTime = time(0);
 
-    bool retry = false;
+    /* The step had a hopefully temporary failure (e.g. network
+       issue). Retry a number of times. */
     if (result.status == RemoteResult::rrMiscFailure) {
+        bool retry;
+        {
             auto step_(step->state.lock());
             retry = step_->tries + 1 < maxTries;
         }
+        if (retry) {
+            pqxx::work txn(*conn);
+            finishBuildStep(txn, result.startTime, result.stopTime, build->id,
+                stepNr, machine->sshName, bssAborted, result.errorMsg);
+            txn.commit();
+            return true;
+        }
+    }
 
     /* Remove this step. After this, incoming builds that depend on
        drvPath will either see that the output paths exist, or will
@@ -1112,10 +1123,8 @@ bool State::doBuildStep(std::shared_ptr<StoreAPI> store, Step::ptr step,
        won't conflict with this one, because we're removing it. In any
        case, the set of dependent builds for ‘step’ can't increase
        anymore because ‘step’ is no longer visible to createStep(). */
-    if (!retry) {
     auto steps_(steps.lock());
     steps_->erase(step->drvPath);
-    }
 
     /* Get the final set of dependent builds. */
     auto dependents = getDependentBuilds(step);
@@ -1159,13 +1168,11 @@ bool State::doBuildStep(std::shared_ptr<StoreAPI> store, Step::ptr step,
                message. */
             if (buildStatus != bsAborted) result.errorMsg = "";
 
-            if (!retry) {
+            /* Create failed build steps for every build that depends
-
+               on this. For cached failures, only create a step for
-                /* Create failed build steps for every build that
+               builds that don't have this step as top-level
-                   depends on this. For cached failures, only create a
+               (otherwise the user won't be able to see what caused
-                   step for builds that don't have this step as
+               the build to fail). */
-                   top-level (otherwise the user won't be able to see
-                   what caused the build to fail). */
             for (auto build2 : dependents) {
                 if (build == build2) continue;
                 if (cachedFailure && build2->drvPath == step->drvPath) continue;
@@ -1173,14 +1180,11 @@ bool State::doBuildStep(std::shared_ptr<StoreAPI> store, Step::ptr step,
                     buildStepStatus, result.errorMsg, build->id);
             }
 
-            }
-
             if (!cachedFailure)
                 finishBuildStep(txn, result.startTime, result.stopTime, build->id,
                     stepNr, machine->sshName, buildStepStatus, result.errorMsg);
 
             /* Mark all builds that depend on this derivation as failed. */
-            if (!retry)
             for (auto build2 : dependents) {
                 printMsg(lvlError, format("marking build %1% as failed") % build2->id);
                 txn.parameterized
@@ -1208,7 +1212,6 @@ bool State::doBuildStep(std::shared_ptr<StoreAPI> store, Step::ptr step,
        is the top-level derivation. In case of failure, destroy all
        dependent Build objects. Any Steps not referenced by other
        Builds will be destroyed as well. */
-    if (!retry)
     for (auto build2 : dependents)
         if (build2->toplevel == step || result.status != RemoteResult::rrSuccess) {
             auto builds_(builds.lock());
@@ -1218,10 +1221,9 @@ bool State::doBuildStep(std::shared_ptr<StoreAPI> store, Step::ptr step,
     /* Remove the step from the graph. In case of success, make
        dependent build steps runnable if they have no other
        dependencies. */
-    if (!retry)
     destroyStep(step, result.status == RemoteResult::rrSuccess);
 
-    return retry;
+    return false;
 }