Systematize the worker protocol derived path serialiser

It was some ad-hoc functions to account for versions, while the already
factored-out serializer just supported the latest version.

Now, we can fold that version-specific logic into the factored out one,
and so we do.

Author: Ericson2314

src/libstore/daemon.cc

@@ -261,18 +261,6 @@ struct ClientSettings
     }
 };
 
-static std::vector<DerivedPath> readDerivedPaths(Store & store, unsigned int 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, unsigned int clientVersion,
     Source & from, BufferedSink & to, WorkerProto::Op op)
@@ -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);
 
@@ -929,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/remote-store.cc

@@ -655,33 +655,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 +684,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 +708,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);
@@ -916,7 +889,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/tests/worker-protocol.cc

@@ -69,9 +69,32 @@ VERSIONED_CHARACTERIZATION_TEST(
 
 VERSIONED_CHARACTERIZATION_TEST(
     WorkerProtoTest,
-    derivedPath,
-    "derived-path",
-    defaultVersion,
+    derivedPath_1_29,
+    "derived-path-1.29",
+    1 << 8 | 29,
+    (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" },

src/libstore/worker-protocol.cc

@@ -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);
+    }
 }