diff --git a/src/libstore-tests/worker-protocol.cc b/src/libstore-tests/worker-protocol.cc index 73c5df330a7..aac5822d01f 100644 --- a/src/libstore-tests/worker-protocol.cc +++ b/src/libstore-tests/worker-protocol.cc @@ -15,6 +15,80 @@ namespace nix { +TEST(WorkerProtoVersionNumber, ordering) +{ + using Number = WorkerProto::Version::Number; + EXPECT_LT((Number{1, 10}), (Number{1, 20})); + EXPECT_GT((Number{1, 30}), (Number{1, 20})); + EXPECT_EQ((Number{1, 10}), (Number{1, 10})); + EXPECT_LT((Number{0, 255}), (Number{1, 0})); +} + +TEST(WorkerProtoVersion, partialOrderingSameFeatures) +{ + using V = WorkerProto::Version; + V v1{.number = {1, 20}, .features = {"a", "b"}}; + V v2{.number = {1, 30}, .features = {"a", "b"}}; + + EXPECT_TRUE(v1 < v2); + EXPECT_TRUE(v2 > v1); + EXPECT_TRUE(v1 <= v2); + EXPECT_TRUE(v2 >= v1); + EXPECT_FALSE(v1 == v2); +} + +TEST(WorkerProtoVersion, partialOrderingSubsetFeatures) +{ + using V = WorkerProto::Version; + V fewer{.number = {1, 30}, .features = {"a"}}; + V more{.number = {1, 30}, .features = {"a", "b"}}; + + // fewer <= more: JUST the features are a subset + EXPECT_TRUE(fewer < more); + EXPECT_TRUE(fewer <= more); + EXPECT_FALSE(fewer > more); + EXPECT_TRUE(fewer != more); +} + +TEST(WorkerProtoVersion, partialOrderingUnordered) +{ + using V = WorkerProto::Version; + // Same number but incomparable features + V v1{.number = {1, 20}, .features = {"a", "c"}}; + V v2{.number = {1, 20}, .features = {"a", "b"}}; + + EXPECT_FALSE(v1 < v2); + EXPECT_FALSE(v1 > v2); + EXPECT_FALSE(v1 <= v2); + EXPECT_FALSE(v1 >= v2); + EXPECT_FALSE(v1 == v2); + EXPECT_TRUE(v1 != v2); +} + +TEST(WorkerProtoVersion, partialOrderingHigherNumberFewerFeatures) +{ + using V = WorkerProto::Version; + // Higher number but fewer features — unordered + V v1{.number = {1, 30}, .features = {"a"}}; + V v2{.number = {1, 20}, .features = {"a", "b"}}; + + EXPECT_FALSE(v1 < v2); + EXPECT_FALSE(v1 > v2); + EXPECT_FALSE(v1 == v2); +} + +TEST(WorkerProtoVersion, partialOrderingEmptyFeatures) +{ + using V = WorkerProto::Version; + V empty{.number = {1, 20}, .features = {}}; + V some{.number = {1, 30}, .features = {"a"}}; + + // empty features is a subset of everything + EXPECT_TRUE(empty < some); + EXPECT_TRUE(empty <= some); + EXPECT_TRUE(empty != some); +} + const char workerProtoDir[] = "worker-protocol"; static constexpr std::string_view defaultStoreDir = "/nix/store"; @@ -26,8 +100,11 @@ struct WorkerProtoTest : VersionedProtoTest * used the oldest one: 1.10. */ WorkerProto::Version defaultVersion = { - .major = 1, - .minor = 10, + .number = + { + .major = 1, + .minor = 10, + }, }; }; @@ -83,8 +160,11 @@ VERSIONED_CHARACTERIZATION_TEST( derivedPath_1_29, "derived-path-1.29", (WorkerProto::Version{ - .major = 1, - .minor = 29, + .number = + { + .major = 1, + .minor = 29, + }, }), (std::tuple{ DerivedPath::Opaque{ @@ -111,8 +191,11 @@ VERSIONED_CHARACTERIZATION_TEST( derivedPath_1_30, "derived-path-1.30", (WorkerProto::Version{ - .major = 1, - .minor = 30, + .number = + { + .major = 1, + .minor = 30, + }, }), (std::tuple{ DerivedPath::Opaque{ @@ -211,8 +294,11 @@ VERSIONED_CHARACTERIZATION_TEST( buildResult_1_27, "build-result-1.27", (WorkerProto::Version{ - .major = 1, - .minor = 27, + .number = + { + .major = 1, + .minor = 27, + }, }), ({ using namespace std::literals::chrono_literals; @@ -237,8 +323,11 @@ VERSIONED_CHARACTERIZATION_TEST( buildResult_1_28, "build-result-1.28", (WorkerProto::Version{ - .major = 1, - .minor = 28, + .number = + { + .major = 1, + .minor = 28, + }, }), ({ using namespace std::literals::chrono_literals; @@ -290,8 +379,11 @@ VERSIONED_CHARACTERIZATION_TEST( buildResult_1_29, "build-result-1.29", (WorkerProto::Version{ - .major = 1, - .minor = 29, + .number = + { + .major = 1, + .minor = 29, + }, }), ({ using namespace std::literals::chrono_literals; @@ -356,8 +448,11 @@ VERSIONED_CHARACTERIZATION_TEST( buildResult_1_37, "build-result-1.37", (WorkerProto::Version{ - .major = 1, - .minor = 37, + .number = + { + .major = 1, + .minor = 37, + }, }), ({ using namespace std::literals::chrono_literals; @@ -424,8 +519,11 @@ VERSIONED_CHARACTERIZATION_TEST( keyedBuildResult_1_29, "keyed-build-result-1.29", (WorkerProto::Version{ - .major = 1, - .minor = 29, + .number = + { + .major = 1, + .minor = 29, + }, }), ({ using namespace std::literals::chrono_literals; @@ -469,8 +567,11 @@ VERSIONED_CHARACTERIZATION_TEST( unkeyedValidPathInfo_1_15, "unkeyed-valid-path-info-1.15", (WorkerProto::Version{ - .major = 1, - .minor = 15, + .number = + { + .major = 1, + .minor = 15, + }, }), (std::tuple{ ({ @@ -506,8 +607,11 @@ VERSIONED_CHARACTERIZATION_TEST( validPathInfo_1_15, "valid-path-info-1.15", (WorkerProto::Version{ - .major = 1, - .minor = 15, + .number = + { + .major = 1, + .minor = 15, + }, }), (std::tuple{ ({ @@ -558,8 +662,11 @@ VERSIONED_CHARACTERIZATION_TEST( validPathInfo_1_16, "valid-path-info-1.16", (WorkerProto::Version{ - .major = 1, - .minor = 16, + .number = + { + .major = 1, + .minor = 16, + }, }), (std::tuple{ ({ @@ -716,8 +823,11 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( clientHandshakeInfo_1_30, "client-handshake-info_1_30", (WorkerProto::Version{ - .major = 1, - .minor = 30, + .number = + { + .major = 1, + .minor = 30, + }, }), (std::tuple{ {}, @@ -728,8 +838,11 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( clientHandshakeInfo_1_33, "client-handshake-info_1_33", (WorkerProto::Version{ - .major = 1, - .minor = 33, + .number = + { + .major = 1, + .minor = 33, + }, }), (std::tuple{ { @@ -745,8 +858,11 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( clientHandshakeInfo_1_35, "client-handshake-info_1_35", (WorkerProto::Version{ - .major = 1, - .minor = 35, + .number = + { + .major = 1, + .minor = 35, + }, }), (std::tuple{ { @@ -774,13 +890,13 @@ TEST_F(WorkerProtoTest, handshake_log) FdSink out{toServer.writeSide.get()}; FdSource in0{toClient.readSide.get()}; TeeSource in{in0, toClientLog}; - clientResult = std::get<0>(WorkerProto::BasicClientConnection::handshake(out, in, defaultVersion, {})); + clientResult = WorkerProto::BasicClientConnection::handshake(out, in, defaultVersion); }); { FdSink out{toClient.writeSide.get()}; FdSource in{toServer.readSide.get()}; - WorkerProto::BasicServerConnection::handshake(out, in, defaultVersion, {}); + WorkerProto::BasicServerConnection::handshake(out, in, defaultVersion); }; thread.join(); @@ -795,7 +911,7 @@ TEST_F(WorkerProtoTest, handshake_features) toClient.create(); toServer.create(); - std::tuple clientResult; + WorkerProto::Version clientResult; auto clientThread = std::thread([&]() { FdSink out{toServer.writeSide.get()}; @@ -804,10 +920,9 @@ TEST_F(WorkerProtoTest, handshake_features) out, in, WorkerProto::Version{ - .major = 1, - .minor = 123, - }, - {"bar", "aap", "mies", "xyzzy"}); + .number = {.major = 1, .minor = 123}, + .features = {"bar", "aap", "mies", "xyzzy"}, + }); }); FdSink out{toClient.writeSide.get()}; @@ -816,21 +931,23 @@ TEST_F(WorkerProtoTest, handshake_features) out, in, WorkerProto::Version{ - .major = 1, - .minor = 200, - }, - {"foo", "bar", "xyzzy"}); + .number = {.major = 1, .minor = 200}, + .features = {"foo", "bar", "xyzzy"}, + }); clientThread.join(); EXPECT_EQ(clientResult, daemonResult); EXPECT_EQ( - std::get<0>(clientResult), + clientResult, (WorkerProto::Version{ - .major = 1, - .minor = 123, + .number = + { + .major = 1, + .minor = 123, + }, + .features = {"bar", "xyzzy"}, })); - EXPECT_EQ(std::get<1>(clientResult), WorkerProto::FeatureSet({"bar", "xyzzy"})); } /// Has to be a `BufferedSink` for handshake. @@ -845,8 +962,7 @@ TEST_F(WorkerProtoTest, handshake_client_replay) NullBufferedSink nullSink; StringSource in{toClientLog}; - auto clientResult = - std::get<0>(WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, {})); + auto clientResult = WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion); EXPECT_EQ(clientResult, defaultVersion); }); @@ -860,11 +976,10 @@ TEST_F(WorkerProtoTest, handshake_client_truncated_replay_throws) auto substring = toClientLog.substr(0, len); StringSource in{substring}; if (len < 8) { - EXPECT_THROW( - WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, {}), EndOfFile); + EXPECT_THROW(WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion), EndOfFile); } else { // Not sure why cannot keep on checking for `EndOfFile`. - EXPECT_THROW(WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, {}), Error); + EXPECT_THROW(WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion), Error); } } }); @@ -884,14 +999,13 @@ TEST_F(WorkerProtoTest, handshake_client_corrupted_throws) if (idx < 4 || idx == 9) { // magic bytes don't match - EXPECT_THROW(WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, {}), Error); + EXPECT_THROW(WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion), Error); } else if (idx < 8 || idx >= 12) { // Number out of bounds EXPECT_THROW( - WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, {}), - SerialisationError); + WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion), SerialisationError); } else { - auto ver = std::get<0>(WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion, {})); + auto ver = WorkerProto::BasicClientConnection::handshake(nullSink, in, defaultVersion); // `std::min` of this and the other version saves us EXPECT_EQ(ver, defaultVersion); } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 045f2259ede..97838999c74 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -133,7 +133,7 @@ struct TunnelLogger : public Logger if (!ex) to << STDERR_LAST; else { - if (clientVersion >= WorkerProto::Version{1, 26}) { + if (clientVersion >= WorkerProto::Version{.number = {1, 26}}) { to << STDERR_ERROR << *ex; } else { to << STDERR_ERROR << ex->what() << ex->info().status; @@ -149,7 +149,7 @@ struct TunnelLogger : public Logger const Fields & fields, ActivityId parent) override { - if (clientVersion < WorkerProto::Version{1, 20}) { + if (clientVersion.number < WorkerProto::Version::Number{1, 20}) { if (!s.empty()) log(lvl, s + "..."); return; @@ -162,7 +162,7 @@ struct TunnelLogger : public Logger void stopActivity(ActivityId act) override { - if (clientVersion < WorkerProto::Version{1, 20}) + if (clientVersion.number < WorkerProto::Version::Number{1, 20}) return; StringSink buf; buf << STDERR_STOP_ACTIVITY << act; @@ -171,7 +171,7 @@ struct TunnelLogger : public Logger void result(ActivityId act, ResultType type, const Fields & fields) override { - if (clientVersion < WorkerProto::Version{1, 20}) + if (clientVersion.number < WorkerProto::Version::Number{1, 20}) return; StringSink buf; buf << STDERR_RESULT << act << type << fields; @@ -325,7 +325,7 @@ static void performOp( auto paths = WorkerProto::Serialise::read(*store, rconn); SubstituteFlag substitute = NoSubstitute; - if (conn.protoVersion >= WorkerProto::Version{1, 27}) { + if (conn.protoVersion >= WorkerProto::Version{.number = {1, 27}}) { substitute = readInt(conn.from) ? Substitute : NoSubstitute; } @@ -402,7 +402,7 @@ static void performOp( } case WorkerProto::Op::AddToStore: { - if (conn.protoVersion >= WorkerProto::Version{1, 25}) { + if (conn.protoVersion >= WorkerProto::Version{.number = {1, 25}}) { auto name = readString(conn.from); auto camStr = readString(conn.from); auto refs = WorkerProto::Serialise::read(*store, rconn); @@ -798,7 +798,7 @@ static void performOp( case WorkerProto::Op::QuerySubstitutablePathInfos: { SubstitutablePathInfos infos; StorePathCAMap pathsMap = {}; - if (conn.protoVersion < WorkerProto::Version{1, 22}) { + if (conn.protoVersion.number < WorkerProto::Version::Number{1, 22}) { auto paths = WorkerProto::Serialise::read(*store, rconn); for (auto & path : paths) pathsMap.emplace(path, std::nullopt); @@ -897,7 +897,7 @@ static void performOp( if (!trusted) info.ultimate = false; - if (conn.protoVersion >= WorkerProto::Version{1, 23}) { + if (conn.protoVersion >= WorkerProto::Version{.number = {1, 23}}) { logger->startWork(); { FramedSource source(conn.from); @@ -909,7 +909,7 @@ static void performOp( else { std::unique_ptr source; StringSink saved; - if (conn.protoVersion >= WorkerProto::Version{1, 21}) + if (conn.protoVersion >= WorkerProto::Version{.number = {1, 21}}) source = std::make_unique(conn.from, conn.to); else { TeeSource tee{conn.from, saved}; @@ -943,7 +943,7 @@ static void performOp( case WorkerProto::Op::RegisterDrvOutput: { logger->startWork(); - if (conn.protoVersion < WorkerProto::Version{1, 31}) { + if (conn.protoVersion.number < WorkerProto::Version::Number{1, 31}) { auto outputId = WorkerProto::Serialise::read(*store, rconn); auto outputPath = StorePath(readString(conn.from)); store->registerDrvOutput(Realisation{{.outPath = outputPath}, outputId}); @@ -960,7 +960,7 @@ static void performOp( auto outputId = WorkerProto::Serialise::read(*store, rconn); auto info = store->queryRealisation(outputId); logger->stopWork(); - if (conn.protoVersion < WorkerProto::Version{1, 31}) { + if (conn.protoVersion.number < WorkerProto::Version::Number{1, 31}) { std::set outPaths; if (info) outPaths.insert(info->outPath); @@ -1015,19 +1015,16 @@ void processConnection(ref store, FdSource && from, FdSink && to, Trusted #endif /* Exchange the greeting. */ - auto [protoVersion, features] = - WorkerProto::BasicServerConnection::handshake(to, from, WorkerProto::latest, WorkerProto::allFeatures); + WorkerProto::BasicServerConnection conn; + conn.protoVersion = WorkerProto::BasicServerConnection::handshake(to, from, WorkerProto::latest); - if (protoVersion < WorkerProto::minimum) + if (conn.protoVersion.number < WorkerProto::minimum.number) throw Error("the Nix client version is too old"); - WorkerProto::BasicServerConnection conn; conn.to = std::move(to); conn.from = std::move(from); - conn.protoVersion = protoVersion; - conn.features = features; - auto tunnelLogger_ = std::make_unique(conn.to, protoVersion); + auto tunnelLogger_ = std::make_unique(conn.to, conn.protoVersion); auto tunnelLogger = tunnelLogger_.get(); std::unique_ptr prevLogger_; auto prevLogger = logger.get(); diff --git a/src/libstore/include/nix/store/worker-protocol-connection.hh b/src/libstore/include/nix/store/worker-protocol-connection.hh index 31436395fe7..4ead3a5da61 100644 --- a/src/libstore/include/nix/store/worker-protocol-connection.hh +++ b/src/libstore/include/nix/store/worker-protocol-connection.hh @@ -23,11 +23,6 @@ struct WorkerProto::BasicConnection */ WorkerProto::Version protoVersion; - /** - * The set of features that both sides support. - */ - FeatureSet features; - /** * Coercion to `WorkerProto::ReadConn`. This makes it easy to use the * factored out serve protocol serializers with a @@ -87,13 +82,11 @@ struct WorkerProto::BasicClientConnection : WorkerProto::BasicConnection * @param from Taken by reference to allow for various error * handling mechanisms. * - * @param localVersion Our version which is sent over. - * - * @param supportedFeatures The protocol features that we support. + * @param localVersion Our version (number + supported features) + * which is sent over. */ // FIXME: this should probably be a constructor. - static std::tuple handshake( - BufferedSink & to, Source & from, WorkerProto::Version localVersion, const FeatureSet & supportedFeatures); + static Version handshake(BufferedSink & to, Source & from, const Version & localVersion); /** * After calling handshake, must call this to exchange some basic @@ -146,13 +139,11 @@ struct WorkerProto::BasicServerConnection : WorkerProto::BasicConnection * @param from Taken by reference to allow for various error * handling mechanisms. * - * @param localVersion Our version which is sent over. - * - * @param supportedFeatures The protocol features that we support. + * @param localVersion Our version (number + supported features) + * which is sent over. */ // FIXME: this should probably be a constructor. - static std::tuple handshake( - BufferedSink & to, Source & from, WorkerProto::Version localVersion, const FeatureSet & supportedFeatures); + static Version handshake(BufferedSink & to, Source & from, const Version & localVersion); /** * After calling handshake, must call this to exchange some basic diff --git a/src/libstore/include/nix/store/worker-protocol.hh b/src/libstore/include/nix/store/worker-protocol.hh index 3babda13daf..4098e8fd912 100644 --- a/src/libstore/include/nix/store/worker-protocol.hh +++ b/src/libstore/include/nix/store/worker-protocol.hh @@ -1,7 +1,9 @@ #pragma once ///@file +#include #include +#include #include "nix/store/common-protocol.hh" @@ -48,48 +50,64 @@ struct WorkerProto /** * Version type for the protocol. * - * @todo Convert to struct with separate major vs minor fields. + * This bundles the protocol version number with the negotiated + * feature set. The version number has a total ordering, but the + * full `Version` (number + features) only has a partial ordering, + * so there is no `operator<=>` on `Version` itself --- callers + * must compare `.number` explicitly. */ struct Version { - unsigned int major; - uint8_t minor; + struct Number + { + unsigned int major; + uint8_t minor; + + constexpr auto operator<=>(const Number &) const = default; + + /** + * Convert to wire format for protocol compatibility. + * Format: (major << 8) | minor + */ + constexpr unsigned int toWire() const + { + return (major << 8) | minor; + } + + /** + * Convert from wire format. + */ + static constexpr Number fromWire(unsigned int wire) + { + return { + .major = (wire & 0xff00) >> 8, + .minor = static_cast(wire & 0x00ff), + }; + }; + } number; - constexpr auto operator<=>(const Version &) const = default; + using Feature = std::string; + using FeatureSet = std::set>; - /** - * Convert to wire format for protocol compatibility. - * Format: (major << 8) | minor - */ - constexpr unsigned int toWire() const - { - return (major << 8) | minor; - } + FeatureSet features = {}; + + bool operator==(const Version &) const = default; /** - * Convert from wire format. + * Partial ordering: v1 <= v2 iff v1.number <= v2.number AND + * v1.features is a subset of v2.features. Two versions with + * incomparable feature sets are unordered. */ - static constexpr Version fromWire(unsigned int wire) - { - return { - .major = (wire & 0xff00) >> 8, - .minor = static_cast(wire & 0x00ff), - }; - } + std::partial_ordering operator<=>(const Version & other) const; }; /** - * @note you generally shouldn't change the protocol version. Define a new - * `WorkerProto::Feature` instead. + * @note you generally shouldn't change the protocol version number. Define a new + * `WorkerProto::Version::Feature` instead. */ - static constexpr Version latest = { - .major = 1, - .minor = 38, - }; - static constexpr Version minimum = { - .major = 1, - .minor = 18, - }; + static const Version latest; + + static const Version minimum; /** * A unidirectional read connection, to be used by the read half of the @@ -98,7 +116,7 @@ struct WorkerProto struct ReadConn { Source & from; - Version version; + const Version & version; }; /** @@ -108,7 +126,7 @@ struct WorkerProto struct WriteConn { Sink & to; - Version version; + const Version & version; }; /** @@ -165,11 +183,6 @@ struct WorkerProto { WorkerProto::Serialise::write(store, conn, t); } - - using Feature = std::string; - using FeatureSet = std::set>; - - static const FeatureSet allFeatures; }; enum struct WorkerProto::Op : uint64_t { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9e0aff16db0..83748e8182d 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1494,7 +1494,7 @@ void LocalStore::verifyPath( unsigned int LocalStore::getProtocol() { - return WorkerProto::latest.toWire(); + return WorkerProto::latest.number.toWire(); } std::optional LocalStore::isTrustedClient() diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 164f87be239..8bc1a5d63bf 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -83,12 +83,9 @@ void RemoteStore::initConnection(Connection & conn) StringSink saved; TeeSource tee(conn.from, saved); try { - auto [protoVersion, features] = WorkerProto::BasicClientConnection::handshake( - conn.to, tee, WorkerProto::latest, WorkerProto::allFeatures); - if (protoVersion < WorkerProto::minimum) + conn.protoVersion = WorkerProto::BasicClientConnection::handshake(conn.to, tee, WorkerProto::latest); + if (conn.protoVersion.number < WorkerProto::minimum.number) throw Error("the Nix daemon version is too old"); - conn.protoVersion = protoVersion; - conn.features = features; } catch (SerialisationError & e) { /* In case the other side is waiting for our input, close it. */ @@ -102,7 +99,7 @@ void RemoteStore::initConnection(Connection & conn) static_cast(conn) = conn.postHandshake(*this); - for (auto & feature : conn.features) + for (auto & feature : conn.protoVersion.features) debug("negotiated feature '%s'", feature); auto ex = conn.processStderrReturn(); @@ -208,7 +205,7 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S auto conn(getConnection()); conn->to << WorkerProto::Op::QuerySubstitutablePathInfos; - if (conn->protoVersion < WorkerProto::Version{1, 22}) { + if (conn->protoVersion.number < WorkerProto::Version::Number{1, 22}) { StorePathSet paths; for (auto & path : pathsMap) paths.insert(path.first); @@ -264,7 +261,7 @@ StorePathSet RemoteStore::queryValidDerivers(const StorePath & path) StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path) { - if (WorkerProto::Version::fromWire(getProtocol()) >= WorkerProto::Version{1, 22}) { + if (WorkerProto::Version::Number::fromWire(getProtocol()) >= WorkerProto::Version::Number{1, 22}) { return Store::queryDerivationOutputs(path); } auto conn(getConnection()); @@ -277,7 +274,7 @@ StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path) std::map> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_) { - if (WorkerProto::Version::fromWire(getProtocol()) >= WorkerProto::Version{1, 22}) { + if (WorkerProto::Version::Number::fromWire(getProtocol()) >= WorkerProto::Version::Number{1, 22}) { if (!evalStore_) { auto conn(getConnection()); conn->to << WorkerProto::Op::QueryDerivationOutputMap; @@ -328,7 +325,7 @@ ref RemoteStore::addCAToStore( std::optional conn_(getConnection()); auto & conn = *conn_; - if (conn->protoVersion >= WorkerProto::Version{1, 25}) { + if (conn->protoVersion >= WorkerProto::Version{.number = {1, 25}}) { conn->to << WorkerProto::Op::AddToStore << name << caMethod.renderWithAlgo(hashAlgo); WorkerProto::write(*this, *conn, references); @@ -447,9 +444,9 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, Repair WorkerProto::write(*this, *conn, info.sigs); conn->to << renderContentAddress(info.ca) << repair << !checkSigs; - if (conn->protoVersion >= WorkerProto::Version{1, 23}) { + if (conn->protoVersion >= WorkerProto::Version{.number = {1, 23}}) { conn.withFramedSink([&](Sink & sink) { copyNAR(source, sink); }); - } else if (conn->protoVersion >= WorkerProto::Version{1, 21}) { + } else if (conn->protoVersion >= WorkerProto::Version{.number = {1, 21}}) { conn.processStderr(0, &source); } else { copyNAR(source, conn->to); @@ -480,7 +477,7 @@ void RemoteStore::addMultipleToStore( *this, WorkerProto::WriteConn{ .to = sink, - .version = 16, + .version = {.number = {.major = 1, .minor = 16}}, }, pathInfo); pathSource->drainInto(sink); @@ -493,7 +490,7 @@ void RemoteStore::addMultipleToStore( void RemoteStore::addMultipleToStore(Source & source, RepairFlag repair, CheckSigsFlag checkSigs) { - if (getConnection()->protoVersion >= WorkerProto::Version{1, 32}) { + if (getConnection()->protoVersion >= WorkerProto::Version{.number = {1, 32}}) { auto conn(getConnection()); conn->to << WorkerProto::Op::AddMultipleToStore << repair << !checkSigs; conn.withFramedSink([&](Sink & sink) { source.drainInto(sink); }); @@ -505,7 +502,7 @@ void RemoteStore::registerDrvOutput(const Realisation & info) { auto conn(getConnection()); conn->to << WorkerProto::Op::RegisterDrvOutput; - if (conn->protoVersion < WorkerProto::Version{1, 31}) { + if (conn->protoVersion.number < WorkerProto::Version::Number{1, 31}) { WorkerProto::write(*this, *conn, info.id); conn->to << std::string(info.outPath.to_string()); } else { @@ -520,7 +517,7 @@ void RemoteStore::queryRealisationUncached( try { auto conn(getConnection()); - if (conn->protoVersion < WorkerProto::Version{1, 27}) { + if (conn->protoVersion.number < WorkerProto::Version::Number{1, 27}) { warn("the daemon is too old to support content-addressing derivations, please upgrade it to 2.4"); return callback(nullptr); } @@ -530,7 +527,7 @@ void RemoteStore::queryRealisationUncached( conn.processStderr(); auto real = [&]() -> std::shared_ptr { - if (conn->protoVersion < WorkerProto::Version{1, 31}) { + if (conn->protoVersion.number < WorkerProto::Version::Number{1, 31}) { auto outPaths = WorkerProto::Serialise>::read(*this, *conn); if (outPaths.empty()) return nullptr; @@ -590,7 +587,7 @@ std::vector RemoteStore::buildPathsWithResults( std::optional conn_(getConnection()); auto & conn = *conn_; - if (conn->protoVersion >= WorkerProto::Version{1, 34}) { + if (conn->protoVersion >= WorkerProto::Version{.number = {1, 34}}) { conn->to << WorkerProto::Op::BuildPathsWithResults; WorkerProto::write(*this, *conn, paths); conn->to << buildMode; @@ -754,7 +751,7 @@ MissingPaths RemoteStore::queryMissing(const std::vector & targets) { { auto conn(getConnection()); - if (conn->protoVersion < WorkerProto::Version{1, 19}) + if (conn->protoVersion.number < WorkerProto::Version::Number{1, 19}) // Don't hold the connection handle in the fallback case // to prevent a deadlock. goto fallback; @@ -796,7 +793,7 @@ void RemoteStore::connect() unsigned int RemoteStore::getProtocol() { auto conn(connections->get()); - return conn->protoVersion.toWire(); + return conn->protoVersion.number.toWire(); } std::optional RemoteStore::isTrustedClient() diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 647787df224..9bb3040552e 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -199,7 +199,7 @@ void Store::addMultipleToStore(Source & source, RepairFlag repair, CheckSigsFlag *this, WorkerProto::ReadConn{ .from = source, - .version = 16, + .version = {.number = {.major = 1, .minor = 16}}, }); info.ultimate = false; addToStore(info, source, repair, checkSigs); diff --git a/src/libstore/worker-protocol-connection.cc b/src/libstore/worker-protocol-connection.cc index c1c6252aada..80ac5242701 100644 --- a/src/libstore/worker-protocol-connection.cc +++ b/src/libstore/worker-protocol-connection.cc @@ -5,8 +5,6 @@ namespace nix { -const WorkerProto::FeatureSet WorkerProto::allFeatures{}; - WorkerProto::BasicClientConnection::~BasicClientConnection() { try { @@ -64,7 +62,7 @@ WorkerProto::BasicClientConnection::processStderrReturn(Sink * sink, Source * so } else if (msg == STDERR_ERROR) { - if (protoVersion >= WorkerProto::Version{1, 26}) { + if (protoVersion >= WorkerProto::Version{.number = {1, 26}}) { ex = std::make_exception_ptr(readError(from)); } else { auto error = readString(from); @@ -122,7 +120,7 @@ WorkerProto::BasicClientConnection::processStderrReturn(Sink * sink, Source * so // explain to users what's going on when their daemon is // older than #4628 (2023). if (experimentalFeatureSettings.isEnabled(Xp::DynamicDerivations) - && protoVersion <= WorkerProto::Version{1, 35}) { + && protoVersion.number < WorkerProto::Version::Number{1, 36}) { auto m = e.msg(); if (m.find("parsing derivation") != std::string::npos && m.find("expected string") != std::string::npos && m.find("Derive([") != std::string::npos) @@ -146,86 +144,87 @@ void WorkerProto::BasicClientConnection::processStderr( } } -static WorkerProto::FeatureSet intersectFeatures(const WorkerProto::FeatureSet & a, const WorkerProto::FeatureSet & b) +static WorkerProto::Version::FeatureSet +intersectFeatures(const WorkerProto::Version::FeatureSet & a, const WorkerProto::Version::FeatureSet & b) { - WorkerProto::FeatureSet res; + WorkerProto::Version::FeatureSet res; for (auto & x : a) if (b.contains(x)) res.insert(x); return res; } -std::tuple WorkerProto::BasicClientConnection::handshake( - BufferedSink & to, - Source & from, - WorkerProto::Version localVersion, - const WorkerProto::FeatureSet & supportedFeatures) +WorkerProto::Version WorkerProto::BasicClientConnection::handshake( + BufferedSink & to, Source & from, const WorkerProto::Version & localVersion) { - to << WORKER_MAGIC_1 << localVersion.toWire(); + to << WORKER_MAGIC_1 << localVersion.number.toWire(); to.flush(); unsigned int magic = readInt(from); if (magic != WORKER_MAGIC_2) throw Error("nix-daemon protocol mismatch from"); - auto daemonVersion = WorkerProto::Version::fromWire(readInt(from)); + auto daemonVersion = WorkerProto::Version::Number::fromWire(readInt(from)); - if (daemonVersion.major != WorkerProto::latest.major) + if (daemonVersion.major != WorkerProto::latest.number.major) throw Error("Nix daemon protocol version not supported"); - if (daemonVersion < WorkerProto::Version{1, 10}) + if (daemonVersion < WorkerProto::Version::Number{1, 10}) throw Error("the Nix daemon version is too old"); - auto protoVersion = std::min(daemonVersion, localVersion); + auto protoVersionNumber = std::min(daemonVersion, localVersion.number); /* Exchange features. */ - WorkerProto::FeatureSet daemonFeatures; - if (protoVersion >= WorkerProto::Version{1, 38}) { - to << supportedFeatures; + WorkerProto::Version::FeatureSet daemonFeatures; + if (protoVersionNumber >= WorkerProto::Version::Number{1, 38}) { + to << localVersion.features; to.flush(); - daemonFeatures = readStrings(from); + daemonFeatures = readStrings(from); } - return {protoVersion, intersectFeatures(daemonFeatures, supportedFeatures)}; + return { + .number = protoVersionNumber, + .features = intersectFeatures(daemonFeatures, localVersion.features), + }; } -std::tuple WorkerProto::BasicServerConnection::handshake( - BufferedSink & to, - Source & from, - WorkerProto::Version localVersion, - const WorkerProto::FeatureSet & supportedFeatures) +WorkerProto::Version WorkerProto::BasicServerConnection::handshake( + BufferedSink & to, Source & from, const WorkerProto::Version & localVersion) { unsigned int magic = readInt(from); if (magic != WORKER_MAGIC_1) throw Error("protocol mismatch"); - to << WORKER_MAGIC_2 << localVersion.toWire(); + to << WORKER_MAGIC_2 << localVersion.number.toWire(); to.flush(); - auto clientVersion = WorkerProto::Version::fromWire(readInt(from)); + auto clientVersion = WorkerProto::Version::Number::fromWire(readInt(from)); - auto protoVersion = std::min(clientVersion, localVersion); + auto protoVersionNumber = std::min(clientVersion, localVersion.number); /* Exchange features. */ - WorkerProto::FeatureSet clientFeatures; - if (protoVersion >= WorkerProto::Version{1, 38}) { - clientFeatures = readStrings(from); - to << supportedFeatures; + WorkerProto::Version::FeatureSet clientFeatures; + if (protoVersionNumber >= WorkerProto::Version::Number{1, 38}) { + clientFeatures = readStrings(from); + to << localVersion.features; to.flush(); } - return {protoVersion, intersectFeatures(clientFeatures, supportedFeatures)}; + return { + .number = protoVersionNumber, + .features = intersectFeatures(clientFeatures, localVersion.features), + }; } WorkerProto::ClientHandshakeInfo WorkerProto::BasicClientConnection::postHandshake(const StoreDirConfig & store) { WorkerProto::ClientHandshakeInfo res; - if (protoVersion >= WorkerProto::Version{1, 14}) { + if (protoVersion >= WorkerProto::Version{.number = {1, 14}}) { // Obsolete CPU affinity. to << 0; } - if (protoVersion >= WorkerProto::Version{1, 11}) + if (protoVersion >= WorkerProto::Version{.number = {1, 11}}) to << false; // obsolete reserveSpace - if (protoVersion >= WorkerProto::Version{1, 33}) + if (protoVersion >= WorkerProto::Version{.number = {1, 33}}) to.flush(); return WorkerProto::Serialise::read(store, *this); @@ -233,12 +232,12 @@ WorkerProto::ClientHandshakeInfo WorkerProto::BasicClientConnection::postHandsha void WorkerProto::BasicServerConnection::postHandshake(const StoreDirConfig & store, const ClientHandshakeInfo & info) { - if (protoVersion >= WorkerProto::Version{1, 14} && readInt(from)) { + if (protoVersion >= WorkerProto::Version{.number = {1, 14}} && readInt(from)) { // Obsolete CPU affinity. readInt(from); } - if (protoVersion >= WorkerProto::Version{1, 11}) + if (protoVersion >= WorkerProto::Version{.number = {1, 11}}) readInt(from); // obsolete reserveSpace WorkerProto::write(store, *this, info); @@ -256,7 +255,7 @@ std::optional WorkerProto::BasicClientConnection::queryPat return std::nullopt; throw; } - if (protoVersion >= WorkerProto::Version{1, 17}) { + if (protoVersion >= WorkerProto::Version{.number = {1, 17}}) { bool valid; from >> valid; if (!valid) @@ -268,10 +267,10 @@ std::optional WorkerProto::BasicClientConnection::queryPat StorePathSet WorkerProto::BasicClientConnection::queryValidPaths( const StoreDirConfig & store, bool * daemonException, const StorePathSet & paths, SubstituteFlag maybeSubstitute) { - assert((protoVersion >= WorkerProto::Version{1, 12})); + assert((protoVersion >= WorkerProto::Version{.number = {1, 12}})); to << WorkerProto::Op::QueryValidPaths; WorkerProto::write(store, *this, paths); - if (protoVersion >= WorkerProto::Version{1, 27}) { + if (protoVersion >= WorkerProto::Version{.number = {1, 27}}) { to << maybeSubstitute; } processStderr(daemonException); diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index 1d07d4e4c66..49ab4c36b1e 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -14,6 +14,37 @@ namespace nix { +const WorkerProto::Version WorkerProto::latest = { + .number = + { + .major = 1, + .minor = 38, + }, +}; + +const WorkerProto::Version WorkerProto::minimum = { + .number = + { + .major = 1, + .minor = 18, + }, +}; + +std::partial_ordering WorkerProto::Version::operator<=>(const WorkerProto::Version & other) const +{ + auto numCmp = number <=> other.number; + bool thisSubsetEq = std::includes(other.features.begin(), other.features.end(), features.begin(), features.end()); + bool otherSubsetEq = std::includes(features.begin(), features.end(), other.features.begin(), other.features.end()); + + if (numCmp == 0 && thisSubsetEq && otherSubsetEq) + return std::partial_ordering::equivalent; + if (numCmp <= 0 && thisSubsetEq) + return std::partial_ordering::less; + if (numCmp >= 0 && otherSubsetEq) + return std::partial_ordering::greater; + return std::partial_ordering::unordered; +} + /* protocol-specific definitions */ BuildMode WorkerProto::Serialise::read(const StoreDirConfig & store, WorkerProto::ReadConn conn) @@ -153,7 +184,7 @@ void WorkerProto::Serialise>::write( DerivedPath WorkerProto::Serialise::read(const StoreDirConfig & store, WorkerProto::ReadConn conn) { auto s = readString(conn.from); - if (conn.version >= WorkerProto::Version{1, 30}) { + if (conn.version >= WorkerProto::Version{.number = {1, 30}}) { return DerivedPath::parseLegacy(store, s); } else { return parsePathWithOutputs(store, s).toDerivedPath(); @@ -163,7 +194,7 @@ DerivedPath WorkerProto::Serialise::read(const StoreDirConfig & sto void WorkerProto::Serialise::write( const StoreDirConfig & store, WorkerProto::WriteConn conn, const DerivedPath & req) { - if (conn.version >= WorkerProto::Version{1, 30}) { + if (conn.version >= WorkerProto::Version{.number = {1, 30}}) { conn.to << req.to_string_legacy(store); } else { auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(req); @@ -174,8 +205,8 @@ void WorkerProto::Serialise::write( throw Error( "trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file", store.printStorePath(drvPath), - conn.version.major, - conn.version.minor); + conn.version.number.major, + conn.version.number.minor); }, [&](std::monostate) { throw Error( @@ -213,17 +244,17 @@ BuildResult WorkerProto::Serialise::read(const StoreDirConfig & sto std::string errorMsg; bool isNonDeterministic = false; - auto status = WorkerProto::Serialise::read(store, {conn.from}); + auto status = WorkerProto::Serialise::read(store, conn); conn.from >> errorMsg; - if (conn.version >= WorkerProto::Version{1, 29}) { + if (conn.version >= WorkerProto::Version{.number = {1, 29}}) { conn.from >> res.timesBuilt >> isNonDeterministic >> res.startTime >> res.stopTime; } - if (conn.version >= WorkerProto::Version{1, 37}) { + if (conn.version >= WorkerProto::Version{.number = {1, 37}}) { res.cpuUser = WorkerProto::Serialise>::read(store, conn); res.cpuSystem = WorkerProto::Serialise>::read(store, conn); } - if (conn.version >= WorkerProto::Version{1, 28}) { + if (conn.version >= WorkerProto::Version{.number = {1, 28}}) { auto builtOutputs = WorkerProto::Serialise::read(store, conn); for (auto && [output, realisation] : builtOutputs) success.builtOutputs.insert_or_assign(std::move(output.outputName), std::move(realisation)); @@ -258,14 +289,14 @@ void WorkerProto::Serialise::write( default value for the fields that don't exist in that case. */ auto common = [&](std::string_view errorMsg, bool isNonDeterministic, const auto & builtOutputs) { conn.to << errorMsg; - if (conn.version >= WorkerProto::Version{1, 29}) { + if (conn.version >= WorkerProto::Version{.number = {1, 29}}) { conn.to << res.timesBuilt << isNonDeterministic << res.startTime << res.stopTime; } - if (conn.version >= WorkerProto::Version{1, 37}) { + if (conn.version >= WorkerProto::Version{.number = {1, 37}}) { WorkerProto::write(store, conn, res.cpuUser); WorkerProto::write(store, conn, res.cpuSystem); } - if (conn.version >= WorkerProto::Version{1, 28}) { + if (conn.version >= WorkerProto::Version{.number = {1, 28}}) { DrvOutputs builtOutputsFullKey; for (auto & [output, realisation] : builtOutputs) builtOutputsFullKey.insert_or_assign(realisation.id, realisation); @@ -275,11 +306,11 @@ void WorkerProto::Serialise::write( std::visit( overloaded{ [&](const BuildResult::Failure & failure) { - WorkerProto::write(store, {conn.to}, BuildResultStatus{failure.status}); + WorkerProto::write(store, conn, BuildResultStatus{failure.status}); common(failure.message(), failure.isNonDeterministic, decltype(BuildResult::Success::builtOutputs){}); }, [&](const BuildResult::Success & success) { - WorkerProto::write(store, {conn.to}, BuildResultStatus{success.status}); + WorkerProto::write(store, conn, BuildResultStatus{success.status}); common(/*errorMsg=*/"", /*isNonDeterministic=*/false, success.builtOutputs); }, }, @@ -310,7 +341,7 @@ UnkeyedValidPathInfo WorkerProto::Serialise::read(const St info.deriver = std::move(deriver); info.references = WorkerProto::Serialise::read(store, conn); conn.from >> info.registrationTime >> info.narSize; - if (conn.version >= WorkerProto::Version{1, 16}) { + if (conn.version >= WorkerProto::Version{.number = {1, 16}}) { conn.from >> info.ultimate; info.sigs = WorkerProto::Serialise>::read(store, conn); info.ca = ContentAddress::parseOpt(readString(conn.from)); @@ -325,7 +356,7 @@ void WorkerProto::Serialise::write( conn.to << pathInfo.narHash.to_string(HashFormat::Base16, false); WorkerProto::write(store, conn, pathInfo.references); conn.to << pathInfo.registrationTime << pathInfo.narSize; - if (conn.version >= WorkerProto::Version{1, 16}) { + if (conn.version >= WorkerProto::Version{.number = {1, 16}}) { conn.to << pathInfo.ultimate; WorkerProto::write(store, conn, pathInfo.sigs); conn.to << renderContentAddress(pathInfo.ca); @@ -337,11 +368,11 @@ WorkerProto::Serialise::read(const StoreDirCon { WorkerProto::ClientHandshakeInfo res; - if (conn.version >= WorkerProto::Version{1, 33}) { + if (conn.version >= WorkerProto::Version{.number = {1, 33}}) { res.daemonNixVersion = readString(conn.from); } - if (conn.version >= WorkerProto::Version{1, 35}) { + if (conn.version >= WorkerProto::Version{.number = {1, 35}}) { res.remoteTrustsUs = WorkerProto::Serialise>::read(store, conn); } else { // We don't know the answer; protocol to old. @@ -354,12 +385,12 @@ WorkerProto::Serialise::read(const StoreDirCon void WorkerProto::Serialise::write( const StoreDirConfig & store, WriteConn conn, const WorkerProto::ClientHandshakeInfo & info) { - if (conn.version >= WorkerProto::Version{1, 33}) { + if (conn.version >= WorkerProto::Version{.number = {1, 33}}) { assert(info.daemonNixVersion); conn.to << *info.daemonNixVersion; } - if (conn.version >= WorkerProto::Version{1, 35}) { + if (conn.version >= WorkerProto::Version{.number = {1, 35}}) { WorkerProto::write(store, conn, info.remoteTrustsUs); } } diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index 53c05890fa7..3308dd0a51a 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -22,7 +22,7 @@ namespace { std::string formatProtocol(unsigned int proto) { if (proto) { - auto version = WorkerProto::Version::fromWire(proto); + auto version = WorkerProto::Version::Number::fromWire(proto); return fmt("%1%.%2%", version.major, version.minor); } return "unknown"; @@ -152,9 +152,10 @@ struct CmdConfigCheck : StoreCommand bool checkStoreProtocol(unsigned int storeProto) { - auto storeVersion = WorkerProto::Version::fromWire(storeProto); - unsigned int clientProto = (storeVersion.major == ServeProto::latest.major) ? ServeProto::latest.toWire() - : WorkerProto::latest.toWire(); + auto storeVersion = WorkerProto::Version::Number::fromWire(storeProto); + unsigned int clientProto = (storeVersion.major == ServeProto::latest.major) + ? ServeProto::latest.toWire() + : WorkerProto::latest.number.toWire(); if (clientProto != storeProto) { std::ostringstream ss;