Factor out serialization for BuildResult

With the differences between the two protocols reconciled, I also factor
out a `worker_proto` for `BuildResult` to be used in the latest versions
of each protocol.

Author: Ericson2314

src/libstore/daemon.cc

@@ -614,13 +614,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
 
         auto res = store->buildDerivation(drvPath, drv, buildMode);
         logger->stopWork();
-        to << res.status << res.errorMsg;
-        if (GET_PROTOCOL_MINOR(clientVersion) >= 29) {
-            to << res.timesBuilt << res.isNonDeterministic << res.startTime << res.stopTime;
-        }
-        if (GET_PROTOCOL_MINOR(clientVersion) >= 28) {
-            worker_proto::write(*store, clientVersion, to, res.builtOutputs);
-        }
+        worker_proto::writeWithoutReq(*store, clientVersion, to, res);
         break;
     }
 

src/libstore/gen-protocol.cc-inc

@@ -90,3 +90,32 @@ void write(const Store & store, unsigned int version, Sink & out, const std::opt
 {
     out << (caOpt ? renderContentAddress(*caOpt) : "");
 }
+
+/* N suffix indicates this is the nth version of them in common. Wrapped downstream
+   by protocol-specific functions which handle the previous versions too. */
+
+BuildResult read0(const Store & store, unsigned int version, Source & from, Phantom<BuildResult> _, DerivedPath req)
+{
+    BuildResult res { .path = req };
+    res.status = (BuildResult::Status) readInt(from);
+    from
+        >> res.errorMsg
+        >> res.timesBuilt
+        >> res.isNonDeterministic
+        >> res.startTime
+        >> res.stopTime;
+    res.builtOutputs = read(store, version, from, Phantom<DrvOutputs> {});
+    return res;
+}
+
+void write0(const Store & store, unsigned int version, Sink & out, const BuildResult & res)
+{
+    out
+        << res.status
+        << res.errorMsg
+        << res.timesBuilt
+        << res.isNonDeterministic
+        << res.startTime
+        << res.stopTime;
+    write(store, version, out, res.builtOutputs);
+}

src/libstore/legacy-ssh-store.cc

@@ -279,16 +279,8 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
 
         conn->to.flush();
 
-        BuildResult status { .path = DerivedPath::Built { .drvPath = drvPath } };
-        status.status = (BuildResult::Status) readInt(conn->from);
-        conn->from >> status.errorMsg;
-
-        if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3)
-            conn->from >> status.timesBuilt >> status.isNonDeterministic >> status.startTime >> status.stopTime;
-        if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 6) {
-            status.builtOutputs = serve_proto::read(*this, conn->remoteVersion, conn->from, Phantom<DrvOutputs> {});
-        }
-        return status;
+        return serve_proto::read(*this, conn->remoteVersion, conn->from, Phantom<BuildResult> {},
+            DerivedPath::Built { .drvPath = drvPath });
     }
 
     void buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMode buildMode, std::shared_ptr<Store> evalStore) override

src/libstore/remote-store.cc

@@ -778,17 +778,8 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD
     writeDerivation(conn->to, *this, drv);
     conn->to << buildMode;
     conn.processStderr();
-    BuildResult res { .path = DerivedPath::Built { .drvPath = drvPath } };
-    res.status = (BuildResult::Status) readInt(conn->from);
-    conn->from >> res.errorMsg;
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) {
-        conn->from >> res.timesBuilt >> res.isNonDeterministic >> res.startTime >> res.stopTime;
-    }
-    if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 28) {
-        auto builtOutputs = worker_proto::read(*this, conn->daemonVersion, conn->from, Phantom<DrvOutputs> {});
-        res.builtOutputs = builtOutputs;
-    }
-    return res;
+    return worker_proto::read(*this, conn->daemonVersion, conn->from, Phantom<BuildResult>{},
+        DerivedPath::Built { .drvPath = drvPath });
 }
 
 

src/libstore/serve-protocol.cc

@@ -2,6 +2,7 @@
 #include "util.hh"
 #include "path-with-outputs.hh"
 #include "store-api.hh"
+#include "build-result.hh"
 #include "serve-protocol.hh"
 #include "serve-protocol-impl.hh"
 #include "archive.hh"
