refactor(tvix): Make Store::buildPaths return a Status

Make Store::buildPaths return a Status with [[nodiscard]] rather than
throwing exceptions to signal failure. This is the beginning of a long
road to refactor the entire store API to be status/statusor based
instead of using exceptions.

Change-Id: I2e32371c95a25b87ad129987c217d49c6d6e0c85
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1745
Tested-by: BuildkiteCI
Reviewed-by: kanepyork <rikingcoding@gmail.com>
This commit is contained in:
Griffin Smith 2020-08-13 22:06:23 -04:00 committed by glittershark
parent aef3607bd3
commit d1c38d9597
18 changed files with 137 additions and 32 deletions

View file

@ -22,6 +22,7 @@
#include "libstore/store-api.hh"
#include "libutil/archive.hh"
#include "libutil/json.hh"
#include "libutil/status.hh"
#include "libutil/util.hh"
namespace nix {
@ -90,7 +91,7 @@ void EvalState::realiseContext(const PathSet& context) {
unsigned long long narSize;
store->queryMissing(drvs, willBuild, willSubstitute, unknown, downloadSize,
narSize);
store->buildPaths(drvs);
nix::util::OkOrThrow(store->buildPaths(drvs));
}
/* Load and evaluate an expression from path specified by the

View file

@ -13,6 +13,7 @@
#include <string>
#include <thread>
#include <absl/status/status.h>
#include <absl/strings/ascii.h>
#include <absl/strings/numbers.h>
#include <absl/strings/str_cat.h>
@ -4686,7 +4687,8 @@ static void primeCache(Store& store, const PathSet& paths) {
}
}
void LocalStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) {
absl::Status LocalStore::buildPaths(const PathSet& drvPaths,
BuildMode buildMode) {
Worker worker(*this);
primeCache(*this, drvPaths);
@ -4717,8 +4719,12 @@ void LocalStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) {
}
if (!failed.empty()) {
throw Error(worker.exitStatus(), "build of %s failed", showPaths(failed));
return absl::Status(
absl::StatusCode::kInternal,
absl::StrFormat("build of %s failed (exit code %d)", showPaths(failed),
worker.exitStatus()));
}
return absl::OkStatus();
}
BuildResult LocalStore::buildDerivation(const Path& drvPath,

View file

@ -5,6 +5,7 @@
#include <string>
#include <unordered_set>
#include <absl/status/status.h>
#include <absl/strings/str_split.h>
#include "libstore/pathlocks.hh"
@ -155,7 +156,7 @@ class LocalStore : public LocalFSStore {
Path addTextToStore(const std::string& name, const std::string& s,
const PathSet& references, RepairFlag repair) override;
void buildPaths(const PathSet& paths, BuildMode buildMode) override;
absl::Status buildPaths(const PathSet& paths, BuildMode buildMode) override;
BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv,
BuildMode buildMode) override;

View file

@ -3,6 +3,7 @@
#include <cerrno>
#include <cstring>
#include <absl/status/status.h>
#include <absl/strings/ascii.h>
#include <fcntl.h>
#include <glog/logging.h>
@ -459,7 +460,8 @@ Path RemoteStore::addTextToStore(const std::string& name, const std::string& s,
return readStorePath(*this, conn->from);
}
void RemoteStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) {
absl::Status RemoteStore::buildPaths(const PathSet& drvPaths,
BuildMode buildMode) {
auto conn(getConnection());
conn->to << wopBuildPaths;
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13) {
@ -470,7 +472,8 @@ void RemoteStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) {
/* Old daemons did not take a 'buildMode' parameter, so we
need to validate it here on the client side. */
if (buildMode != bmNormal) {
throw Error(
return absl::Status(
absl::StatusCode::kInvalidArgument,
"repairing or checking is not supported when building through the "
"Nix daemon");
}
@ -485,6 +488,8 @@ void RemoteStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) {
}
conn.processStderr();
readInt(conn->from);
return absl::OkStatus();
}
BuildResult RemoteStore::buildDerivation(const Path& drvPath,

View file

@ -71,7 +71,7 @@ class RemoteStore : public virtual Store {
Path addTextToStore(const std::string& name, const std::string& s,
const PathSet& references, RepairFlag repair) override;
void buildPaths(const PathSet& paths, BuildMode buildMode) override;
absl::Status buildPaths(const PathSet& paths, BuildMode buildMode) override;
BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv,
BuildMode buildMode) override;

View file

@ -317,15 +317,17 @@ Path RpcStore::addTextToStore(const std::string& name,
return result.path();
}
void RpcStore::buildPaths(const PathSet& paths, BuildMode buildMode) {
absl::Status RpcStore::buildPaths(const PathSet& paths, BuildMode buildMode) {
ClientContext ctx;
proto::BuildPathsRequest request;
for (const auto& path : paths) {
request.add_drvs(path);
}
google::protobuf::Empty response;
request.set_mode(nix::BuildModeToProto(buildMode));
SuccessOrThrow(stub_->BuildPaths(&ctx, request, &response), __FUNCTION__);
return nix::util::proto::GRPCStatusToAbsl(
stub_->BuildPaths(&ctx, request, &response));
}
BuildResult RpcStore::buildDerivation(const Path& drvPath,

View file

@ -67,8 +67,8 @@ class RpcStore : public LocalFSStore, public virtual Store {
const PathSet& references,
RepairFlag repair = NoRepair) override;
virtual void buildPaths(const PathSet& paths,
BuildMode buildMode = bmNormal) override;
virtual absl::Status buildPaths(const PathSet& paths,
BuildMode buildMode = bmNormal) override;
virtual BuildResult buildDerivation(const Path& drvPath,
const BasicDerivation& drv,

View file

@ -3,6 +3,7 @@
#include <future>
#include <utility>
#include <absl/status/status.h>
#include <absl/strings/match.h>
#include <absl/strings/numbers.h>
#include <absl/strings/str_cat.h>
@ -700,16 +701,20 @@ const Store::Stats& Store::getStats() {
return stats;
}
void Store::buildPaths(const PathSet& paths, BuildMode buildMode) {
absl::Status Store::buildPaths(const PathSet& paths, BuildMode) {
for (auto& path : paths) {
if (isDerivation(path)) {
unsupported("buildPaths");
return absl::Status(absl::StatusCode::kUnimplemented,
"buildPaths is unsupported");
}
}
if (queryValidPaths(paths).size() != paths.size()) {
unsupported("buildPaths");
return absl::Status(absl::StatusCode::kUnimplemented,
"buildPaths is unsupported");
}
return absl::OkStatus();
}
void copyStorePath(ref<Store> srcStore, const ref<Store>& dstStore,

View file

@ -445,7 +445,8 @@ class Store : public std::enable_shared_from_this<Store>, public Config {
output paths can be created by running the builder, after
recursively building any sub-derivations. For inputs that are
not derivations, substitute them. */
virtual void buildPaths(const PathSet& paths, BuildMode buildMode = bmNormal);
[[nodiscard]] virtual absl::Status buildPaths(const PathSet& paths,
BuildMode buildMode = bmNormal);
/* Build a single non-materialized derivation (i.e. not from an
on-disk .drv file). Note that drvPath is only used for

View file

@ -21,6 +21,7 @@ set(HEADER_FILES
proto.hh
ref.hh
serialise.hh
status.hh
sync.hh
thread-pool.hh
types.hh

View file

@ -2,19 +2,20 @@
#include <absl/status/status.h>
#include <grpcpp/impl/codegen/status.h>
#include <grpcpp/impl/codegen/status_code_enum.h>
#include "libproto/worker.pb.h"
#include "libutil/types.hh"
namespace nix::util::proto {
::nix::proto::StorePath StorePath(const Path& path) {
inline ::nix::proto::StorePath StorePath(const Path& path) {
::nix::proto::StorePath store_path;
store_path.set_path(path);
return store_path;
}
::nix::proto::StorePaths StorePaths(const PathSet& paths) {
inline ::nix::proto::StorePaths StorePaths(const PathSet& paths) {
::nix::proto::StorePaths result;
for (const auto& path : paths) {
result.add_paths(path);
@ -70,6 +71,47 @@ constexpr absl::StatusCode GRPCStatusCodeToAbsl(grpc::StatusCode code) {
}
}
constexpr grpc::StatusCode AbslStatusCodeToGRPC(absl::StatusCode code) {
switch (code) {
case absl::StatusCode::kOk:
return grpc::StatusCode::OK;
case absl::StatusCode::kCancelled:
return grpc::StatusCode::CANCELLED;
case absl::StatusCode::kUnknown:
return grpc::StatusCode::UNKNOWN;
case absl::StatusCode::kInvalidArgument:
return grpc::StatusCode::INVALID_ARGUMENT;
case absl::StatusCode::kDeadlineExceeded:
return grpc::StatusCode::DEADLINE_EXCEEDED;
case absl::StatusCode::kNotFound:
return grpc::StatusCode::NOT_FOUND;
case absl::StatusCode::kAlreadyExists:
return grpc::StatusCode::ALREADY_EXISTS;
case absl::StatusCode::kPermissionDenied:
return grpc::StatusCode::PERMISSION_DENIED;
case absl::StatusCode::kUnauthenticated:
return grpc::StatusCode::UNAUTHENTICATED;
case absl::StatusCode::kResourceExhausted:
return grpc::StatusCode::RESOURCE_EXHAUSTED;
case absl::StatusCode::kFailedPrecondition:
return grpc::StatusCode::FAILED_PRECONDITION;
case absl::StatusCode::kAborted:
return grpc::StatusCode::ABORTED;
case absl::StatusCode::kOutOfRange:
return grpc::StatusCode::OUT_OF_RANGE;
case absl::StatusCode::kUnimplemented:
return grpc::StatusCode::UNIMPLEMENTED;
case absl::StatusCode::kInternal:
return grpc::StatusCode::INTERNAL;
case absl::StatusCode::kUnavailable:
return grpc::StatusCode::UNAVAILABLE;
case absl::StatusCode::kDataLoss:
return grpc::StatusCode::DATA_LOSS;
default:
return grpc::StatusCode::INTERNAL;
}
}
constexpr absl::string_view GRPCStatusCodeDescription(grpc::StatusCode code) {
switch (code) {
case grpc::StatusCode::OK:
@ -111,4 +153,22 @@ constexpr absl::string_view GRPCStatusCodeDescription(grpc::StatusCode code) {
};
}
inline absl::Status GRPCStatusToAbsl(grpc::Status status) {
if (status.ok()) {
return absl::OkStatus();
}
return absl::Status(GRPCStatusCodeToAbsl(status.error_code()),
status.error_message());
}
inline grpc::Status AbslToGRPCStatus(absl::Status status) {
if (status.ok()) {
return grpc::Status::OK;
}
return grpc::Status(AbslStatusCodeToGRPC(status.code()),
std::string(status.message()));
}
} // namespace nix::util::proto

17
third_party/nix/src/libutil/status.hh vendored Normal file
View file

@ -0,0 +1,17 @@
#pragma once
#include <absl/status/status.h>
#include <absl/strings/str_format.h>
#include <absl/strings/string_view.h>
#include "libutil/types.hh"
namespace nix::util {
inline void OkOrThrow(absl::Status status) {
if (!status.ok()) {
throw Error(absl::StrFormat("Operation failed: %s", status.ToString()));
}
}
} // namespace nix::util

View file

@ -19,6 +19,7 @@
#include "libstore/globals.hh"
#include "libstore/store-api.hh"
#include "libutil/affinity.hh"
#include "libutil/status.hh"
#include "libutil/util.hh"
#include "nix/legacy.hh"
@ -358,7 +359,7 @@ static void _main(int argc, char** argv) {
}
if (!dryRun) {
store->buildPaths(paths, buildMode);
util::OkOrThrow(store->buildPaths(paths, buildMode));
}
};

View file

@ -21,6 +21,7 @@
#include "libstore/store-api.hh"
#include "libutil/archive.hh"
#include "libutil/hash.hh"
#include "libutil/proto.hh"
#include "libutil/serialise.hh"
#include "libutil/types.hh"
@ -288,9 +289,8 @@ class WorkerServiceImpl final : public WorkerService::Service {
// TODO(grfn): If mode is repair and not trusted, we need to return an
// error here (but we can't yet because we don't know anything about
// trusted users)
store_->buildPaths(drvs, mode.value());
return Status::OK;
return nix::util::proto::AbslToGRPCStatus(
store_->buildPaths(drvs, mode.value()));
},
__FUNCTION__);
}

View file

@ -23,6 +23,7 @@
#include "libstore/profiles.hh"
#include "libstore/store-api.hh"
#include "libutil/json.hh"
#include "libutil/status.hh"
#include "libutil/util.hh"
#include "libutil/xml-writer.hh"
#include "nix-env/user-env.hh"
@ -720,8 +721,8 @@ static void opSet(Globals& globals, Strings opFlags, Strings opArgs) {
if (globals.dryRun) {
return;
}
globals.state->store->buildPaths(
paths, globals.state->repair != 0u ? bmRepair : bmNormal);
nix::util::OkOrThrow(globals.state->store->buildPaths(
paths, globals.state->repair != 0u ? bmRepair : bmNormal));
} else {
printMissing(globals.state->store, {drv.queryOutPath()});
if (globals.dryRun) {

View file

@ -9,6 +9,7 @@
#include "libstore/globals.hh"
#include "libstore/profiles.hh"
#include "libstore/store-api.hh"
#include "libutil/status.hh"
#include "libutil/util.hh"
namespace nix {
@ -37,8 +38,8 @@ bool createUserEnv(EvalState& state, DrvInfos& elems, const Path& profile,
}
DLOG(INFO) << "building user environment dependencies";
state.store->buildPaths(drvsToBuild,
state.repair != 0u ? bmRepair : bmNormal);
util::OkOrThrow(state.store->buildPaths(
drvsToBuild, state.repair != 0u ? bmRepair : bmNormal));
/* Construct the whole top level derivation. */
PathSet references;
@ -137,8 +138,8 @@ bool createUserEnv(EvalState& state, DrvInfos& elems, const Path& profile,
/* Realise the resulting store expression. */
DLOG(INFO) << "building user environment";
state.store->buildPaths({topLevelDrv},
state.repair != 0u ? bmRepair : bmNormal);
util::OkOrThrow(state.store->buildPaths(
{topLevelDrv}, state.repair != 0u ? bmRepair : bmNormal));
/* Switch the current user environment to the output path. */
auto store2 = state.store.dynamic_pointer_cast<LocalFSStore>();

View file

@ -16,6 +16,7 @@
#include "libstore/worker-protocol.hh"
#include "libutil/archive.hh"
#include "libutil/monitor-fd.hh"
#include "libutil/status.hh"
#include "libutil/util.hh"
#include "nix-store/dotgraph.hh"
#include "nix-store/graphml.hh"
@ -68,7 +69,7 @@ static PathSet realisePath(Path path, bool build = true) {
if (isDerivation(p.first)) {
if (build) {
store->buildPaths({path});
util::OkOrThrow(store->buildPaths({path}));
}
Derivation drv = store->derivationFromPath(p.first);
rootNr++;
@ -184,7 +185,8 @@ static void opRealise(Strings opFlags, Strings opArgs) {
}
/* Build all paths at the same time to exploit parallelism. */
store->buildPaths(PathSet(paths.begin(), paths.end()), buildMode);
util::OkOrThrow(
store->buildPaths(PathSet(paths.begin(), paths.end()), buildMode));
if (!ignoreUnknown) {
for (auto& i : paths) {
@ -1002,7 +1004,7 @@ static void opServe(Strings opFlags, Strings opArgs) {
does one path at a time. */
if (!willSubstitute.empty()) {
try {
store->buildPaths(willSubstitute);
util::OkOrThrow(store->buildPaths(willSubstitute));
} catch (Error& e) {
LOG(WARNING) << e.msg();
}
@ -1064,7 +1066,7 @@ static void opServe(Strings opFlags, Strings opArgs) {
try {
MonitorFdHup monitor(in.fd);
store->buildPaths(paths);
util::OkOrThrow(store->buildPaths(paths));
out << 0;
} catch (Error& e) {
assert(e.status);

View file

@ -9,6 +9,7 @@
#include "libmain/shared.hh"
#include "libstore/derivations.hh"
#include "libstore/store-api.hh"
#include "libutil/status.hh"
#include "nix/command.hh"
namespace nix {
@ -273,7 +274,7 @@ Buildables build(
if (mode == DryRun) {
printMissing(store, pathsToBuild);
} else if (mode == Build) {
store->buildPaths(pathsToBuild);
util::OkOrThrow(store->buildPaths(pathsToBuild));
}
return buildables;