Merge pull request #6223 from obsidiansystems/worker-proto-with-version

Give `nix daemon` and `nix-store --serve` protocols separate serializers with version info

(cherry picked from commit 8b68bbb777)
Change-Id: Ia3d3b9fbaf9f0ae62ab225020b7d14790e793655

src/libstore/daemon.cc

@@ -261,18 +261,6 @@ struct ClientSettings
     }
 };
 
-static std::vector<DerivedPath> readDerivedPaths(Store & store, WorkerProto::Version clientVersion, WorkerProto::ReadConn conn)
-{
-    std::vector<DerivedPath> reqs;
-    if (GET_PROTOCOL_MINOR(clientVersion) >= 30) {
-        reqs = WorkerProto::Serialise<std::vector<DerivedPath>>::read(store, conn);
-    } else {
-        for (auto & s : readStrings<Strings>(conn.from))
-            reqs.push_back(parsePathWithOutputs(store, s).toDerivedPath());
-    }
-    return reqs;
-}
-
 static void performOp(TunnelLogger * logger, ref<Store> store,
     TrustedFlag trusted, RecursiveFlag recursive, WorkerProto::Version clientVersion,
     Source & from, BufferedSink & to, WorkerProto::Op op)
@@ -434,7 +422,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
             }();
             logger->stopWork();
 
-            pathInfo->write(to, *store, GET_PROTOCOL_MINOR(clientVersion));
+            WorkerProto::Serialise<ValidPathInfo>::write(*store, wconn, *pathInfo);
         } else {
             HashType hashAlgo;
             std::string baseName;
@@ -538,7 +526,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
     }
 
     case WorkerProto::Op::BuildPaths: {
-        auto drvs = readDerivedPaths(*store, clientVersion, rconn);
+        auto drvs = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
         BuildMode mode = bmNormal;
         if (GET_PROTOCOL_MINOR(clientVersion) >= 15) {
             mode = (BuildMode) readInt(from);
@@ -563,7 +551,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
     }
 
     case WorkerProto::Op::BuildPathsWithResults: {
-        auto drvs = readDerivedPaths(*store, clientVersion, rconn);
+        auto drvs = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
         BuildMode mode = bmNormal;
         mode = (BuildMode) readInt(from);
 
@@ -647,16 +635,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
 
         auto res = store->buildDerivation(drvPath, drv, buildMode);
         logger->stopWork();
-        to << res.status << res.errorMsg;
+        WorkerProto::write(*store, wconn, res);
-        if (GET_PROTOCOL_MINOR(clientVersion) >= 29) {
-            to << res.timesBuilt << res.isNonDeterministic << res.startTime << res.stopTime;
-        }
-        if (GET_PROTOCOL_MINOR(clientVersion) >= 28) {
-            DrvOutputs builtOutputs;
-            for (auto & [output, realisation] : res.builtOutputs)
-                builtOutputs.insert_or_assign(realisation.id, realisation);
-            WorkerProto::write(*store, wconn, builtOutputs);
-        }
         break;
     }
 
@@ -840,7 +819,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         if (info) {
             if (GET_PROTOCOL_MINOR(clientVersion) >= 17)
                 to << 1;
-            info->write(to, *store, GET_PROTOCOL_MINOR(clientVersion), false);
+            WorkerProto::write(*store, wconn, static_cast<const UnkeyedValidPathInfo &>(*info));
         } else {
             assert(GET_PROTOCOL_MINOR(clientVersion) >= 17);
             to << 0;
@@ -938,7 +917,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
     }
 
     case WorkerProto::Op::QueryMissing: {
-        auto targets = readDerivedPaths(*store, clientVersion, rconn);
+        auto targets = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
         logger->startWork();
         StorePathSet willBuild, willSubstitute, unknown;
         uint64_t downloadSize, narSize;

src/libstore/legacy-ssh-store.cc

@@ -319,20 +319,7 @@ public:
 
         conn->to.flush();
 
-        BuildResult status;
+        return ServeProto::Serialise<BuildResult>::read(*this, *conn);
-        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) {
-            auto builtOutputs = ServeProto::Serialise<DrvOutputs>::read(*this, *conn);
-            for (auto && [output, realisation] : builtOutputs)
-                status.builtOutputs.insert_or_assign(
-                    std::move(output.outputName),
-                    std::move(realisation));
-        }
-        return status;
     }
 
     void buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMode buildMode, std::shared_ptr<Store> evalStore) override

src/libstore/path-info.cc

@@ -1,10 +1,27 @@
 #include "path-info.hh"
