Start factoring out the serve protocol for Hydra to share

Factor out `ServeProto::BasicClientConnection` for Hydra to share

- `handshake` Separate error handling logic a good bit and make (I
  hope!) more robust.

- `queryValidPaths`: Hydra uses the lock argument differently than Nix,
  so we un-hard-code it.

- `buildDerivationRequest`: Just the request half, as Hydra does some
  things between requesting and responding.

src/libstore/legacy-ssh-store.cc

@@ -40,45 +40,10 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
     // the documentation
     const Setting<int> logFD{this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"};
 
-    struct Connection
+    struct Connection : public ServeProto::BasicClientConnection
     {
         std::unique_ptr<SSHMaster::Connection> sshConn;
-        FdSink to;
-        FdSource from;
-        ServeProto::Version remoteVersion;
         bool good = true;
-
-        /**
-         * Coercion to `ServeProto::ReadConn`. This makes it easy to use the
-         * factored out serve protocol searlizers with a
-         * `LegacySSHStore::Connection`.
-         *
-         * The serve protocol connection types are unidirectional, unlike
-         * this type.
-         */
-        operator ServeProto::ReadConn ()
-        {
-            return ServeProto::ReadConn {
-                .from = from,
-                .version = remoteVersion,
-            };
-        }
-
-        /*
-         * Coercion to `ServeProto::WriteConn`. This makes it easy to use the
-         * factored out serve protocol searlizers with a
-         * `LegacySSHStore::Connection`.
-         *
-         * The serve protocol connection types are unidirectional, unlike
-         * this type.
-         */
-        operator ServeProto::WriteConn ()
-        {
-            return ServeProto::WriteConn {
-                .to = to,
-                .version = remoteVersion,
-            };
-        }
     };
 
     std::string host;
@@ -120,28 +85,21 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
         conn->to = FdSink(conn->sshConn->in.get());
         conn->from = FdSource(conn->sshConn->out.get());
 
+        StringSink saved;
+        TeeSource tee(conn->from, saved);
         try {
-            conn->to << SERVE_MAGIC_1 << SERVE_PROTOCOL_VERSION;
-            conn->to.flush();
-
-            StringSink saved;
-            try {
-                TeeSource tee(conn->from, saved);
-                unsigned int magic = readInt(tee);
-                if (magic != SERVE_MAGIC_2)
-                    throw Error("'nix-store --serve' protocol mismatch from '%s'", host);
-            } catch (SerialisationError & e) {
-                /* In case the other side is waiting for our input,
-                   close it. */
-                conn->sshConn->in.close();
-                auto msg = conn->from.drain();
-                throw Error("'nix-store --serve' protocol mismatch from '%s', got '%s'",
-                    host, chomp(saved.s + msg));
+            conn->remoteVersion = ServeProto::BasicClientConnection::handshake(
+                conn->to, tee, SERVE_PROTOCOL_VERSION, host);
+        } catch (SerialisationError & e) {
+            /* In case the other side is waiting for our input,
+               close it. */
+            conn->sshConn->in.close();
+            {
+                NullSink nullSink;
+                conn->from.drainInto(nullSink);
             }
-            conn->remoteVersion = readInt(conn->from);
-            if (GET_PROTOCOL_MAJOR(conn->remoteVersion) != 0x200)
-                throw Error("unsupported 'nix-store --serve' protocol version on '%s'", host);
-
+            throw Error("'nix-store --serve' protocol mismatch from '%s', got '%s'",
+                host, chomp(saved.s));
         } catch (EndOfFile & e) {
             throw Error("cannot connect to '%1%'", host);
         }
@@ -273,16 +231,16 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
 
 private:
 
-    void putBuildSettings(Connection & conn)
+    ServeProto::BuildOptions buildSettings()
     {
-        ServeProto::write(*this, conn, ServeProto::BuildOptions {
+        return {
             .maxSilentTime = settings.maxSilentTime,
             .buildTimeout = settings.buildTimeout,
             .maxLogSize = settings.maxLogSize,
             .nrRepeats = 0, // buildRepeat hasn't worked for ages anyway
             .enforceDeterminism = 0,
             .keepFailed = settings.keepFailed,
-        });
+        };
     }
 
 public:
@@ -292,14 +250,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
     {
         auto conn(connections->get());
 
-        conn->to
-            << ServeProto::Command::BuildDerivation
-            << printStorePath(drvPath);
-        writeDerivation(conn->to, *this, drv);
-
-        putBuildSettings(*conn);
-
-        conn->to.flush();
+        conn->buildDerivationRequest(*this, drvPath, drv, buildSettings());
 
         return ServeProto::Serialise<BuildResult>::read(*this, *conn);
     }
@@ -329,7 +280,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
         }
         conn->to << ss;
 
-        putBuildSettings(*conn);
+        ServeProto::write(*this, *conn, buildSettings());
 
         conn->to.flush();
 
@@ -384,15 +335,8 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
         SubstituteFlag maybeSubstitute = NoSubstitute) override
     {
         auto conn(connections->get());
-
-        conn->to
-            << ServeProto::Command::QueryValidPaths
-            << false // lock
-            << maybeSubstitute;
-        ServeProto::write(*this, *conn, paths);
-        conn->to.flush();
-
-        return ServeProto::Serialise<StorePathSet>::read(*this, *conn);
+        return conn->queryValidPaths(*this,
+            false, paths, maybeSubstitute);
     }
 
     void connect() override

