chore(tvix): Thread a std::ostream through Store::buildPaths

This part of the store API needs to carry a handle to the log sink
from now on, so that it can be passed in as appropriate from the gRPC
handlers.

In all places where there is no such handler available at the moment,
the discarding log sink has been inserted. This can be used as a
convenient grep target in the future.

Change-Id: I26628e30b4c6437dccdf8f722ca2e8ed827dfc19
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1797
Tested-by: BuildkiteCI
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Reviewed-by: glittershark <grfn@gws.fyi>
This commit is contained in:
Vincent Ambo 2020-08-20 03:28:35 +01:00 committed by tazjin
parent f7fa77f14d
commit 33e8b0f975
15 changed files with 63 additions and 43 deletions

View file

@ -91,7 +91,12 @@ void EvalState::realiseContext(const PathSet& context) {
unsigned long long narSize; unsigned long long narSize;
store->queryMissing(drvs, willBuild, willSubstitute, unknown, downloadSize, store->queryMissing(drvs, willBuild, willSubstitute, unknown, downloadSize,
narSize); narSize);
nix::util::OkOrThrow(store->buildPaths(drvs));
// TODO(tazjin): Figure out where these logs are supposed to go ...
// unless we keep a per-store stream open persistently there's no
// "generic" way to send logs anywhere for cases like this (IFD).
auto discard_logs = DiscardLogsSink();
nix::util::OkOrThrow(store->buildPaths(discard_logs, drvs));
} }
/* Load and evaluate an expression from path specified by the /* Load and evaluate an expression from path specified by the

View file

@ -7,6 +7,7 @@
#include <iostream> #include <iostream>
#include <map> #include <map>
#include <memory> #include <memory>
#include <ostream>
#include <queue> #include <queue>
#include <regex> #include <regex>
#include <sstream> #include <sstream>
@ -4687,8 +4688,9 @@ static void primeCache(Store& store, const PathSet& paths) {
} }
} }
absl::Status LocalStore::buildPaths(const PathSet& drvPaths, absl::Status LocalStore::buildPaths(std::ostream& /* log_sink */,
BuildMode buildMode) { const PathSet& drvPaths,
BuildMode build_mode) {
Worker worker(*this); Worker worker(*this);
primeCache(*this, drvPaths); primeCache(*this, drvPaths);
@ -4697,10 +4699,10 @@ absl::Status LocalStore::buildPaths(const PathSet& drvPaths,
for (auto& i : drvPaths) { for (auto& i : drvPaths) {
DrvPathWithOutputs i2 = parseDrvPathWithOutputs(i); DrvPathWithOutputs i2 = parseDrvPathWithOutputs(i);
if (isDerivation(i2.first)) { if (isDerivation(i2.first)) {
goals.insert(worker.makeDerivationGoal(i2.first, i2.second, buildMode)); goals.insert(worker.makeDerivationGoal(i2.first, i2.second, build_mode));
} else { } else {
goals.insert(worker.makeSubstitutionGoal( goals.insert(worker.makeSubstitutionGoal(
i, buildMode == bmRepair ? Repair : NoRepair)); i, build_mode == bmRepair ? Repair : NoRepair));
} }
} }

View file

@ -156,7 +156,8 @@ class LocalStore : public LocalFSStore {
Path addTextToStore(const std::string& name, const std::string& s, Path addTextToStore(const std::string& name, const std::string& s,
const PathSet& references, RepairFlag repair) override; const PathSet& references, RepairFlag repair) override;
absl::Status buildPaths(const PathSet& paths, BuildMode buildMode) override; absl::Status buildPaths(std::ostream& log_sink, const PathSet& paths,
BuildMode build_mode) override;
BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv, BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv,
BuildMode buildMode) override; BuildMode buildMode) override;

View file

@ -460,18 +460,19 @@ Path RemoteStore::addTextToStore(const std::string& name, const std::string& s,
return readStorePath(*this, conn->from); return readStorePath(*this, conn->from);
} }
absl::Status RemoteStore::buildPaths(const PathSet& drvPaths, absl::Status RemoteStore::buildPaths(std::ostream& /* log_sink */,
BuildMode buildMode) { const PathSet& drvPaths,
BuildMode build_mode) {
auto conn(getConnection()); auto conn(getConnection());
conn->to << wopBuildPaths; conn->to << wopBuildPaths;
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13) { if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13) {
conn->to << drvPaths; conn->to << drvPaths;
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15) { if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15) {
conn->to << buildMode; conn->to << build_mode;
} else } else
/* Old daemons did not take a 'buildMode' parameter, so we /* Old daemons did not take a 'buildMode' parameter, so we
need to validate it here on the client side. */ need to validate it here on the client side. */
if (buildMode != bmNormal) { if (build_mode != bmNormal) {
return absl::Status( return absl::Status(
absl::StatusCode::kInvalidArgument, absl::StatusCode::kInvalidArgument,
"repairing or checking is not supported when building through the " "repairing or checking is not supported when building through the "

View file

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

View file

@ -4,6 +4,7 @@
#include <filesystem> #include <filesystem>
#include <memory> #include <memory>
#include <optional> #include <optional>
#include <ostream>
#include <absl/status/status.h> #include <absl/status/status.h>
#include <absl/strings/str_cat.h> #include <absl/strings/str_cat.h>
@ -318,7 +319,8 @@ Path RpcStore::addTextToStore(const std::string& name,
return result.path(); return result.path();
} }
absl::Status RpcStore::buildPaths(const PathSet& paths, BuildMode buildMode) { absl::Status RpcStore::buildPaths(std::ostream& log_sink, const PathSet& paths,
BuildMode build_mode) {
ClientContext ctx; ClientContext ctx;
proto::BuildPathsRequest request; proto::BuildPathsRequest request;
for (const auto& path : paths) { for (const auto& path : paths) {
@ -326,14 +328,7 @@ absl::Status RpcStore::buildPaths(const PathSet& paths, BuildMode buildMode) {
} }
google::protobuf::Empty response; google::protobuf::Empty response;
request.set_mode(nix::BuildModeToProto(buildMode)); request.set_mode(nix::BuildModeToProto(build_mode));
// TODO(tazjin): Temporary no-op sink used to discard build output,
// but satisfy the compiler. A real one is needed.
//
// Maybe this should just be stderr, considering that this is the
// *client*, but I'm not sure.
std::ostream discard_logs = DiscardLogsSink();
std::unique_ptr<grpc::ClientReader<proto::BuildEvent>> reader = std::unique_ptr<grpc::ClientReader<proto::BuildEvent>> reader =
stub_->BuildPaths(&ctx, request); stub_->BuildPaths(&ctx, request);
@ -342,11 +337,11 @@ absl::Status RpcStore::buildPaths(const PathSet& paths, BuildMode buildMode) {
while (reader->Read(&event)) { while (reader->Read(&event)) {
if (event.has_build_log()) { if (event.has_build_log()) {
// TODO(tazjin): Include .path()? // TODO(tazjin): Include .path()?
discard_logs << event.build_log().line(); log_sink << event.build_log().line();
} else { } else {
discard_logs << std::endl log_sink << std::endl
<< "Building path: " << event.building_path().path() << "Building path: " << event.building_path().path()
<< std::endl; << std::endl;
} }
// has_result() is not in use in this call (for now) // has_result() is not in use in this call (for now)

View file

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

View file

@ -714,7 +714,8 @@ const Store::Stats& Store::getStats() {
return stats; return stats;
} }
absl::Status Store::buildPaths(const PathSet& paths, BuildMode) { absl::Status Store::buildPaths(std::ostream& /* log_sink */,
const PathSet& paths, BuildMode) {
for (auto& path : paths) { for (auto& path : paths) {
if (isDerivation(path)) { if (isDerivation(path)) {
return absl::Status(absl::StatusCode::kUnimplemented, return absl::Status(absl::StatusCode::kUnimplemented,

View file

@ -455,11 +455,13 @@ class Store : public std::enable_shared_from_this<Store>, public Config {
output paths can be created by running the builder, after output paths can be created by running the builder, after
recursively building any sub-derivations. For inputs that are recursively building any sub-derivations. For inputs that are
not derivations, substitute them. */ not derivations, substitute them. */
[[nodiscard]] virtual absl::Status buildPaths(const PathSet& paths, [[nodiscard]] virtual absl::Status buildPaths(std::ostream& log_sink,
BuildMode buildMode); const PathSet& paths,
BuildMode build_mode);
[[nodiscard]] absl::Status buildPaths(const PathSet& paths) { [[nodiscard]] absl::Status buildPaths(std::ostream& log_sink,
return buildPaths(paths, bmNormal); const PathSet& paths) {
return buildPaths(log_sink, paths, bmNormal);
} }
/* Build a single non-materialized derivation (i.e. not from an /* Build a single non-materialized derivation (i.e. not from an

View file

@ -359,7 +359,8 @@ static void _main(int argc, char** argv) {
} }
if (!dryRun) { if (!dryRun) {
util::OkOrThrow(store->buildPaths(paths, buildMode)); auto discard_logs = DiscardLogsSink();
util::OkOrThrow(store->buildPaths(discard_logs, paths, buildMode));
} }
}; };

View file

@ -290,7 +290,7 @@ class WorkerServiceImpl final : public WorkerService::Service {
Status BuildPaths( Status BuildPaths(
grpc::ServerContext*, const nix::proto::BuildPathsRequest* request, grpc::ServerContext*, const nix::proto::BuildPathsRequest* request,
grpc::ServerWriter<nix::proto::BuildEvent>* /* writer */) override { grpc::ServerWriter<nix::proto::BuildEvent>* writer) override {
return HandleExceptions( return HandleExceptions(
[&]() -> Status { [&]() -> Status {
PathSet drvs; PathSet drvs;
@ -303,11 +303,14 @@ class WorkerServiceImpl final : public WorkerService::Service {
return Status(grpc::StatusCode::INTERNAL, "Invalid build mode"); return Status(grpc::StatusCode::INTERNAL, "Invalid build mode");
} }
BuildLogStreambuf log_buffer(writer);
std::ostream log_sink(&log_buffer);
// TODO(grfn): If mode is repair and not trusted, we need to return an // 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 // error here (but we can't yet because we don't know anything about
// trusted users) // trusted users)
return nix::util::proto::AbslToGRPCStatus( return nix::util::proto::AbslToGRPCStatus(
store_->buildPaths(drvs, mode.value())); store_->buildPaths(log_sink, drvs, mode.value()));
}, },
__FUNCTION__); __FUNCTION__);
} }

View file

@ -722,8 +722,10 @@ static void opSet(Globals& globals, Strings opFlags, Strings opArgs) {
if (globals.dryRun) { if (globals.dryRun) {
return; return;
} }
auto discard_logs = DiscardLogsSink();
nix::util::OkOrThrow(globals.state->store->buildPaths( nix::util::OkOrThrow(globals.state->store->buildPaths(
paths, globals.state->repair != 0u ? bmRepair : bmNormal)); discard_logs, paths,
globals.state->repair != 0u ? bmRepair : bmNormal));
} else { } else {
printMissing(globals.state->store, {drv.queryOutPath()}); printMissing(globals.state->store, {drv.queryOutPath()});
if (globals.dryRun) { if (globals.dryRun) {

View file

@ -38,8 +38,9 @@ bool createUserEnv(EvalState& state, DrvInfos& elems, const Path& profile,
} }
DLOG(INFO) << "building user environment dependencies"; DLOG(INFO) << "building user environment dependencies";
auto discard_logs = DiscardLogsSink();
util::OkOrThrow(state.store->buildPaths( util::OkOrThrow(state.store->buildPaths(
drvsToBuild, state.repair != 0u ? bmRepair : bmNormal)); discard_logs, drvsToBuild, state.repair != 0u ? bmRepair : bmNormal));
/* Construct the whole top level derivation. */ /* Construct the whole top level derivation. */
PathSet references; PathSet references;
@ -139,7 +140,7 @@ bool createUserEnv(EvalState& state, DrvInfos& elems, const Path& profile,
/* Realise the resulting store expression. */ /* Realise the resulting store expression. */
DLOG(INFO) << "building user environment"; DLOG(INFO) << "building user environment";
util::OkOrThrow(state.store->buildPaths( util::OkOrThrow(state.store->buildPaths(
{topLevelDrv}, state.repair != 0u ? bmRepair : bmNormal)); discard_logs, {topLevelDrv}, state.repair != 0u ? bmRepair : bmNormal));
/* Switch the current user environment to the output path. */ /* Switch the current user environment to the output path. */
auto store2 = state.store.dynamic_pointer_cast<LocalFSStore>(); auto store2 = state.store.dynamic_pointer_cast<LocalFSStore>();

View file

@ -69,7 +69,8 @@ static PathSet realisePath(Path path, bool build = true) {
if (isDerivation(p.first)) { if (isDerivation(p.first)) {
if (build) { if (build) {
util::OkOrThrow(store->buildPaths({path})); auto discard_logs = DiscardLogsSink();
util::OkOrThrow(store->buildPaths(discard_logs, {path}));
} }
Derivation drv = store->derivationFromPath(p.first); Derivation drv = store->derivationFromPath(p.first);
rootNr++; rootNr++;
@ -185,8 +186,9 @@ static void opRealise(Strings opFlags, Strings opArgs) {
} }
/* Build all paths at the same time to exploit parallelism. */ /* Build all paths at the same time to exploit parallelism. */
util::OkOrThrow( auto discard_logs = DiscardLogsSink();
store->buildPaths(PathSet(paths.begin(), paths.end()), buildMode)); util::OkOrThrow(store->buildPaths(
discard_logs, PathSet(paths.begin(), paths.end()), buildMode));
if (!ignoreUnknown) { if (!ignoreUnknown) {
for (auto& i : paths) { for (auto& i : paths) {
@ -1004,7 +1006,8 @@ static void opServe(Strings opFlags, Strings opArgs) {
does one path at a time. */ does one path at a time. */
if (!willSubstitute.empty()) { if (!willSubstitute.empty()) {
try { try {
util::OkOrThrow(store->buildPaths(willSubstitute)); auto discard_logs = DiscardLogsSink();
util::OkOrThrow(store->buildPaths(discard_logs, willSubstitute));
} catch (Error& e) { } catch (Error& e) {
LOG(WARNING) << e.msg(); LOG(WARNING) << e.msg();
} }
@ -1066,7 +1069,8 @@ static void opServe(Strings opFlags, Strings opArgs) {
try { try {
MonitorFdHup monitor(in.fd); MonitorFdHup monitor(in.fd);
util::OkOrThrow(store->buildPaths(paths)); auto discard_logs = DiscardLogsSink();
util::OkOrThrow(store->buildPaths(discard_logs, paths));
out << 0; out << 0;
} catch (Error& e) { } catch (Error& e) {
assert(e.status); assert(e.status);

View file

@ -274,7 +274,8 @@ Buildables build(
if (mode == DryRun) { if (mode == DryRun) {
printMissing(store, pathsToBuild); printMissing(store, pathsToBuild);
} else if (mode == Build) { } else if (mode == Build) {
util::OkOrThrow(store->buildPaths(pathsToBuild)); auto discard_logs = DiscardLogsSink();
util::OkOrThrow(store->buildPaths(discard_logs, pathsToBuild));
} }
return buildables; return buildables;