Factor out serialization for BuildResult

src/libstore/build-result.hh

@@ -64,9 +64,6 @@ struct BuildResult
        non-determinism.) */
     bool isNonDeterministic = false;
 
-    /* The derivation we built or the store path we substituted. */
-    DerivedPath path;
-
     /* For derivations, a mapping from the names of the wanted outputs
        to actual paths. */
     DrvOutputs builtOutputs;
@@ -86,4 +83,11 @@ struct BuildResult
     }
 };
 
+/* A build result together with it's "primary key" */
+struct KeyedBuildResult : BuildResult
+{
+    /* The derivation we built or the store path we substituted. */
+    DerivedPath path;
+};
+
 }

src/libstore/build/derivation-goal.cc

@@ -9,7 +9,8 @@
 #include "archive.hh"
 #include "json.hh"
 #include "compression.hh"
-#include "worker-protocol.hh"
+#include "common-protocol.hh"
+#include "common-protocol-impl.hh"
 #include "topo-sort.hh"
 #include "callback.hh"
 #include "local-store.hh" // TODO remove, along with remaining downcasts
@@ -309,11 +310,14 @@ void DerivationGoal::outputsSubstitutionTried()
     gaveUpOnSubstitution();
 }
 
-
 /* At least one of the output paths could not be
    produced using a substitute.  So we have to build instead. */
 void DerivationGoal::gaveUpOnSubstitution()
 {
+    /* Make sure checkPathValidity() from now on checks all
+       outputs. */
+    wantedOutputs.clear();
+
     /* The inputs must be built before we can build this goal. */
     if (useDerivation)
         for (auto & i : dynamic_cast<Derivation *>(drv.get())->inputDrvs)
@@ -523,8 +527,6 @@ void DerivationGoal::inputsRealised()
        build hook. */
     state = &DerivationGoal::tryToBuild;
     worker.wakeUp(shared_from_this());
-
-    buildResult = BuildResult { .path = buildResult.path };
 }
 
 void DerivationGoal::started()
@@ -1068,9 +1070,11 @@ HookReply DerivationGoal::tryBuildHook()
         throw;
     }
 
+    common_proto::WriteConn conn { hook->sink };
+
     /* Tell the hook all the inputs that have to be copied to the
        remote system. */
-    worker_proto::write(worker.store, hook->sink, inputPaths);
+    common_proto::write(worker.store, conn, inputPaths);
 
     /* Tell the hooks the missing outputs that have to be copied back
        from the remote system. */
@@ -1081,7 +1085,7 @@ HookReply DerivationGoal::tryBuildHook()
             if (buildMode != bmCheck && status.known && status.known->isValid()) continue;
             missingOutputs.insert(outputName);
         }
-        worker_proto::write(worker.store, hook->sink, missingOutputs);
+        common_proto::write(worker.store, conn, missingOutputs);
     }
 
     hook->sink = FdSink();

src/libstore/build/entry-points.cc

@@ -47,31 +47,57 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
     }
 }
 
-std::vector<BuildResult> Store::buildPathsWithResults(
+std::vector<KeyedBuildResult> Store::buildPathsWithResults(
     const std::vector<DerivedPath> & reqs,
     BuildMode buildMode,
     std::shared_ptr<Store> evalStore)
 {
     Worker worker(*this, evalStore ? *evalStore : *this);
 
     Goals goals;
-    for (const auto & br : reqs) {
-        std::visit(overloaded {
-            [&](const DerivedPath::Built & bfd) {
-                goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode));
+    std::vector<std::pair<GoalPtr, const DerivedPath &>> reqs2;
+
+    for (const auto & req : reqs) {
+        auto g = std::visit(overloaded {
+            [&](const DerivedPath::Built & bfd) -> GoalPtr {
+                return worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode);
             },
-            [&](const DerivedPath::Opaque & bo) {
-                goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair));
+            [&](const DerivedPath::Opaque & bo) -> GoalPtr {
+                return worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair);
             },
-        }, br.raw());
+        }, req.raw());
+        goals.insert(g);
+        reqs2.push_back({g, req});
     }
 
     worker.run(goals);
 
-    std::vector<BuildResult> results;
+    std::vector<KeyedBuildResult> results;
 
-    for (auto & i : goals)
-        results.push_back(i->buildResult);
+    for (auto & [gp, req] : reqs2) {
+        auto & br = results.emplace_back(KeyedBuildResult {
+            gp->buildResult,
+            .path = req
+        });
+
+        auto pbp = std::get_if<DerivedPath::Built>(&req);
+        if (!pbp) continue;
+        auto & bp = *pbp;
+
+        /* Because goals are in general shared between derived paths
+           that share the same derivation, we need to filter their
+           results to get back just the results we care about.
+         */
+
+        auto & bos = br.builtOutputs;
+
+        for (auto it = bos.begin(); it != bos.end();) {
+            if (wantOutput(it->first.outputName, bp.outputs))
+                ++it;
+            else
+                it = bos.erase(it);
+        }
+    }
 
     return results;
 }
@@ -89,7 +115,6 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat
         return BuildResult {
             .status = BuildResult::MiscFailure,
             .errorMsg = e.msg(),
-            .path = DerivedPath::Built { .drvPath = drvPath },
         };
     };
 }

src/libstore/build/goal.hh

@@ -64,7 +64,6 @@ struct Goal : public std::enable_shared_from_this<Goal>
 
     Goal(Worker & worker, DerivedPath path)
         : worker(worker)
-        , buildResult { .path = std::move(path) }
     { }
 
     virtual ~Goal()

