From 96c58550b839ca415a2cef0f7a5ae51f3e86f028 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 16 Oct 2023 01:45:06 -0400 Subject: [PATCH 1/5] Test more derived paths --- src/libstore/tests/worker-protocol.cc | 11 ++++++++++- .../libstore/worker-protocol/derived-path.bin | Bin 120 -> 248 bytes 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libstore/tests/worker-protocol.cc b/src/libstore/tests/worker-protocol.cc index 63fecce96..b10192cc1 100644 --- a/src/libstore/tests/worker-protocol.cc +++ b/src/libstore/tests/worker-protocol.cc @@ -72,10 +72,19 @@ VERSIONED_CHARACTERIZATION_TEST( derivedPath, "derived-path", defaultVersion, - (std::tuple { + (std::tuple { DerivedPath::Opaque { .path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, }, + DerivedPath::Opaque { + .path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, + }, + DerivedPath::Built { + .drvPath = makeConstantStorePathRef(StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }), + .outputs = OutputsSpec::All { }, + }, DerivedPath::Built { .drvPath = makeConstantStorePathRef(StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", diff --git a/unit-test-data/libstore/worker-protocol/derived-path.bin b/unit-test-data/libstore/worker-protocol/derived-path.bin index bb1a81ac6d096cddf510a457e7506e6a81ee66a3..0729b2690e2f99dc8122b013e4ceaa4a51ebd3d6 100644 GIT binary patch delta 39 mcmb>U!8pN!(Rd=G8JAv4Q5gdWm`(%= Date: Fri, 26 May 2023 14:48:11 -0400 Subject: [PATCH 2/5] Systematize the worker protocol derived path serialiser It was some ad-hoc functions to account for versions, while the already factored-out serializer just supported the latest version. Now, we can fold that version-specific logic into the factored out one, and so we do. --- src/libstore/daemon.cc | 18 ++-------- src/libstore/remote-store.cc | 33 ++---------------- src/libstore/tests/worker-protocol.cc | 29 +++++++++++++-- src/libstore/worker-protocol.cc | 26 ++++++++++++-- .../worker-protocol/derived-path-1.29.bin | Bin 0 -> 184 bytes ...derived-path.bin => derived-path-1.30.bin} | Bin 6 files changed, 56 insertions(+), 50 deletions(-) create mode 100644 unit-test-data/libstore/worker-protocol/derived-path-1.29.bin rename unit-test-data/libstore/worker-protocol/{derived-path.bin => derived-path-1.30.bin} (100%) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index e117b507f..27c0b6431 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -261,18 +261,6 @@ struct ClientSettings } }; -static std::vector readDerivedPaths(Store & store, WorkerProto::Version clientVersion, WorkerProto::ReadConn conn) -{ - std::vector reqs; - if (GET_PROTOCOL_MINOR(clientVersion) >= 30) { - reqs = WorkerProto::Serialise>::read(store, conn); - } else { - for (auto & s : readStrings(conn.from)) - reqs.push_back(parsePathWithOutputs(store, s).toDerivedPath()); - } - return reqs; -} - static void performOp(TunnelLogger * logger, ref store, TrustedFlag trusted, RecursiveFlag recursive, WorkerProto::Version clientVersion, Source & from, BufferedSink & to, WorkerProto::Op op) @@ -538,7 +526,7 @@ static void performOp(TunnelLogger * logger, ref store, } case WorkerProto::Op::BuildPaths: { - auto drvs = readDerivedPaths(*store, clientVersion, rconn); + auto drvs = WorkerProto::Serialise::read(*store, rconn); BuildMode mode = bmNormal; if (GET_PROTOCOL_MINOR(clientVersion) >= 15) { mode = (BuildMode) readInt(from); @@ -563,7 +551,7 @@ static void performOp(TunnelLogger * logger, ref store, } case WorkerProto::Op::BuildPathsWithResults: { - auto drvs = readDerivedPaths(*store, clientVersion, rconn); + auto drvs = WorkerProto::Serialise::read(*store, rconn); BuildMode mode = bmNormal; mode = (BuildMode) readInt(from); @@ -938,7 +926,7 @@ static void performOp(TunnelLogger * logger, ref store, } case WorkerProto::Op::QueryMissing: { - auto targets = readDerivedPaths(*store, clientVersion, rconn); + auto targets = WorkerProto::Serialise::read(*store, rconn); logger->startWork(); StorePathSet willBuild, willSubstitute, unknown; uint64_t downloadSize, narSize; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 1704bfbb0..482c2ae01 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -655,33 +655,6 @@ void RemoteStore::queryRealisationUncached(const DrvOutput & id, } catch (...) { return callback.rethrow(); } } -static void writeDerivedPaths(RemoteStore & store, RemoteStore::Connection & conn, const std::vector & reqs) -{ - if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 30) { - WorkerProto::write(store, conn, reqs); - } else { - Strings ss; - for (auto & p : reqs) { - auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(p); - std::visit(overloaded { - [&](const StorePathWithOutputs & s) { - ss.push_back(s.to_string(store)); - }, - [&](const StorePath & drvPath) { - throw Error("trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file", - store.printStorePath(drvPath), - GET_PROTOCOL_MAJOR(conn.daemonVersion), - GET_PROTOCOL_MINOR(conn.daemonVersion)); - }, - [&](std::monostate) { - throw Error("wanted to build a derivation that is itself a build product, but the legacy 'ssh://' protocol doesn't support that. Try using 'ssh-ng://'"); - }, - }, sOrDrvPath); - } - conn.to << ss; - } -} - void RemoteStore::copyDrvsFromEvalStore( const std::vector & paths, std::shared_ptr evalStore) @@ -711,7 +684,7 @@ void RemoteStore::buildPaths(const std::vector & drvPaths, BuildMod auto conn(getConnection()); conn->to << WorkerProto::Op::BuildPaths; assert(GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13); - writeDerivedPaths(*this, *conn, drvPaths); + WorkerProto::write(*this, *conn, drvPaths); if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15) conn->to << buildMode; else @@ -735,7 +708,7 @@ std::vector RemoteStore::buildPathsWithResults( if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 34) { conn->to << WorkerProto::Op::BuildPathsWithResults; - writeDerivedPaths(*this, *conn, paths); + WorkerProto::write(*this, *conn, paths); conn->to << buildMode; conn.processStderr(); return WorkerProto::Serialise>::read(*this, *conn); @@ -929,7 +902,7 @@ void RemoteStore::queryMissing(const std::vector & targets, // to prevent a deadlock. goto fallback; conn->to << WorkerProto::Op::QueryMissing; - writeDerivedPaths(*this, *conn, targets); + WorkerProto::write(*this, *conn, targets); conn.processStderr(); willBuild = WorkerProto::Serialise::read(*this, *conn); willSubstitute = WorkerProto::Serialise::read(*this, *conn); diff --git a/src/libstore/tests/worker-protocol.cc b/src/libstore/tests/worker-protocol.cc index b10192cc1..b3e857bc4 100644 --- a/src/libstore/tests/worker-protocol.cc +++ b/src/libstore/tests/worker-protocol.cc @@ -69,9 +69,32 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - derivedPath, - "derived-path", - defaultVersion, + derivedPath_1_29, + "derived-path-1.29", + 1 << 8 | 29, + (std::tuple { + DerivedPath::Opaque { + .path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + DerivedPath::Built { + .drvPath = makeConstantStorePathRef(StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }), + .outputs = OutputsSpec::All { }, + }, + DerivedPath::Built { + .drvPath = makeConstantStorePathRef(StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }), + .outputs = OutputsSpec::Names { "x", "y" }, + }, + })) + +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + derivedPath_1_30, + "derived-path-1.30", + 1 << 8 | 30, (std::tuple { DerivedPath::Opaque { .path = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index 415e66f16..0f3f20ca5 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -51,12 +51,34 @@ void WorkerProto::Serialise>::write(const Store & sto DerivedPath WorkerProto::Serialise::read(const Store & store, WorkerProto::ReadConn conn) { auto s = readString(conn.from); - return DerivedPath::parseLegacy(store, s); + if (GET_PROTOCOL_MINOR(conn.version) >= 30) { + return DerivedPath::parseLegacy(store, s); + } else { + return parsePathWithOutputs(store, s).toDerivedPath(); + } } void WorkerProto::Serialise::write(const Store & store, WorkerProto::WriteConn conn, const DerivedPath & req) { - conn.to << req.to_string_legacy(store); + if (GET_PROTOCOL_MINOR(conn.version) >= 30) { + conn.to << req.to_string_legacy(store); + } else { + auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(req); + std::visit(overloaded { + [&](const StorePathWithOutputs & s) { + conn.to << s.to_string(store); + }, + [&](const StorePath & drvPath) { + throw Error("trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file", + store.printStorePath(drvPath), + GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MINOR(conn.version)); + }, + [&](std::monostate) { + throw Error("wanted to build a derivation that is itself a build product, but protocols do not support that. Try upgrading the Nix on the other end of this connection"); + }, + }, sOrDrvPath); + } } diff --git a/unit-test-data/libstore/worker-protocol/derived-path-1.29.bin b/unit-test-data/libstore/worker-protocol/derived-path-1.29.bin new file mode 100644 index 0000000000000000000000000000000000000000..05ea7678aa058344a5ca4cb1e9e7333e9cf8fe20 GIT binary patch literal 184 zcmdOAfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7Eu*F&X-j5{vXwipsz`&B@oVSfNwN F001Y$HlP3i literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/worker-protocol/derived-path.bin b/unit-test-data/libstore/worker-protocol/derived-path-1.30.bin similarity index 100% rename from unit-test-data/libstore/worker-protocol/derived-path.bin rename to unit-test-data/libstore/worker-protocol/derived-path-1.30.bin From ab822af0dfee341be131ad285a2cba5362a3f2dc Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 28 Feb 2021 18:42:46 +0000 Subject: [PATCH 3/5] Factor out serialization for `BuildResult` Worker Protocol: Note that the worker protocol already had a serialization for `BuildResult`; this was added in a4604f19284254ac98f19a13ff7c2216de7fe176. It didn't have any versioning support because at that time reusable seralizers were not away for the protocol version. It could thus only be used for new messages also introduced in that commit. Now that we do support versioning in reusable serializers, we can expand it to support all known versions and use it in many more places. The exist test data becomes the version 1.29 tests: note that those files' contents are unchanged. 1.28 and 1.27 tests are added to cover the older code-paths. The keyered build result test only has 1.29 because the keying was also added in a4604f19284254ac98f19a13ff7c2216de7fe176; the older serializations are always used unkeyed. Serve Protocol: Conversely, no attempt was made to factor out such a serializer for the serve protocol, so our work there in this commit for that protocol proceeds from scratch. --- src/libstore/daemon.cc | 11 +- src/libstore/legacy-ssh-store.cc | 15 +-- src/libstore/remote-store.cc | 15 +-- src/libstore/serve-protocol.cc | 43 +++++++ src/libstore/serve-protocol.hh | 6 + src/libstore/tests/serve-protocol.cc | 113 +++++++++++++++++- src/libstore/tests/worker-protocol.cc | 80 ++++++++++++- src/libstore/worker-protocol.cc | 49 ++++---- src/nix-store/nix-store.cc | 12 +- .../serve-protocol/build-result-2.2.bin | Bin 0 -> 80 bytes .../serve-protocol/build-result-2.3.bin | Bin 0 -> 176 bytes .../build-result-2.6.bin} | Bin .../worker-protocol/build-result-1.27.bin | Bin 0 -> 80 bytes .../worker-protocol/build-result-1.28.bin | Bin 0 -> 648 bytes .../worker-protocol/build-result-1.29.bin | Bin 0 -> 744 bytes ...result.bin => keyed-build-result-1.29.bin} | Bin 16 files changed, 268 insertions(+), 76 deletions(-) create mode 100644 unit-test-data/libstore/serve-protocol/build-result-2.2.bin create mode 100644 unit-test-data/libstore/serve-protocol/build-result-2.3.bin rename unit-test-data/libstore/{worker-protocol/build-result.bin => serve-protocol/build-result-2.6.bin} (100%) create mode 100644 unit-test-data/libstore/worker-protocol/build-result-1.27.bin create mode 100644 unit-test-data/libstore/worker-protocol/build-result-1.28.bin create mode 100644 unit-test-data/libstore/worker-protocol/build-result-1.29.bin rename unit-test-data/libstore/worker-protocol/{keyed-build-result.bin => keyed-build-result-1.29.bin} (100%) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 27c0b6431..19afe4388 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -635,16 +635,7 @@ static void performOp(TunnelLogger * logger, ref store, auto res = store->buildDerivation(drvPath, drv, buildMode); logger->stopWork(); - to << res.status << res.errorMsg; - if (GET_PROTOCOL_MINOR(clientVersion) >= 29) { - to << res.timesBuilt << res.isNonDeterministic << res.startTime << res.stopTime; - } - if (GET_PROTOCOL_MINOR(clientVersion) >= 28) { - DrvOutputs builtOutputs; - for (auto & [output, realisation] : res.builtOutputs) - builtOutputs.insert_or_assign(realisation.id, realisation); - WorkerProto::write(*store, wconn, builtOutputs); - } + WorkerProto::write(*store, wconn, res); break; } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index d6e24521a..c712f7eb1 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -319,20 +319,7 @@ public: conn->to.flush(); - BuildResult status; - status.status = (BuildResult::Status) readInt(conn->from); - conn->from >> status.errorMsg; - - 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 = ServeProto::Serialise::read(*this, *conn); - for (auto && [output, realisation] : builtOutputs) - status.builtOutputs.insert_or_assign( - std::move(output.outputName), - std::move(realisation)); - } - return status; + return ServeProto::Serialise::read(*this, *conn); } void buildPaths(const std::vector & drvPaths, BuildMode buildMode, std::shared_ptr evalStore) override diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 482c2ae01..ef648e01b 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -788,20 +788,7 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD writeDerivation(conn->to, *this, drv); conn->to << buildMode; conn.processStderr(); - BuildResult res; - res.status = (BuildResult::Status) readInt(conn->from); - conn->from >> res.errorMsg; - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) { - conn->from >> res.timesBuilt >> res.isNonDeterministic >> res.startTime >> res.stopTime; - } - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 28) { - auto builtOutputs = WorkerProto::Serialise::read(*this, *conn); - for (auto && [output, realisation] : builtOutputs) - res.builtOutputs.insert_or_assign( - std::move(output.outputName), - std::move(realisation)); - } - return res; + return WorkerProto::Serialise::read(*this, *conn); } diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc index 16a62b5bc..97a0ddf0e 100644 --- a/src/libstore/serve-protocol.cc +++ b/src/libstore/serve-protocol.cc @@ -2,6 +2,7 @@ #include "util.hh" #include "path-with-outputs.hh" #include "store-api.hh" +#include "build-result.hh" #include "serve-protocol.hh" #include "serve-protocol-impl.hh" #include "archive.hh" @@ -12,4 +13,46 @@ namespace nix { /* protocol-specific definitions */ +BuildResult ServeProto::Serialise::read(const Store & store, ServeProto::ReadConn conn) +{ + BuildResult status; + status.status = (BuildResult::Status) readInt(conn.from); + conn.from >> status.errorMsg; + + if (GET_PROTOCOL_MINOR(conn.version) >= 3) + conn.from + >> status.timesBuilt + >> status.isNonDeterministic + >> status.startTime + >> status.stopTime; + if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + auto builtOutputs = ServeProto::Serialise::read(store, conn); + for (auto && [output, realisation] : builtOutputs) + status.builtOutputs.insert_or_assign( + std::move(output.outputName), + std::move(realisation)); + } + return status; +} + +void ServeProto::Serialise::write(const Store & store, ServeProto::WriteConn conn, const BuildResult & status) +{ + conn.to + << status.status + << status.errorMsg; + + if (GET_PROTOCOL_MINOR(conn.version) >= 3) + conn.to + << status.timesBuilt + << status.isNonDeterministic + << status.startTime + << status.stopTime; + if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + DrvOutputs builtOutputs; + for (auto & [output, realisation] : status.builtOutputs) + builtOutputs.insert_or_assign(realisation.id, realisation); + ServeProto::write(store, conn, builtOutputs); + } +} + } diff --git a/src/libstore/serve-protocol.hh b/src/libstore/serve-protocol.hh index a627c6ad6..ba159f6e9 100644 --- a/src/libstore/serve-protocol.hh +++ b/src/libstore/serve-protocol.hh @@ -16,6 +16,9 @@ namespace nix { class Store; struct Source; +// items being serialised +struct BuildResult; + /** * The "serve protocol", used by ssh:// stores. @@ -136,6 +139,9 @@ inline std::ostream & operator << (std::ostream & s, ServeProto::Command op) static void write(const Store & store, ServeProto::WriteConn conn, const T & t); \ }; +template<> +DECLARE_SERVE_SERIALISER(BuildResult); + template DECLARE_SERVE_SERIALISER(std::vector); template diff --git a/src/libstore/tests/serve-protocol.cc b/src/libstore/tests/serve-protocol.cc index ba798ab1c..c8ac87a04 100644 --- a/src/libstore/tests/serve-protocol.cc +++ b/src/libstore/tests/serve-protocol.cc @@ -19,7 +19,7 @@ struct ServeProtoTest : VersionedProtoTest * For serializers that don't care about the minimum version, we * used the oldest one: 1.0. */ - ServeProto::Version defaultVersion = 1 << 8 | 0; + ServeProto::Version defaultVersion = 2 << 8 | 0; }; VERSIONED_CHARACTERIZATION_TEST( @@ -114,6 +114,117 @@ VERSIONED_CHARACTERIZATION_TEST( }, })) +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_2, + "build-result-2.2", + 2 << 8 | 2, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::Built, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_3, + "build-result-2.3", + 2 << 8 | 3, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + .timesBuilt = 3, + .isNonDeterministic = true, + .startTime = 30, + .stopTime = 50, + }, + BuildResult { + .status = BuildResult::Built, + .startTime = 30, + .stopTime = 50, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_6, + "build-result-2.6", + 2 << 8 | 6, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + .timesBuilt = 3, + .isNonDeterministic = true, + .startTime = 30, + .stopTime = 50, + }, + BuildResult { + .status = BuildResult::Built, + .timesBuilt = 1, + .builtOutputs = { + { + "foo", + { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "foo", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + }, + { + "bar", + { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "bar", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, + }, + }, + }, + .startTime = 30, + .stopTime = 50, +#if 0 + // These fields are not yet serialized. + // FIXME Include in next version of protocol or document + // why they are skipped. + .cpuUser = std::chrono::milliseconds(500s), + .cpuSystem = std::chrono::milliseconds(604s), +#endif + }, + }; + t; + })) + VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, vector, diff --git a/src/libstore/tests/worker-protocol.cc b/src/libstore/tests/worker-protocol.cc index b3e857bc4..e0ce340d8 100644 --- a/src/libstore/tests/worker-protocol.cc +++ b/src/libstore/tests/worker-protocol.cc @@ -167,9 +167,77 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - buildResult, - "build-result", - defaultVersion, + buildResult_1_27, + "build-result-1.27", + 1 << 8 | 27, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::Built, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + buildResult_1_28, + "build-result-1.28", + 1 << 8 | 28, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::Built, + .builtOutputs = { + { + "foo", + { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "foo", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + }, + { + "bar", + { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "bar", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, + }, + }, + }, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + buildResult_1_29, + "build-result-1.29", + 1 << 8 | 29, ({ using namespace std::literals::chrono_literals; std::tuple t { @@ -226,9 +294,9 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - keyedBuildResult, - "keyed-build-result", - defaultVersion, + keyedBuildResult_1_29, + "keyed-build-result-1.29", + 1 << 8 | 29, ({ using namespace std::literals::chrono_literals; std::tuple t { diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index 0f3f20ca5..f69322a61 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -103,17 +103,21 @@ BuildResult WorkerProto::Serialise::read(const Store & store, Worke { BuildResult res; res.status = (BuildResult::Status) readInt(conn.from); - conn.from - >> res.errorMsg - >> res.timesBuilt - >> res.isNonDeterministic - >> res.startTime - >> res.stopTime; - auto builtOutputs = WorkerProto::Serialise::read(store, conn); - for (auto && [output, realisation] : builtOutputs) - res.builtOutputs.insert_or_assign( - std::move(output.outputName), - std::move(realisation)); + conn.from >> res.errorMsg; + if (GET_PROTOCOL_MINOR(conn.version) >= 29) { + conn.from + >> res.timesBuilt + >> res.isNonDeterministic + >> res.startTime + >> res.stopTime; + } + if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + auto builtOutputs = WorkerProto::Serialise::read(store, conn); + for (auto && [output, realisation] : builtOutputs) + res.builtOutputs.insert_or_assign( + std::move(output.outputName), + std::move(realisation)); + } return res; } @@ -121,15 +125,20 @@ void WorkerProto::Serialise::write(const Store & store, WorkerProto { conn.to << res.status - << res.errorMsg - << res.timesBuilt - << res.isNonDeterministic - << res.startTime - << res.stopTime; - DrvOutputs builtOutputs; - for (auto & [output, realisation] : res.builtOutputs) - builtOutputs.insert_or_assign(realisation.id, realisation); - WorkerProto::write(store, conn, builtOutputs); + << res.errorMsg; + if (GET_PROTOCOL_MINOR(conn.version) >= 29) { + conn.to + << res.timesBuilt + << res.isNonDeterministic + << res.startTime + << res.stopTime; + } + if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + DrvOutputs builtOutputs; + for (auto & [output, realisation] : res.builtOutputs) + builtOutputs.insert_or_assign(realisation.id, realisation); + WorkerProto::write(store, conn, builtOutputs); + } } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index e78720f92..e4dd94585 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -959,17 +959,7 @@ static void opServe(Strings opFlags, Strings opArgs) MonitorFdHup monitor(in.fd); auto status = store->buildDerivation(drvPath, drv); - out << status.status << status.errorMsg; - - if (GET_PROTOCOL_MINOR(clientVersion) >= 3) - out << status.timesBuilt << status.isNonDeterministic << status.startTime << status.stopTime; - if (GET_PROTOCOL_MINOR(clientVersion) >= 6) { - DrvOutputs builtOutputs; - for (auto & [output, realisation] : status.builtOutputs) - builtOutputs.insert_or_assign(realisation.id, realisation); - ServeProto::write(*store, wconn, builtOutputs); - } - + ServeProto::write(*store, wconn, status); break; } diff --git a/unit-test-data/libstore/serve-protocol/build-result-2.2.bin b/unit-test-data/libstore/serve-protocol/build-result-2.2.bin new file mode 100644 index 0000000000000000000000000000000000000000..ae684778bc26addba4bf1b3e49cc30edf5f038fa GIT binary patch literal 80 gcmZQ&fBf0AiU4H~;_u literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/serve-protocol/build-result-2.3.bin b/unit-test-data/libstore/serve-protocol/build-result-2.3.bin new file mode 100644 index 0000000000000000000000000000000000000000..d51e08dfc0d1715210e8a616a5403f03f4b2a46c GIT binary patch literal 176 vcmZQ&fBf0AiU4H~;_u literal 0 HcmV?d00001 diff --git a/unit-test-data/libstore/worker-protocol/build-result-1.28.bin b/unit-test-data/libstore/worker-protocol/build-result-1.28.bin new file mode 100644 index 0000000000000000000000000000000000000000..74bcd5cf98b828fb63a931ef7810f7fabc805ca5 GIT binary patch literal 648 zcmc&wK?=e!5EQ|aK0%e!+`)6%Si?(*`6_8xaxz$KI6Xx-2t!SGa{k;2f8i+RMR2A;`6>RitBjDY7{lnANJD3OVh8WW_c zkTRG+zDX!Yj%68orEsd#Y$KG=*{FoWL-7`MFAQl%7RmZ0!PYe3jk66aF4r+L$O`tu z&uq-x(J#Q)LAOdzsy>VTJDdcofzX)BfXe~O6CwQ^%woR?}>#sX4il$bfs;n zmv%`YOQ~vvTo;t-%xH@l(n4vSK8bRZQHc`kI^B)Ih0TkNGRhXy8rqZN5BnYj(ieFo zAJ+t*u7l`;??iPt&V)lzi91dfGZFf@g4iVAZN4+jUVRU7o>ol_o!fedeM@Pl_mAVf T^ROZOQyyvZZF!s Date: Tue, 8 Mar 2022 23:09:26 +0000 Subject: [PATCH 4/5] Move `ValidPathInfo` serialization code to `worker-protocol.{cc.hh}` It does not belong with the data type itself. This also materializes the fact that `copyPath` does not do any version negotiation just just hard-codes "16". The non-standard interface of these serializers makes it harder to test, but this is fixed in the next commit which then adds those tests. --- src/libstore/daemon.cc | 4 +-- src/libstore/path-info.cc | 54 --------------------------------- src/libstore/path-info.hh | 5 --- src/libstore/remote-store.cc | 11 +++++-- src/libstore/store-api.cc | 11 ++++++- src/libstore/worker-protocol.cc | 45 ++++++++++++++++++++++++++- src/libstore/worker-protocol.hh | 12 ++++++++ 7 files changed, 76 insertions(+), 66 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 19afe4388..0afaaccba 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -422,7 +422,7 @@ static void performOp(TunnelLogger * logger, ref store, }(); logger->stopWork(); - pathInfo->write(to, *store, GET_PROTOCOL_MINOR(clientVersion)); + WorkerProto::Serialise::write(*store, wconn, *pathInfo); } else { HashType hashAlgo; std::string baseName; @@ -819,7 +819,7 @@ static void performOp(TunnelLogger * logger, ref store, if (info) { if (GET_PROTOCOL_MINOR(clientVersion) >= 17) to << 1; - info->write(to, *store, GET_PROTOCOL_MINOR(clientVersion), false); + WorkerProto::Serialise::write(*store, wconn, *info, false); } else { assert(GET_PROTOCOL_MINOR(clientVersion) >= 17); to << 0; diff --git a/src/libstore/path-info.cc b/src/libstore/path-info.cc index fb507a847..51fb5f02a 100644 --- a/src/libstore/path-info.cc +++ b/src/libstore/path-info.cc @@ -1,6 +1,4 @@ #include "path-info.hh" -#include "worker-protocol.hh" -#include "worker-protocol-impl.hh" #include "store-api.hh" namespace nix { @@ -99,7 +97,6 @@ Strings ValidPathInfo::shortRefs() const return refs; } - ValidPathInfo::ValidPathInfo( const Store & store, std::string_view name, @@ -128,55 +125,4 @@ ValidPathInfo::ValidPathInfo( }, std::move(ca).raw); } - -ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format) -{ - return read(source, store, format, store.parseStorePath(readString(source))); -} - -ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned int format, StorePath && path) -{ - auto deriver = readString(source); - auto narHash = Hash::parseAny(readString(source), htSHA256); - ValidPathInfo info(path, narHash); - if (deriver != "") info.deriver = store.parseStorePath(deriver); - info.references = WorkerProto::Serialise::read(store, - WorkerProto::ReadConn { - .from = source, - .version = format, - }); - source >> info.registrationTime >> info.narSize; - if (format >= 16) { - source >> info.ultimate; - info.sigs = readStrings(source); - info.ca = ContentAddress::parseOpt(readString(source)); - } - return info; -} - - -void ValidPathInfo::write( - Sink & sink, - const Store & store, - unsigned int format, - bool includePath) const -{ - if (includePath) - sink << store.printStorePath(path); - sink << (deriver ? store.printStorePath(*deriver) : "") - << narHash.to_string(HashFormat::Base16, false); - WorkerProto::write(store, - WorkerProto::WriteConn { - .to = sink, - .version = format, - }, - references); - sink << registrationTime << narSize; - if (format >= 16) { - sink << ultimate - << sigs - << renderContentAddress(ca); - } -} - } diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh index fea6d0e5f..919fe32f1 100644 --- a/src/libstore/path-info.hh +++ b/src/libstore/path-info.hh @@ -121,11 +121,6 @@ struct ValidPathInfo std::string_view name, ContentAddressWithReferences && ca, Hash narHash); virtual ~ValidPathInfo() { } - - static ValidPathInfo read(Source & source, const Store & store, unsigned int format); - static ValidPathInfo read(Source & source, const Store & store, unsigned int format, StorePath && path); - - void write(Sink & sink, const Store & store, unsigned int format, bool includePath = true) const; }; typedef std::map ValidPathInfos; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index ef648e01b..aaea5c4a2 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -332,7 +332,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path)); } info = std::make_shared( - ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion), StorePath{path})); + WorkerProto::Serialise::read(*this, *conn, StorePath{path})); } callback(std::move(info)); } catch (...) { callback.rethrow(); } @@ -445,7 +445,7 @@ ref RemoteStore::addCAToStore( } return make_ref( - ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion))); + WorkerProto::Serialise::read(*this, *conn)); } else { if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25"); @@ -570,7 +570,12 @@ void RemoteStore::addMultipleToStore( auto source = sinkToSource([&](Sink & sink) { sink << pathsToCopy.size(); for (auto & [pathInfo, pathSource] : pathsToCopy) { - pathInfo.write(sink, *this, 16); + WorkerProto::Serialise::write(*this, + WorkerProto::WriteConn { + .to = sink, + .version = 16, + }, + pathInfo); pathSource->drainInto(sink); } }); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 0c58cca0c..785b26062 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -11,6 +11,9 @@ #include "archive.hh" #include "callback.hh" #include "remote-store.hh" +// FIXME this should not be here, see TODO below on +// `addMultipleToStore`. +#include "worker-protocol.hh" #include #include @@ -357,7 +360,13 @@ void Store::addMultipleToStore( { auto expected = readNum(source); for (uint64_t i = 0; i < expected; ++i) { - auto info = ValidPathInfo::read(source, *this, 16); + // FIXME we should not be using the worker protocol here, let + // alone the worker protocol with a hard-coded version! + auto info = WorkerProto::Serialise::read(*this, + WorkerProto::ReadConn { + .from = source, + .version = 16, + }); info.ultimate = false; addToStore(info, source, repair, checkSigs); } diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index f69322a61..94f8c8811 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -6,7 +6,7 @@ #include "worker-protocol.hh" #include "worker-protocol-impl.hh" #include "archive.hh" -#include "derivations.hh" +#include "path-info.hh" #include @@ -142,4 +142,47 @@ 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)); +} + +ValidPathInfo WorkerProto::Serialise::read(const Store & store, ReadConn conn, StorePath && path) +{ + auto deriver = readString(conn.from); + auto narHash = Hash::parseAny(readString(conn.from), htSHA256); + ValidPathInfo info(path, narHash); + if (deriver != "") info.deriver = store.parseStorePath(deriver); + info.references = WorkerProto::Serialise::read(store, conn); + conn.from >> info.registrationTime >> info.narSize; + if (GET_PROTOCOL_MINOR(conn.version) >= 16) { + conn.from >> info.ultimate; + info.sigs = readStrings(conn.from); + info.ca = ContentAddress::parseOpt(readString(conn.from)); + } + return info; +} + +void WorkerProto::Serialise::write( + const Store & store, + WriteConn conn, + const ValidPathInfo & pathInfo, + bool includePath) +{ + if (includePath) + conn.to << store.printStorePath(pathInfo.path); + conn.to + << (pathInfo.deriver ? store.printStorePath(*pathInfo.deriver) : "") + << pathInfo.narHash.to_string(HashFormat::Base16, false); + WorkerProto::write(store, conn, pathInfo.references); + conn.to << pathInfo.registrationTime << pathInfo.narSize; + if (GET_PROTOCOL_MINOR(conn.version) >= 16) { + conn.to + << pathInfo.ultimate + << pathInfo.sigs + << renderContentAddress(pathInfo.ca); + } +} + } diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index 2d55d926a..d35e3c678 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -31,6 +31,7 @@ struct Source; struct DerivedPath; struct BuildResult; struct KeyedBuildResult; +struct ValidPathInfo; enum TrustedFlag : bool; @@ -220,4 +221,15 @@ 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); +}; + } From 70f8b96c11af275b5766dca0a49737803d1e0339 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 17 Apr 2023 17:13:44 -0400 Subject: [PATCH 5/5] 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