From 7c7cc8c059343ea0a27029b6af58075b63c432d4 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Mon, 12 Aug 2019 15:50:43 +0200
Subject: [PATCH] Fix duplicate step_finished notification

---
 src/hydra-queue-runner/builder.cc | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc
index ce65f117..edd4b1f7 100644
--- a/src/hydra-queue-runner/builder.cc
+++ b/src/hydra-queue-runner/builder.cc
@@ -228,6 +228,11 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore,
     time_t stepStopTime = time(0);
     if (!result.stopTime) result.stopTime = stepStopTime;
 
+    /* For standard failures, we don't care about the error
+       message. */
+    if (result.stepStatus != bsAborted)
+        result.errorMsg = "";
+
     /* Account the time we spent building this step by dividing it
        among the jobsets that depend on it. */
     {
@@ -240,6 +245,13 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore,
         }
     }
 
+    /* Finish the step in the database. */
+    if (stepNr) {
+        pqxx::work txn(*conn);
+        finishBuildStep(txn, result, buildId, stepNr, machine->sshName);
+        txn.commit();
+    }
+
     /* The step had a hopefully temporary failure (e.g. network
        issue). Retry a number of times. */
     if (result.canRetry) {
@@ -253,11 +265,6 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore,
         }
         if (retry) {
             auto mc = startDbUpdate();
-            {
-            pqxx::work txn(*conn);
-            finishBuildStep(txn, result, buildId, stepNr, machine->sshName);
-            txn.commit();
-            }
             stepFinished = true;
             if (quit) exit(1);
             return sRetry;
@@ -312,8 +319,6 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore,
 
                 pqxx::work txn(*conn);
 
-                finishBuildStep(txn, result, buildId, stepNr, machine->sshName);
-
                 for (auto & b : direct) {
                     printMsg(lvlInfo, format("marking build %1% as succeeded") % b->id);
                     markSucceededBuild(txn, b, res, buildId != b->id || result.isCached,
@@ -370,11 +375,6 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore,
 
     } else {
 
-        /* For standard failures, we don't care about the error
-           message. */
-        if (result.stepStatus != bsAborted)
-            result.errorMsg = "";
-
         /* Register failure in the database for all Build objects that
            directly or indirectly depend on this step. */
 
@@ -420,11 +420,6 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore,
                         result.stepStatus, result.errorMsg, buildId == build2->id ? 0 : buildId);
                 }
 
-                if (result.stepStatus != bsCachedFailure && !stepFinished) {
-                    assert(stepNr);
-                    finishBuildStep(txn, result, buildId, stepNr, machine->sshName);
-                }
-
                 /* Mark all builds that depend on this derivation as failed. */
                 for (auto & build2 : indirect) {
                     if (build2->finishedInDB) continue;