From 6a54ab24e26b4e387c41e6d8958e7a9ec36c8398 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 7 Dec 2023 02:00:22 -0500
Subject: [PATCH] Use factored-out `BuildResult` serializer

For the record, here is the Nix 2.19 version:
https://github.com/NixOS/nix/blob/2.19-maintenance/src/libstore/serve-protocol.cc,
which is what we would initially use.

It is a more complete version of what Hydra has today except for one
thing: it always unconditionally sets the start/stop times.

I think that is correct at the other end seems to unconditionally
measure them, but just to be extra careful, I reproduced the old
behavior of falling back on Hydra's own measurements if `startTime` is
0.

The only difference is that the fallback `stopTime` is now measured from
after the entire `BuildResult` is transferred over the wire, but I think
that should be negligible if it is measurable at all. (And remember,
this is fallback case I already suspect is dead code.)
---
 src/hydra-queue-runner/build-remote.cc | 36 ++++++++++----------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc
index 73f46a53..5206ae5a 100644
--- a/src/hydra-queue-runner/build-remote.cc
+++ b/src/hydra-queue-runner/build-remote.cc
@@ -286,9 +286,6 @@ static BuildResult performBuild(
     counter & nrStepsBuilding
 )
 {
-
-    BuildResult result;
-
     conn.to << ServeProto::Command::BuildDerivation << localStore.printStorePath(drvPath);
     writeDerivation(conn.to, localStore, drv);
     conn.to << options.maxSilentTime << options.buildTimeout;
@@ -301,30 +298,25 @@ static BuildResult performBuild(
     }
     conn.to.flush();
 
-    result.startTime = time(0);
+    BuildResult result;
 
+    time_t startTime, stopTime;
+
+    startTime = time(0);
     {
         MaintainCount<counter> mc(nrStepsBuilding);
-        result.status = (BuildResult::Status)readInt(conn.from);
+        result = ServeProto::Serialise<BuildResult>::read(localStore, conn);
     }
-    result.stopTime = time(0);
+    stopTime = time(0);
 
-
-    result.errorMsg = readString(conn.from);
-    if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 3) {
-        result.timesBuilt = readInt(conn.from);
-        result.isNonDeterministic = readInt(conn.from);
-        auto start = readInt(conn.from);
-        auto stop = readInt(conn.from);
-        if (start && start) {
-            /* Note: this represents the duration of a single
-                round, rather than all rounds. */
-            result.startTime = start;
-            result.stopTime = stop;
-        }
-    }
-    if (GET_PROTOCOL_MINOR(conn.remoteVersion) >= 6) {
-        ServeProto::Serialise<DrvOutputs>::read(localStore, conn);
+    if (!result.startTime) {
+        // If the builder gave `startTime = 0`, use our measurements
+        // instead of the builder's.
+        //
+        // Note: this represents the duration of a single round, rather
+        // than all rounds.
+        result.startTime = startTime;
+        result.stopTime = stopTime;
     }
 
     return result;