-#include "worker-protocol.hh"
-#include "worker-protocol-impl.hh"
 #include "store-api.hh"
 
 namespace nix {
 
+GENERATE_CMP_EXT(
+    ,
+    UnkeyedValidPathInfo,
+    me->deriver,
+    me->narHash,
+    me->references,
+    me->registrationTime,
+    me->narSize,
+    //me->id,
+    me->ultimate,
+    me->sigs,
+    me->ca);
+
+GENERATE_CMP_EXT(
+    ,
+    ValidPathInfo,
+    me->path,
+    static_cast<const UnkeyedValidPathInfo &>(*me));
+
 std::string ValidPathInfo::fingerprint(const Store & store) const
 {
     if (narSize == 0)
@@ -99,14 +116,13 @@ Strings ValidPathInfo::shortRefs() const
     return refs;
 }
 
-
 ValidPathInfo::ValidPathInfo(
     const Store & store,
     std::string_view name,
     ContentAddressWithReferences && ca,
     Hash narHash)
-      : path(store.makeFixedOutputPathFromCA(name, ca))
+      : UnkeyedValidPathInfo(narHash)
-      , narHash(narHash)
+      , path(store.makeFixedOutputPathFromCA(name, ca))
 {
     std::visit(overloaded {
         [this](TextInfo && ti) {
@@ -128,55 +144,4 @@ ValidPathInfo::ValidPathInfo(
     }, std::move(ca).raw);
 }
 
-
-ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format)
-{
-    return read(source, store, format, store.parseStorePath(readString(source)));
-}
-
-ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format, StorePath && path)
-{
-    auto deriver = readString(source);
-    auto narHash = Hash::parseAny(readString(source), htSHA256);
-    ValidPathInfo info(path, narHash);
-    if (deriver != "") info.deriver = store.parseStorePath(deriver);
-    info.references = WorkerProto::Serialise<StorePathSet>::read(store,
-        WorkerProto::ReadConn {
-            .from = source,
-            .version = format,
-        });
-    source >> info.registrationTime >> info.narSize;
-    if (format >= 16) {
-        source >> info.ultimate;
-        info.sigs = readStrings<StringSet>(source);
-        info.ca = ContentAddress::parseOpt(readString(source));
-    }
-    return info;
-}
-
-
-void ValidPathInfo::write(
-    Sink & sink,
-    const Store & store,
-    unsigned int format,
-    bool includePath) const
-{
-    if (includePath)
-        sink << store.printStorePath(path);
-    sink << (deriver ? store.printStorePath(*deriver) : "")
-         << narHash.to_string(Base16, false);
-    WorkerProto::write(store,
-        WorkerProto::WriteConn {
-            .to = sink,
-            .version = format,
-        },
-        references);
-    sink << registrationTime << narSize;
-    if (format >= 16) {
-        sink << ultimate
-             << sigs
-             << renderContentAddress(ca);
-    }
-}
-
 }

src/libstore/path-info.hh

@@ -32,9 +32,8 @@ struct SubstitutablePathInfo
 typedef std::map<StorePath, SubstitutablePathInfo> SubstitutablePathInfos;
 
 