src/libstore/serve-protocol-impl.cc

@@ -0,0 +1,57 @@
+#include "serve-protocol-impl.hh"
+#include "build-result.hh"
+#include "derivations.hh"
+
+namespace nix {
+
+ServeProto::Version ServeProto::BasicClientConnection::handshake(
+    BufferedSink & to,
+    Source & from,
+    ServeProto::Version localVersion,
+    std::string_view host)
+{
+    to << SERVE_MAGIC_1 << localVersion;
+    to.flush();
+
+    unsigned int magic = readInt(from);
+    if (magic != SERVE_MAGIC_2)
+        throw Error("'nix-store --serve' protocol mismatch from '%s'", host);
+    auto remoteVersion = readInt(from);
+    if (GET_PROTOCOL_MAJOR(remoteVersion) != 0x200)
+        throw Error("unsupported 'nix-store --serve' protocol version on '%s'", host);
+    return remoteVersion;
+}
+
+
+StorePathSet ServeProto::BasicClientConnection::queryValidPaths(
+    const Store & store,
+    bool lock, const StorePathSet & paths,
+    SubstituteFlag maybeSubstitute)
+{
+    to
+        << ServeProto::Command::QueryValidPaths
+        << lock
+        << maybeSubstitute;
+    write(store, *this, paths);
+    to.flush();
+
+    return Serialise<StorePathSet>::read(store, *this);
+}
+
+
+void ServeProto::BasicClientConnection::buildDerivationRequest(
+    const Store & store,
+    const StorePath & drvPath, const BasicDerivation & drv,
+    const ServeProto::BuildOptions & options)
+{
+    to
+        << ServeProto::Command::BuildDerivation
+        << store.printStorePath(drvPath);
+    writeDerivation(to, store, drv);
+
+    ServeProto::write(store, *this, options);
+
+    to.flush();
+}
+
+}

src/libstore/serve-protocol-impl.hh

@@ -10,6 +10,7 @@
 
 #include "serve-protocol.hh"
 #include "length-prefixed-protocol-helper.hh"
+#include "store-api.hh"
 
 namespace nix {
 
@@ -56,4 +57,79 @@ struct ServeProto::Serialise
 
 /* protocol-specific templates */
 
+struct ServeProto::BasicClientConnection
+{
+    FdSink to;
+    FdSource from;
+    ServeProto::Version remoteVersion;
+
+    /**
+     * Establishes connection, negotiating version.
+     *
+     * @return the version provided by the other side of the
+     * connection.
+     *
+     * @param to Taken by reference to allow for various error handling
+     * mechanismsm.
+     *
+     * @param from Taken by reference to allow for various error
+     * handling mechanisms.
+     *
+     * @param localVersion Our version which is sent over
+     *
+     * @param host Just used to add context to thrown exceptions.
+     */
+    static ServeProto::Version handshake(
+        BufferedSink & to,
+        Source & from,
+        ServeProto::Version localVersion,
+        std::string_view host);
+
+    /**
+     * Coercion to `ServeProto::ReadConn`. This makes it easy to use the
+     * factored out serve protocol searlizers with a
+     * `LegacySSHStore::Connection`.
+     *
+     * The serve protocol connection types are unidirectional, unlike
+     * this type.
+     */
+    operator ServeProto::ReadConn ()
+    {
+        return ServeProto::ReadConn {
+            .from = from,
+            .version = remoteVersion,
+        };
+    }
+
+    /**
+     * Coercion to `ServeProto::WriteConn`. This makes it easy to use the
+     * factored out serve protocol searlizers with a
+     * `LegacySSHStore::Connection`.
+     *
+     * The serve protocol connection types are unidirectional, unlike
+     * this type.
+     */
+    operator ServeProto::WriteConn ()
+    {
+        return ServeProto::WriteConn {
+            .to = to,
+            .version = remoteVersion,
+        };
+    }
+
+    StorePathSet queryValidPaths(
+        const Store & remoteStore,
+        bool lock, const StorePathSet & paths,
+        SubstituteFlag maybeSubstitute);
+
+    /**
+     * Just the request half, because Hydra may do other things between
+     * issuing the request and reading the `BuildResult` response.
+     */
+    void buildDerivationRequest(
+        const Store & store,
+        const StorePath & drvPath, const BasicDerivation & drv,
+        const ServeProto::BuildOptions & options);
+};
+
 }

src/libstore/serve-protocol.hh

@@ -59,6 +59,13 @@ struct ServeProto
         Version version;
     };
 
+    /**
+     * Stripped down serialization logic suitable for sharing with Hydra.
+     *
+     * @todo remove once Hydra uses Store abstraction consistently.
+     */
+    struct BasicClientConnection;
+
     /**
      * Data type for canonical pairs of serialisers for the serve protocol.
      *