diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 27c0b6431..19afe4388 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -635,16 +635,7 @@ static void performOp(TunnelLogger * logger, ref store, auto res = store->buildDerivation(drvPath, drv, buildMode); logger->stopWork(); - to << res.status << res.errorMsg; - if (GET_PROTOCOL_MINOR(clientVersion) >= 29) { - to << res.timesBuilt << res.isNonDeterministic << res.startTime << res.stopTime; - } - if (GET_PROTOCOL_MINOR(clientVersion) >= 28) { - DrvOutputs builtOutputs; - for (auto & [output, realisation] : res.builtOutputs) - builtOutputs.insert_or_assign(realisation.id, realisation); - WorkerProto::write(*store, wconn, builtOutputs); - } + WorkerProto::write(*store, wconn, res); break; } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index d6e24521a..c712f7eb1 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -319,20 +319,7 @@ public: conn->to.flush(); - BuildResult status; - status.status = (BuildResult::Status) readInt(conn->from); - conn->from >> status.errorMsg; - - if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3) - conn->from >> status.timesBuilt >> status.isNonDeterministic >> status.startTime >> status.stopTime; - if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 6) { - auto builtOutputs = ServeProto::Serialise::read(*this, *conn); - for (auto && [output, realisation] : builtOutputs) - status.builtOutputs.insert_or_assign( - std::move(output.outputName), - std::move(realisation)); - } - return status; + return ServeProto::Serialise::read(*this, *conn); } void buildPaths(const std::vector & drvPaths, BuildMode buildMode, std::shared_ptr evalStore) override diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 482c2ae01..ef648e01b 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -788,20 +788,7 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD writeDerivation(conn->to, *this, drv); conn->to << buildMode; conn.processStderr(); - BuildResult res; - res.status = (BuildResult::Status) readInt(conn->from); - conn->from >> res.errorMsg; - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) { - conn->from >> res.timesBuilt >> res.isNonDeterministic >> res.startTime >> res.stopTime; - } - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 28) { - auto builtOutputs = WorkerProto::Serialise::read(*this, *conn); - for (auto && [output, realisation] : builtOutputs) - res.builtOutputs.insert_or_assign( - std::move(output.outputName), - std::move(realisation)); - } - return res; + return WorkerProto::Serialise::read(*this, *conn); } diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc index 16a62b5bc..97a0ddf0e 100644 --- a/src/libstore/serve-protocol.cc +++ b/src/libstore/serve-protocol.cc @@ -2,6 +2,7 @@ #include "util.hh" #include "path-with-outputs.hh" #include "store-api.hh" +#include "build-result.hh" #include "serve-protocol.hh" #include "serve-protocol-impl.hh" #include "archive.hh" @@ -12,4 +13,46 @@ namespace nix { /* protocol-specific definitions */ +BuildResult ServeProto::Serialise::read(const Store & store, ServeProto::ReadConn conn) +{ + BuildResult status; + status.status = (BuildResult::Status) readInt(conn.from); + conn.from >> status.errorMsg; + + if (GET_PROTOCOL_MINOR(conn.version) >= 3) + conn.from + >> status.timesBuilt + >> status.isNonDeterministic + >> status.startTime + >> status.stopTime; + if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + auto builtOutputs = ServeProto::Serialise::read(store, conn); + for (auto && [output, realisation] : builtOutputs) + status.builtOutputs.insert_or_assign( + std::move(output.outputName), + std::move(realisation)); + } + return status; +} + +void ServeProto::Serialise::write(const Store & store, ServeProto::WriteConn conn, const BuildResult & status) +{ + conn.to + << status.status + << status.errorMsg; + + if (GET_PROTOCOL_MINOR(conn.version) >= 3) + conn.to + << status.timesBuilt + << status.isNonDeterministic + << status.startTime + << status.stopTime; + if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + DrvOutputs builtOutputs; + for (auto & [output, realisation] : status.builtOutputs) + builtOutputs.insert_or_assign(realisation.id, realisation); + ServeProto::write(store, conn, builtOutputs); + } +} + } diff --git a/src/libstore/serve-protocol.hh b/src/libstore/serve-protocol.hh index a627c6ad6..ba159f6e9 100644 --- a/src/libstore/serve-protocol.hh +++ b/src/libstore/serve-protocol.hh @@ -16,6 +16,9 @@ namespace nix { class Store; struct Source; +// items being serialised +struct BuildResult; + /** * The "serve protocol", used by ssh:// stores. @@ -136,6 +139,9 @@ inline std::ostream & operator << (std::ostream & s, ServeProto::Command op) static void write(const Store & store, ServeProto::WriteConn conn, const T & t); \ }; +template<> +DECLARE_SERVE_SERIALISER(BuildResult); + template DECLARE_SERVE_SERIALISER(std::vector); template diff --git a/src/libstore/tests/serve-protocol.cc b/src/libstore/tests/serve-protocol.cc index ba798ab1c..c8ac87a04 100644 --- a/src/libstore/tests/serve-protocol.cc +++ b/src/libstore/tests/serve-protocol.cc @@ -19,7 +19,7 @@ struct ServeProtoTest : VersionedProtoTest * For serializers that don't care about the minimum version, we * used the oldest one: 1.0. */ - ServeProto::Version defaultVersion = 1 << 8 | 0; + ServeProto::Version defaultVersion = 2 << 8 | 0; }; VERSIONED_CHARACTERIZATION_TEST( @@ -114,6 +114,117 @@ VERSIONED_CHARACTERIZATION_TEST( }, })) +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_2, + "build-result-2.2", + 2 << 8 | 2, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::Built, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_3, + "build-result-2.3", + 2 << 8 | 3, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + .timesBuilt = 3, + .isNonDeterministic = true, + .startTime = 30, + .stopTime = 50, + }, + BuildResult { + .status = BuildResult::Built, + .startTime = 30, + .stopTime = 50, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + buildResult_2_6, + "build-result-2.6", + 2 << 8 | 6, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + .timesBuilt = 3, + .isNonDeterministic = true, + .startTime = 30, + .stopTime = 50, + }, + BuildResult { + .status = BuildResult::Built, + .timesBuilt = 1, + .builtOutputs = { + { + "foo", + { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "foo", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + }, + { + "bar", + { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "bar", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, + }, + }, + }, + .startTime = 30, + .stopTime = 50, +#if 0 + // These fields are not yet serialized. + // FIXME Include in next version of protocol or document + // why they are skipped. + .cpuUser = std::chrono::milliseconds(500s), + .cpuSystem = std::chrono::milliseconds(604s), +#endif + }, + }; + t; + })) + VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, vector, diff --git a/src/libstore/tests/worker-protocol.cc b/src/libstore/tests/worker-protocol.cc index b3e857bc4..e0ce340d8 100644 --- a/src/libstore/tests/worker-protocol.cc +++ b/src/libstore/tests/worker-protocol.cc @@ -167,9 +167,77 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - buildResult, - "build-result", - defaultVersion, + buildResult_1_27, + "build-result-1.27", + 1 << 8 | 27, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::Built, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + buildResult_1_28, + "build-result-1.28", + 1 << 8 | 28, + ({ + using namespace std::literals::chrono_literals; + std::tuple t { + BuildResult { + .status = BuildResult::OutputRejected, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::NotDeterministic, + .errorMsg = "no idea why", + }, + BuildResult { + .status = BuildResult::Built, + .builtOutputs = { + { + "foo", + { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "foo", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo" }, + }, + }, + { + "bar", + { + .id = DrvOutput { + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "bar", + }, + .outPath = StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar" }, + }, + }, + }, + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, + buildResult_1_29, + "build-result-1.29", + 1 << 8 | 29, ({ using namespace std::literals::chrono_literals; std::tuple t { @@ -226,9 +294,9 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - keyedBuildResult, - "keyed-build-result", - defaultVersion, + keyedBuildResult_1_29, + "keyed-build-result-1.29", + 1 << 8 | 29, ({ using namespace std::literals::chrono_literals; std::tuple t { diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index 0f3f20ca5..f69322a61 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -103,17 +103,21 @@ BuildResult WorkerProto::Serialise::read(const Store & store, Worke { BuildResult res; res.status = (BuildResult::Status) readInt(conn.from); - conn.from - >> res.errorMsg - >> res.timesBuilt - >> res.isNonDeterministic - >> res.startTime - >> res.stopTime; - auto builtOutputs = WorkerProto::Serialise::read(store, conn); - for (auto && [output, realisation] : builtOutputs) - res.builtOutputs.insert_or_assign( - std::move(output.outputName), - std::move(realisation)); + conn.from >> res.errorMsg; + if (GET_PROTOCOL_MINOR(conn.version) >= 29) { + conn.from + >> res.timesBuilt + >> res.isNonDeterministic + >> res.startTime + >> res.stopTime; + } + if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + auto builtOutputs = WorkerProto::Serialise::read(store, conn); + for (auto && [output, realisation] : builtOutputs) + res.builtOutputs.insert_or_assign( + std::move(output.outputName), + std::move(realisation)); + } return res; } @@ -121,15 +125,20 @@ void WorkerProto::Serialise::write(const Store & store, WorkerProto { conn.to << res.status - << res.errorMsg - << res.timesBuilt - << res.isNonDeterministic - << res.startTime - << res.stopTime; - DrvOutputs builtOutputs; - for (auto & [output, realisation] : res.builtOutputs) - builtOutputs.insert_or_assign(realisation.id, realisation); - WorkerProto::write(store, conn, builtOutputs); + << res.errorMsg; + if (GET_PROTOCOL_MINOR(conn.version) >= 29) { + conn.to + << res.timesBuilt + << res.isNonDeterministic + << res.startTime + << res.stopTime; + } + if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + DrvOutputs builtOutputs; + for (auto & [output, realisation] : res.builtOutputs) + builtOutputs.insert_or_assign(realisation.id, realisation); + WorkerProto::write(store, conn, builtOutputs); + } } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index e78720f92..e4dd94585 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -959,17 +959,7 @@ static void opServe(Strings opFlags, Strings opArgs) MonitorFdHup monitor(in.fd); auto status = store->buildDerivation(drvPath, drv); - out << status.status << status.errorMsg; - - if (GET_PROTOCOL_MINOR(clientVersion) >= 3) - out << status.timesBuilt << status.isNonDeterministic << status.startTime << status.stopTime; - if (GET_PROTOCOL_MINOR(clientVersion) >= 6) { - DrvOutputs builtOutputs; - for (auto & [output, realisation] : status.builtOutputs) - builtOutputs.insert_or_assign(realisation.id, realisation); - ServeProto::write(*store, wconn, builtOutputs); - } - + ServeProto::write(*store, wconn, status); break; } diff --git a/unit-test-data/libstore/serve-protocol/build-result-2.2.bin b/unit-test-data/libstore/serve-protocol/build-result-2.2.bin new file mode 100644 index 000000000..ae684778b Binary files /dev/null and b/unit-test-data/libstore/serve-protocol/build-result-2.2.bin differ diff --git a/unit-test-data/libstore/serve-protocol/build-result-2.3.bin b/unit-test-data/libstore/serve-protocol/build-result-2.3.bin new file mode 100644 index 000000000..d51e08dfc Binary files /dev/null and b/unit-test-data/libstore/serve-protocol/build-result-2.3.bin differ diff --git a/unit-test-data/libstore/worker-protocol/build-result.bin b/unit-test-data/libstore/serve-protocol/build-result-2.6.bin similarity index 100% rename from unit-test-data/libstore/worker-protocol/build-result.bin rename to unit-test-data/libstore/serve-protocol/build-result-2.6.bin diff --git a/unit-test-data/libstore/worker-protocol/build-result-1.27.bin b/unit-test-data/libstore/worker-protocol/build-result-1.27.bin new file mode 100644 index 000000000..ae684778b Binary files /dev/null and b/unit-test-data/libstore/worker-protocol/build-result-1.27.bin differ diff --git a/unit-test-data/libstore/worker-protocol/build-result-1.28.bin b/unit-test-data/libstore/worker-protocol/build-result-1.28.bin new file mode 100644 index 000000000..74bcd5cf9 Binary files /dev/null and b/unit-test-data/libstore/worker-protocol/build-result-1.28.bin differ diff --git a/unit-test-data/libstore/worker-protocol/build-result-1.29.bin b/unit-test-data/libstore/worker-protocol/build-result-1.29.bin new file mode 100644 index 000000000..b02c706ea Binary files /dev/null and b/unit-test-data/libstore/worker-protocol/build-result-1.29.bin differ diff --git a/unit-test-data/libstore/worker-protocol/keyed-build-result.bin b/unit-test-data/libstore/worker-protocol/keyed-build-result-1.29.bin similarity index 100% rename from unit-test-data/libstore/worker-protocol/keyed-build-result.bin rename to unit-test-data/libstore/worker-protocol/keyed-build-result-1.29.bin