From 49024be05644d4fac252e2191e9de74e0ffd4fe3 Mon Sep 17 00:00:00 2001 From: Griffin Smith Date: Fri, 27 Nov 2020 18:12:44 -0500 Subject: feat(tvix): Thread a log sink through calls to buildDerivation Similarly to how we did for buildPaths, add a std::ostream& log_sink parameter to the build_derivation method on Store, and pass it std::cerr when called at the top level by nix commands - most notably, the build-remote hook binary, so that we get build logs when using tvix as a remote builder. Change-Id: I0f8f729ba8429d4838a0a135a5c2ac1e1a95d575 Reviewed-on: https://cl.tvl.fyi/c/depot/+/2176 Tested-by: BuildkiteCI Reviewed-by: andi Reviewed-by: kanepyork --- third_party/nix/src/build-remote/build-remote.cc | 2 +- third_party/nix/src/libstore/binary-cache-store.hh | 3 ++- third_party/nix/src/libstore/build.cc | 6 +++--- third_party/nix/src/libstore/legacy-ssh-store.cc | 3 ++- third_party/nix/src/libstore/local-store.hh | 3 ++- third_party/nix/src/libstore/remote-store.cc | 3 ++- third_party/nix/src/libstore/remote-store.hh | 3 ++- third_party/nix/src/libstore/rpc-store.cc | 5 +++-- third_party/nix/src/libstore/rpc-store.hh | 3 ++- third_party/nix/src/libstore/store-api.hh | 8 +++++--- third_party/nix/src/nix-daemon/nix-daemon-proto.cc | 5 ++++- third_party/nix/src/nix-store/nix-store.cc | 2 +- third_party/nix/src/tests/dummy-store.hh | 3 ++- 13 files changed, 31 insertions(+), 18 deletions(-) diff --git a/third_party/nix/src/build-remote/build-remote.cc b/third_party/nix/src/build-remote/build-remote.cc index 00bd833ac38e..43564a5eb74a 100644 --- a/third_party/nix/src/build-remote/build-remote.cc +++ b/third_party/nix/src/build-remote/build-remote.cc @@ -246,7 +246,7 @@ static int _main(int argc, char* argv[]) { readDerivation(store->realStoreDir + "/" + baseNameOf(drvPath))); drv.inputSrcs = inputs; - auto result = sshStore->buildDerivation(drvPath, drv); + auto result = sshStore->buildDerivation(std::cerr, drvPath, drv); if (!result.success()) { throw Error("build of '%s' on '%s' failed: %s", drvPath, storeUri, diff --git a/third_party/nix/src/libstore/binary-cache-store.hh b/third_party/nix/src/libstore/binary-cache-store.hh index 0b788152f0bc..40c636f60ac2 100644 --- a/third_party/nix/src/libstore/binary-cache-store.hh +++ b/third_party/nix/src/libstore/binary-cache-store.hh @@ -93,7 +93,8 @@ class BinaryCacheStore : public Store { void narFromPath(const Path& path, Sink& sink) override; - BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv, + BuildResult buildDerivation(std::ostream& /*log_sink*/, const Path& drvPath, + const BasicDerivation& drv, BuildMode buildMode) override { unsupported("buildDerivation"); } diff --git a/third_party/nix/src/libstore/build.cc b/third_party/nix/src/libstore/build.cc index 15ed3b05146c..1f5752a168c8 100644 --- a/third_party/nix/src/libstore/build.cc +++ b/third_party/nix/src/libstore/build.cc @@ -4754,11 +4754,11 @@ absl::Status LocalStore::buildPaths(std::ostream& log_sink, return absl::OkStatus(); } -BuildResult LocalStore::buildDerivation(const Path& drvPath, +BuildResult LocalStore::buildDerivation(std::ostream& log_sink, + const Path& drvPath, const BasicDerivation& drv, BuildMode buildMode) { - auto discard_logs = DiscardLogsSink(); - Worker worker(*this, discard_logs); + Worker worker(*this, log_sink); auto goal = worker.makeBasicDerivationGoal(drvPath, drv, buildMode); BuildResult result; diff --git a/third_party/nix/src/libstore/legacy-ssh-store.cc b/third_party/nix/src/libstore/legacy-ssh-store.cc index 19603115a81c..8163258179a1 100644 --- a/third_party/nix/src/libstore/legacy-ssh-store.cc +++ b/third_party/nix/src/libstore/legacy-ssh-store.cc @@ -199,7 +199,8 @@ struct LegacySSHStore : public Store { unsupported("addTextToStore"); } - BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv, + BuildResult buildDerivation(std::ostream& /*log_sink*/, const Path& drvPath, + const BasicDerivation& drv, BuildMode buildMode) override { auto conn(connections->get()); diff --git a/third_party/nix/src/libstore/local-store.hh b/third_party/nix/src/libstore/local-store.hh index cfcfb35cc3cc..a7c49079d22f 100644 --- a/third_party/nix/src/libstore/local-store.hh +++ b/third_party/nix/src/libstore/local-store.hh @@ -160,7 +160,8 @@ class LocalStore : public LocalFSStore { absl::Status buildPaths(std::ostream& log_sink, const PathSet& paths, BuildMode build_mode) override; - BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv, + BuildResult buildDerivation(std::ostream& log_sink, const Path& drvPath, + const BasicDerivation& drv, BuildMode buildMode) override; void ensurePath(const Path& path) override; diff --git a/third_party/nix/src/libstore/remote-store.cc b/third_party/nix/src/libstore/remote-store.cc index c4fd856a2cb0..cb6cc808c610 100644 --- a/third_party/nix/src/libstore/remote-store.cc +++ b/third_party/nix/src/libstore/remote-store.cc @@ -492,7 +492,8 @@ absl::Status RemoteStore::buildPaths(std::ostream& /* log_sink */, return absl::OkStatus(); } -BuildResult RemoteStore::buildDerivation(const Path& drvPath, +BuildResult RemoteStore::buildDerivation(std::ostream& /*log_sink*/, + const Path& drvPath, const BasicDerivation& drv, BuildMode buildMode) { auto conn(getConnection()); diff --git a/third_party/nix/src/libstore/remote-store.hh b/third_party/nix/src/libstore/remote-store.hh index 60da6045a81a..c360055b6e6b 100644 --- a/third_party/nix/src/libstore/remote-store.hh +++ b/third_party/nix/src/libstore/remote-store.hh @@ -74,7 +74,8 @@ class RemoteStore : public virtual Store { absl::Status buildPaths(std::ostream& log_sink, const PathSet& paths, BuildMode build_mode) override; - BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv, + BuildResult buildDerivation(std::ostream& log_sink, const Path& drvPath, + const BasicDerivation& drv, BuildMode buildMode) override; void ensurePath(const Path& path) override; diff --git a/third_party/nix/src/libstore/rpc-store.cc b/third_party/nix/src/libstore/rpc-store.cc index afaeca4a660e..c29bd059de9b 100644 --- a/third_party/nix/src/libstore/rpc-store.cc +++ b/third_party/nix/src/libstore/rpc-store.cc @@ -374,7 +374,8 @@ absl::Status RpcStore::buildPaths(std::ostream& log_sink, const PathSet& paths, return nix::util::proto::GRPCStatusToAbsl(reader->Finish()); } -BuildResult RpcStore::buildDerivation(const Path& drvPath, +BuildResult RpcStore::buildDerivation(std::ostream& log_sink, + const Path& drvPath, const BasicDerivation& drv, BuildMode buildMode) { ClientContext ctx; @@ -392,7 +393,7 @@ BuildResult RpcStore::buildDerivation(const Path& drvPath, proto::BuildEvent event; while (reader->Read(&event)) { if (event.has_build_log()) { - LOG(INFO) << event.build_log().line(); + log_sink << event.build_log().line(); } else if (event.has_result()) { result = BuildResult::FromProto(event.result()); } diff --git a/third_party/nix/src/libstore/rpc-store.hh b/third_party/nix/src/libstore/rpc-store.hh index 3a156e9e1afe..679ceac7af9c 100644 --- a/third_party/nix/src/libstore/rpc-store.hh +++ b/third_party/nix/src/libstore/rpc-store.hh @@ -70,7 +70,8 @@ class RpcStore : public LocalFSStore, public virtual Store { absl::Status buildPaths(std::ostream& log_sink, const PathSet& paths, BuildMode build_mode) override; - virtual BuildResult buildDerivation(const Path& drvPath, + virtual BuildResult buildDerivation(std::ostream& log_sink, + const Path& drvPath, const BasicDerivation& drv, BuildMode buildMode) override; diff --git a/third_party/nix/src/libstore/store-api.hh b/third_party/nix/src/libstore/store-api.hh index 76c757bd6939..eb18511e60df 100644 --- a/third_party/nix/src/libstore/store-api.hh +++ b/third_party/nix/src/libstore/store-api.hh @@ -468,12 +468,14 @@ class Store : public std::enable_shared_from_this, public Config { on-disk .drv file). Note that ‘drvPath’ is only used for informational purposes. */ // TODO(tazjin): Thread std::ostream through here, too. - virtual BuildResult buildDerivation(const Path& drvPath, + virtual BuildResult buildDerivation(std::ostream& log_sink, + const Path& drvPath, const BasicDerivation& drv, BuildMode buildMode) = 0; - BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv) { - return buildDerivation(drvPath, drv, bmNormal); + BuildResult buildDerivation(std::ostream& log_sink, const Path& drvPath, + const BasicDerivation& drv) { + return buildDerivation(log_sink, drvPath, drv, bmNormal); } /* Ensure that a path is valid. If it is not currently valid, it 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 597ef2434a07..d6498e77c241 100644 --- a/third_party/nix/src/nix-daemon/nix-daemon-proto.cc +++ b/third_party/nix/src/nix-daemon/nix-daemon-proto.cc @@ -687,7 +687,10 @@ class WorkerServiceImpl final : public WorkerService::Service { return Status(grpc::StatusCode::INTERNAL, "Invalid build mode"); } - BuildResult res = store_->buildDerivation(drv_path, drv, *build_mode); + BuildLogStreambuf log_buffer(writer); + std::ostream log_sink(&log_buffer); + BuildResult res = + store_->buildDerivation(log_sink, drv_path, drv, *build_mode); proto::BuildResult proto_res{}; proto_res.set_status(res.status_to_proto()); diff --git a/third_party/nix/src/nix-store/nix-store.cc b/third_party/nix/src/nix-store/nix-store.cc index 535903d823ed..532f60b7b7e3 100644 --- a/third_party/nix/src/nix-store/nix-store.cc +++ b/third_party/nix/src/nix-store/nix-store.cc @@ -1088,7 +1088,7 @@ static void opServe(Strings opFlags, Strings opArgs) { getBuildSettings(); MonitorFdHup monitor(in.fd); - auto status = store->buildDerivation(drvPath, drv); + auto status = store->buildDerivation(std::cerr, drvPath, drv); out << status.status << status.errorMsg; diff --git a/third_party/nix/src/tests/dummy-store.hh b/third_party/nix/src/tests/dummy-store.hh index b2c49364b741..8047d25727e1 100644 --- a/third_party/nix/src/tests/dummy-store.hh +++ b/third_party/nix/src/tests/dummy-store.hh @@ -36,7 +36,8 @@ class DummyStore final : public Store { void narFromPath(const Path& path, Sink& sink) {} - BuildResult buildDerivation(const Path& drvPath, const BasicDerivation& drv, + BuildResult buildDerivation(std::ostream& log_sink, const Path& drvPath, + const BasicDerivation& drv, BuildMode buildMode = bmNormal) { return BuildResult{}; } -- cgit 1.4.1