From 70f8b96c11af275b5766dca0a49737803d1e0339 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 17 Apr 2023 17:13:44 -0400 Subject: [PATCH] Factor out `UnkeyedValidPathInfo` and test This makes the path info serialisers ideomatic again, which allows me to test them. --- src/libstore/daemon.cc | 2 +- src/libstore/path-info.cc | 23 ++- src/libstore/path-info.hh | 31 ++-- src/libstore/remote-store.cc | 3 +- src/libstore/tests/worker-protocol.cc | 153 ++++++++++++++++++ src/libstore/worker-protocol.cc | 24 +-- src/libstore/worker-protocol.hh | 16 +- .../unkeyed-valid-path-info-1.15.bin | Bin 0 -> 328 bytes .../worker-protocol/valid-path-info-1.15.bin | Bin 0 -> 488 bytes .../worker-protocol/valid-path-info-1.16.bin | Bin 0 -> 952 bytes 10 files changed, 218 insertions(+), 34 deletions(-) create mode 100644 unit-test-data/libstore/worker-protocol/unkeyed-valid-path-info-1.15.bin create mode 100644 unit-test-data/libstore/worker-protocol/valid-path-info-1.15.bin create mode 100644 unit-test-data/libstore/worker-protocol/valid-path-info-1.16.bin diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 0afaaccba..007ffc05a 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -819,7 +819,7 @@ static void performOp(TunnelLogger * logger, ref store, if (info) { if (GET_PROTOCOL_MINOR(clientVersion) >= 17) to << 1; - WorkerProto::Serialise::write(*store, wconn, *info, false); + WorkerProto::write(*store, wconn, static_cast(*info)); } else { assert(GET_PROTOCOL_MINOR(clientVersion) >= 17); to << 0; diff --git a/src/libstore/path-info.cc b/src/libstore/path-info.cc index 51fb5f02a..ab39e71f4 100644 --- a/src/libstore/path-info.cc +++ b/src/libstore/path-info.cc @@ -3,6 +3,25 @@ namespace nix { +GENERATE_CMP_EXT( + , + UnkeyedValidPathInfo, + me->deriver, + me->narHash, + me->references, + me->registrationTime, + me->narSize, + //me->id, + me->ultimate, + me->sigs, + me->ca); + +GENERATE_CMP_EXT( + , + ValidPathInfo, + me->path, + static_cast(*me)); + std::string ValidPathInfo::fingerprint(const Store & store) const { if (narSize == 0) @@ -102,8 +121,8 @@ ValidPathInfo::ValidPathInfo( std::string_view name, ContentAddressWithReferences && ca, Hash narHash) - : path(store.makeFixedOutputPathFromCA(name, ca)) - , narHash(narHash) + : UnkeyedValidPathInfo(narHash) + , path(store.makeFixedOutputPathFromCA(name, ca)) { std::visit(overloaded { [this](TextInfo && ti) { diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh index 919fe32f1..a82e643ae 100644 --- a/src/libstore/path-info.hh +++ b/src/libstore/path-info.hh @@ -32,9 +32,8 @@ struct SubstitutablePathInfo typedef std::map SubstitutablePathInfos; -struct ValidPathInfo +struct UnkeyedValidPathInfo { - StorePath path; std::optional deriver; /** * \todo document this @@ -72,6 +71,20 @@ struct ValidPathInfo */ std::optional ca; + UnkeyedValidPathInfo(const UnkeyedValidPathInfo & other) = default; + + UnkeyedValidPathInfo(Hash narHash) : narHash(narHash) { }; + + DECLARE_CMP(UnkeyedValidPathInfo); + + virtual ~UnkeyedValidPathInfo() { } +}; + +struct ValidPathInfo : UnkeyedValidPathInfo { + StorePath path; + + DECLARE_CMP(ValidPathInfo); + /** * Return a fingerprint of the store path to be used in binary * cache signatures. It contains the store path, the base-32 @@ -84,11 +97,11 @@ struct ValidPathInfo void sign(const Store & store, const SecretKey & secretKey); - /** - * @return The `ContentAddressWithReferences` that determines the - * store path for a content-addressed store object, `std::nullopt` - * for an input-addressed store object. - */ + /** + * @return The `ContentAddressWithReferences` that determines the + * store path for a content-addressed store object, `std::nullopt` + * for an input-addressed store object. + */ std::optional contentAddressWithReferences() const; /** @@ -114,8 +127,8 @@ struct ValidPathInfo ValidPathInfo(const ValidPathInfo & other) = default; - ValidPathInfo(StorePath && path, Hash narHash) : path(std::move(path)), narHash(narHash) { }; - ValidPathInfo(const StorePath & path, Hash narHash) : path(path), narHash(narHash) { }; + ValidPathInfo(StorePath && path, UnkeyedValidPathInfo info) : UnkeyedValidPathInfo(info), path(std::move(path)) { }; + ValidPathInfo(const StorePath & path, UnkeyedValidPathInfo info) : UnkeyedValidPathInfo(info), path(path) { }; ValidPathInfo(const Store & store, std::string_view name, ContentAddressWithReferences && ca, Hash narHash); diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index aaea5c4a2..7bdc25433 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -332,7 +332,8 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path)); } info = std::make_shared( - WorkerProto::Serialise::read(*this, *conn, StorePath{path})); + StorePath{path}, + WorkerProto::Serialise::read(*this, *conn)); } callback(std::move(info)); } catch (...) { callback.rethrow(); } diff --git a/src/libstore/tests/worker-protocol.cc b/src/libstore/tests/worker-protocol.cc index e0ce340d8..ad5943c69 100644 --- a/src/libstore/tests/worker-protocol.cc +++ b/src/libstore/tests/worker-protocol.cc @@ -329,6 +329,159 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + unkeyedValidPathInfo_1_15, + "unkeyed-valid-path-info-1.15", + 1 << 8 | 15, + (std::tuple { + ({ + UnkeyedValidPathInfo info { + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }; + info.registrationTime = 23423; + info.narSize = 34878; + info; + }), + ({ + UnkeyedValidPathInfo info { + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }; + info.deriver = StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }; + info.references = { + StorePath { + "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo.drv", + }, + }; + info.registrationTime = 23423; + info.narSize = 34878; + info; + }), + })) + +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + validPathInfo_1_15, + "valid-path-info-1.15", + 1 << 8 | 15, + (std::tuple { + ({ + ValidPathInfo info { + StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar", + }, + UnkeyedValidPathInfo { + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }, + }; + info.registrationTime = 23423; + info.narSize = 34878; + info; + }), + ({ + ValidPathInfo info { + StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar", + }, + UnkeyedValidPathInfo { + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }, + }; + info.deriver = StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }; + info.references = { + // other reference + StorePath { + "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo", + }, + // self reference + StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar", + }, + }; + info.registrationTime = 23423; + info.narSize = 34878; + info; + }), + })) + +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + validPathInfo_1_16, + "valid-path-info-1.16", + 1 << 8 | 16, + (std::tuple { + ({ + ValidPathInfo info { + StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar", + }, + UnkeyedValidPathInfo { + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }, + }; + info.registrationTime = 23423; + info.narSize = 34878; + info.ultimate = true; + info; + }), + ({ + ValidPathInfo info { + StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar", + }, + UnkeyedValidPathInfo { + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }, + }; + info.deriver = StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }; + info.references = { + // other reference + StorePath { + "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo", + }, + // self reference + StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar", + }, + }; + info.registrationTime = 23423; + info.narSize = 34878; + info.sigs = { + "fake-sig-1", + "fake-sig-2", + }, + 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.registrationTime = 23423; + info.narSize = 34878; + info; + }), + })) + VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, optionalTrustedFlag, diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index 94f8c8811..d618b9bd8 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -145,14 +145,24 @@ void WorkerProto::Serialise::write(const Store & store, WorkerProto ValidPathInfo WorkerProto::Serialise::read(const Store & store, ReadConn conn) { auto path = WorkerProto::Serialise::read(store, conn); - return WorkerProto::Serialise::read(store, conn, std::move(path)); + return ValidPathInfo { + std::move(path), + WorkerProto::Serialise::read(store, conn), + }; } -ValidPathInfo WorkerProto::Serialise::read(const Store & store, ReadConn conn, StorePath && path) +void WorkerProto::Serialise::write(const Store & store, WriteConn conn, const ValidPathInfo & pathInfo) +{ + WorkerProto::write(store, conn, pathInfo.path); + WorkerProto::write(store, conn, static_cast(pathInfo)); +} + + +UnkeyedValidPathInfo WorkerProto::Serialise::read(const Store & store, ReadConn conn) { auto deriver = readString(conn.from); auto narHash = Hash::parseAny(readString(conn.from), htSHA256); - ValidPathInfo info(path, narHash); + UnkeyedValidPathInfo info(narHash); if (deriver != "") info.deriver = store.parseStorePath(deriver); info.references = WorkerProto::Serialise::read(store, conn); conn.from >> info.registrationTime >> info.narSize; @@ -164,14 +174,8 @@ ValidPathInfo WorkerProto::Serialise::read(const Store & store, R return info; } -void WorkerProto::Serialise::write( - const Store & store, - WriteConn conn, - const ValidPathInfo & pathInfo, - bool includePath) +void WorkerProto::Serialise::write(const Store & store, WriteConn conn, const UnkeyedValidPathInfo & pathInfo) { - if (includePath) - conn.to << store.printStorePath(pathInfo.path); conn.to << (pathInfo.deriver ? store.printStorePath(*pathInfo.deriver) : "") << pathInfo.narHash.to_string(HashFormat::Base16, false); diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index d35e3c678..dcd54ad16 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -32,6 +32,7 @@ struct DerivedPath; struct BuildResult; struct KeyedBuildResult; struct ValidPathInfo; +struct UnkeyedValidPathInfo; enum TrustedFlag : bool; @@ -207,6 +208,10 @@ DECLARE_WORKER_SERIALISER(BuildResult); template<> DECLARE_WORKER_SERIALISER(KeyedBuildResult); template<> +DECLARE_WORKER_SERIALISER(ValidPathInfo); +template<> +DECLARE_WORKER_SERIALISER(UnkeyedValidPathInfo); +template<> DECLARE_WORKER_SERIALISER(std::optional); template @@ -221,15 +226,4 @@ template DECLARE_WORKER_SERIALISER(std::map); #undef COMMA_ -/* These are a non-standard form for historical reasons. */ - -template<> -struct WorkerProto::Serialise -{ - static ValidPathInfo read(const Store & store, WorkerProto::ReadConn conn); - static ValidPathInfo read(const Store & store, WorkerProto::ReadConn conn, StorePath && path); - - static void write(const Store & store, WriteConn conn, const ValidPathInfo & pathInfo, bool includePath = true); -}; - } diff --git a/unit-test-data/libstore/worker-protocol/unkeyed-valid-path-info-1.15.bin b/unit-test-data/libstore/worker-protocol/unkeyed-valid-path-info-1.15.bin new file mode 100644 index 0000000000000000000000000000000000000000..e69ccbe83862b29e3f79810c0c2ff4561a42f86f GIT binary patch literal 328 zcmbu4%?`pK5QOo8D!&WM#q>Qq0R{CUX=_wYUVUsEAs(9a)(w2^a=MH48Q8;TGRcmlTf(6rfXPFjSJ46(yf&k8}#9A2z znmI2!XwnU&AmvqJYIhQk7+x#{!gQkU@$Il_5ceP*O@N zxu!r+#$?P>4g#ry<$$?w`2MnGPahlJ`HLM!?tH6m@;lyRZZ9kI;P`*_d+&94ym9_l z{ZG@h-`{zOWuKw$x?n$F_k7^>Jh*~Rnvu&5}L}Qp+gUA>+#+bF!C(UNw&;S4c literal 0 HcmV?d00001