-struct ValidPathInfo
+struct UnkeyedValidPathInfo
 {
-    StorePath path;
     std::optional<StorePath> deriver;
     /**
      * \todo document this
@@ -72,13 +71,19 @@ struct ValidPathInfo
      */
     std::optional<ContentAddress> ca;
 
-    bool operator == (const ValidPathInfo & i) const
+    UnkeyedValidPathInfo(const UnkeyedValidPathInfo & other) = default;
-    {
+
-        return
+    UnkeyedValidPathInfo(Hash narHash) : narHash(narHash) { };
-            path == i.path
+
-            && narHash == i.narHash
+    DECLARE_CMP(UnkeyedValidPathInfo);
-            && references == i.references;
+
-    }
+    virtual ~UnkeyedValidPathInfo() { }
+};
+
+struct ValidPathInfo : UnkeyedValidPathInfo {
+    StorePath path;
+
+    DECLARE_CMP(ValidPathInfo);
 
     /**
      * Return a fingerprint of the store path to be used in binary
@@ -92,11 +97,11 @@ struct ValidPathInfo
 
     void sign(const Store & store, const SecretKey & secretKey);
 
-	/**
+    /**
-	 * @return The `ContentAddressWithReferences` that determines the
+     * @return The `ContentAddressWithReferences` that determines the
-	 * store path for a content-addressed store object, `std::nullopt`
+     * store path for a content-addressed store object, `std::nullopt`
-	 * for an input-addressed store object.
+     * for an input-addressed store object.
-	 */
+     */
     std::optional<ContentAddressWithReferences> contentAddressWithReferences() const;
 
     /**
@@ -122,18 +127,13 @@ struct ValidPathInfo
 
     ValidPathInfo(const ValidPathInfo & other) = default;
 
-    ValidPathInfo(StorePath && path, Hash narHash) : path(std::move(path)), narHash(narHash) { };
+    ValidPathInfo(StorePath && path, UnkeyedValidPathInfo info) : UnkeyedValidPathInfo(info), path(std::move(path)) { };
-    ValidPathInfo(const StorePath & path, Hash narHash) : path(path), narHash(narHash) { };
+    ValidPathInfo(const StorePath & path, UnkeyedValidPathInfo info) : UnkeyedValidPathInfo(info), path(path) { };
 
     ValidPathInfo(const Store & store,
         std::string_view name, ContentAddressWithReferences && ca, Hash narHash);
 
     virtual ~ValidPathInfo() { }
-
-    static ValidPathInfo read(Source & source, const Store & store, unsigned int format);
-    static ValidPathInfo read(Source & source, const Store & store, unsigned int format, StorePath && path);
-
-    void write(Sink & sink, const Store & store, unsigned int format, bool includePath = true) const;
 };
 
 typedef std::map<StorePath, ValidPathInfo> ValidPathInfos;

src/libstore/remote-store.cc

@@ -332,7 +332,8 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path,
                 if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path));
             }
             info = std::make_shared<ValidPathInfo>(
-                ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion), StorePath{path}));
+                StorePath{path},
+                WorkerProto::Serialise<UnkeyedValidPathInfo>::read(*this, *conn));
         }
         callback(std::move(info));
     } catch (...) { callback.rethrow(); }
@@ -445,7 +446,7 @@ ref<const ValidPathInfo> RemoteStore::addCAToStore(
         }
 
         return make_ref<ValidPathInfo>(
-            ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion)));
+            WorkerProto::Serialise<ValidPathInfo>::read(*this, *conn));
     }
     else {
         if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25");
@@ -570,7 +571,12 @@ void RemoteStore::addMultipleToStore(
     auto source = sinkToSource([&](Sink & sink) {
         sink << pathsToCopy.size();
         for (auto & [pathInfo, pathSource] : pathsToCopy) {
-            pathInfo.write(sink, *this, 16);
+            WorkerProto::Serialise<ValidPathInfo>::write(*this,
+                 WorkerProto::WriteConn {
+                     .to = sink,
+                     .version = 16,
+                 },
+                 pathInfo);
             pathSource->drainInto(sink);
         }
     });
@@ -655,33 +661,6 @@ void RemoteStore::queryRealisationUncached(const DrvOutput & id,
     } catch (...) { return callback.rethrow(); }
 }
 
-static void writeDerivedPaths(RemoteStore & store, RemoteStore::Connection & conn, const std::vector<DerivedPath> & reqs)
-{
-    if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 30) {
-        WorkerProto::write(store, conn, reqs);
-    } else {
-        Strings ss;
-        for (auto & p : reqs) {
-            auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(p);
-            std::visit(overloaded {
-                [&](const StorePathWithOutputs & s) {
-                    ss.push_back(s.to_string(store));
-                },
-                [&](const StorePath & drvPath) {
-                    throw Error("trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file",
-                        store.printStorePath(drvPath),
-                        GET_PROTOCOL_MAJOR(conn.daemonVersion),
-                        GET_PROTOCOL_MINOR(conn.daemonVersion));
-                },
-                [&](std::monostate) {
-                    throw Error("wanted to build a derivation that is itself a build product, but the legacy 'ssh://' protocol doesn't support that. Try using 'ssh-ng://'");
-                },
-            }, sOrDrvPath);
-        }
-        conn.to << ss;
-    }
-}
-
 void RemoteStore::copyDrvsFromEvalStore(
     const std::vector<DerivedPath> & paths,
     std::shared_ptr<Store> evalStore)
@@ -711,7 +690,7 @@ void RemoteStore::buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMod
     auto conn(getConnection());
     conn->to << WorkerProto::Op::BuildPaths;
     assert(GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13);
-    writeDerivedPaths(*this, *conn, drvPaths);
+    WorkerProto::write(*this, *conn, drvPaths);
     if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15)
         conn->to << buildMode;
     else
@@ -735,7 +714,7 @@ std::vector<KeyedBuildResult> RemoteStore::buildPathsWithResults(
 
     if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 34) {
         conn->to << WorkerProto::Op::BuildPathsWithResults;
-        writeDerivedPaths(*this, *conn, paths);
+        WorkerProto::write(*this, *conn, paths);
         conn->to << buildMode;
         conn.processStderr();
         return WorkerProto::Serialise<std::vector<KeyedBuildResult>>::read(*this, *conn);
@@ -815,20 +794,7 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD
     writeDerivation(conn->to, *this, drv);
     conn->to << buildMode;
     conn.processStderr();
-    BuildResult res;
+    return WorkerProto::Serialise<BuildResult>::read(*this, *conn);
-    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 = WorkerProto::Serialise<DrvOutputs>::read(*this, *conn);
-        for (auto && [output, realisation] : builtOutputs)
-            res.builtOutputs.insert_or_assign(
-                std::move(output.outputName),
-                std::move(realisation));
-    }
-    return res;
 }
 
 
@@ -929,7 +895,7 @@ void RemoteStore::queryMissing(const std::vector<DerivedPath> & targets,
             // to prevent a deadlock.
             goto fallback;
         conn->to << WorkerProto::Op::QueryMissing;
-        writeDerivedPaths(*this, *conn, targets);
+        WorkerProto::write(*this, *conn, targets);
         conn.processStderr();
         willBuild = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);
         willSubstitute = WorkerProto::Serialise<StorePathSet>::read(*this, *conn);

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"
@@ -12,4 +13,46 @@ namespace nix {
 
 /* protocol-specific definitions */
 
+BuildResult ServeProto::Serialise<BuildResult>::read(const Store & store, ServeProto::ReadConn conn)
+{
+    BuildResult status;
+    status.status = (BuildResult::Status) readInt(conn.from);
+    conn.from >> status.errorMsg;
+
+    if (GET_PROTOCOL_MINOR(conn.version) >= 3)
+        conn.from
+            >> status.timesBuilt
+            >> status.isNonDeterministic
+            >> status.startTime
+            >> status.stopTime;
+    if (GET_PROTOCOL_MINOR(conn.version) >= 6) {
+        auto builtOutputs = ServeProto::Serialise<DrvOutputs>::read(store, conn);
+        for (auto && [output, realisation] : builtOutputs)
+            status.builtOutputs.insert_or_assign(
+                std::move(output.outputName),
+                std::move(realisation));
+    }
+    return status;
+}
+
+void ServeProto::Serialise<BuildResult>::write(const Store & store, ServeProto::WriteConn conn, const BuildResult & status)
+{
+    conn.to
+        << status.status
+        << status.errorMsg;
+
+    if (GET_PROTOCOL_MINOR(conn.version) >= 3)
+        conn.to
+            << status.timesBuilt
+            << status.isNonDeterministic
+            << status.startTime
+            << status.stopTime;
+    if (GET_PROTOCOL_MINOR(conn.version) >= 6) {
+        DrvOutputs builtOutputs;
+        for (auto & [output, realisation] : status.builtOutputs)
+            builtOutputs.insert_or_assign(realisation.id, realisation);
+        ServeProto::write(store, conn, builtOutputs);
+    }
+}
+
 }