src/libstore/build/local-derivation-goal.cc

@@ -11,7 +11,8 @@
 #include "json.hh"
 #include "compression.hh"
 #include "daemon.hh"
-#include "worker-protocol.hh"
+#include "common-protocol.hh"
+#include "common-protocol-impl.hh"
 #include "topo-sort.hh"
 #include "callback.hh"
 
@@ -1266,7 +1267,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
                 result.rethrow();
     }
 
-    std::vector<BuildResult> buildPathsWithResults(
+    std::vector<KeyedBuildResult> buildPathsWithResults(
         const std::vector<DerivedPath> & paths,
         BuildMode buildMode = bmNormal,
         std::shared_ptr<Store> evalStore = nullptr) override

src/libstore/common-protocol-impl.hh

@@ -0,0 +1,22 @@
+#pragma once
+
+#include "meta-protocol-templates.hh"
+
+namespace nix {
+namespace common_proto {
+
+/* protocol-agnostic templates */
+
+WRAP_META_PROTO(template<typename T>, std::vector<T>)
+WRAP_META_PROTO(template<typename T>, std::set<T>)
+
+#define X_ template<typename K, typename V>
+#define Y_ std::map<K, V>
+WRAP_META_PROTO(X_, Y_)
+#undef X_
+#undef Y_
+
+/* protocol-specific templates */
+
+}
+}

src/libstore/common-protocol.cc

@@ -0,0 +1,139 @@
+#include "serialise.hh"
+#include "util.hh"
+#include "path-with-outputs.hh"
+#include "store-api.hh"
+#include "build-result.hh"
+#include "common-protocol.hh"
+#include "common-protocol-impl.hh"
+#include "archive.hh"
+#include "derivations.hh"
+
+#include <nlohmann/json.hpp>
+
+namespace nix {
+namespace common_proto {
+
+/* protocol-agnostic definitions */
+
+std::string read(const Store & store, ReadConn conn, Phantom<std::string> _)
+{
+    return readString(conn.from);
+}
+
+void write(const Store & store, WriteConn conn, const std::string & str)
+{
+    conn.to << str;
+}
+
+
+StorePath read(const Store & store, ReadConn conn, Phantom<StorePath> _)
+{
+    return store.parseStorePath(readString(conn.from));
+}
+
+void write(const Store & store, WriteConn conn, const StorePath & storePath)
+{
+    conn.to << store.printStorePath(storePath);
+}
+
+
+ContentAddress read(const Store & store, ReadConn conn, Phantom<ContentAddress> _)
+{
+    return parseContentAddress(readString(conn.from));
+}
+
+void write(const Store & store, WriteConn conn, const ContentAddress & ca)
+{
+    conn.to << renderContentAddress(ca);
+}
+
+DerivedPath read(const Store & store, ReadConn conn, Phantom<DerivedPath> _)
+{
+    auto s = readString(conn.from);
+    return DerivedPath::parse(store, s);
+}
+
+void write(const Store & store, WriteConn conn, const DerivedPath & req)
+{
+    conn.to << req.to_string(store);
+}
+
+
+Realisation read(const Store & store, ReadConn conn, Phantom<Realisation> _)
+{
+    std::string rawInput = readString(conn.from);
+    return Realisation::fromJSON(
+        nlohmann::json::parse(rawInput),
+        "remote-protocol"
+    );
+}
+
+void write(const Store & store, WriteConn conn, const Realisation & realisation)
+{
+    conn.to << realisation.toJSON().dump();
+}
+
+
+DrvOutput read(const Store & store, ReadConn conn, Phantom<DrvOutput> _)
+{
+    return DrvOutput::parse(readString(conn.from));
+}
+
+void write(const Store & store, WriteConn conn, const DrvOutput & drvOutput)
+{
+    conn.to << drvOutput.to_string();
+}
+
+
+std::optional<StorePath> read(const Store & store, ReadConn conn, Phantom<std::optional<StorePath>> _)
+{
+    auto s = readString(conn.from);
+    return s == "" ? std::optional<StorePath> {} : store.parseStorePath(s);
+}
+
+void write(const Store & store, WriteConn conn, const std::optional<StorePath> & storePathOpt)
+{
+    conn.to << (storePathOpt ? store.printStorePath(*storePathOpt) : "");
+}
+
+
+std::optional<ContentAddress> read(const Store & store, ReadConn conn, Phantom<std::optional<ContentAddress>> _)
+{
+    return parseContentAddressOpt(readString(conn.from));
+}
+
+void write(const Store & store, WriteConn conn, const std::optional<ContentAddress> & caOpt)
+{
+    conn.to << (caOpt ? renderContentAddress(*caOpt) : "");
+}
+
+// Helpers for downstream
+
+BuildResult read0(const Store & store, ReadConn conn, Phantom<BuildResult> _)
+{
+    BuildResult res;
+    res.status = (BuildResult::Status) readInt(conn.from);
+    conn.from
+        >> res.errorMsg
+        >> res.timesBuilt
+        >> res.isNonDeterministic
+        >> res.startTime
+        >> res.stopTime;
+    res.builtOutputs = read(store, conn, Phantom<DrvOutputs> {});
+    return res;
+}
+
+void write0(const Store & store, WriteConn conn, const BuildResult & res)
+{
+    conn.to
+        << res.status
+        << res.errorMsg
+        << res.timesBuilt
+        << res.isNonDeterministic
+        << res.startTime
+        << res.stopTime;
+    write(store, conn, res.builtOutputs);
+}
+
+}
+}