From cccc9440d77e7d4bc38814edef46f453685bf6b6 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 30 Jan 2026 15:56:23 -0500 Subject: [PATCH] worker-protocol: replace version bit-shifting with structs This commit replaces the `GET_PROTOCOL_MINOR(version)` macros with a proper `WorkerProto::Version` struct. As a bonus, this also fixes some version checks that were incorrectly ignoring the major version number. Co-authored-by: Amaan Qureshi --- .../include/nix/store/tests/protocol.hh | 32 +-- src/libstore-tests/serve-protocol.cc | 150 +++++++----- src/libstore-tests/worker-protocol.cc | 219 ++++++++++++------ src/libstore/daemon.cc | 26 +-- .../include/nix/store/serve-protocol.hh | 37 ++- .../include/nix/store/worker-protocol.hh | 48 +++- src/libstore/legacy-ssh-store.cc | 4 +- src/libstore/local-store.cc | 2 +- src/libstore/remote-store.cc | 32 +-- src/libstore/serve-protocol-connection.cc | 10 +- src/libstore/serve-protocol.cc | 24 +- src/libstore/worker-protocol-connection.cc | 36 +-- src/libstore/worker-protocol.cc | 32 +-- src/nix/config-check.cc | 11 +- src/nix/nix-store/nix-store.cc | 8 +- 15 files changed, 428 insertions(+), 243 deletions(-) diff --git a/src/libstore-test-support/include/nix/store/tests/protocol.hh b/src/libstore-test-support/include/nix/store/tests/protocol.hh index f65f12c53..4d62a0078 100644 --- a/src/libstore-test-support/include/nix/store/tests/protocol.hh +++ b/src/libstore-test-support/include/nix/store/tests/protocol.hh @@ -100,26 +100,26 @@ public: writeProtoTest(STEM, VERSION, VALUE); \ } -#define VERSIONED_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ - VERSIONED_READ_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ - VERSIONED_WRITE_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) +#define VERSIONED_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_READ_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, (VERSION), VALUE) \ + VERSIONED_WRITE_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, (VERSION), VALUE) -#define VERSIONED_READ_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ - VERSIONED_READ_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ - TEST_F(FIXTURE, NAME##_json_read) \ - { \ - readJsonTest(STEM, VALUE); \ +#define VERSIONED_READ_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_READ_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, (VERSION), VALUE) \ + TEST_F(FIXTURE, NAME##_json_read) \ + { \ + readJsonTest(STEM, VALUE); \ } -#define VERSIONED_WRITE_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ - VERSIONED_WRITE_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ - TEST_F(FIXTURE, NAME##_json_write) \ - { \ - writeJsonTest(STEM, VALUE); \ +#define VERSIONED_WRITE_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_WRITE_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, (VERSION), VALUE) \ + TEST_F(FIXTURE, NAME##_json_write) \ + { \ + writeJsonTest(STEM, VALUE); \ } -#define VERSIONED_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ - VERSIONED_READ_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ - VERSIONED_WRITE_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) +#define VERSIONED_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_READ_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, (VERSION), VALUE) \ + VERSIONED_WRITE_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, (VERSION), VALUE) } // namespace nix diff --git a/src/libstore-tests/serve-protocol.cc b/src/libstore-tests/serve-protocol.cc index b7be7b56d..159abeb60 100644 --- a/src/libstore-tests/serve-protocol.cc +++ b/src/libstore-tests/serve-protocol.cc @@ -25,7 +25,10 @@ struct ServeProtoTest : VersionedProtoTest * For serializers that don't care about the minimum version, we * used the oldest one: 2.5. */ - ServeProto::Version defaultVersion = 2 << 8 | 5; + ServeProto::Version defaultVersion = { + .major = 2, + .minor = 5, + }; }; VERSIONED_CHARACTERIZATION_TEST( @@ -144,54 +147,77 @@ VERSIONED_READ_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{.inner{BuildResult::Failure{{ - .status = BuildResult::Failure::OutputRejected, - .msg = HintFmt("no idea why"), - }}}}, - BuildResult{.inner{BuildResult::Failure{{ - .status = BuildResult::Failure::NotDeterministic, - .msg = HintFmt("no idea why"), - }}}}, - BuildResult{.inner{BuildResult::Success{ - .status = BuildResult::Success::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{.inner{BuildResult::Failure{{ - .status = BuildResult::Failure::OutputRejected, - .msg = HintFmt("no idea why"), - }}}}, - BuildResult{ - .inner{BuildResult::Failure{{ - .status = BuildResult::Failure::NotDeterministic, - .msg = HintFmt("no idea why"), - .isNonDeterministic = true, - }}}, - .timesBuilt = 3, - .startTime = 30, - .stopTime = 50, - }, - BuildResult{ - .inner{BuildResult::Success{ - .status = BuildResult::Success::Built, - }}, - .startTime = 30, - .stopTime = 50, - }, - }; - t; - })) +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_2, + "build-result-2.2", + (ServeProto::Version{ + .major = 2, + .minor = 2, + }), + ({ + using namespace std::literals::chrono_literals; + std::tuple t{ + BuildResult{.inner{BuildResult::Failure{{ + .status = BuildResult::Failure::OutputRejected, + .msg = HintFmt("no idea why"), + }}}}, + BuildResult{.inner{BuildResult::Failure{{ + .status = BuildResult::Failure::NotDeterministic, + .msg = HintFmt("no idea why"), + }}}}, + BuildResult{.inner{BuildResult::Success{ + .status = BuildResult::Success::Built, + }}}, + }; + t; + })) VERSIONED_CHARACTERIZATION_TEST( - ServeProtoTest, buildResult_2_6, "build-result-2.6", 2 << 8 | 6, ({ + ServeProtoTest, + buildResult_2_3, + "build-result-2.3", + (ServeProto::Version{ + .major = 2, + .minor = 3, + }), + ({ + using namespace std::literals::chrono_literals; + std::tuple t{ + BuildResult{.inner{BuildResult::Failure{{ + .status = BuildResult::Failure::OutputRejected, + .msg = HintFmt("no idea why"), + }}}}, + BuildResult{ + .inner{BuildResult::Failure{{ + .status = BuildResult::Failure::NotDeterministic, + .msg = HintFmt("no idea why"), + .isNonDeterministic = true, + }}}, + .timesBuilt = 3, + .startTime = 30, + .stopTime = 50, + }, + BuildResult{ + .inner{BuildResult::Success{ + .status = BuildResult::Success::Built, + }}, + .startTime = 30, + .stopTime = 50, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_6, + "build-result-2.6", + (ServeProto::Version{ + .major = 2, + .minor = 6, + }), + ({ using namespace std::literals::chrono_literals; std::tuple t{ BuildResult{.inner{BuildResult::Failure{{ @@ -260,7 +286,10 @@ VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, unkeyedValidPathInfo_2_3, "unkeyed-valid-path-info-2.3", - 2 << 8 | 3, + (ServeProto::Version{ + .major = 2, + .minor = 3, + }), (std::tuple{ ({ UnkeyedValidPathInfo info{std::string{defaultStoreDir}, Hash::dummy}; @@ -286,7 +315,10 @@ VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, unkeyedValidPathInfo_2_4, "unkeyed-valid-path-info-2.4", - 2 << 8 | 4, + (ServeProto::Version{ + .major = 2, + .minor = 4, + }), (std::tuple{ ({ UnkeyedValidPathInfo info{ @@ -340,7 +372,10 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( ServeProtoTest, build_options_2_1, "build-options-2.1", - 2 << 8 | 1, + (ServeProto::Version{ + .major = 2, + .minor = 1, + }), (ServeProto::BuildOptions{ .maxSilentTime = 5, .buildTimeout = 6, @@ -350,7 +385,10 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( ServeProtoTest, build_options_2_2, "build-options-2.2", - 2 << 8 | 2, + (ServeProto::Version{ + .major = 2, + .minor = 2, + }), (ServeProto::BuildOptions{ .maxSilentTime = 5, .buildTimeout = 6, @@ -361,7 +399,10 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( ServeProtoTest, build_options_2_3, "build-options-2.3", - 2 << 8 | 3, + (ServeProto::Version{ + .major = 2, + .minor = 3, + }), (ServeProto::BuildOptions{ .maxSilentTime = 5, .buildTimeout = 6, @@ -374,7 +415,10 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( ServeProtoTest, build_options_2_7, "build-options-2.7", - 2 << 8 | 7, + (ServeProto::Version{ + .major = 2, + .minor = 7, + }), (ServeProto::BuildOptions{ .maxSilentTime = 5, .buildTimeout = 6, diff --git a/src/libstore-tests/worker-protocol.cc b/src/libstore-tests/worker-protocol.cc index ea3be80ef..73c5df330 100644 --- a/src/libstore-tests/worker-protocol.cc +++ b/src/libstore-tests/worker-protocol.cc @@ -25,7 +25,10 @@ struct WorkerProtoTest : VersionedProtoTest * For serializers that don't care about the minimum version, we * used the oldest one: 1.10. */ - WorkerProto::Version defaultVersion = 1 << 8 | 10; + WorkerProto::Version defaultVersion = { + .major = 1, + .minor = 10, + }; }; VERSIONED_CHARACTERIZATION_TEST( @@ -79,7 +82,10 @@ VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, derivedPath_1_29, "derived-path-1.29", - 1 << 8 | 29, + (WorkerProto::Version{ + .major = 1, + .minor = 29, + }), (std::tuple{ DerivedPath::Opaque{ .path = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, @@ -104,7 +110,10 @@ VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, derivedPath_1_30, "derived-path-1.30", - 1 << 8 | 30, + (WorkerProto::Version{ + .major = 1, + .minor = 30, + }), (std::tuple{ DerivedPath::Opaque{ .path = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, @@ -197,26 +206,41 @@ VERSIONED_READ_CHARACTERIZATION_TEST( }, })) -VERSIONED_CHARACTERIZATION_TEST(WorkerProtoTest, buildResult_1_27, "build-result-1.27", 1 << 8 | 27, ({ - using namespace std::literals::chrono_literals; - std::tuple t{ - BuildResult{.inner{BuildResult::Failure{{ - .status = BuildResult::Failure::OutputRejected, - .msg = HintFmt("no idea why"), - }}}}, - BuildResult{.inner{BuildResult::Failure{{ - .status = BuildResult::Failure::NotDeterministic, - .msg = HintFmt("no idea why"), - }}}}, - BuildResult{.inner{BuildResult::Success{ - .status = BuildResult::Success::Built, - }}}, - }; - t; - })) +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + buildResult_1_27, + "build-result-1.27", + (WorkerProto::Version{ + .major = 1, + .minor = 27, + }), + ({ + using namespace std::literals::chrono_literals; + std::tuple t{ + BuildResult{.inner{BuildResult::Failure{{ + .status = BuildResult::Failure::OutputRejected, + .msg = HintFmt("no idea why"), + }}}}, + BuildResult{.inner{BuildResult::Failure{{ + .status = BuildResult::Failure::NotDeterministic, + .msg = HintFmt("no idea why"), + }}}}, + BuildResult{.inner{BuildResult::Success{ + .status = BuildResult::Success::Built, + }}}, + }; + t; + })) VERSIONED_CHARACTERIZATION_TEST( - WorkerProtoTest, buildResult_1_28, "build-result-1.28", 1 << 8 | 28, ({ + WorkerProtoTest, + buildResult_1_28, + "build-result-1.28", + (WorkerProto::Version{ + .major = 1, + .minor = 28, + }), + ({ using namespace std::literals::chrono_literals; std::tuple t{ BuildResult{.inner{BuildResult::Failure{{ @@ -262,7 +286,14 @@ VERSIONED_CHARACTERIZATION_TEST( })) VERSIONED_CHARACTERIZATION_TEST( - WorkerProtoTest, buildResult_1_29, "build-result-1.29", 1 << 8 | 29, ({ + WorkerProtoTest, + buildResult_1_29, + "build-result-1.29", + (WorkerProto::Version{ + .major = 1, + .minor = 29, + }), + ({ using namespace std::literals::chrono_literals; std::tuple t{ BuildResult{.inner{BuildResult::Failure{{ @@ -321,7 +352,14 @@ VERSIONED_CHARACTERIZATION_TEST( })) VERSIONED_CHARACTERIZATION_TEST( - WorkerProtoTest, buildResult_1_37, "build-result-1.37", 1 << 8 | 37, ({ + WorkerProtoTest, + buildResult_1_37, + "build-result-1.37", + (WorkerProto::Version{ + .major = 1, + .minor = 37, + }), + ({ using namespace std::literals::chrono_literals; std::tuple t{ BuildResult{.inner{BuildResult::Failure{{ @@ -381,48 +419,59 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) -VERSIONED_CHARACTERIZATION_TEST(WorkerProtoTest, keyedBuildResult_1_29, "keyed-build-result-1.29", 1 << 8 | 29, ({ - using namespace std::literals::chrono_literals; - std::tuple t{ - KeyedBuildResult{ - BuildResult{.inner{KeyedBuildResult::Failure{{ - .status = KeyedBuildResult::Failure::OutputRejected, - .msg = HintFmt("no idea why"), - }}}}, - /* .path = */ - DerivedPath::Opaque{ - StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-xxx"}, - }, - }, - KeyedBuildResult{ - BuildResult{ - .inner{KeyedBuildResult::Failure{{ - .status = KeyedBuildResult::Failure::NotDeterministic, - .msg = HintFmt("no idea why"), - .isNonDeterministic = true, - }}}, - .timesBuilt = 3, - .startTime = 30, - .stopTime = 50, - }, - /* .path = */ - DerivedPath::Built{ - .drvPath = makeConstantStorePathRef( - StorePath{ - "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", - }), - .outputs = OutputsSpec::Names{"out"}, - }, - }, - }; - t; - })) +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + keyedBuildResult_1_29, + "keyed-build-result-1.29", + (WorkerProto::Version{ + .major = 1, + .minor = 29, + }), + ({ + using namespace std::literals::chrono_literals; + std::tuple t{ + KeyedBuildResult{ + BuildResult{.inner{KeyedBuildResult::Failure{{ + .status = KeyedBuildResult::Failure::OutputRejected, + .msg = HintFmt("no idea why"), + }}}}, + /* .path = */ + DerivedPath::Opaque{ + StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-xxx"}, + }, + }, + KeyedBuildResult{ + BuildResult{ + .inner{KeyedBuildResult::Failure{{ + .status = KeyedBuildResult::Failure::NotDeterministic, + .msg = HintFmt("no idea why"), + .isNonDeterministic = true, + }}}, + .timesBuilt = 3, + .startTime = 30, + .stopTime = 50, + }, + /* .path = */ + DerivedPath::Built{ + .drvPath = makeConstantStorePathRef( + StorePath{ + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }), + .outputs = OutputsSpec::Names{"out"}, + }, + }, + }; + t; + })) VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, unkeyedValidPathInfo_1_15, "unkeyed-valid-path-info-1.15", - 1 << 8 | 15, + (WorkerProto::Version{ + .major = 1, + .minor = 15, + }), (std::tuple{ ({ UnkeyedValidPathInfo info{ @@ -456,7 +505,10 @@ VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, validPathInfo_1_15, "valid-path-info-1.15", - 1 << 8 | 15, + (WorkerProto::Version{ + .major = 1, + .minor = 15, + }), (std::tuple{ ({ ValidPathInfo info{ @@ -505,7 +557,10 @@ VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, validPathInfo_1_16, "valid-path-info-1.16", - 1 << 8 | 16, + (WorkerProto::Version{ + .major = 1, + .minor = 16, + }), (std::tuple{ ({ ValidPathInfo info{ @@ -660,7 +715,10 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( WorkerProtoTest, clientHandshakeInfo_1_30, "client-handshake-info_1_30", - 1 << 8 | 30, + (WorkerProto::Version{ + .major = 1, + .minor = 30, + }), (std::tuple{ {}, })) @@ -669,7 +727,10 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( WorkerProtoTest, clientHandshakeInfo_1_33, "client-handshake-info_1_33", - 1 << 8 | 33, + (WorkerProto::Version{ + .major = 1, + .minor = 33, + }), (std::tuple{ { .daemonNixVersion = std::optional{"foo"}, @@ -683,7 +744,10 @@ VERSIONED_CHARACTERIZATION_TEST_NO_JSON( WorkerProtoTest, clientHandshakeInfo_1_35, "client-handshake-info_1_35", - 1 << 8 | 35, + (WorkerProto::Version{ + .major = 1, + .minor = 35, + }), (std::tuple{ { .daemonNixVersion = std::optional{"foo"}, @@ -736,17 +800,36 @@ TEST_F(WorkerProtoTest, handshake_features) auto clientThread = std::thread([&]() { FdSink out{toServer.writeSide.get()}; FdSource in{toClient.readSide.get()}; - clientResult = WorkerProto::BasicClientConnection::handshake(out, in, 123, {"bar", "aap", "mies", "xyzzy"}); + clientResult = WorkerProto::BasicClientConnection::handshake( + out, + in, + WorkerProto::Version{ + .major = 1, + .minor = 123, + }, + {"bar", "aap", "mies", "xyzzy"}); }); FdSink out{toClient.writeSide.get()}; FdSource in{toServer.readSide.get()}; - auto daemonResult = WorkerProto::BasicServerConnection::handshake(out, in, 456, {"foo", "bar", "xyzzy"}); + auto daemonResult = WorkerProto::BasicServerConnection::handshake( + out, + in, + WorkerProto::Version{ + .major = 1, + .minor = 200, + }, + {"foo", "bar", "xyzzy"}); clientThread.join(); EXPECT_EQ(clientResult, daemonResult); - EXPECT_EQ(std::get<0>(clientResult), 123u); + EXPECT_EQ( + std::get<0>(clientResult), + (WorkerProto::Version{ + .major = 1, + .minor = 123, + })); EXPECT_EQ(std::get<1>(clientResult), WorkerProto::FeatureSet({"bar", "xyzzy"})); } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 481f7dc11..045f2259e 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 (GET_PROTOCOL_MINOR(clientVersion) >= 26) { + if (clientVersion >= WorkerProto::Version{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 (GET_PROTOCOL_MINOR(clientVersion) < 20) { + if (clientVersion < WorkerProto::Version{1, 20}) { if (!s.empty()) log(lvl, s + "..."); return; @@ -162,7 +162,7 @@ struct TunnelLogger : public Logger void stopActivity(ActivityId act) override { - if (GET_PROTOCOL_MINOR(clientVersion) < 20) + if (clientVersion < WorkerProto::Version{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 (GET_PROTOCOL_MINOR(clientVersion) < 20) + if (clientVersion < WorkerProto::Version{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 (GET_PROTOCOL_MINOR(conn.protoVersion) >= 27) { + if (conn.protoVersion >= WorkerProto::Version{1, 27}) { substitute = readInt(conn.from) ? Substitute : NoSubstitute; } @@ -402,7 +402,7 @@ static void performOp( } case WorkerProto::Op::AddToStore: { - if (GET_PROTOCOL_MINOR(conn.protoVersion) >= 25) { + if (conn.protoVersion >= WorkerProto::Version{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 (GET_PROTOCOL_MINOR(conn.protoVersion) < 22) { + if (conn.protoVersion < WorkerProto::Version{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 (GET_PROTOCOL_MINOR(conn.protoVersion) >= 23) { + if (conn.protoVersion >= WorkerProto::Version{1, 23}) { logger->startWork(); { FramedSource source(conn.from); @@ -909,7 +909,7 @@ static void performOp( else { std::unique_ptr source; StringSink saved; - if (GET_PROTOCOL_MINOR(conn.protoVersion) >= 21) + if (conn.protoVersion >= WorkerProto::Version{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 (GET_PROTOCOL_MINOR(conn.protoVersion) < 31) { + if (conn.protoVersion < WorkerProto::Version{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 (GET_PROTOCOL_MINOR(conn.protoVersion) < 31) { + if (conn.protoVersion < WorkerProto::Version{1, 31}) { std::set outPaths; if (info) outPaths.insert(info->outPath); @@ -1016,9 +1016,9 @@ void processConnection(ref store, FdSource && from, FdSink && to, Trusted /* Exchange the greeting. */ auto [protoVersion, features] = - WorkerProto::BasicServerConnection::handshake(to, from, PROTOCOL_VERSION, WorkerProto::allFeatures); + WorkerProto::BasicServerConnection::handshake(to, from, WorkerProto::latest, WorkerProto::allFeatures); - if (protoVersion < MINIMUM_PROTOCOL_VERSION) + if (protoVersion < WorkerProto::minimum) throw Error("the Nix client version is too old"); WorkerProto::BasicServerConnection conn; diff --git a/src/libstore/include/nix/store/serve-protocol.hh b/src/libstore/include/nix/store/serve-protocol.hh index 974bf42d5..dba05a345 100644 --- a/src/libstore/include/nix/store/serve-protocol.hh +++ b/src/libstore/include/nix/store/serve-protocol.hh @@ -8,10 +8,6 @@ namespace nix { #define SERVE_MAGIC_1 0x390c9deb #define SERVE_MAGIC_2 0x5452eecb -#define SERVE_PROTOCOL_VERSION (2 << 8 | 7) -#define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) -#define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) - struct StoreDirConfig; struct Source; @@ -37,7 +33,38 @@ struct ServeProto * * @todo Convert to struct with separate major vs minor fields. */ - using Version = unsigned int; + struct Version + { + unsigned int major; + uint8_t minor; + + constexpr auto operator<=>(const Version &) 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 Version fromWire(unsigned int wire) + { + return { + .major = (wire & 0xff00) >> 8, + .minor = static_cast(wire & 0x00ff), + }; + } + }; + + static constexpr Version latest = { + .major = 2, + .minor = 7, + }; /** * A unidirectional read connection, to be used by the read half of the diff --git a/src/libstore/include/nix/store/worker-protocol.hh b/src/libstore/include/nix/store/worker-protocol.hh index 110c50ce6..3babda13d 100644 --- a/src/libstore/include/nix/store/worker-protocol.hh +++ b/src/libstore/include/nix/store/worker-protocol.hh @@ -10,13 +10,6 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f -/* Note: you generally shouldn't change the protocol version. Define a - new `WorkerProto::Feature` instead. */ -#define PROTOCOL_VERSION (1 << 8 | 38) -#define MINIMUM_PROTOCOL_VERSION (1 << 8 | 18) -#define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) -#define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) - #define STDERR_NEXT 0x6f6c6d67 #define STDERR_READ 0x64617461 // data needed from source #define STDERR_WRITE 0x64617416 // data for sink @@ -57,7 +50,46 @@ struct WorkerProto * * @todo Convert to struct with separate major vs minor fields. */ - using Version = unsigned int; + struct Version + { + unsigned int major; + uint8_t minor; + + constexpr auto operator<=>(const Version &) 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 Version fromWire(unsigned int wire) + { + return { + .major = (wire & 0xff00) >> 8, + .minor = static_cast(wire & 0x00ff), + }; + } + }; + + /** + * @note you generally shouldn't change the protocol version. Define a new + * `WorkerProto::Feature` instead. + */ + static constexpr Version latest = { + .major = 1, + .minor = 38, + }; + static constexpr Version minimum = { + .major = 1, + .minor = 18, + }; /** * A unidirectional read connection, to be used by the read half of the diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index fe6bb6d0b..830582753 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -73,7 +73,7 @@ ref LegacySSHStore::openConnection() TeeSource tee(conn->from, saved); try { conn->remoteVersion = - ServeProto::BasicClientConnection::handshake(conn->to, tee, SERVE_PROTOCOL_VERSION, config->authority.host); + ServeProto::BasicClientConnection::handshake(conn->to, tee, ServeProto::latest, config->authority.host); } catch (SerialisationError & e) { // in.close(): Don't let the remote block on us not writing. conn->sshConn->in.close(); @@ -290,7 +290,7 @@ void LegacySSHStore::connect() unsigned int LegacySSHStore::getProtocol() { auto conn(connections->get()); - return conn->remoteVersion; + return conn->remoteVersion.toWire(); } pid_t LegacySSHStore::getConnectionPid() diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 91c3867e5..06d4d2b4f 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1496,7 +1496,7 @@ void LocalStore::verifyPath( unsigned int LocalStore::getProtocol() { - return PROTOCOL_VERSION; + return WorkerProto::latest.toWire(); } std::optional LocalStore::isTrustedClient() diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 357ea55f9..164f87be2 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -83,9 +83,9 @@ void RemoteStore::initConnection(Connection & conn) StringSink saved; TeeSource tee(conn.from, saved); try { - auto [protoVersion, features] = - WorkerProto::BasicClientConnection::handshake(conn.to, tee, PROTOCOL_VERSION, WorkerProto::allFeatures); - if (protoVersion < MINIMUM_PROTOCOL_VERSION) + auto [protoVersion, features] = WorkerProto::BasicClientConnection::handshake( + conn.to, tee, WorkerProto::latest, WorkerProto::allFeatures); + if (protoVersion < WorkerProto::minimum) throw Error("the Nix daemon version is too old"); conn.protoVersion = protoVersion; conn.features = features; @@ -208,7 +208,7 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S auto conn(getConnection()); conn->to << WorkerProto::Op::QuerySubstitutablePathInfos; - if (GET_PROTOCOL_MINOR(conn->protoVersion) < 22) { + if (conn->protoVersion < WorkerProto::Version{1, 22}) { StorePathSet paths; for (auto & path : pathsMap) paths.insert(path.first); @@ -264,7 +264,7 @@ StorePathSet RemoteStore::queryValidDerivers(const StorePath & path) StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path) { - if (GET_PROTOCOL_MINOR(getProtocol()) >= 0x16) { + if (WorkerProto::Version::fromWire(getProtocol()) >= WorkerProto::Version{1, 22}) { return Store::queryDerivationOutputs(path); } auto conn(getConnection()); @@ -277,7 +277,7 @@ StorePathSet RemoteStore::queryDerivationOutputs(const StorePath & path) std::map> RemoteStore::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_) { - if (GET_PROTOCOL_MINOR(getProtocol()) >= 0x16) { + if (WorkerProto::Version::fromWire(getProtocol()) >= WorkerProto::Version{1, 22}) { if (!evalStore_) { auto conn(getConnection()); conn->to << WorkerProto::Op::QueryDerivationOutputMap; @@ -328,7 +328,7 @@ ref RemoteStore::addCAToStore( std::optional conn_(getConnection()); auto & conn = *conn_; - if (GET_PROTOCOL_MINOR(conn->protoVersion) >= 25) { + if (conn->protoVersion >= WorkerProto::Version{1, 25}) { conn->to << WorkerProto::Op::AddToStore << name << caMethod.renderWithAlgo(hashAlgo); WorkerProto::write(*this, *conn, references); @@ -447,9 +447,9 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, Repair WorkerProto::write(*this, *conn, info.sigs); conn->to << renderContentAddress(info.ca) << repair << !checkSigs; - if (GET_PROTOCOL_MINOR(conn->protoVersion) >= 23) { + if (conn->protoVersion >= WorkerProto::Version{1, 23}) { conn.withFramedSink([&](Sink & sink) { copyNAR(source, sink); }); - } else if (GET_PROTOCOL_MINOR(conn->protoVersion) >= 21) { + } else if (conn->protoVersion >= WorkerProto::Version{1, 21}) { conn.processStderr(0, &source); } else { copyNAR(source, conn->to); @@ -493,7 +493,7 @@ void RemoteStore::addMultipleToStore( void RemoteStore::addMultipleToStore(Source & source, RepairFlag repair, CheckSigsFlag checkSigs) { - if (GET_PROTOCOL_MINOR(getConnection()->protoVersion) >= 32) { + if (getConnection()->protoVersion >= WorkerProto::Version{1, 32}) { auto conn(getConnection()); conn->to << WorkerProto::Op::AddMultipleToStore << repair << !checkSigs; conn.withFramedSink([&](Sink & sink) { source.drainInto(sink); }); @@ -505,7 +505,7 @@ void RemoteStore::registerDrvOutput(const Realisation & info) { auto conn(getConnection()); conn->to << WorkerProto::Op::RegisterDrvOutput; - if (GET_PROTOCOL_MINOR(conn->protoVersion) < 31) { + if (conn->protoVersion < WorkerProto::Version{1, 31}) { WorkerProto::write(*this, *conn, info.id); conn->to << std::string(info.outPath.to_string()); } else { @@ -520,7 +520,7 @@ void RemoteStore::queryRealisationUncached( try { auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->protoVersion) < 27) { + if (conn->protoVersion < WorkerProto::Version{1, 27}) { warn("the daemon is too old to support content-addressing derivations, please upgrade it to 2.4"); return callback(nullptr); } @@ -530,7 +530,7 @@ void RemoteStore::queryRealisationUncached( conn.processStderr(); auto real = [&]() -> std::shared_ptr { - if (GET_PROTOCOL_MINOR(conn->protoVersion) < 31) { + if (conn->protoVersion < WorkerProto::Version{1, 31}) { auto outPaths = WorkerProto::Serialise>::read(*this, *conn); if (outPaths.empty()) return nullptr; @@ -590,7 +590,7 @@ std::vector RemoteStore::buildPathsWithResults( std::optional conn_(getConnection()); auto & conn = *conn_; - if (GET_PROTOCOL_MINOR(conn->protoVersion) >= 34) { + if (conn->protoVersion >= WorkerProto::Version{1, 34}) { conn->to << WorkerProto::Op::BuildPathsWithResults; WorkerProto::write(*this, *conn, paths); conn->to << buildMode; @@ -754,7 +754,7 @@ MissingPaths RemoteStore::queryMissing(const std::vector & targets) { { auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->protoVersion) < 19) + if (conn->protoVersion < WorkerProto::Version{1, 19}) // Don't hold the connection handle in the fallback case // to prevent a deadlock. goto fallback; @@ -796,7 +796,7 @@ void RemoteStore::connect() unsigned int RemoteStore::getProtocol() { auto conn(connections->get()); - return conn->protoVersion; + return conn->protoVersion.toWire(); } std::optional RemoteStore::isTrustedClient() diff --git a/src/libstore/serve-protocol-connection.cc b/src/libstore/serve-protocol-connection.cc index baa3bf0ce..9625777d2 100644 --- a/src/libstore/serve-protocol-connection.cc +++ b/src/libstore/serve-protocol-connection.cc @@ -8,14 +8,14 @@ namespace nix { ServeProto::Version ServeProto::BasicClientConnection::handshake( BufferedSink & to, Source & from, ServeProto::Version localVersion, std::string_view host) { - to << SERVE_MAGIC_1 << localVersion; + to << SERVE_MAGIC_1 << localVersion.toWire(); to.flush(); unsigned int magic = readInt(from); if (magic != SERVE_MAGIC_2) throw Error("'nix-store --serve' protocol mismatch from '%s'", host); - auto remoteVersion = readInt(from); - if (GET_PROTOCOL_MAJOR(remoteVersion) != 0x200 || GET_PROTOCOL_MINOR(remoteVersion) < 5) + auto remoteVersion = ServeProto::Version::fromWire(readInt(from)); + if (remoteVersion.major != 2 || remoteVersion < ServeProto::Version{2, 5}) throw Error("unsupported 'nix-store --serve' protocol version on '%s'", host); return std::min(remoteVersion, localVersion); } @@ -26,9 +26,9 @@ ServeProto::BasicServerConnection::handshake(BufferedSink & to, Source & from, S unsigned int magic = readInt(from); if (magic != SERVE_MAGIC_1) throw Error("protocol mismatch"); - to << SERVE_MAGIC_2 << localVersion; + to << SERVE_MAGIC_2 << localVersion.toWire(); to.flush(); - auto remoteVersion = readInt(from); + auto remoteVersion = ServeProto::Version::fromWire(readInt(from)); return std::min(remoteVersion, localVersion); } diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc index a1250fee4..ec7b85ed8 100644 --- a/src/libstore/serve-protocol.cc +++ b/src/libstore/serve-protocol.cc @@ -26,9 +26,9 @@ BuildResult ServeProto::Serialise::read(const StoreDirConfig & stor auto status = ServeProto::Serialise::read(store, {conn.from}); conn.from >> errorMsg; - if (GET_PROTOCOL_MINOR(conn.version) >= 3) + if (conn.version >= ServeProto::Version{2, 3}) conn.from >> res.timesBuilt >> isNonDeterministic >> res.startTime >> res.stopTime; - if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + if (conn.version >= ServeProto::Version{2, 6}) { auto builtOutputs = ServeProto::Serialise::read(store, conn); for (auto && [output, realisation] : builtOutputs) success.builtOutputs.insert_or_assign(std::move(output.outputName), std::move(realisation)); @@ -63,9 +63,9 @@ void ServeProto::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 (GET_PROTOCOL_MINOR(conn.version) >= 3) + if (conn.version >= ServeProto::Version{2, 3}) conn.to << res.timesBuilt << isNonDeterministic << res.startTime << res.stopTime; - if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + if (conn.version >= ServeProto::Version{2, 6}) { DrvOutputs builtOutputsFullKey; for (auto & [output, realisation] : builtOutputs) builtOutputsFullKey.insert_or_assign(realisation.id, realisation); @@ -100,7 +100,7 @@ UnkeyedValidPathInfo ServeProto::Serialise::read(const Sto readLongLong(conn.from); // download size, unused info.narSize = readLongLong(conn.from); - if (GET_PROTOCOL_MINOR(conn.version) >= 4) { + if (conn.version >= ServeProto::Version{2, 4}) { auto s = readString(conn.from); if (!s.empty()) info.narHash = Hash::parseAnyPrefixed(s); @@ -120,7 +120,7 @@ void ServeProto::Serialise::write( // !!! Maybe we want compression? conn.to << info.narSize // downloadSize, lie a little << info.narSize; - if (GET_PROTOCOL_MINOR(conn.version) >= 4) { + if (conn.version >= ServeProto::Version{2, 4}) { conn.to << info.narHash.to_string(HashFormat::Nix32, true) << renderContentAddress(info.ca); ServeProto::write(store, conn, info.sigs); } @@ -132,13 +132,13 @@ ServeProto::Serialise::read(const StoreDirConfig & sto BuildOptions options; options.maxSilentTime = readInt(conn.from); options.buildTimeout = readInt(conn.from); - if (GET_PROTOCOL_MINOR(conn.version) >= 2) + if (conn.version >= ServeProto::Version{2, 2}) options.maxLogSize = readNum(conn.from); - if (GET_PROTOCOL_MINOR(conn.version) >= 3) { + if (conn.version >= ServeProto::Version{2, 3}) { options.nrRepeats = readInt(conn.from); options.enforceDeterminism = readInt(conn.from); } - if (GET_PROTOCOL_MINOR(conn.version) >= 7) { + if (conn.version >= ServeProto::Version{2, 7}) { options.keepFailed = (bool) readInt(conn.from); } return options; @@ -148,12 +148,12 @@ void ServeProto::Serialise::write( const StoreDirConfig & store, WriteConn conn, const ServeProto::BuildOptions & options) { conn.to << options.maxSilentTime << options.buildTimeout; - if (GET_PROTOCOL_MINOR(conn.version) >= 2) + if (conn.version >= ServeProto::Version{2, 2}) conn.to << options.maxLogSize; - if (GET_PROTOCOL_MINOR(conn.version) >= 3) + if (conn.version >= ServeProto::Version{2, 3}) conn.to << options.nrRepeats << options.enforceDeterminism; - if (GET_PROTOCOL_MINOR(conn.version) >= 7) { + if (conn.version >= ServeProto::Version{2, 7}) { conn.to << ((int) options.keepFailed); } } diff --git a/src/libstore/worker-protocol-connection.cc b/src/libstore/worker-protocol-connection.cc index 8a3766290..c1c6252aa 100644 --- a/src/libstore/worker-protocol-connection.cc +++ b/src/libstore/worker-protocol-connection.cc @@ -64,7 +64,7 @@ WorkerProto::BasicClientConnection::processStderrReturn(Sink * sink, Source * so } else if (msg == STDERR_ERROR) { - if (GET_PROTOCOL_MINOR(protoVersion) >= 26) { + if (protoVersion >= WorkerProto::Version{1, 26}) { ex = std::make_exception_ptr(readError(from)); } else { auto error = readString(from); @@ -122,7 +122,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) - && GET_PROTOCOL_MINOR(protoVersion) <= 35) { + && protoVersion <= WorkerProto::Version{1, 35}) { 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) @@ -161,24 +161,24 @@ std::tuple WorkerProto::BasicClie WorkerProto::Version localVersion, const WorkerProto::FeatureSet & supportedFeatures) { - to << WORKER_MAGIC_1 << localVersion; + to << WORKER_MAGIC_1 << localVersion.toWire(); to.flush(); unsigned int magic = readInt(from); if (magic != WORKER_MAGIC_2) throw Error("nix-daemon protocol mismatch from"); - auto daemonVersion = readInt(from); + auto daemonVersion = WorkerProto::Version::fromWire(readInt(from)); - if (GET_PROTOCOL_MAJOR(daemonVersion) != GET_PROTOCOL_MAJOR(PROTOCOL_VERSION)) + if (daemonVersion.major != WorkerProto::latest.major) throw Error("Nix daemon protocol version not supported"); - if (GET_PROTOCOL_MINOR(daemonVersion) < 10) + if (daemonVersion < WorkerProto::Version{1, 10}) throw Error("the Nix daemon version is too old"); auto protoVersion = std::min(daemonVersion, localVersion); /* Exchange features. */ WorkerProto::FeatureSet daemonFeatures; - if (GET_PROTOCOL_MINOR(protoVersion) >= 38) { + if (protoVersion >= WorkerProto::Version{1, 38}) { to << supportedFeatures; to.flush(); daemonFeatures = readStrings(from); @@ -196,15 +196,15 @@ std::tuple WorkerProto::BasicServ unsigned int magic = readInt(from); if (magic != WORKER_MAGIC_1) throw Error("protocol mismatch"); - to << WORKER_MAGIC_2 << localVersion; + to << WORKER_MAGIC_2 << localVersion.toWire(); to.flush(); - auto clientVersion = readInt(from); + auto clientVersion = WorkerProto::Version::fromWire(readInt(from)); auto protoVersion = std::min(clientVersion, localVersion); /* Exchange features. */ WorkerProto::FeatureSet clientFeatures; - if (GET_PROTOCOL_MINOR(protoVersion) >= 38) { + if (protoVersion >= WorkerProto::Version{1, 38}) { clientFeatures = readStrings(from); to << supportedFeatures; to.flush(); @@ -217,15 +217,15 @@ WorkerProto::ClientHandshakeInfo WorkerProto::BasicClientConnection::postHandsha { WorkerProto::ClientHandshakeInfo res; - if (GET_PROTOCOL_MINOR(protoVersion) >= 14) { + if (protoVersion >= WorkerProto::Version{1, 14}) { // Obsolete CPU affinity. to << 0; } - if (GET_PROTOCOL_MINOR(protoVersion) >= 11) + if (protoVersion >= WorkerProto::Version{1, 11}) to << false; // obsolete reserveSpace - if (GET_PROTOCOL_MINOR(protoVersion) >= 33) + if (protoVersion >= WorkerProto::Version{1, 33}) to.flush(); return WorkerProto::Serialise::read(store, *this); @@ -233,12 +233,12 @@ WorkerProto::ClientHandshakeInfo WorkerProto::BasicClientConnection::postHandsha void WorkerProto::BasicServerConnection::postHandshake(const StoreDirConfig & store, const ClientHandshakeInfo & info) { - if (GET_PROTOCOL_MINOR(protoVersion) >= 14 && readInt(from)) { + if (protoVersion >= WorkerProto::Version{1, 14} && readInt(from)) { // Obsolete CPU affinity. readInt(from); } - if (GET_PROTOCOL_MINOR(protoVersion) >= 11) + if (protoVersion >= WorkerProto::Version{1, 11}) readInt(from); // obsolete reserveSpace WorkerProto::write(store, *this, info); @@ -256,7 +256,7 @@ std::optional WorkerProto::BasicClientConnection::queryPat return std::nullopt; throw; } - if (GET_PROTOCOL_MINOR(protoVersion) >= 17) { + if (protoVersion >= WorkerProto::Version{1, 17}) { bool valid; from >> valid; if (!valid) @@ -268,10 +268,10 @@ std::optional WorkerProto::BasicClientConnection::queryPat StorePathSet WorkerProto::BasicClientConnection::queryValidPaths( const StoreDirConfig & store, bool * daemonException, const StorePathSet & paths, SubstituteFlag maybeSubstitute) { - assert(GET_PROTOCOL_MINOR(protoVersion) >= 12); + assert((protoVersion >= WorkerProto::Version{1, 12})); to << WorkerProto::Op::QueryValidPaths; WorkerProto::write(store, *this, paths); - if (GET_PROTOCOL_MINOR(protoVersion) >= 27) { + if (protoVersion >= WorkerProto::Version{1, 27}) { to << maybeSubstitute; } processStderr(daemonException); diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index a78bb87be..1d07d4e4c 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -153,7 +153,7 @@ void WorkerProto::Serialise>::write( DerivedPath WorkerProto::Serialise::read(const StoreDirConfig & store, WorkerProto::ReadConn conn) { auto s = readString(conn.from); - if (GET_PROTOCOL_MINOR(conn.version) >= 30) { + if (conn.version >= WorkerProto::Version{1, 30}) { return DerivedPath::parseLegacy(store, s); } else { return parsePathWithOutputs(store, s).toDerivedPath(); @@ -163,7 +163,7 @@ DerivedPath WorkerProto::Serialise::read(const StoreDirConfig & sto void WorkerProto::Serialise::write( const StoreDirConfig & store, WorkerProto::WriteConn conn, const DerivedPath & req) { - if (GET_PROTOCOL_MINOR(conn.version) >= 30) { + if (conn.version >= WorkerProto::Version{1, 30}) { conn.to << req.to_string_legacy(store); } else { auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(req); @@ -174,8 +174,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), - GET_PROTOCOL_MAJOR(conn.version), - GET_PROTOCOL_MINOR(conn.version)); + conn.version.major, + conn.version.minor); }, [&](std::monostate) { throw Error( @@ -216,14 +216,14 @@ BuildResult WorkerProto::Serialise::read(const StoreDirConfig & sto auto status = WorkerProto::Serialise::read(store, {conn.from}); conn.from >> errorMsg; - if (GET_PROTOCOL_MINOR(conn.version) >= 29) { + if (conn.version >= WorkerProto::Version{1, 29}) { conn.from >> res.timesBuilt >> isNonDeterministic >> res.startTime >> res.stopTime; } - if (GET_PROTOCOL_MINOR(conn.version) >= 37) { + if (conn.version >= WorkerProto::Version{1, 37}) { res.cpuUser = WorkerProto::Serialise>::read(store, conn); res.cpuSystem = WorkerProto::Serialise>::read(store, conn); } - if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + if (conn.version >= WorkerProto::Version{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 +258,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 (GET_PROTOCOL_MINOR(conn.version) >= 29) { + if (conn.version >= WorkerProto::Version{1, 29}) { conn.to << res.timesBuilt << isNonDeterministic << res.startTime << res.stopTime; } - if (GET_PROTOCOL_MINOR(conn.version) >= 37) { + if (conn.version >= WorkerProto::Version{1, 37}) { WorkerProto::write(store, conn, res.cpuUser); WorkerProto::write(store, conn, res.cpuSystem); } - if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + if (conn.version >= WorkerProto::Version{1, 28}) { DrvOutputs builtOutputsFullKey; for (auto & [output, realisation] : builtOutputs) builtOutputsFullKey.insert_or_assign(realisation.id, realisation); @@ -310,7 +310,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 (GET_PROTOCOL_MINOR(conn.version) >= 16) { + if (conn.version >= WorkerProto::Version{1, 16}) { conn.from >> info.ultimate; info.sigs = WorkerProto::Serialise>::read(store, conn); info.ca = ContentAddress::parseOpt(readString(conn.from)); @@ -325,7 +325,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 (GET_PROTOCOL_MINOR(conn.version) >= 16) { + if (conn.version >= WorkerProto::Version{1, 16}) { conn.to << pathInfo.ultimate; WorkerProto::write(store, conn, pathInfo.sigs); conn.to << renderContentAddress(pathInfo.ca); @@ -337,11 +337,11 @@ WorkerProto::Serialise::read(const StoreDirCon { WorkerProto::ClientHandshakeInfo res; - if (GET_PROTOCOL_MINOR(conn.version) >= 33) { + if (conn.version >= WorkerProto::Version{1, 33}) { res.daemonNixVersion = readString(conn.from); } - if (GET_PROTOCOL_MINOR(conn.version) >= 35) { + if (conn.version >= WorkerProto::Version{1, 35}) { res.remoteTrustsUs = WorkerProto::Serialise>::read(store, conn); } else { // We don't know the answer; protocol to old. @@ -354,12 +354,12 @@ WorkerProto::Serialise::read(const StoreDirCon void WorkerProto::Serialise::write( const StoreDirConfig & store, WriteConn conn, const WorkerProto::ClientHandshakeInfo & info) { - if (GET_PROTOCOL_MINOR(conn.version) >= 33) { + if (conn.version >= WorkerProto::Version{1, 33}) { assert(info.daemonNixVersion); conn.to << *info.daemonNixVersion; } - if (GET_PROTOCOL_MINOR(conn.version) >= 35) { + if (conn.version >= WorkerProto::Version{1, 35}) { WorkerProto::write(store, conn, info.remoteTrustsUs); } } diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index 999d916f7..53c05890f 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -22,9 +22,8 @@ namespace { std::string formatProtocol(unsigned int proto) { if (proto) { - auto major = GET_PROTOCOL_MAJOR(proto) >> 8; - auto minor = GET_PROTOCOL_MINOR(proto); - return fmt("%1%.%2%", major, minor); + auto version = WorkerProto::Version::fromWire(proto); + return fmt("%1%.%2%", version.major, version.minor); } return "unknown"; } @@ -153,9 +152,9 @@ struct CmdConfigCheck : StoreCommand bool checkStoreProtocol(unsigned int storeProto) { - unsigned int clientProto = GET_PROTOCOL_MAJOR(SERVE_PROTOCOL_VERSION) == GET_PROTOCOL_MAJOR(storeProto) - ? SERVE_PROTOCOL_VERSION - : PROTOCOL_VERSION; + auto storeVersion = WorkerProto::Version::fromWire(storeProto); + unsigned int clientProto = (storeVersion.major == ServeProto::latest.major) ? ServeProto::latest.toWire() + : WorkerProto::latest.toWire(); if (clientProto != storeProto) { std::ostringstream ss; diff --git a/src/nix/nix-store/nix-store.cc b/src/nix/nix-store/nix-store.cc index e0c5c6b7a..e8480d615 100644 --- a/src/nix/nix-store/nix-store.cc +++ b/src/nix/nix-store/nix-store.cc @@ -881,7 +881,7 @@ static void opServe(Strings opFlags, Strings opArgs) FdSink out(getStandardOutput()); /* Exchange the greeting. */ - ServeProto::Version clientVersion = ServeProto::BasicServerConnection::handshake(out, in, SERVE_PROTOCOL_VERSION); + ServeProto::Version clientVersion = ServeProto::BasicServerConnection::handshake(out, in, ServeProto::latest); ServeProto::ReadConn rconn{ .from = in, @@ -908,9 +908,9 @@ static void opServe(Strings opFlags, Strings opArgs) // these conditions. settings.maxSilentTime = options.maxSilentTime; settings.buildTimeout = options.buildTimeout; - if (GET_PROTOCOL_MINOR(clientVersion) >= 2) + if (clientVersion >= ServeProto::Version{2, 2}) settings.maxLogSize = options.maxLogSize; - if (GET_PROTOCOL_MINOR(clientVersion) >= 3) { + if (clientVersion >= ServeProto::Version{2, 3}) { if (options.nrRepeats != 0) { throw Error("client requested repeating builds, but this is not currently implemented"); } @@ -923,7 +923,7 @@ static void opServe(Strings opFlags, Strings opArgs) // client asked for. settings.runDiffHook = true; } - if (GET_PROTOCOL_MINOR(clientVersion) >= 7) { + if (clientVersion >= ServeProto::Version{2, 7}) { settings.keepFailed = options.keepFailed; } };