src/libstore/serve-protocol.hh

@@ -16,6 +16,9 @@ namespace nix {
 class Store;
 struct Source;
 
+// items being serialised
+struct BuildResult;
+
 
 /**
  * The "serve protocol", used by ssh:// stores.
@@ -136,6 +139,9 @@ inline std::ostream & operator << (std::ostream & s, ServeProto::Command op)
         static void write(const Store & store, ServeProto::WriteConn conn, const T & t); \
     };
 
+template<>
+DECLARE_SERVE_SERIALISER(BuildResult);
+
 template<typename T>
 DECLARE_SERVE_SERIALISER(std::vector<T>);
 template<typename T>

src/libstore/store-api.cc

@@ -11,6 +11,9 @@
 #include "archive.hh"
 #include "callback.hh"
 #include "remote-store.hh"
+// FIXME this should not be here, see TODO below on
+// `addMultipleToStore`.
+#include "worker-protocol.hh"
 
 #include <nlohmann/json.hpp>
 #include <regex>
@@ -357,7 +360,13 @@ void Store::addMultipleToStore(
 {
     auto expected = readNum<uint64_t>(source);
     for (uint64_t i = 0; i < expected; ++i) {
-        auto info = ValidPathInfo::read(source, *this, 16);
+        // FIXME we should not be using the worker protocol here, let
+        // alone the worker protocol with a hard-coded version!
+        auto info = WorkerProto::Serialise<ValidPathInfo>::read(*this,
+            WorkerProto::ReadConn {
+                .from = source,
+                .version = 16,
+            });
         info.ultimate = false;
         addToStore(info, source, repair, checkSigs);
     }

src/libstore/worker-protocol.cc

@@ -6,7 +6,7 @@
 #include "worker-protocol.hh"
 #include "worker-protocol-impl.hh"
 #include "archive.hh"
-#include "derivations.hh"
+#include "path-info.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -51,12 +51,34 @@ void WorkerProto::Serialise<std::optional<TrustedFlag>>::write(const Store & sto
 DerivedPath WorkerProto::Serialise<DerivedPath>::read(const Store & store, WorkerProto::ReadConn conn)
 {
     auto s = readString(conn.from);
-    return DerivedPath::parseLegacy(store, s);
+    if (GET_PROTOCOL_MINOR(conn.version) >= 30) {
+        return DerivedPath::parseLegacy(store, s);
+    } else {
+        return parsePathWithOutputs(store, s).toDerivedPath();
+    }
 }
 
 void WorkerProto::Serialise<DerivedPath>::write(const Store & store, WorkerProto::WriteConn conn, const DerivedPath & req)
 {
-    conn.to << req.to_string_legacy(store);
+    if (GET_PROTOCOL_MINOR(conn.version) >= 30) {
+        conn.to << req.to_string_legacy(store);
+    } else {
+        auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(req);
+        std::visit(overloaded {
+            [&](const StorePathWithOutputs & s) {
+                conn.to << s.to_string(store);
+            },
+            [&](const StorePath & drvPath) {
+                throw Error("trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file",
+                    store.printStorePath(drvPath),
+                    GET_PROTOCOL_MAJOR(conn.version),
+                    GET_PROTOCOL_MINOR(conn.version));
+            },
+            [&](std::monostate) {
+                throw Error("wanted to build a derivation that is itself a build product, but protocols do not support that. Try upgrading the Nix on the other end of this connection");
+            },
+        }, sOrDrvPath);
+    }
 }
 
 
@@ -81,17 +103,21 @@ BuildResult WorkerProto::Serialise<BuildResult>::read(const Store & store, Worke
 {
     BuildResult res;
     res.status = (BuildResult::Status) readInt(conn.from);
-    conn.from
+    conn.from >> res.errorMsg;
-        >> res.errorMsg
+    if (GET_PROTOCOL_MINOR(conn.version) >= 29) {
-        >> res.timesBuilt
+        conn.from
-        >> res.isNonDeterministic
+            >> res.timesBuilt
-        >> res.startTime
+            >> res.isNonDeterministic
-        >> res.stopTime;
+            >> res.startTime
-    auto builtOutputs = WorkerProto::Serialise<DrvOutputs>::read(store, conn);
+            >> res.stopTime;
-    for (auto && [output, realisation] : builtOutputs)
+    }
-        res.builtOutputs.insert_or_assign(
+    if (GET_PROTOCOL_MINOR(conn.version) >= 28) {
-            std::move(output.outputName),
+        auto builtOutputs = WorkerProto::Serialise<DrvOutputs>::read(store, conn);
-            std::move(realisation));
+        for (auto && [output, realisation] : builtOutputs)
+            res.builtOutputs.insert_or_assign(
+                std::move(output.outputName),
+                std::move(realisation));
+    }
     return res;
 }
 
@@ -99,16 +125,68 @@ void WorkerProto::Serialise<BuildResult>::write(const Store & store, WorkerProto
 {
     conn.to
         << res.status
-        << res.errorMsg
+        << res.errorMsg;
-        << res.timesBuilt
+    if (GET_PROTOCOL_MINOR(conn.version) >= 29) {
-        << res.isNonDeterministic
+        conn.to
-        << res.startTime
+            << res.timesBuilt
-        << res.stopTime;
+            << res.isNonDeterministic
-    DrvOutputs builtOutputs;
+            << res.startTime
-    for (auto & [output, realisation] : res.builtOutputs)
+            << res.stopTime;
-        builtOutputs.insert_or_assign(realisation.id, realisation);
+    }
-    WorkerProto::write(store, conn, builtOutputs);
+    if (GET_PROTOCOL_MINOR(conn.version) >= 28) {
+        DrvOutputs builtOutputs;
+        for (auto & [output, realisation] : res.builtOutputs)
+            builtOutputs.insert_or_assign(realisation.id, realisation);
+        WorkerProto::write(store, conn, builtOutputs);
+    }
 }
 
 
+ValidPathInfo WorkerProto::Serialise<ValidPathInfo>::read(const Store & store, ReadConn conn)
+{
+    auto path = WorkerProto::Serialise<StorePath>::read(store, conn);
+    return ValidPathInfo {
+        std::move(path),
+        WorkerProto::Serialise<UnkeyedValidPathInfo>::read(store, conn),
+    };
+}
+
+void WorkerProto::Serialise<ValidPathInfo>::write(const Store & store, WriteConn conn, const ValidPathInfo & pathInfo)
+{
+    WorkerProto::write(store, conn, pathInfo.path);
+    WorkerProto::write(store, conn, static_cast<const UnkeyedValidPathInfo &>(pathInfo));
+}
+
+
+UnkeyedValidPathInfo WorkerProto::Serialise<UnkeyedValidPathInfo>::read(const Store & store, ReadConn conn)
+{
+    auto deriver = readString(conn.from);
+    auto narHash = Hash::parseAny(readString(conn.from), htSHA256);
+    UnkeyedValidPathInfo info(narHash);
+    if (deriver != "") info.deriver = store.parseStorePath(deriver);
+    info.references = WorkerProto::Serialise<StorePathSet>::read(store, conn);
+    conn.from >> info.registrationTime >> info.narSize;
+    if (GET_PROTOCOL_MINOR(conn.version) >= 16) {
+        conn.from >> info.ultimate;
+        info.sigs = readStrings<StringSet>(conn.from);
+        info.ca = ContentAddress::parseOpt(readString(conn.from));
+    }
+    return info;
+}
+
+void WorkerProto::Serialise<UnkeyedValidPathInfo>::write(const Store & store, WriteConn conn, const UnkeyedValidPathInfo & pathInfo)
+{
+    conn.to
+        << (pathInfo.deriver ? store.printStorePath(*pathInfo.deriver) : "")
+        << pathInfo.narHash.to_string(Base16, false);
+    WorkerProto::write(store, conn, pathInfo.references);
+    conn.to << pathInfo.registrationTime << pathInfo.narSize;
+    if (GET_PROTOCOL_MINOR(conn.version) >= 16) {
+        conn.to
+            << pathInfo.ultimate
+            << pathInfo.sigs
+            << renderContentAddress(pathInfo.ca);
+    }
+}
+
 }

src/libstore/worker-protocol.hh

@@ -31,6 +31,8 @@ struct Source;
 struct DerivedPath;
 struct BuildResult;
 struct KeyedBuildResult;
+struct ValidPathInfo;
+struct UnkeyedValidPathInfo;
 enum TrustedFlag : bool;
 
 
@@ -206,6 +208,10 @@ DECLARE_WORKER_SERIALISER(BuildResult);
 template<>
 DECLARE_WORKER_SERIALISER(KeyedBuildResult);
 template<>
+DECLARE_WORKER_SERIALISER(ValidPathInfo);
+template<>
+DECLARE_WORKER_SERIALISER(UnkeyedValidPathInfo);
+template<>
 DECLARE_WORKER_SERIALISER(std::optional<TrustedFlag>);
 
 template<typename T>

src/nix-store/nix-store.cc

@@ -959,17 +959,7 @@ static void opServe(Strings opFlags, Strings opArgs)
                 MonitorFdHup monitor(in.fd);
                 auto status = store->buildDerivation(drvPath, drv);
 
-                out << status.status << status.errorMsg;
+                ServeProto::write(*store, wconn, status);
-
-                if (GET_PROTOCOL_MINOR(clientVersion) >= 3)
-                    out << status.timesBuilt << status.isNonDeterministic << status.startTime << status.stopTime;
-                if (GET_PROTOCOL_MINOR(clientVersion) >= 6) {
-                    DrvOutputs builtOutputs;
-                    for (auto & [output, realisation] : status.builtOutputs)
-                        builtOutputs.insert_or_assign(realisation.id, realisation);
-                    ServeProto::write(*store, wconn, builtOutputs);
-                }
-
                 break;
             }
 

tests/unit/libstore/serve-protocol.cc

@@ -19,7 +19,7 @@ struct ServeProtoTest : VersionedProtoTest<ServeProto, serveProtoDir>
      * For serializers that don't care about the minimum version, we
      * used the oldest one: 1.0.
      */
-    ServeProto::Version defaultVersion = 1 << 8 | 0;
+    ServeProto::Version defaultVersion = 2 << 8 | 0;
 };
 
 VERSIONED_CHARACTERIZATION_TEST(
@@ -114,6 +114,117 @@ VERSIONED_CHARACTERIZATION_TEST(
         },
     }))
 
