Merge pull request #6312 from obsidiansystems/keyed-build-result

Shuffle `BuildResult` data definition, make state machine clearer, introduce `SingleDrvOutputs`

1 files changed, 47 insertions, 23 deletions

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index d0ce0bce9..a6e8b9577 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -125,10 +125,26 @@ void write(const Store & store, Sink & out, const DrvOutput & drvOutput)
 }
 
 
-BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _)
+KeyedBuildResult read(const Store & store, Source & from, Phantom<KeyedBuildResult> _)
 {
     auto path = worker_proto::read(store, from, Phantom<DerivedPath> {});
-    BuildResult res { .path = path };
+    auto br = worker_proto::read(store, from, Phantom<BuildResult> {});
+    return KeyedBuildResult {
+        std::move(br),
+        /* .path = */ std::move(path),
+    };
+}
+
+void write(const Store & store, Sink & to, const KeyedBuildResult & res)
+{
+    worker_proto::write(store, to, res.path);
+    worker_proto::write(store, to, static_cast<const BuildResult &>(res));
+}
+
+
+BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _)
+{
+    BuildResult res;
     res.status = (BuildResult::Status) readInt(from);
     from
         >> res.errorMsg
@@ -136,13 +152,16 @@ BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _)
         >> res.isNonDeterministic
         >> res.startTime
         >> res.stopTime;
-    res.builtOutputs = worker_proto::read(store, from, Phantom<DrvOutputs> {});
+    auto builtOutputs = worker_proto::read(store, from, Phantom<DrvOutputs> {});
+    for (auto && [output, realisation] : builtOutputs)
+        res.builtOutputs.insert_or_assign(
+            std::move(output.outputName),
+            std::move(realisation));
     return res;
 }
 
 void write(const Store & store, Sink & to, const BuildResult & res)
 {
-    worker_proto::write(store, to, res.path);
     to
         << res.status
         << res.errorMsg
@@ -150,7 +169,10 @@ void write(const Store & store, Sink & to, const BuildResult & res)
         << res.isNonDeterministic
         << res.startTime
         << res.stopTime;
-    worker_proto::write(store, to, res.builtOutputs);
+    DrvOutputs builtOutputs;
+    for (auto & [output, realisation] : res.builtOutputs)
+        builtOutputs.insert_or_assign(realisation.id, realisation);
+    worker_proto::write(store, to, builtOutputs);
 }
 
 
@@ -865,7 +887,7 @@ void RemoteStore::buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMod
     readInt(conn->from);
 }
 
-std::vector<BuildResult> RemoteStore::buildPathsWithResults(
+std::vector<KeyedBuildResult> RemoteStore::buildPathsWithResults(
     const std::vector<DerivedPath> & paths,
     BuildMode buildMode,
     std::shared_ptr<Store> evalStore)
@@ -880,7 +902,7 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults(
         writeDerivedPaths(*this, conn, paths);
         conn->to << buildMode;
         conn.processStderr();
-        return worker_proto::read(*this, conn->from, Phantom<std::vector<BuildResult>> {});
+        return worker_proto::read(*this, conn->from, Phantom<std::vector<KeyedBuildResult>> {});
     } else {
         // Avoid deadlock.
         conn_.reset();
@@ -889,21 +911,25 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults(
         // fails, but meh.
         buildPaths(paths, buildMode, evalStore);
 
-        std::vector<BuildResult> results;
+        std::vector<KeyedBuildResult> results;
 
         for (auto & path : paths) {
             std::visit(
                 overloaded {
                     [&](const DerivedPath::Opaque & bo) {
-                        results.push_back(BuildResult {
-                            .status = BuildResult::Substituted,
-                            .path = bo,
+                        results.push_back(KeyedBuildResult {
+                            {
+                                .status = BuildResult::Substituted,
+                            },
+                            /* .path = */ bo,
                         });
                     },
                     [&](const DerivedPath::Built & bfd) {
-                        BuildResult res {
-                            .status = BuildResult::Built,
-                            .path = bfd,
+                        KeyedBuildResult res {
+                            {
+                                .status = BuildResult::Built
+                            },
+                            /* .path = */ bfd,
                         };
 
                         OutputPathMap outputs;
@@ -922,10 +948,10 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults(
                                     queryRealisation(outputId);
                                 if (!realisation)
                                     throw MissingRealisation(outputId);
-                                res.builtOutputs.emplace(realisation->id, *realisation);
+                                res.builtOutputs.emplace(output, *realisation);
                             } else {
                                 res.builtOutputs.emplace(
-                                    outputId,
+                                    output,
                                     Realisation {
                                         .id = outputId,
                                         .outPath = outputPath,
@@ -952,12 +978,7 @@ 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,
-            .outputs = OutputsSpec::All { },
-        },
-    };
+    BuildResult res;
     res.status = (BuildResult::Status) readInt(conn->from);
     conn->from >> res.errorMsg;
     if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) {
@@ -965,7 +986,10 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD
     }
     if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 28) {
         auto builtOutputs = worker_proto::read(*this, conn->from, Phantom<DrvOutputs> {});
-        res.builtOutputs = builtOutputs;
+        for (auto && [output, realisation] : builtOutputs)
+            res.builtOutputs.insert_or_assign(
+                std::move(output.outputName),
+                std::move(realisation));
     }
     return res;
 }