From e12e9f2452681a3036884e9e778154b35246d39e Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 4 Mar 2024 04:35:39 +0100 Subject: [PATCH] Merge pull request #9137 from obsidiansystems/serve-protocol Introduce separate Serve protocol serialisers (cherry picked from commit d070d8b7460f412a657745698dba291c66792402) Change-Id: Ibcc8639e8997bcd07f7a5318330a147bcadc411b --- src/libstore/legacy-ssh-store.cc | 45 +++--- src/libstore/serve-protocol-impl.hh | 59 +++++++ src/libstore/serve-protocol.cc | 15 ++ src/libstore/serve-protocol.hh | 87 ++++++++++ src/nix-store/nix-store.cc | 25 ++- tests/unit/libstore/serve-protocol.cc | 152 ++++++++++++++++++ .../serve-protocol/content-address.bin | Bin 0 -> 208 bytes .../libstore/serve-protocol/drv-output.bin | Bin 0 -> 176 bytes .../optional-content-address.bin | Bin 0 -> 64 bytes .../serve-protocol/optional-store-path.bin | Bin 0 -> 72 bytes .../libstore/serve-protocol/realisation.bin | Bin 0 -> 520 bytes .../libstore/serve-protocol/set.bin | Bin 0 -> 152 bytes .../libstore/serve-protocol/store-path.bin | Bin 0 -> 120 bytes .../libstore/serve-protocol/string.bin | Bin 0 -> 88 bytes .../libstore/serve-protocol/vector.bin | Bin 0 -> 152 bytes 15 files changed, 344 insertions(+), 39 deletions(-) create mode 100644 src/libstore/serve-protocol-impl.hh create mode 100644 src/libstore/serve-protocol.cc create mode 100644 tests/unit/libstore/serve-protocol.cc create mode 100644 unit-test-data/libstore/serve-protocol/content-address.bin create mode 100644 unit-test-data/libstore/serve-protocol/drv-output.bin create mode 100644 unit-test-data/libstore/serve-protocol/optional-content-address.bin create mode 100644 unit-test-data/libstore/serve-protocol/optional-store-path.bin create mode 100644 unit-test-data/libstore/serve-protocol/realisation.bin create mode 100644 unit-test-data/libstore/serve-protocol/set.bin create mode 100644 unit-test-data/libstore/serve-protocol/store-path.bin create mode 100644 unit-test-data/libstore/serve-protocol/string.bin create mode 100644 unit-test-data/libstore/serve-protocol/vector.bin diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 7bf4476d0..703ded0b2 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -3,11 +3,10 @@ #include "pool.hh" #include "remote-store.hh" #include "serve-protocol.hh" +#include "serve-protocol-impl.hh" #include "build-result.hh" #include "store-api.hh" #include "path-with-outputs.hh" -#include "common-protocol.hh" -#include "common-protocol-impl.hh" #include "ssh.hh" #include "derivations.hh" #include "callback.hh" @@ -50,37 +49,31 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor bool good = true; /** - * Coercion to `CommonProto::ReadConn`. This makes it easy to use the - * factored out common protocol serialisers with a + * Coercion to `ServeProto::ReadConn`. This makes it easy to use the + * factored out serve protocol searlizers with a * `LegacySSHStore::Connection`. * - * The common protocol connection types are unidirectional, unlike + * The serve protocol connection types are unidirectional, unlike * this type. - * - * @todo Use server protocol serializers, not common protocol - * serializers, once we have made that distiction. */ - operator CommonProto::ReadConn () + operator ServeProto::ReadConn () { - return CommonProto::ReadConn { + return ServeProto::ReadConn { .from = from, }; } /* - * Coercion to `CommonProto::WriteConn`. This makes it easy to use the - * factored out common protocol searlizers with a + * Coercion to `ServeProto::WriteConn`. This makes it easy to use the + * factored out serve protocol searlizers with a * `LegacySSHStore::Connection`. * - * The common protocol connection types are unidirectional, unlike + * The serve protocol connection types are unidirectional, unlike * this type. - * - * @todo Use server protocol serializers, not common protocol - * serializers, once we have made that distiction. */ - operator CommonProto::WriteConn () + operator ServeProto::WriteConn () { - return CommonProto::WriteConn { + return ServeProto::WriteConn { .to = to, }; } @@ -183,7 +176,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor auto deriver = readString(conn->from); if (deriver != "") info->deriver = parseStorePath(deriver); - info->references = CommonProto::Serialise::read(*this, *conn); + info->references = ServeProto::Serialise::read(*this, *conn); readLongLong(conn->from); // download size info->narSize = readLongLong(conn->from); @@ -217,7 +210,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor << printStorePath(info.path) << (info.deriver ? printStorePath(*info.deriver) : "") << info.narHash.to_string(Base16, false); - CommonProto::write(*this, *conn, info.references); + ServeProto::write(*this, *conn, info.references); conn->to << info.registrationTime << info.narSize @@ -246,7 +239,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor conn->to << exportMagic << printStorePath(info.path); - CommonProto::write(*this, *conn, info.references); + ServeProto::write(*this, *conn, info.references); conn->to << (info.deriver ? printStorePath(*info.deriver) : "") << 0 @@ -331,7 +324,7 @@ public: if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3) conn->from >> status.timesBuilt >> status.isNonDeterministic >> status.startTime >> status.stopTime; if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 6) { - auto builtOutputs = CommonProto::Serialise::read(*this, *conn); + auto builtOutputs = ServeProto::Serialise::read(*this, *conn); for (auto && [output, realisation] : builtOutputs) status.builtOutputs.insert_or_assign( std::move(output.outputName), @@ -409,10 +402,10 @@ public: conn->to << ServeProto::Command::QueryClosure << includeOutputs; - CommonProto::write(*this, *conn, paths); + ServeProto::write(*this, *conn, paths); conn->to.flush(); - for (auto & i : CommonProto::Serialise::read(*this, *conn)) + for (auto & i : ServeProto::Serialise::read(*this, *conn)) out.insert(i); } @@ -425,10 +418,10 @@ public: << ServeProto::Command::QueryValidPaths << false // lock << maybeSubstitute; - CommonProto::write(*this, *conn, paths); + ServeProto::write(*this, *conn, paths); conn->to.flush(); - return CommonProto::Serialise::read(*this, *conn); + return ServeProto::Serialise::read(*this, *conn); } void connect() override diff --git a/src/libstore/serve-protocol-impl.hh b/src/libstore/serve-protocol-impl.hh new file mode 100644 index 000000000..a3ce81026 --- /dev/null +++ b/src/libstore/serve-protocol-impl.hh @@ -0,0 +1,59 @@ +#pragma once +/** + * @file + * + * Template implementations (as opposed to mere declarations). + * + * This file is an exmample of the "impl.hh" pattern. See the + * contributing guide. + */ + +#include "serve-protocol.hh" +#include "length-prefixed-protocol-helper.hh" + +namespace nix { + +/* protocol-agnostic templates */ + +#define SERVE_USE_LENGTH_PREFIX_SERIALISER(TEMPLATE, T) \ + TEMPLATE T ServeProto::Serialise< T >::read(const Store & store, ServeProto::ReadConn conn) \ + { \ + return LengthPrefixedProtoHelper::read(store, conn); \ + } \ + TEMPLATE void ServeProto::Serialise< T >::write(const Store & store, ServeProto::WriteConn conn, const T & t) \ + { \ + LengthPrefixedProtoHelper::write(store, conn, t); \ + } + +SERVE_USE_LENGTH_PREFIX_SERIALISER(template, std::vector) +SERVE_USE_LENGTH_PREFIX_SERIALISER(template, std::set) +SERVE_USE_LENGTH_PREFIX_SERIALISER(template, std::tuple) + +#define COMMA_ , +SERVE_USE_LENGTH_PREFIX_SERIALISER( + template, + std::map) +#undef COMMA_ + +/** + * Use `CommonProto` where possible. + */ +template +struct ServeProto::Serialise +{ + static T read(const Store & store, ServeProto::ReadConn conn) + { + return CommonProto::Serialise::read(store, + CommonProto::ReadConn { .from = conn.from }); + } + static void write(const Store & store, ServeProto::WriteConn conn, const T & t) + { + CommonProto::Serialise::write(store, + CommonProto::WriteConn { .to = conn.to }, + t); + } +}; + +/* protocol-specific templates */ + +} diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc new file mode 100644 index 000000000..16a62b5bc --- /dev/null +++ b/src/libstore/serve-protocol.cc @@ -0,0 +1,15 @@ +#include "serialise.hh" +#include "util.hh" +#include "path-with-outputs.hh" +#include "store-api.hh" +#include "serve-protocol.hh" +#include "serve-protocol-impl.hh" +#include "archive.hh" + +#include + +namespace nix { + +/* protocol-specific definitions */ + +} diff --git a/src/libstore/serve-protocol.hh b/src/libstore/serve-protocol.hh index 7e43b3969..e2345d450 100644 --- a/src/libstore/serve-protocol.hh +++ b/src/libstore/serve-protocol.hh @@ -1,6 +1,8 @@ #pragma once ///@file +#include "common-protocol.hh" + namespace nix { #define SERVE_MAGIC_1 0x390c9deb @@ -10,6 +12,11 @@ namespace nix { #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) + +class Store; +struct Source; + + /** * The "serve protocol", used by ssh:// stores. * @@ -22,6 +29,57 @@ struct ServeProto * Enumeration of all the request types for the protocol. */ enum struct Command : uint64_t; + + /** + * 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; + }; + + /** + * 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; + }; + + /** + * Data type for canonical pairs of serialisers for the serve protocol. + * + * See https://en.cppreference.com/w/cpp/language/adl for the broader + * concept of what is going on here. + */ + template + struct Serialise; + // This is the definition of `Serialise` we *want* to put here, but + // do not do so. + // + // See `worker-protocol.hh` for a longer explanation. +#if 0 + { + static T read(const Store & store, ReadConn conn); + static void write(const Store & store, WriteConn conn, const T & t); + }; +#endif + + /** + * Wrapper function around `ServeProto::Serialise::write` that allows us to + * infer the type instead of having to write it down explicitly. + */ + template + static void write(const Store & store, WriteConn conn, const T & t) + { + ServeProto::Serialise::write(store, conn, t); + } }; enum struct ServeProto::Command : uint64_t @@ -58,4 +116,33 @@ inline std::ostream & operator << (std::ostream & s, ServeProto::Command op) return s << (uint64_t) op; } +/** + * Declare a canonical serialiser pair for the worker protocol. + * + * We specialise the struct merely to indicate that we are implementing + * the function for the given type. + * + * Some sort of `template<...>` must be used with the caller for this to + * be legal specialization syntax. See below for what that looks like in + * practice. + */ +#define DECLARE_SERVE_SERIALISER(T) \ + struct ServeProto::Serialise< T > \ + { \ + static T read(const Store & store, ServeProto::ReadConn conn); \ + static void write(const Store & store, ServeProto::WriteConn conn, const T & t); \ + }; + +template +DECLARE_SERVE_SERIALISER(std::vector); +template +DECLARE_SERVE_SERIALISER(std::set); +template +DECLARE_SERVE_SERIALISER(std::tuple); + +#define COMMA_ , +template +DECLARE_SERVE_SERIALISER(std::map); +#undef COMMA_ + } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 6fc22214a..9b6c80a75 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -9,10 +9,9 @@ #include "local-store.hh" #include "monitor-fd.hh" #include "serve-protocol.hh" +#include "serve-protocol-impl.hh" #include "shared.hh" #include "util.hh" -#include "common-protocol.hh" -#include "common-protocol-impl.hh" #include "graphml.hh" #include "legacy.hh" #include "path-with-outputs.hh" @@ -821,8 +820,8 @@ static void opServe(Strings opFlags, Strings opArgs) out.flush(); unsigned int clientVersion = readInt(in); - CommonProto::ReadConn rconn { .from = in }; - CommonProto::WriteConn wconn { .to = out }; + ServeProto::ReadConn rconn { .from = in }; + ServeProto::WriteConn wconn { .to = out }; auto getBuildSettings = [&]() { // FIXME: changing options here doesn't work if we're @@ -867,7 +866,7 @@ static void opServe(Strings opFlags, Strings opArgs) case ServeProto::Command::QueryValidPaths: { bool lock = readInt(in); bool substitute = readInt(in); - auto paths = CommonProto::Serialise::read(*store, rconn); + auto paths = ServeProto::Serialise::read(*store, rconn); if (lock && writeAllowed) for (auto & path : paths) store->addTempRoot(path); @@ -876,19 +875,19 @@ static void opServe(Strings opFlags, Strings opArgs) store->substitutePaths(paths); } - CommonProto::write(*store, wconn, store->queryValidPaths(paths)); + ServeProto::write(*store, wconn, store->queryValidPaths(paths)); break; } case ServeProto::Command::QueryPathInfos: { - auto paths = CommonProto::Serialise::read(*store, rconn); + auto paths = ServeProto::Serialise::read(*store, rconn); // !!! Maybe we want a queryPathInfos? for (auto & i : paths) { try { auto info = store->queryPathInfo(i); out << store->printStorePath(info->path) << (info->deriver ? store->printStorePath(*info->deriver) : ""); - CommonProto::write(*store, wconn, info->references); + ServeProto::write(*store, wconn, info->references); // !!! Maybe we want compression? out << info->narSize // downloadSize << info->narSize; @@ -916,7 +915,7 @@ static void opServe(Strings opFlags, Strings opArgs) case ServeProto::Command::ExportPaths: { readInt(in); // obsolete - store->exportPaths(CommonProto::Serialise::read(*store, rconn), out); + store->exportPaths(ServeProto::Serialise::read(*store, rconn), out); break; } @@ -962,7 +961,7 @@ static void opServe(Strings opFlags, Strings opArgs) DrvOutputs builtOutputs; for (auto & [output, realisation] : status.builtOutputs) builtOutputs.insert_or_assign(realisation.id, realisation); - CommonProto::write(*store, wconn, builtOutputs); + ServeProto::write(*store, wconn, builtOutputs); } break; @@ -971,9 +970,9 @@ static void opServe(Strings opFlags, Strings opArgs) case ServeProto::Command::QueryClosure: { bool includeOutputs = readInt(in); StorePathSet closure; - store->computeFSClosure(CommonProto::Serialise::read(*store, rconn), + store->computeFSClosure(ServeProto::Serialise::read(*store, rconn), closure, false, includeOutputs); - CommonProto::write(*store, wconn, closure); + ServeProto::write(*store, wconn, closure); break; } @@ -988,7 +987,7 @@ static void opServe(Strings opFlags, Strings opArgs) }; if (deriver != "") info.deriver = store->parseStorePath(deriver); - info.references = CommonProto::Serialise::read(*store, rconn); + info.references = ServeProto::Serialise::read(*store, rconn); in >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(in); info.ca = ContentAddress::parseOpt(readString(in)); diff --git a/tests/unit/libstore/serve-protocol.cc b/tests/unit/libstore/serve-protocol.cc new file mode 100644 index 000000000..dd7d83e0a --- /dev/null +++ b/tests/unit/libstore/serve-protocol.cc @@ -0,0 +1,152 @@ +#include + +#include +#include + +#include "serve-protocol.hh" +#include "serve-protocol-impl.hh" +#include "build-result.hh" +#include "protocol.hh" +#include "characterization.hh" + +namespace nix { + +const char commonProtoDir[] = "serve-protocol"; + +using ServeProtoTest = ProtoTest; + +CHARACTERIZATION_TEST( + ServeProtoTest, + string, + "string", + (std::tuple { + "", + "hi", + "white rabbit", + "大白兔", + "oh no \0\0\0 what was that!", + })) + +CHARACTERIZATION_TEST( + ServeProtoTest, + storePath, + "store-path", + (std::tuple { + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo-bar" }, + })) + +CHARACTERIZATION_TEST( + ServeProtoTest, + contentAddress, + "content-address", + (std::tuple { + ContentAddress { + .method = TextIngestionMethod {}, + .hash = hashString(HashType::htSHA256, "Derive(...)"), + }, + ContentAddress { + .method = FileIngestionMethod::Flat, + .hash = hashString(HashType::htSHA1, "blob blob..."), + }, + ContentAddress { + .method = FileIngestionMethod::Recursive, + .hash = hashString(HashType::htSHA256, "(...)"), + }, + })) + +CHARACTERIZATION_TEST( + ServeProtoTest, + drvOutput, + "drv-output", + (std::tuple { + { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "quux", + }, + })) + +CHARACTERIZATION_TEST( + ServeProtoTest, + realisation, + "realisation", + (std::tuple { + Realisation { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + .signatures = { "asdf", "qwer" }, + }, + Realisation { + .id = { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + .signatures = { "asdf", "qwer" }, + .dependentRealisations = { + { + DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "quux", + }, + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + }, + }, + })) + +CHARACTERIZATION_TEST( + ServeProtoTest, + vector, + "vector", + (std::tuple, std::vector, std::vector, std::vector>> { + { }, + { "" }, + { "", "foo", "bar" }, + { {}, { "" }, { "", "1", "2" } }, + })) + +CHARACTERIZATION_TEST( + ServeProtoTest, + set, + "set", + (std::tuple, std::set, std::set, std::set>> { + { }, + { "" }, + { "", "foo", "bar" }, + { {}, { "" }, { "", "1", "2" } }, + })) + +CHARACTERIZATION_TEST( + ServeProtoTest, + optionalStorePath, + "optional-store-path", + (std::tuple, std::optional> { + std::nullopt, + std::optional { + StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo-bar" }, + }, + })) + +CHARACTERIZATION_TEST( + ServeProtoTest, + optionalContentAddress, + "optional-content-address", + (std::tuple, std::optional> { + std::nullopt, + std::optional { + ContentAddress { + .method = FileIngestionMethod::Flat, + .hash = hashString(HashType::htSHA1, "blob blob..."), + }, + }, + })) + +} diff --git a/unit-test-data/libstore/serve-protocol/content-address.bin b/unit-test-data/libstore/serve-protocol/content-address.bin new file mode 100644 index 0000000000000000000000000000000000000000..8f14bcdb3e50cc72d87106ce159a6fd7b9f75713 GIT binary patch literal 208 zcmY+;K@P$o5QSmy;|7WYrYjRqQfMm$+LWPzDW_MfG4bucKks(>Y#V56lkFOiEzi24 zUMdC5VUCa86OJ&gL0BF^B{HBQEY%f|I<6v7#q+l_PBirI5N~-md%37WE|g33QTE2k zc~-WF&R}7Oxc@o)T?ojpD!+*5=xpO;%IoCKSV5FMh={>xePK|8<${>E)*huNHZ(pX literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/serve-protocol/drv-output.bin b/unit-test-data/libstore/serve-protocol/drv-output.bin new file mode 100644 index 0000000000000000000000000000000000000000..800a45fd8757a15e8810066aed48be56a600b7d3 GIT binary patch literal 176 zcmY++xeWp_5I|8{*$(Wn=b{B@!gGlfp_LHTb8N(qe)KM%U7Sq@}q)U|6h9n2j1!MT?w+C~^gO WO9=?G&ojrYjy`x4ZufnEe#tjMpDPdm literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/serve-protocol/optional-content-address.bin b/unit-test-data/libstore/serve-protocol/optional-content-address.bin new file mode 100644 index 0000000000000000000000000000000000000000..f8cfe65ba27fd78ec7787eed7ebec8dfdead2fba GIT binary patch literal 64 zcmZQzfB#ECTBU06lLKJOBUy literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/serve-protocol/realisation.bin b/unit-test-data/libstore/serve-protocol/realisation.bin new file mode 100644 index 0000000000000000000000000000000000000000..2176c6c4afd96b32fe372de6084ac7f4c7a11d49 GIT binary patch literal 520 zcmdUrO-{o=429t+opq7s&z_l_03#3x z?rW}!>Uc!$VBY>Sr2mUC`<2<)qY;)1KN Po44)luetw6d9AunL$;dR literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/serve-protocol/set.bin b/unit-test-data/libstore/serve-protocol/set.bin new file mode 100644 index 0000000000000000000000000000000000000000..ce11ede7fe7aa061a30b211a18cd7a336f879de8 GIT binary patch literal 152 ucmZQzfB;4)4WpQ03@8obCnXkvMPU52{CpHXOdBEdVDg4g4KThDln(&+h63;a literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/serve-protocol/store-path.bin b/unit-test-data/libstore/serve-protocol/store-path.bin new file mode 100644 index 0000000000000000000000000000000000000000..3fc05f2981d3398ab30ec34125c903cdcefa8729 GIT binary patch literal 120 tcmdOAfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7Eu*F?srQlM;)-IsqP{BM|@q literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/serve-protocol/string.bin b/unit-test-data/libstore/serve-protocol/string.bin new file mode 100644 index 0000000000000000000000000000000000000000..aa7b5a604745b735473ec34283177e69320776e4 GIT binary patch literal 88 zcmZQzfB+^aoskJ)@Id+H8JQ)i3Pp)YNtq=eAx^0H(