From 7f590ea7096d1e1bbbe73697358fef962d0fb494 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 4 Mar 2024 04:59:31 +0100 Subject: [PATCH] Merge pull request #6223 from obsidiansystems/worker-proto-with-version Give `nix daemon` and `nix-store --serve` protocols separate serializers with version info (cherry picked from commit 8b68bbb77745fda0d14939b6c23d31cc89da41ce) Change-Id: Ia3d3b9fbaf9f0ae62ab225020b7d14790e793655 --- src/libstore/daemon.cc | 33 +-- src/libstore/legacy-ssh-store.cc | 15 +- src/libstore/path-info.cc | 77 ++--- src/libstore/path-info.hh | 42 +-- src/libstore/remote-store.cc | 60 +--- src/libstore/serve-protocol.cc | 43 +++ src/libstore/serve-protocol.hh | 6 + src/libstore/store-api.cc | 11 +- src/libstore/worker-protocol.cc | 124 ++++++-- src/libstore/worker-protocol.hh | 6 + src/nix-store/nix-store.cc | 12 +- tests/unit/libstore/serve-protocol.cc | 113 +++++++- tests/unit/libstore/worker-protocol.cc | 273 +++++++++++++++++- .../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 .../worker-protocol/derived-path-1.29.bin | Bin 0 -> 184 bytes .../worker-protocol/derived-path-1.30.bin | Bin 0 -> 248 bytes .../libstore/worker-protocol/derived-path.bin | Bin 120 -> 0 bytes ...result.bin => keyed-build-result-1.29.bin} | Bin .../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 26 files changed, 604 insertions(+), 211 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 create mode 100644 unit-test-data/libstore/worker-protocol/derived-path-1.29.bin create mode 100644 unit-test-data/libstore/worker-protocol/derived-path-1.30.bin delete mode 100644 unit-test-data/libstore/worker-protocol/derived-path.bin rename unit-test-data/libstore/worker-protocol/{keyed-build-result.bin => keyed-build-result-1.29.bin} (100%) 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 d61e97a64..3e44f6e31 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) @@ -434,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; @@ -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); @@ -647,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; } @@ -840,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::write(*store, wconn, static_cast(*info)); } else { assert(GET_PROTOCOL_MINOR(clientVersion) >= 17); to << 0; @@ -938,7 +917,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/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 9143be72e..6c482ea13 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/path-info.cc b/src/libstore/path-info.cc index a9231ce8d..336a9fcfc 100644 --- a/src/libstore/path-info.cc +++ b/src/libstore/path-info.cc @@ -1,10 +1,27 @@ #include "path-info.hh" -#include "worker-protocol.hh" -#include "worker-protocol-impl.hh" #include "store-api.hh" 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) @@ -99,14 +116,13 @@ Strings ValidPathInfo::shortRefs() const return refs; } - ValidPathInfo::ValidPathInfo( const Store & store, 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) { @@ -128,55 +144,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(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 221523622..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,13 +71,19 @@ struct ValidPathInfo */ std::optional ca; - bool operator == (const ValidPathInfo & i) const - { - return - path == i.path - && narHash == i.narHash - && references == i.references; - } + 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 @@ -92,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; /** @@ -122,18 +127,13 @@ 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); 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 a639346d1..3dcfb2d1d 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( - ValidPathInfo::read(conn->from, *this, GET_PROTOCOL_MINOR(conn->daemonVersion), StorePath{path})); + StorePath{path}, + WorkerProto::Serialise::read(*this, *conn)); } callback(std::move(info)); } catch (...) { callback.rethrow(); } @@ -445,7 +446,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 +571,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); } }); @@ -655,33 +661,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 +690,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 +714,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); @@ -815,20 +794,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); } @@ -929,7 +895,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/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/store-api.cc b/src/libstore/store-api.cc index 28689e100..b15765b73 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 415e66f16..ad94c79ee 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 @@ -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); + } } @@ -81,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; } @@ -99,16 +125,68 @@ 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); + } } +ValidPathInfo WorkerProto::Serialise::read(const Store & store, ReadConn conn) +{ + auto path = WorkerProto::Serialise::read(store, conn); + return ValidPathInfo { + std::move(path), + WorkerProto::Serialise::read(store, conn), + }; +} + +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); + UnkeyedValidPathInfo info(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 UnkeyedValidPathInfo & pathInfo) +{ + conn.to + << (pathInfo.deriver ? store.printStorePath(*pathInfo.deriver) : "") + << pathInfo.narHash.to_string(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..dcd54ad16 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -31,6 +31,8 @@ struct Source; struct DerivedPath; struct BuildResult; struct KeyedBuildResult; +struct ValidPathInfo; +struct UnkeyedValidPathInfo; enum TrustedFlag : bool; @@ -206,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 diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 11d2e86e3..79002626d 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/tests/unit/libstore/serve-protocol.cc b/tests/unit/libstore/serve-protocol.cc index 8146ea895..b67b19dbf 100644 --- a/tests/unit/libstore/serve-protocol.cc +++ b/tests/unit/libstore/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/tests/unit/libstore/worker-protocol.cc b/tests/unit/libstore/worker-protocol.cc index 6e7f47168..1389e2bac 100644 --- a/tests/unit/libstore/worker-protocol.cc +++ b/tests/unit/libstore/worker-protocol.cc @@ -69,13 +69,45 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - derivedPath, - "derived-path", - defaultVersion, - (std::tuple { + 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" }, + }, + 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", @@ -135,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 { @@ -194,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 { @@ -229,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/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!sQq0R{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