@@ -17,6 +18,33 @@ namespace serve_proto {
 
 /* protocol-specific definitions */
 
+BuildResult read(const Store & store, unsigned int version, Source & from, Phantom<BuildResult> _, DerivedPath req)
+{
+    if (GET_PROTOCOL_MINOR(version) < 6) {
+        BuildResult status { .path = req };
+        status.status = (BuildResult::Status) readInt(from);
+        from >> status.errorMsg;
+
+        if (GET_PROTOCOL_MINOR(version) >= 3)
+            from >> status.timesBuilt >> status.isNonDeterministic >> status.startTime >> status.stopTime;
+        return status;
+    } else
+        return serve_proto::read0(store, version, from, Phantom<BuildResult> {}, req);
+}
+
+void writeWithoutReq(const Store & store, unsigned int version, Sink & out, const BuildResult & status)
+{
+    if (GET_PROTOCOL_MINOR(version < 6)) {
+        out << status.status << status.errorMsg;
+
+        if (GET_PROTOCOL_MINOR(version) >= 3)
+            out << status.timesBuilt << status.isNonDeterministic << status.startTime << status.stopTime;
+    } else {
+        serve_proto::write0(store, version, out, status);
+    }
+}
+
+
 ValidPathInfo readValidPathInfo(const Store & store, unsigned int version, Source & source)
 {
     auto path = read(store, version, source, Phantom<StorePath>{});

src/libstore/serve-protocol.hh

@@ -33,6 +33,7 @@ struct Source;
 struct DerivedPath;
 struct DrvOutput;
 struct Realisation;
+struct BuildResult;
 struct PathInfo;
 
 
@@ -66,6 +67,9 @@ MAKE_SERVE_PROTO(, std::optional<ContentAddress>);
 
 /* These are a non-standard form for historical reasons. */
 
+BuildResult read(const Store & store, unsigned int version, Source & from, Phantom<BuildResult> _, DerivedPath req);
+void writeWithoutReq(const Store & store, unsigned int version, Sink & out, const BuildResult & str);
+
 ValidPathInfo readValidPathInfo(const Store & store, unsigned int version, Source & source);
 ValidPathInfo readValidPathInfo(const Store & store, unsigned int version, Source & source, StorePath && path);
 

src/libstore/worker-protocol.cc

@@ -18,32 +18,42 @@ namespace worker_proto {
 
 /* protocol-specific definitions */
 
+BuildResult read(const Store & store, unsigned int version, Source & from, Phantom<BuildResult> _, DerivedPath req)
+{
+    if (GET_PROTOCOL_MINOR(version) < 29) {
+        BuildResult res { .path = req };
+        res.status = (BuildResult::Status) readInt(from);
+        from >> res.errorMsg;
+        if (GET_PROTOCOL_MINOR(version) >= 28) {
+            auto builtOutputs = read(store, version, from, Phantom<DrvOutputs> {});
+            res.builtOutputs = builtOutputs;
+        }
+        return res;
+    } else
+        return read0(store, version, from, Phantom<BuildResult>{}, req);
+}
+
+void writeWithoutReq(const Store & store, unsigned int version, Sink & out, const BuildResult & res)
+{
+    if (GET_PROTOCOL_MINOR(version) < 29) {
+        out << res.status << res.errorMsg;
+        if (GET_PROTOCOL_MINOR(version) >= 28) {
+            write(store, version, out, res.builtOutputs);
+        }
+    } else
+        write0(store, version, out, res);
+}
+
 BuildResult read(const Store & store, unsigned int version, Source & from, Phantom<BuildResult> _)
 {
-    auto path = worker_proto::read(store, version, from, Phantom<DerivedPath> {});
-    BuildResult res { .path = path };
-    res.status = (BuildResult::Status) readInt(from);
-    from
-        >> res.errorMsg
-        >> res.timesBuilt
-        >> res.isNonDeterministic
-        >> res.startTime
-        >> res.stopTime;
-    res.builtOutputs = worker_proto::read(store, version, from, Phantom<DrvOutputs> {});
-    return res;
+    auto req = read(store, version, from, Phantom<DerivedPath> {});
+    return read(store, version, from, Phantom<BuildResult> {}, req);
 }
 
 void write(const Store & store, unsigned int version, Sink & to, const BuildResult & res)
 {
-    worker_proto::write(store, version, to, res.path);
-    to
-        << res.status
-        << res.errorMsg
-        << res.timesBuilt
-        << res.isNonDeterministic
-        << res.startTime
-        << res.stopTime;
-    worker_proto::write(store, version, to, res.builtOutputs);
+    write(store, version, to, res.path);
+    writeWithoutReq(store, version, to, res);
 }
 
 

src/libstore/worker-protocol.hh

@@ -124,6 +124,9 @@ MAKE_WORKER_PROTO(, std::optional<ContentAddress>);
 
 /* These are a non-standard form for historical reasons. */
 
+BuildResult read(const Store & store, unsigned int version, Source & from, Phantom<BuildResult> _, DerivedPath req);
+void writeWithoutReq(const Store & store, unsigned int version, Sink & out, const BuildResult & str);
+
 ValidPathInfo readValidPathInfo(const Store & store, unsigned int version, Source & source);
 ValidPathInfo readValidPathInfo(const Store & store, unsigned int version, Source & source, StorePath && path);
 

src/nix-store/nix-store.cc

@@ -914,14 +914,7 @@ static void opServe(Strings opFlags, Strings opArgs)
                 MonitorFdHup monitor(in.fd);
                 auto status = store->buildDerivation(drvPath, drv);
 
-                out << status.status << status.errorMsg;
-
-                if (GET_PROTOCOL_MINOR(clientVersion) >= 3)
-                    out << status.timesBuilt << status.isNonDeterministic << status.startTime << status.stopTime;
-                if (GET_PROTOCOL_MINOR(clientVersion >= 6)) {
-                    serve_proto::write(*store, clientVersion, out, status.builtOutputs);
-                }
-
+                serve_proto::writeWithoutReq(*store, clientVersion, out, status);
 
                 break;
             }