diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 4b55668cb..c89b5f5b3 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -47,10 +47,15 @@ struct TunnelLogger : public Logger Sync state_; - WorkerProto::Version clientVersion; + /** + * Worker protocol version of the other side. May be newer than this daemon. + */ + const WorkerProto::Version clientVersion; TunnelLogger(FdSink & to, WorkerProto::Version clientVersion) - : to(to), clientVersion(clientVersion) { } + : to(to), clientVersion(clientVersion) { + assert(clientVersion >= MIN_SUPPORTED_WORKER_PROTO_VERSION); + } void enqueueMsg(const std::string & s) { @@ -129,12 +134,6 @@ struct TunnelLogger : public Logger void startActivity(ActivityId act, Verbosity lvl, ActivityType type, const std::string & s, const Fields & fields, ActivityId parent) override { - if (GET_PROTOCOL_MINOR(clientVersion) < 20) { - if (!s.empty()) - log(lvl, s + "..."); - return; - } - StringSink buf; buf << STDERR_START_ACTIVITY << act << lvl << type << s << fields << parent; enqueueMsg(buf.s); @@ -142,7 +141,6 @@ struct TunnelLogger : public Logger void stopActivity(ActivityId act) override { - if (GET_PROTOCOL_MINOR(clientVersion) < 20) return; StringSink buf; buf << STDERR_STOP_ACTIVITY << act; enqueueMsg(buf.s); @@ -150,7 +148,6 @@ struct TunnelLogger : public Logger void result(ActivityId act, ResultType type, const Fields & fields) override { - if (GET_PROTOCOL_MINOR(clientVersion) < 20) return; StringSink buf; buf << STDERR_RESULT << act << type << fields; enqueueMsg(buf.s); @@ -267,14 +264,8 @@ static void performOp(TunnelLogger * logger, ref store, TrustedFlag trusted, RecursiveFlag recursive, WorkerProto::Version clientVersion, Source & from, BufferedSink & to, WorkerProto::Op op) { - WorkerProto::ReadConn rconn { - .from = from, - .version = clientVersion, - }; - WorkerProto::WriteConn wconn { - .to = to, - .version = clientVersion, - }; + WorkerProto::ReadConn rconn{from, clientVersion}; + WorkerProto::WriteConn wconn{to, clientVersion}; switch (op) { @@ -527,22 +518,19 @@ static void performOp(TunnelLogger * logger, ref store, case WorkerProto::Op::BuildPaths: { auto drvs = WorkerProto::Serialise::read(*store, rconn); - BuildMode mode = bmNormal; - if (GET_PROTOCOL_MINOR(clientVersion) >= 15) { - mode = buildModeFromInteger(readInt(from)); + BuildMode mode = buildModeFromInteger(readInt(from)); - /* Repairing is not atomic, so disallowed for "untrusted" - clients. + /* Repairing is not atomic, so disallowed for "untrusted" + clients. - FIXME: layer violation in this message: the daemon code (i.e. - this file) knows whether a client/connection is trusted, but it - does not how how the client was authenticated. The mechanism - need not be getting the UID of the other end of a Unix Domain - Socket. - */ - if (mode == bmRepair && !trusted) - throw Error("repairing is not allowed because you are not in 'trusted-users'"); - } + FIXME: layer violation in this message: the daemon code (i.e. + this file) knows whether a client/connection is trusted, but it + does not how how the client was authenticated. The mechanism + need not be getting the UID of the other end of a Unix Domain + Socket. + */ + if (mode == bmRepair && !trusted) + throw Error("repairing is not allowed because you are not in 'trusted-users'"); logger->startWork(); store->buildPaths(drvs, mode); logger->stopWork(); @@ -746,13 +734,11 @@ static void performOp(TunnelLogger * logger, ref store, clientSettings.buildCores = readInt(from); clientSettings.useSubstitutes = readInt(from); - if (GET_PROTOCOL_MINOR(clientVersion) >= 12) { - unsigned int n = readInt(from); - for (unsigned int i = 0; i < n; i++) { - auto name = readString(from); - auto value = readString(from); - clientSettings.overrides.emplace(name, value); - } + unsigned int n = readInt(from); + for (unsigned int i = 0; i < n; i++) { + auto name = readString(from); + auto value = readString(from); + clientSettings.overrides.emplace(name, value); } logger->startWork(); @@ -822,15 +808,14 @@ static void performOp(TunnelLogger * logger, ref store, try { info = store->queryPathInfo(path); } catch (InvalidPath &) { - if (GET_PROTOCOL_MINOR(clientVersion) < 17) throw; + // The path being invalid isn't fatal here since it will just be + // sent as not present. } logger->stopWork(); if (info) { - if (GET_PROTOCOL_MINOR(clientVersion) >= 17) - to << 1; + to << 1; WorkerProto::write(*store, wconn, static_cast(*info)); } else { - assert(GET_PROTOCOL_MINOR(clientVersion) >= 17); to << 0; } break; @@ -904,12 +889,7 @@ static void performOp(TunnelLogger * logger, ref store, else { std::unique_ptr source; StringSink saved; - if (GET_PROTOCOL_MINOR(clientVersion) >= 21) - source = std::make_unique(from, to); - else { - copyNAR(from, saved); - source = std::make_unique(saved.s); - } + source = std::make_unique(from, to); logger->startWork(); @@ -1011,7 +991,7 @@ void processConnection( to.flush(); WorkerProto::Version clientVersion = readInt(from); - if (clientVersion < 0x10a) + if (clientVersion < MIN_SUPPORTED_WORKER_PROTO_VERSION) throw Error("the Nix client version is too old"); auto tunnelLogger = new TunnelLogger(to, clientVersion); @@ -1027,13 +1007,13 @@ void processConnection( printMsgUsing(prevLogger, lvlDebug, "%d operations", opCount); }); - if (GET_PROTOCOL_MINOR(clientVersion) >= 14 && readInt(from)) { + // FIXME: what is *supposed* to be in this even? + if (readInt(from)) { // Obsolete CPU affinity. readInt(from); } - if (GET_PROTOCOL_MINOR(clientVersion) >= 11) - readInt(from); // obsolete reserveSpace + readInt(from); // obsolete reserveSpace if (GET_PROTOCOL_MINOR(clientVersion) >= 33) to << nixVersion; @@ -1044,10 +1024,7 @@ void processConnection( auto temp = trusted ? store->isTrustedClient() : std::optional { NotTrusted }; - WorkerProto::WriteConn wconn { - .to = to, - .version = clientVersion, - }; + WorkerProto::WriteConn wconn {to, clientVersion}; WorkerProto::write(*store, wconn, temp); } diff --git a/src/libstore/remote-store-connection.hh b/src/libstore/remote-store-connection.hh index 44328b06b..1647afd0d 100644 --- a/src/libstore/remote-store-connection.hh +++ b/src/libstore/remote-store-connection.hh @@ -27,11 +27,8 @@ struct RemoteStore::Connection FdSource from; /** - * Worker protocol version used for the connection. - * - * Despite its name, I think it is actually the maximum version both - * sides support. (If the maximum doesn't exist, we would fail to - * establish a connection and produce a value of this type.) + * The worker protocol version of the connected daemon. This may be newer + * than this Lix supports. */ WorkerProto::Version daemonVersion; @@ -71,10 +68,7 @@ struct RemoteStore::Connection */ operator WorkerProto::ReadConn () { - return WorkerProto::ReadConn { - .from = from, - .version = daemonVersion, - }; + return WorkerProto::ReadConn {from, daemonVersion}; } /** @@ -87,10 +81,7 @@ struct RemoteStore::Connection */ operator WorkerProto::WriteConn () { - return WorkerProto::WriteConn { - .to = to, - .version = daemonVersion, - }; + return WorkerProto::WriteConn {to, daemonVersion}; } virtual ~Connection(); diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 966dd3fe0..575078289 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -75,17 +75,14 @@ void RemoteStore::initConnection(Connection & conn) conn.from >> conn.daemonVersion; if (GET_PROTOCOL_MAJOR(conn.daemonVersion) != GET_PROTOCOL_MAJOR(PROTOCOL_VERSION)) throw Error("Nix daemon protocol version not supported"); - if (GET_PROTOCOL_MINOR(conn.daemonVersion) < 10) + if (GET_PROTOCOL_MINOR(conn.daemonVersion) < MIN_SUPPORTED_MINOR_WORKER_PROTO_VERSION) throw Error("the Nix daemon version is too old"); conn.to << PROTOCOL_VERSION; - if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 14) { - // Obsolete CPU affinity. - conn.to << 0; - } + // Obsolete CPU affinity. + conn.to << 0; - if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 11) - conn.to << false; // obsolete reserveSpace + conn.to << false; // obsolete reserveSpace if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 33) { conn.to.flush(); @@ -126,24 +123,22 @@ void RemoteStore::setOptions(Connection & conn) << settings.buildCores << settings.useSubstitutes; - if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 12) { - std::map overrides; - settings.getSettings(overrides, true); // libstore settings - fileTransferSettings.getSettings(overrides, true); - overrides.erase(settings.keepFailed.name); - overrides.erase(settings.keepGoing.name); - overrides.erase(settings.tryFallback.name); - overrides.erase(settings.maxBuildJobs.name); - overrides.erase(settings.maxSilentTime.name); - overrides.erase(settings.buildCores.name); - overrides.erase(settings.useSubstitutes.name); - overrides.erase(loggerSettings.showTrace.name); - overrides.erase(experimentalFeatureSettings.experimentalFeatures.name); - overrides.erase(settings.pluginFiles.name); - conn.to << overrides.size(); - for (auto & i : overrides) - conn.to << i.first << i.second.value; - } + std::map overrides; + settings.getSettings(overrides, true); // libstore settings + fileTransferSettings.getSettings(overrides, true); + overrides.erase(settings.keepFailed.name); + overrides.erase(settings.keepGoing.name); + overrides.erase(settings.tryFallback.name); + overrides.erase(settings.maxBuildJobs.name); + overrides.erase(settings.maxSilentTime.name); + overrides.erase(settings.buildCores.name); + overrides.erase(settings.useSubstitutes.name); + overrides.erase(loggerSettings.showTrace.name); + overrides.erase(experimentalFeatureSettings.experimentalFeatures.name); + overrides.erase(settings.pluginFiles.name); + conn.to << overrides.size(); + for (auto & i : overrides) + conn.to << i.first << i.second.value; auto ex = conn.processStderr(); if (ex) std::rethrow_exception(ex); @@ -207,20 +202,13 @@ bool RemoteStore::isValidPathUncached(const StorePath & path) StorePathSet RemoteStore::queryValidPaths(const StorePathSet & paths, SubstituteFlag maybeSubstitute) { auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) { - StorePathSet res; - for (auto & i : paths) - if (isValidPath(i)) res.insert(i); - return res; - } else { - conn->to << WorkerProto::Op::QueryValidPaths; - WorkerProto::write(*this, *conn, paths); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 27) { - conn->to << maybeSubstitute; - } - conn.processStderr(); - return WorkerProto::Serialise::read(*this, *conn); + conn->to << WorkerProto::Op::QueryValidPaths; + WorkerProto::write(*this, *conn, paths); + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 27) { + conn->to << maybeSubstitute; } + conn.processStderr(); + return WorkerProto::Serialise::read(*this, *conn); } @@ -236,20 +224,10 @@ StorePathSet RemoteStore::queryAllValidPaths() StorePathSet RemoteStore::querySubstitutablePaths(const StorePathSet & paths) { auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) { - StorePathSet res; - for (auto & i : paths) { - conn->to << WorkerProto::Op::HasSubstitutes << printStorePath(i); - conn.processStderr(); - if (readInt(conn->from)) res.insert(i); - } - return res; - } else { - conn->to << WorkerProto::Op::QuerySubstitutablePaths; - WorkerProto::write(*this, *conn, paths); - conn.processStderr(); - return WorkerProto::Serialise::read(*this, *conn); - } + conn->to << WorkerProto::Op::QuerySubstitutablePaths; + WorkerProto::write(*this, *conn, paths); + conn.processStderr(); + return WorkerProto::Serialise::read(*this, *conn); } @@ -259,45 +237,25 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) { - - for (auto & i : pathsMap) { - SubstitutablePathInfo info; - conn->to << WorkerProto::Op::QuerySubstitutablePathInfo << printStorePath(i.first); - conn.processStderr(); - unsigned int reply = readInt(conn->from); - if (reply == 0) continue; - auto deriver = readString(conn->from); - if (deriver != "") - info.deriver = parseStorePath(deriver); - info.references = WorkerProto::Serialise::read(*this, *conn); - info.downloadSize = readLongLong(conn->from); - info.narSize = readLongLong(conn->from); - infos.insert_or_assign(i.first, std::move(info)); - } - - } else { - - conn->to << WorkerProto::Op::QuerySubstitutablePathInfos; - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 22) { - StorePathSet paths; - for (auto & path : pathsMap) - paths.insert(path.first); - WorkerProto::write(*this, *conn, paths); - } else - WorkerProto::write(*this, *conn, pathsMap); - conn.processStderr(); - size_t count = readNum(conn->from); - for (size_t n = 0; n < count; n++) { - SubstitutablePathInfo & info(infos[parseStorePath(readString(conn->from))]); - auto deriver = readString(conn->from); - if (deriver != "") - info.deriver = parseStorePath(deriver); - info.references = WorkerProto::Serialise::read(*this, *conn); - info.downloadSize = readLongLong(conn->from); - info.narSize = readLongLong(conn->from); - } + conn->to << WorkerProto::Op::QuerySubstitutablePathInfos; + if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 22) { + StorePathSet paths; + for (auto & path : pathsMap) + paths.insert(path.first); + WorkerProto::write(*this, *conn, paths); + } else + WorkerProto::write(*this, *conn, pathsMap); + conn.processStderr(); + size_t count = readNum(conn->from); + for (size_t n = 0; n < count; n++) { + SubstitutablePathInfo & info(infos[parseStorePath(readString(conn->from))]); + auto deriver = readString(conn->from); + if (deriver != "") + info.deriver = parseStorePath(deriver); + info.references = WorkerProto::Serialise::read(*this, *conn); + info.downloadSize = readLongLong(conn->from); + info.narSize = readLongLong(conn->from); } } @@ -314,10 +272,10 @@ std::shared_ptr RemoteStore::queryPathInfoUncached(const St return nullptr; throw; } - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) { - bool valid; conn->from >> valid; - if (!valid) return nullptr; - } + + bool valid; conn->from >> valid; + if (!valid) return nullptr; + return std::make_shared( StorePath{path}, WorkerProto::Serialise::read(*this, *conn)); @@ -346,7 +304,7 @@ StorePathSet RemoteStore::queryValidDerivers(const StorePath & path) StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path) { - if (GET_PROTOCOL_MINOR(getProtocol()) >= 0x16) { + if (GET_PROTOCOL_MINOR(getProtocol()) >= 22) { return Store::queryDerivationOutputs(path); } auto conn(getConnection()); @@ -358,7 +316,7 @@ StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path) std::map> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_) { - if (GET_PROTOCOL_MINOR(getProtocol()) >= 0x16) { + if (GET_PROTOCOL_MINOR(getProtocol()) >= 22) { if (!evalStore_) { auto conn(getConnection()); conn->to << WorkerProto::Op::QueryDerivationOutputMap << printStorePath(path); @@ -498,51 +456,21 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, { auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 18) { - conn->to << WorkerProto::Op::ImportPaths; + conn->to << WorkerProto::Op::AddToStoreNar + << printStorePath(info.path) + << (info.deriver ? printStorePath(*info.deriver) : "") + << info.narHash.to_string(Base16, false); + WorkerProto::write(*this, *conn, info.references); + conn->to << info.registrationTime << info.narSize + << info.ultimate << info.sigs << renderContentAddress(info.ca) + << repair << !checkSigs; - auto source2 = sinkToSource([&](Sink & sink) { - sink << 1 // == path follows - ; + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) { + conn.withFramedSink([&](Sink & sink) { copyNAR(source, sink); - sink - << exportMagic - << printStorePath(info.path); - WorkerProto::WriteConn nested { .to = sink, .version = conn->daemonVersion }; - WorkerProto::write(*this, nested, info.references); - sink - << (info.deriver ? printStorePath(*info.deriver) : "") - << 0 // == no legacy signature - << 0 // == no path follows - ; }); - - conn.processStderr(0, source2.get()); - - auto importedPaths = WorkerProto::Serialise::read(*this, *conn); - assert(importedPaths.size() <= 1); - } - - else { - conn->to << WorkerProto::Op::AddToStoreNar - << printStorePath(info.path) - << (info.deriver ? printStorePath(*info.deriver) : "") - << info.narHash.to_string(Base16, false); - WorkerProto::write(*this, *conn, info.references); - conn->to << info.registrationTime << info.narSize - << info.ultimate << info.sigs << renderContentAddress(info.ca) - << repair << !checkSigs; - - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) { - conn.withFramedSink([&](Sink & sink) { - copyNAR(source, sink); - }); - } else if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21) { - conn.processStderr(0, &source); - } else { - copyNAR(source, conn->to); - conn.processStderr(0, nullptr); - } + } else { + conn.processStderr(0, &source); } } @@ -553,14 +481,13 @@ void RemoteStore::addMultipleToStore( RepairFlag repair, CheckSigsFlag checkSigs) { + auto remoteVersion = getProtocol(); + auto source = sinkToSource([&](Sink & sink) { sink << pathsToCopy.size(); for (auto & [pathInfo, pathSource] : pathsToCopy) { WorkerProto::Serialise::write(*this, - WorkerProto::WriteConn { - .to = sink, - .version = 16, - }, + WorkerProto::WriteConn {sink, remoteVersion}, pathInfo); pathSource->drainInto(sink); } @@ -667,15 +594,8 @@ void RemoteStore::buildPaths(const std::vector & drvPaths, BuildMod auto conn(getConnection()); conn->to << WorkerProto::Op::BuildPaths; - assert(GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13); WorkerProto::write(*this, *conn, drvPaths); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15) - conn->to << buildMode; - else - /* Old daemons did not take a 'buildMode' parameter, so we - need to validate it here on the client side. */ - if (buildMode != bmNormal) - throw Error("repairing or checking is not supported when building through the Nix daemon"); + conn->to << buildMode; conn.processStderr(); readInt(conn->from); } @@ -866,25 +786,14 @@ void RemoteStore::queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, uint64_t & downloadSize, uint64_t & narSize) { - { - auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 19) - // Don't hold the connection handle in the fallback case - // to prevent a deadlock. - goto fallback; - conn->to << WorkerProto::Op::QueryMissing; - WorkerProto::write(*this, *conn, targets); - conn.processStderr(); - willBuild = WorkerProto::Serialise::read(*this, *conn); - willSubstitute = WorkerProto::Serialise::read(*this, *conn); - unknown = WorkerProto::Serialise::read(*this, *conn); - conn->from >> downloadSize >> narSize; - return; - } - - fallback: - return Store::queryMissing(targets, willBuild, willSubstitute, - unknown, downloadSize, narSize); + auto conn(getConnection()); + conn->to << WorkerProto::Op::QueryMissing; + WorkerProto::write(*this, *conn, targets); + conn.processStderr(); + willBuild = WorkerProto::Serialise::read(*this, *conn); + willSubstitute = WorkerProto::Serialise::read(*this, *conn); + unknown = WorkerProto::Serialise::read(*this, *conn); + conn->from >> downloadSize >> narSize; } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index e0e842060..a10e5df0a 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -368,15 +368,14 @@ void Store::addMultipleToStore( RepairFlag repair, CheckSigsFlag checkSigs) { + auto remoteVersion = getProtocol(); + auto expected = readNum(source); for (uint64_t i = 0; i < expected; ++i) { - // FIXME we should not be using the worker protocol here, let - // alone the worker protocol with a hard-coded version! + // FIXME we should not be using the worker protocol here at all! auto info = WorkerProto::Serialise::read(*this, - WorkerProto::ReadConn { - .from = source, - .version = 16, - }); + WorkerProto::ReadConn {source, remoteVersion} + ); info.ultimate = false; addToStore(info, source, repair, checkSigs); } diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index d4954aaa8..08c5c6b70 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -165,11 +165,11 @@ UnkeyedValidPathInfo WorkerProto::Serialise::read(const St 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)); - } + + conn.from >> info.ultimate; + info.sigs = readStrings(conn.from); + info.ca = ContentAddress::parseOpt(readString(conn.from)); + return info; } @@ -180,12 +180,11 @@ void WorkerProto::Serialise::write(const Store & store, Wr << 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); - } + + conn.to + << pathInfo.ultimate + << pathInfo.sigs + << renderContentAddress(pathInfo.ca); } } diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index dcd54ad16..5f24c189f 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -9,7 +9,16 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f +// This must remain 1.35 (Nix 2.18) forever in Lix, since the protocol has +// diverged in CppNix such that we cannot assign newer versions ourselves, the +// protocol is bad in design and implementation and Lix intends to replace it +// entirely. #define PROTOCOL_VERSION (1 << 8 | 35) +// Nix 2.3 is protocol 1.21 (see RemoteStore::initConnection for client, +// processConnection for server). +#define MIN_SUPPORTED_MINOR_WORKER_PROTO_VERSION 21 +#define MIN_SUPPORTED_WORKER_PROTO_VERSION (1 << 8 | MIN_SUPPORTED_MINOR_WORKER_PROTO_VERSION) + #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) @@ -63,6 +72,10 @@ struct WorkerProto struct ReadConn { Source & from; Version version; + + ReadConn(Source & from, Version version) : from(from), version(version) { + assert(version >= MIN_SUPPORTED_WORKER_PROTO_VERSION); + } }; /** @@ -72,6 +85,10 @@ struct WorkerProto struct WriteConn { Sink & to; Version version; + + WriteConn(Sink & to, Version version) : to(to), version(version) { + assert(version >= MIN_SUPPORTED_WORKER_PROTO_VERSION); + } }; /** diff --git a/tests/unit/libstore/data/libstore/worker-protocol/unkeyed-valid-path-info-1.15.bin b/tests/unit/libstore/data/libstore/worker-protocol/unkeyed-valid-path-info-1.15.bin deleted file mode 100644 index e69ccbe83..000000000 Binary files a/tests/unit/libstore/data/libstore/worker-protocol/unkeyed-valid-path-info-1.15.bin and /dev/null differ diff --git a/tests/unit/libstore/data/libstore/worker-protocol/unkeyed-valid-path-info.bin b/tests/unit/libstore/data/libstore/worker-protocol/unkeyed-valid-path-info.bin new file mode 100644 index 000000000..7bf72e0f4 Binary files /dev/null and b/tests/unit/libstore/data/libstore/worker-protocol/unkeyed-valid-path-info.bin differ diff --git a/tests/unit/libstore/data/libstore/worker-protocol/valid-path-info-1.15.bin b/tests/unit/libstore/data/libstore/worker-protocol/valid-path-info-1.15.bin deleted file mode 100644 index 7adc8dd44..000000000 Binary files a/tests/unit/libstore/data/libstore/worker-protocol/valid-path-info-1.15.bin and /dev/null differ diff --git a/tests/unit/libstore/data/libstore/worker-protocol/valid-path-info-1.16.bin b/tests/unit/libstore/data/libstore/worker-protocol/valid-path-info.bin similarity index 100% rename from tests/unit/libstore/data/libstore/worker-protocol/valid-path-info-1.16.bin rename to tests/unit/libstore/data/libstore/worker-protocol/valid-path-info.bin diff --git a/tests/unit/libstore/protocol.hh b/tests/unit/libstore/protocol.hh index 7fdd3e11c..f480f4ad1 100644 --- a/tests/unit/libstore/protocol.hh +++ b/tests/unit/libstore/protocol.hh @@ -39,10 +39,8 @@ public: StringSource from { expected }; Proto::template Serialise::read( *LibStoreTest::store, - typename Proto::ReadConn { - .from = from, - .version = version, - }); + typename Proto::ReadConn {from, version} + ); }); ASSERT_EQ(got, value); @@ -60,10 +58,7 @@ public: StringSink to; Proto::write( *LibStoreTest::store, - typename Proto::WriteConn { - .to = to, - .version = version, - }, + typename Proto::WriteConn {to, version}, value); if (testAccept()) diff --git a/tests/unit/libstore/worker-protocol.cc b/tests/unit/libstore/worker-protocol.cc index 2d6fbd58f..f36de9cf9 100644 --- a/tests/unit/libstore/worker-protocol.cc +++ b/tests/unit/libstore/worker-protocol.cc @@ -18,9 +18,9 @@ struct WorkerProtoTest : VersionedProtoTest { /** * For serializers that don't care about the minimum version, we - * used the oldest one: 1.0. + * have to use the minimum supported to not throw an assert. */ - WorkerProto::Version defaultVersion = 1 << 8 | 0; + WorkerProto::Version defaultVersion = MIN_SUPPORTED_WORKER_PROTO_VERSION; }; @@ -331,9 +331,9 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - unkeyedValidPathInfo_1_15, - "unkeyed-valid-path-info-1.15", - 1 << 8 | 15, + unkeyedValidPathInfo, + "unkeyed-valid-path-info", + defaultVersion, (std::tuple { ({ UnkeyedValidPathInfo info { @@ -363,56 +363,9 @@ VERSIONED_CHARACTERIZATION_TEST( 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, + validPathInfo, + "valid-path-info", + defaultVersion, (std::tuple { ({ ValidPathInfo info {