Add protocol versions to {Worker,Serve}Proto::*Conn

src/libstore/daemon.cc

@@ -45,9 +45,9 @@ struct TunnelLogger : public Logger
 
     Sync<State> state_;
 
-    unsigned int clientVersion;
+    WorkerProto::Version clientVersion;
 
-    TunnelLogger(FdSink & to, unsigned int clientVersion)
+    TunnelLogger(FdSink & to, WorkerProto::Version clientVersion)
         : to(to), clientVersion(clientVersion) { }
 
     void enqueueMsg(const std::string & s)
@@ -261,7 +261,7 @@ struct ClientSettings
     }
 };
 
-static std::vector<DerivedPath> readDerivedPaths(Store & store, unsigned int clientVersion, WorkerProto::ReadConn conn)
+static std::vector<DerivedPath> readDerivedPaths(Store & store, WorkerProto::Version clientVersion, WorkerProto::ReadConn conn)
 {
     std::vector<DerivedPath> reqs;
     if (GET_PROTOCOL_MINOR(clientVersion) >= 30) {
@@ -274,11 +274,17 @@ static std::vector<DerivedPath> readDerivedPaths(Store & store, unsigned int cli
 }
 
 static void performOp(TunnelLogger * logger, ref<Store> store,
-    TrustedFlag trusted, RecursiveFlag recursive, unsigned int clientVersion,
+    TrustedFlag trusted, RecursiveFlag recursive, WorkerProto::Version clientVersion,
     Source & from, BufferedSink & to, WorkerProto::Op op)
 {
-    WorkerProto::ReadConn rconn { .from = from };
-    WorkerProto::WriteConn wconn { .to = to };
+    WorkerProto::ReadConn rconn {
+        .from = from,
+        .version = clientVersion,
+    };
+    WorkerProto::WriteConn wconn {
+        .to = to,
+        .version = clientVersion,
+    };
 
     switch (op) {
 
@@ -1017,7 +1023,7 @@ void processConnection(
     if (magic != WORKER_MAGIC_1) throw Error("protocol mismatch");
     to << WORKER_MAGIC_2 << PROTOCOL_VERSION;
     to.flush();
-    unsigned int clientVersion = readInt(from);
+    WorkerProto::Version clientVersion = readInt(from);
 
     if (clientVersion < 0x10a)
         throw Error("the Nix client version is too old");
@@ -1052,7 +1058,10 @@ void processConnection(
         auto temp = trusted
             ? store->isTrustedClient()
             : std::optional { NotTrusted };
-        WorkerProto::WriteConn wconn { .to = to };
+        WorkerProto::WriteConn wconn {
+            .to = to,
+            .version = clientVersion,
+        };
         WorkerProto::write(*store, wconn, temp);
     }
 

src/libstore/legacy-ssh-store.cc

@@ -45,7 +45,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
         std::unique_ptr<SSHMaster::Connection> sshConn;
         FdSink to;
         FdSource from;
-        int remoteVersion;
+        ServeProto::Version remoteVersion;
         bool good = true;
 
         /**
@@ -60,6 +60,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
         {
             return ServeProto::ReadConn {
                 .from = from,
+                .version = remoteVersion,
             };
         }
 
@@ -75,6 +76,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
         {
             return ServeProto::WriteConn {
                 .to = to,
+                .version = remoteVersion,
             };
         }
     };

src/libstore/path-info.cc

@@ -141,7 +141,10 @@ ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned
     ValidPathInfo info(path, narHash);
     if (deriver != "") info.deriver = store.parseStorePath(deriver);
     info.references = WorkerProto::Serialise<StorePathSet>::read(store,
-        WorkerProto::ReadConn { .from = source });
+        WorkerProto::ReadConn {
+            .from = source,
+            .version = format,
+        });
     source >> info.registrationTime >> info.narSize;
     if (format >= 16) {
         source >> info.ultimate;
@@ -163,7 +166,10 @@ void ValidPathInfo::write(
     sink << (deriver ? store.printStorePath(*deriver) : "")
          << narHash.to_string(Base16, false);
     WorkerProto::write(store,
-        WorkerProto::WriteConn { .to = sink },
+        WorkerProto::WriteConn {
+            .to = sink,
+            .version = format,
+        },
         references);
     sink << registrationTime << narSize;
     if (format >= 16) {

src/libstore/remote-store-connection.hh

@@ -30,7 +30,7 @@ struct RemoteStore::Connection
      * sides support. (If the maximum doesn't exist, we would fail to
      * establish a connection and produce a value of this type.)
      */
-    unsigned int daemonVersion;
+    WorkerProto::Version daemonVersion;
 
     /**
      * Whether the remote side trusts us or not.
@@ -70,6 +70,7 @@ struct RemoteStore::Connection
     {
         return WorkerProto::ReadConn {
             .from = from,
+            .version = daemonVersion,
         };
     }
 
@@ -85,6 +86,7 @@ struct RemoteStore::Connection
     {
         return WorkerProto::WriteConn {
             .to = to,
+            .version = daemonVersion,
         };
     }
 

src/libstore/serve-protocol.hh

@@ -30,26 +30,29 @@ struct ServeProto
      */
     enum struct Command : uint64_t;
 
+    /**
+     * Version type for the protocol.
+     *
+     * @todo Convert to struct with separate major vs minor fields.
+     */
+    using Version = unsigned int;
+
     /**
      * A unidirectional read connection, to be used by the read half of the
      * canonical serializers below.
-     *
-     * This currently is just a `Source &`, but more fields will be added
-     * later.
      */
     struct ReadConn {
         Source & from;
+        Version version;
     };
 
     /**
      * A unidirectional write connection, to be used by the write half of the
      * canonical serializers below.
-     *
-     * This currently is just a `Sink &`, but more fields will be added
-     * later.
      */
     struct WriteConn {
         Sink & to;
+        Version version;
     };
 
     /**

src/libstore/tests/common-protocol.cc

@@ -13,10 +13,71 @@ namespace nix {
 
 const char commonProtoDir[] = "common-protocol";
 
-using CommonProtoTest = ProtoTest<CommonProto, commonProtoDir>;
+class CommonProtoTest : public ProtoTest<CommonProto, commonProtoDir>
+{
+public:
+    /**
+     * Golden test for `T` reading
+     */
+    template<typename T>
+    void readTest(PathView testStem, T value)
+    {
+        if (testAccept())
+        {
+            GTEST_SKIP() << "Cannot read golden master because another test is also updating it";
+        }
+        else
+        {
+            auto expected = readFile(goldenMaster(testStem));
+
+            T got = ({
+                StringSource from { expected };
+                CommonProto::Serialise<T>::read(
+                    *store,
+                    CommonProto::ReadConn { .from = from });
+            });
+
+            ASSERT_EQ(got, value);
+        }
+    }
+
+    /**
+     * Golden test for `T` write
+     */
+    template<typename T>
+    void writeTest(PathView testStem, const T & value)
+    {
+        auto file = goldenMaster(testStem);
+
+        StringSink to;
+        CommonProto::write(
+            *store,
+            CommonProto::WriteConn { .to = to },
+            value);
+
+        if (testAccept())
+        {
+            createDirs(dirOf(file));
+            writeFile(file, to.s);
+            GTEST_SKIP() << "Updating golden master";
+        }
+        else
+        {
+            auto expected = readFile(file);
+            ASSERT_EQ(to.s, expected);
+        }
+    }
+};
+
+#define CHARACTERIZATION_TEST(NAME, STEM, VALUE) \
+    TEST_F(CommonProtoTest, NAME ## _read) { \
+        readTest(STEM, VALUE); \
+    } \
+    TEST_F(CommonProtoTest, NAME ## _write) { \
+        writeTest(STEM, VALUE); \
+    }
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     string,
     "string",
     (std::tuple<std::string, std::string, std::string, std::string, std::string> {
@@ -28,7 +89,6 @@ CHARACTERIZATION_TEST(
     }))
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     storePath,
     "store-path",
     (std::tuple<StorePath, StorePath> {
@@ -37,7 +97,6 @@ CHARACTERIZATION_TEST(
     }))
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     contentAddress,
     "content-address",
     (std::tuple<ContentAddress, ContentAddress, ContentAddress> {
@@ -56,7 +115,6 @@ CHARACTERIZATION_TEST(
     }))
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     drvOutput,
     "drv-output",
     (std::tuple<DrvOutput, DrvOutput> {
@@ -71,7 +129,6 @@ CHARACTERIZATION_TEST(
     }))
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     realisation,
     "realisation",
     (std::tuple<Realisation, Realisation> {
@@ -103,7 +160,6 @@ CHARACTERIZATION_TEST(
     }))
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     vector,
     "vector",
     (std::tuple<std::vector<std::string>, std::vector<std::string>, std::vector<std::string>, std::vector<std::vector<std::string>>> {
@@ -114,7 +170,6 @@ CHARACTERIZATION_TEST(
     }))
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     set,
     "set",
     (std::tuple<std::set<std::string>, std::set<std::string>, std::set<std::string>, std::set<std::set<std::string>>> {
@@ -125,7 +180,6 @@ CHARACTERIZATION_TEST(
     }))
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     optionalStorePath,
     "optional-store-path",
     (std::tuple<std::optional<StorePath>, std::optional<StorePath>> {
@@ -136,7 +190,6 @@ CHARACTERIZATION_TEST(
     }))
 
 CHARACTERIZATION_TEST(
-    CommonProtoTest,
     optionalContentAddress,
     "optional-content-address",
     (std::tuple<std::optional<ContentAddress>, std::optional<ContentAddress>> {