From a79df261b498473ae7c6d4a04f32c50d5954124f Mon Sep 17 00:00:00 2001 From: Griffin Smith Date: Fri, 17 Jul 2020 09:30:04 -0400 Subject: [PATCH] feat(3p/nix/nix-daemon): Implement Worker::BuildDerivation handler Implement the proto handler on the server side for Worker::BuildDerivation. This includes several additions to the proto which I had missed on the first pass, including the actual proto definition for the Derivation itself and a few sequence number reorderings which are fine because this is all provisional and not deployed yet. A couple things to note - I implemented a couple constructors for nix classes that initialize themselves based on their proto variants, which felt nice and didn't end up causing any issues. - I've made the conversions between the enum types in nix and in proto explicit via switch statements rather than using a static_cast, out of an abundance of caution that the error would get mismatched in the future and we'd convert the wrong thing to the wrong thing - this is verbose, but exceptionally future proof. Change-Id: Iecf6b88e76bc37e49efa05fd65d6cd0cb0deffed Reviewed-on: https://cl.tvl.fyi/c/depot/+/1249 Tested-by: BuildkiteCI Reviewed-by: tazjin Reviewed-by: Kane York --- third_party/nix/src/libstore/derivations.cc | 22 ++++++++++ third_party/nix/src/libstore/derivations.hh | 19 ++++++++ third_party/nix/src/libstore/store-api.cc | 44 +++++++++++++++++++ third_party/nix/src/libstore/store-api.hh | 9 ++++ .../nix/src/nix-daemon/nix-daemon-proto.cc | 23 ++++++++++ third_party/nix/src/proto/worker.proto | 17 ++++++- 6 files changed, 133 insertions(+), 1 deletion(-) diff --git a/third_party/nix/src/libstore/derivations.cc b/third_party/nix/src/libstore/derivations.cc index 59b2a505a..419fc584f 100644 --- a/third_party/nix/src/libstore/derivations.cc +++ b/third_party/nix/src/libstore/derivations.cc @@ -30,6 +30,28 @@ void DerivationOutput::parseHashInfo(bool& recursive, Hash& hash) const { hash = Hash(this->hash, hashType); } +BasicDerivation BasicDerivation::from_proto( + const nix::proto::Derivation* proto_derivation, const nix::Store* store) { + BasicDerivation result; + result.platform = proto_derivation->platform(); + result.builder = proto_derivation->builder().path(); + store->assertStorePath(result.builder); + + result.outputs.insert(proto_derivation->outputs().begin(), + proto_derivation->outputs().end()); + + result.inputSrcs.insert(proto_derivation->input_sources().paths().begin(), + proto_derivation->input_sources().paths().end()); + + result.args.insert(result.args.end(), proto_derivation->args().begin(), + proto_derivation->args().end()); + + result.env.insert(proto_derivation->env().begin(), + proto_derivation->env().end()); + + return result; +} + Path BasicDerivation::findOutput(const std::string& id) const { auto i = outputs.find(id); if (i == outputs.end()) { diff --git a/third_party/nix/src/libstore/derivations.hh b/third_party/nix/src/libstore/derivations.hh index 7574aa90c..d8a5dbf09 100644 --- a/third_party/nix/src/libstore/derivations.hh +++ b/third_party/nix/src/libstore/derivations.hh @@ -2,6 +2,7 @@ #include +#include "libproto/worker.pb.h" #include "libstore/store-api.hh" #include "libutil/hash.hh" #include "libutil/types.hh" @@ -18,20 +19,31 @@ struct DerivationOutput { std::string hashAlgo; /* hash used for expected hash computation */ std::string hash; /* expected hash, may be null */ DerivationOutput() {} + // TODO(grfn): Make explicit DerivationOutput(Path path, std::string hashAlgo, std::string hash) { this->path = path; this->hashAlgo = hashAlgo; this->hash = hash; } + + explicit DerivationOutput( + const nix::proto::Derivation_DerivationOutput& proto_derivation_output) + : path(proto_derivation_output.path().path()), + hashAlgo(proto_derivation_output.hash_algo()), + hash(proto_derivation_output.hash()) {} + void parseHashInfo(bool& recursive, Hash& hash) const; }; +// TODO(grfn): change to absl::flat_hash_map typedef std::map DerivationOutputs; /* For inputs that are sub-derivations, we specify exactly which output IDs we are interested in. */ +// TODO(grfn): change to absl::flat_hash_map typedef std::map DerivationInputs; +// TODO(grfn): change to absl::flat_hash_map typedef std::map StringPairs; struct BasicDerivation { @@ -42,6 +54,13 @@ struct BasicDerivation { Strings args; StringPairs env; + BasicDerivation(){}; + + // Convert the given proto derivation to a BasicDerivation in the given + // nix::Store. + static BasicDerivation from_proto( + const nix::proto::Derivation* proto_derivation, const nix::Store* store); + virtual ~BasicDerivation(){}; /* Return the path corresponding to the output identifier `id' in diff --git a/third_party/nix/src/libstore/store-api.cc b/third_party/nix/src/libstore/store-api.cc index dd1b199d9..47f85e6e2 100644 --- a/third_party/nix/src/libstore/store-api.cc +++ b/third_party/nix/src/libstore/store-api.cc @@ -18,6 +18,50 @@ namespace nix { +std::optional build_mode_from(nix::proto::BuildMode mode) { + switch (mode) { + case nix::proto::BuildMode::Normal: + return BuildMode::bmNormal; + case nix::proto::BuildMode::Repair: + return BuildMode::bmRepair; + case nix::proto::BuildMode::Check: + return BuildMode::bmCheck; + default: + return {}; + } +} + +nix::proto::BuildStatus BuildResult::status_to_proto() { + switch (status) { + case BuildResult::Status::Built: + return proto::BuildStatus::Built; + case BuildResult::Status::Substituted: + return proto::BuildStatus::Substituted; + case BuildResult::Status::AlreadyValid: + return proto::BuildStatus::AlreadyValid; + case BuildResult::Status::PermanentFailure: + return proto::BuildStatus::PermanentFailure; + case BuildResult::Status::InputRejected: + return proto::BuildStatus::InputRejected; + case BuildResult::Status::OutputRejected: + return proto::BuildStatus::OutputRejected; + case BuildResult::Status::TransientFailure: + return proto::BuildStatus::TransientFailure; + case BuildResult::Status::CachedFailure: + return proto::BuildStatus::CachedFailure; + case BuildResult::Status::TimedOut: + return proto::BuildStatus::TimedOut; + case BuildResult::Status::MiscFailure: + return proto::BuildStatus::MiscFailure; + case BuildResult::Status::DependencyFailed: + return proto::BuildStatus::DependencyFailed; + case BuildResult::Status::LogLimitExceeded: + return proto::BuildStatus::LogLimitExceeded; + case BuildResult::Status::NotDeterministic: + return proto::BuildStatus::NotDeterministic; + } +} + bool Store::isInStore(const Path& path) const { return isInDir(path, storeDir); } diff --git a/third_party/nix/src/libstore/store-api.hh b/third_party/nix/src/libstore/store-api.hh index c6f8d75b6..f5076e458 100644 --- a/third_party/nix/src/libstore/store-api.hh +++ b/third_party/nix/src/libstore/store-api.hh @@ -8,6 +8,7 @@ #include #include +#include "libproto/worker.pb.h" #include "libstore/crypto.hh" #include "libstore/globals.hh" #include "libutil/config.hh" @@ -181,6 +182,10 @@ typedef std::list ValidPathInfos; enum BuildMode { bmNormal, bmRepair, bmCheck }; +// Convert the proto version of a `nix::proto::BuildMode` to its corresponding +// nix `BuildMode` +std::optional build_mode_from(nix::proto::BuildMode mode); + struct BuildResult { /* Note: don't remove status codes, and only add new status codes at the end of the list, to prevent client/server @@ -218,6 +223,10 @@ struct BuildResult { bool success() { return status == Built || status == Substituted || status == AlreadyValid; } + + // Convert the status of this `BuildResult` to its corresponding + // `nix::proto::BuildStatus` + nix::proto::BuildStatus status_to_proto(); }; class Store : public std::enable_shared_from_this, public Config { diff --git a/third_party/nix/src/nix-daemon/nix-daemon-proto.cc b/third_party/nix/src/nix-daemon/nix-daemon-proto.cc index 1ba440522..26ae538f2 100644 --- a/third_party/nix/src/nix-daemon/nix-daemon-proto.cc +++ b/third_party/nix/src/nix-daemon/nix-daemon-proto.cc @@ -6,11 +6,13 @@ #include "libproto/worker.grpc.pb.h" #include "libproto/worker.pb.h" +#include "libstore/derivations.hh" #include "libstore/store-api.hh" namespace nix::daemon { using ::grpc::Status; +using ::nix::proto::BuildStatus; using ::nix::proto::PathInfo; using ::nix::proto::StorePath; using ::nix::proto::StorePaths; @@ -204,6 +206,27 @@ class WorkerServiceImpl final : public WorkerService::Service { return Status::OK; } + Status BuildDerivation( + grpc::ServerContext* context, + const nix::proto::BuildDerivationRequest* request, + nix::proto::BuildDerivationResponse* response) override { + auto drv_path = request->drv_path().path(); + store_->assertStorePath(drv_path); + auto drv = BasicDerivation::from_proto(&request->derivation(), store_); + + auto build_mode = nix::build_mode_from(request->build_mode()); + if (!build_mode) { + return Status(grpc::StatusCode::INTERNAL, "Invalid build mode"); + } + + auto res = store_->buildDerivation(drv_path, drv, *build_mode); + + response->set_status(res.status_to_proto()); + response->set_error_message(res.errorMsg); + + return Status::OK; + } + Status QueryMissing(grpc::ServerContext* context, const StorePaths* request, nix::proto::QueryMissingResponse* response) override { std::set targets; diff --git a/third_party/nix/src/proto/worker.proto b/third_party/nix/src/proto/worker.proto index 8e629da70..3dd83d998 100644 --- a/third_party/nix/src/proto/worker.proto +++ b/third_party/nix/src/proto/worker.proto @@ -288,10 +288,25 @@ message VerifyStoreResponse { bool errors = 1; } +message Derivation { + message DerivationOutput { + StorePath path = 1; + string hash_algo = 2; + bytes hash = 3; + } + map outputs = 1; + StorePaths input_sources = 2; + string platform = 3; + StorePath builder = 4; + repeated string args = 5; + map env = 6; +} + message BuildDerivationRequest { // Only used for informational purposes. StorePath drv_path = 1; - BuildMode build_mode = 2; + Derivation derivation = 2; + BuildMode build_mode = 3; } message BuildDerivationResponse {