feat(tvix): Thread a log sink through calls to buildDerivation r/1946

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 <andi@notmuch.email>
Reviewed-by: kanepyork <rikingcoding@gmail.com>

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 00bd833ac3..43564a5eb7 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 0b788152f0..40c636f60a 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 15ed3b0514..1f5752a168 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 19603115a8..8163258179 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 cfcfb35cc3..a7c49079d2 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 c4fd856a2c..cb6cc808c6 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 60da6045a8..c360055b6e 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 afaeca4a66..c29bd059de 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 3a156e9e1a..679ceac7af 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 76c757bd69..eb18511e60 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<Store>, 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 597ef2434a..d6498e77c2 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 535903d823..532f60b7b7 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 b2c49364b7..8047d25727 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{};
   }