+VERSIONED_CHARACTERIZATION_TEST(
+    ServeProtoTest,
+    buildResult_2_2,
+    "build-result-2.2",
+    2 << 8 | 2,
+    ({
+        using namespace std::literals::chrono_literals;
+        std::tuple<BuildResult, BuildResult, BuildResult> t {
+            BuildResult {
+                .status = BuildResult::OutputRejected,
+                .errorMsg = "no idea why",
+            },
+            BuildResult {
+                .status = BuildResult::NotDeterministic,
+                .errorMsg = "no idea why",
+            },
+            BuildResult {
+                .status = BuildResult::Built,
+            },
+        };
+        t;
+    }))
+
+VERSIONED_CHARACTERIZATION_TEST(
+    ServeProtoTest,
+    buildResult_2_3,
+    "build-result-2.3",
+    2 << 8 | 3,
+    ({
+        using namespace std::literals::chrono_literals;
+        std::tuple<BuildResult, BuildResult, BuildResult> t {
+            BuildResult {
+                .status = BuildResult::OutputRejected,
+                .errorMsg = "no idea why",
+            },
+            BuildResult {
+                .status = BuildResult::NotDeterministic,
+                .errorMsg = "no idea why",
+                .timesBuilt = 3,
+                .isNonDeterministic = true,
+                .startTime = 30,
+                .stopTime = 50,
+            },
+            BuildResult {
+                .status = BuildResult::Built,
+                .startTime = 30,
+                .stopTime = 50,
+            },
+        };
+        t;
+    }))
+
+VERSIONED_CHARACTERIZATION_TEST(
+    ServeProtoTest,
+    buildResult_2_6,
+    "build-result-2.6",
+    2 << 8 | 6,
+    ({
+        using namespace std::literals::chrono_literals;
+        std::tuple<BuildResult, BuildResult, BuildResult> t {
+            BuildResult {
+                .status = BuildResult::OutputRejected,
+                .errorMsg = "no idea why",
+            },
+            BuildResult {
+                .status = BuildResult::NotDeterministic,
+                .errorMsg = "no idea why",
+                .timesBuilt = 3,
+                .isNonDeterministic = true,
+                .startTime = 30,
+                .stopTime = 50,
+            },
+            BuildResult {
+                .status = BuildResult::Built,
+                .timesBuilt = 1,
+                .builtOutputs = {
+                    {
+                        "foo",
+                        {
+                            .id = DrvOutput {
+                                .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="),
+                                .outputName = "foo",
+                            },
+                            .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" },
+                        },
+                    },
+                    {
+                        "bar",
+                        {
+                            .id = DrvOutput {
+                                .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="),
+                                .outputName = "bar",
+                            },
+                            .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" },
+                        },
+                    },
+                },
+                .startTime = 30,
+                .stopTime = 50,
+#if 0
+                // These fields are not yet serialized.
+                // FIXME Include in next version of protocol or document
+                // why they are skipped.
+                .cpuUser = std::chrono::milliseconds(500s),
+                .cpuSystem = std::chrono::milliseconds(604s),
+#endif
+            },
+        };
+        t;
+    }))
+
 VERSIONED_CHARACTERIZATION_TEST(
     ServeProtoTest,
     vector,

tests/unit/libstore/worker-protocol.cc

@@ -69,13 +69,45 @@ VERSIONED_CHARACTERIZATION_TEST(
 
 VERSIONED_CHARACTERIZATION_TEST(
     WorkerProtoTest,
-    derivedPath,
+    derivedPath_1_29,
-    "derived-path",
+    "derived-path-1.29",
-    defaultVersion,
+    1 << 8 | 29,
-    (std::tuple<DerivedPath, DerivedPath> {
+    (std::tuple<DerivedPath, DerivedPath, DerivedPath> {
         DerivedPath::Opaque {
             .path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" },
         },
+        DerivedPath::Built {
+            .drvPath = makeConstantStorePathRef(StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            }),
+            .outputs = OutputsSpec::All { },
+        },
+        DerivedPath::Built {
+            .drvPath = makeConstantStorePathRef(StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            }),
+            .outputs = OutputsSpec::Names { "x", "y" },
+        },
+    }))
+
+VERSIONED_CHARACTERIZATION_TEST(
+    WorkerProtoTest,
+    derivedPath_1_30,
+    "derived-path-1.30",
+    1 << 8 | 30,
+    (std::tuple<DerivedPath, DerivedPath, DerivedPath, DerivedPath> {
+        DerivedPath::Opaque {
+            .path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" },
+        },
+        DerivedPath::Opaque {
+            .path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" },
+        },
+        DerivedPath::Built {
+            .drvPath = makeConstantStorePathRef(StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            }),
+            .outputs = OutputsSpec::All { },
+        },
         DerivedPath::Built {
             .drvPath = makeConstantStorePathRef(StorePath {
                 "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
@@ -135,9 +167,77 @@ VERSIONED_CHARACTERIZATION_TEST(
 
 VERSIONED_CHARACTERIZATION_TEST(
     WorkerProtoTest,
-    buildResult,
+    buildResult_1_27,
-    "build-result",
+    "build-result-1.27",
-    defaultVersion,
+    1 << 8 | 27,
+    ({
+        using namespace std::literals::chrono_literals;
+        std::tuple<BuildResult, BuildResult, BuildResult> t {
+            BuildResult {
+                .status = BuildResult::OutputRejected,
+                .errorMsg = "no idea why",
+            },
+            BuildResult {
+                .status = BuildResult::NotDeterministic,
+                .errorMsg = "no idea why",
+            },
+            BuildResult {
+                .status = BuildResult::Built,
+            },
+        };
+        t;
+    }))
+
+VERSIONED_CHARACTERIZATION_TEST(
+    WorkerProtoTest,
+    buildResult_1_28,
+    "build-result-1.28",
+    1 << 8 | 28,
+    ({
+        using namespace std::literals::chrono_literals;
+        std::tuple<BuildResult, BuildResult, BuildResult> t {
+            BuildResult {
+                .status = BuildResult::OutputRejected,
+                .errorMsg = "no idea why",
+            },
+            BuildResult {
+                .status = BuildResult::NotDeterministic,
+                .errorMsg = "no idea why",
+            },
+            BuildResult {
+                .status = BuildResult::Built,
+                .builtOutputs = {
+                    {
+                        "foo",
+                        {
+                            .id = DrvOutput {
+                                .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="),
+                                .outputName = "foo",
+                            },
+                            .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" },
+                        },
+                    },
+                    {
+                        "bar",
+                        {
+                            .id = DrvOutput {
+                                .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="),
+                                .outputName = "bar",
+                            },
+                            .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" },
+                        },
+                    },
+                },
+            },
+        };
+        t;
+    }))
+
+VERSIONED_CHARACTERIZATION_TEST(
+    WorkerProtoTest,
+    buildResult_1_29,
+    "build-result-1.29",
+    1 << 8 | 29,
     ({
         using namespace std::literals::chrono_literals;
         std::tuple<BuildResult, BuildResult, BuildResult> t {
@@ -194,9 +294,9 @@ VERSIONED_CHARACTERIZATION_TEST(
 
 VERSIONED_CHARACTERIZATION_TEST(
     WorkerProtoTest,
-    keyedBuildResult,
+    keyedBuildResult_1_29,
-    "keyed-build-result",
+    "keyed-build-result-1.29",
-    defaultVersion,
+    1 << 8 | 29,
     ({
         using namespace std::literals::chrono_literals;
         std::tuple<KeyedBuildResult, KeyedBuildResult/*, KeyedBuildResult*/> t {
@@ -229,6 +329,159 @@ VERSIONED_CHARACTERIZATION_TEST(
         t;
     }))
 
+VERSIONED_CHARACTERIZATION_TEST(
+    WorkerProtoTest,
+    unkeyedValidPathInfo_1_15,
+    "unkeyed-valid-path-info-1.15",
+    1 << 8 | 15,
+    (std::tuple<UnkeyedValidPathInfo, UnkeyedValidPathInfo> {
+        ({
+            UnkeyedValidPathInfo info {
+                Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+            };
+            info.registrationTime = 23423;
+            info.narSize = 34878;
+            info;
+        }),
+        ({
+            UnkeyedValidPathInfo info {
+                Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+            };
+            info.deriver = StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            };
+            info.references = {
+                StorePath {
+                    "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo.drv",
+                },
+            };
+            info.registrationTime = 23423;
+            info.narSize = 34878;
+            info;
+        }),
+    }))
+
+VERSIONED_CHARACTERIZATION_TEST(
+    WorkerProtoTest,
+    validPathInfo_1_15,
+    "valid-path-info-1.15",
+    1 << 8 | 15,
+    (std::tuple<ValidPathInfo, ValidPathInfo> {
+        ({
+            ValidPathInfo info {
+                StorePath {
+                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
+                },
+                UnkeyedValidPathInfo {
+                    Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+                },
+            };
+            info.registrationTime = 23423;
+            info.narSize = 34878;
+            info;
+        }),
+        ({
+            ValidPathInfo info {
+                StorePath {
+                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
+                },
+                UnkeyedValidPathInfo {
+                    Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+                },
+            };
+            info.deriver = StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            };
+            info.references = {
+                // other reference
+                StorePath {
+                    "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo",
+                },
+                // self reference
+                StorePath {
+                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
+                },
+            };
+            info.registrationTime = 23423;
+            info.narSize = 34878;
+            info;
+        }),
+    }))
+
+VERSIONED_CHARACTERIZATION_TEST(
+    WorkerProtoTest,
+    validPathInfo_1_16,
+    "valid-path-info-1.16",
+    1 << 8 | 16,
+    (std::tuple<ValidPathInfo, ValidPathInfo, ValidPathInfo> {
+        ({
+            ValidPathInfo info {
+                StorePath {
+                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
+                },
+                UnkeyedValidPathInfo {
+                    Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+                },
+            };
+            info.registrationTime = 23423;
+            info.narSize = 34878;
+            info.ultimate = true;
+            info;
+        }),
+        ({
+            ValidPathInfo info {
+                StorePath {
+                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
+                },
+                UnkeyedValidPathInfo {
+                    Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+                },
+            };
+            info.deriver = StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            };
+            info.references = {
+                // other reference
+                StorePath {
+                    "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo",
+                },
+                // self reference
+                StorePath {
+                    "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
+                },
+            };
+            info.registrationTime = 23423;
+            info.narSize = 34878;
+            info.sigs = {
+                "fake-sig-1",
+                "fake-sig-2",
+            },
+            info;
+        }),
+        ({
+            ValidPathInfo info {
+                *LibStoreTest::store,
+                "foo",
+                FixedOutputInfo {
+                    .method = FileIngestionMethod::Recursive,
+                    .hash = hashString(HashType::htSHA256, "(...)"),
+                    .references = {
+                        .others = {
+                            StorePath {
+                                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
+                            },
+                        },
+                        .self = true,
+                    },
+                },
+                Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+            };
+            info.registrationTime = 23423;
+            info.narSize = 34878;
+            info;
+        }),
+    }))
+
 VERSIONED_CHARACTERIZATION_TEST(
     WorkerProtoTest,
     optionalTrustedFlag,