Merge pull request #9560 from obsidiansystems/serve-proto-unkeyed-valid-path-info-serializer

Factor out `ServeProto::Serialiser<UnkeyedValidPathInfo>` and test

(cherry picked from commit 139982997eec493a0f74105c427953f6be77da6d)
Change-Id: I28e4ba5a681a90d81915a56e6dbaa5456d64f96d

src/libstore/legacy-ssh-store.cc

@@ -172,24 +172,12 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
             if (p.empty()) return callback(nullptr);
             auto path2 = parseStorePath(p);
             assert(path == path2);
-            /* Hash will be set below. FIXME construct ValidPathInfo at end. */
-            auto info = std::make_shared<ValidPathInfo>(path, Hash::dummy);
+            auto info = std::make_shared<ValidPathInfo>(
+                path,
+                ServeProto::Serialise<UnkeyedValidPathInfo>::read(*this, *conn));
 
-            auto deriver = readString(conn->from);
-            if (deriver != "")
-                info->deriver = parseStorePath(deriver);
-            info->references = ServeProto::Serialise<StorePathSet>::read(*this, *conn);
-            readLongLong(conn->from); // download size
-            info->narSize = readLongLong(conn->from);
-
-            {
-                auto s = readString(conn->from);
-                if (s == "")
+            if (info->narHash == Hash::dummy)
                 throw Error("NAR hash is now mandatory");
-                info->narHash = Hash::parseAnyPrefixed(s);
-            }
-            info->ca = ContentAddress::parseOpt(readString(conn->from));
-            info->sigs = readStrings<StringSet>(conn->from);
 
             auto s = readString(conn->from);
             assert(s == "");

src/libstore/serve-protocol.cc

@@ -6,6 +6,7 @@
 #include "serve-protocol.hh"
 #include "serve-protocol-impl.hh"
 #include "archive.hh"
+#include "path-info.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -55,4 +56,47 @@ void ServeProto::Serialise<BuildResult>::write(const Store & store, ServeProto::
     }
 }
 
+
+UnkeyedValidPathInfo ServeProto::Serialise<UnkeyedValidPathInfo>::read(const Store & store, ReadConn conn)
+{
+    /* Hash should be set below unless very old `nix-store --serve`.
+       Caller should assert that it did set it. */
+    UnkeyedValidPathInfo info { Hash::dummy };
+
+    auto deriver = readString(conn.from);
+    if (deriver != "")
+        info.deriver = store.parseStorePath(deriver);
+    info.references = ServeProto::Serialise<StorePathSet>::read(store, conn);
+
+    readLongLong(conn.from); // download size, unused
+    info.narSize = readLongLong(conn.from);
+
+    if (GET_PROTOCOL_MINOR(conn.version) >= 4) {
+        auto s = readString(conn.from);
+        if (!s.empty())
+            info.narHash = Hash::parseAnyPrefixed(s);
+        info.ca = ContentAddress::parseOpt(readString(conn.from));
+        info.sigs = readStrings<StringSet>(conn.from);
+    }
+
+    return info;
+}
+
+void ServeProto::Serialise<UnkeyedValidPathInfo>::write(const Store & store, WriteConn conn, const UnkeyedValidPathInfo & info)
+{
+    conn.to
+        << (info.deriver ? store.printStorePath(*info.deriver) : "");
+
+    ServeProto::write(store, conn, info.references);
+    // !!! Maybe we want compression?
+    conn.to
+        << info.narSize // downloadSize, lie a little
+        << info.narSize;
+    if (GET_PROTOCOL_MINOR(conn.version) >= 4)
+        conn.to
+            << info.narHash.to_string(Base32, true)
+            << renderContentAddress(info.ca)
+            << info.sigs;
+}
+
 }

src/libstore/serve-protocol.hh

@@ -18,6 +18,7 @@ struct Source;
 
 // items being serialised
 struct BuildResult;
+struct UnkeyedValidPathInfo;
 
 
 /**
@@ -141,6 +142,8 @@ inline std::ostream & operator << (std::ostream & s, ServeProto::Command op)
 
 template<>
 DECLARE_SERVE_SERIALISER(BuildResult);
+template<>
+DECLARE_SERVE_SERIALISER(UnkeyedValidPathInfo);
 
 template<typename T>
 DECLARE_SERVE_SERIALISER(std::vector<T>);

src/nix-store/nix-store.cc

@@ -891,16 +891,8 @@ static void opServe(Strings opFlags, Strings opArgs)
                 for (auto & i : paths) {
                     try {
                         auto info = store->queryPathInfo(i);
-                        out << store->printStorePath(info->path)
-                            << (info->deriver ? store->printStorePath(*info->deriver) : "");
-                        ServeProto::write(*store, wconn, info->references);
-                        // !!! Maybe we want compression?
-                        out << info->narSize // downloadSize
-                            << info->narSize;
-                        if (GET_PROTOCOL_MINOR(clientVersion) >= 4)
-                            out << info->narHash.to_string(Base32, true)
-                                << renderContentAddress(info->ca)
-                                << info->sigs;
+                        out << store->printStorePath(info->path);
+                        ServeProto::write(*store, wconn, static_cast<const UnkeyedValidPathInfo &>(*info));
                     } catch (InvalidPath &) {
                     }
                 }

tests/unit/libstore/serve-protocol.cc

@@ -225,6 +225,83 @@ VERSIONED_CHARACTERIZATION_TEST(
         t;
     }))
 
+VERSIONED_CHARACTERIZATION_TEST(
+    ServeProtoTest,
+    unkeyedValidPathInfo_2_3,
+    "unkeyed-valid-path-info-2.3",
+    2 << 8 | 3,
+    (std::tuple<UnkeyedValidPathInfo, UnkeyedValidPathInfo> {
+        ({
+            UnkeyedValidPathInfo info { Hash::dummy };
+            info.narSize = 34878;
+            info;
+        }),
+        ({
+            UnkeyedValidPathInfo info { Hash::dummy };
+            info.deriver = StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            };
+            info.references = {
+                StorePath {
+                    "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo.drv",
+                },
+            };
+            info.narSize = 34878;
+            info;
+        }),
+    }))
+
+VERSIONED_CHARACTERIZATION_TEST(
+    ServeProtoTest,
+    unkeyedValidPathInfo_2_4,
+    "unkeyed-valid-path-info-2.4",
+    2 << 8 | 4,
+    (std::tuple<UnkeyedValidPathInfo, UnkeyedValidPathInfo> {
+        ({
+            UnkeyedValidPathInfo info {
+                Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+            };
+            info.deriver = StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            };
+            info.references = {
+                StorePath {
+                    "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo.drv",
+                },
+            };
+            info.narSize = 34878;
+            info;
+        }),
+        ({
+            ValidPathInfo info {
+                *LibStoreTest::store,
+                "foo",
+                FixedOutputInfo {
+                    .method = FileIngestionMethod::Recursive,
+                    .hash = hashString(HashType::htSHA256, "(...)"),
+                    .references = {
+                        .others = {
+                            StorePath {
+                                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar",
+                            },
+                        },
+                        .self = true,
+                    },
+                },
+                Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="),
+            };
+            info.deriver = StorePath {
+                "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv",
+            };
+            info.narSize = 34878;
+            info.sigs = {
+                "fake-sig-1",
+                "fake-sig-2",
+            },
+            static_cast<UnkeyedValidPathInfo>(std::move(info));
+        }),
+    }))
+
 VERSIONED_CHARACTERIZATION_TEST(
     ServeProtoTest,
     vector,