refactor(tvix): Make Store::buildPaths return a Status r/1649

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>

8 files changed, 34 insertions, 14 deletions

diff --git a/third_party/nix/src/libstore/build.cc b/third_party/nix/src/libstore/build.cc
index 6aad99b37a..a9d40991d3 100644
--- a/third_party/nix/src/libstore/build.cc
+++ b/third_party/nix/src/libstore/build.cc
@@ -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,
diff --git a/third_party/nix/src/libstore/local-store.hh b/third_party/nix/src/libstore/local-store.hh
index 731cf1764c..f9e08e5e6b 100644
--- a/third_party/nix/src/libstore/local-store.hh
+++ b/third_party/nix/src/libstore/local-store.hh
@@ -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;
diff --git a/third_party/nix/src/libstore/remote-store.cc b/third_party/nix/src/libstore/remote-store.cc
index 41881f8ef9..6948d4e61d 100644
--- a/third_party/nix/src/libstore/remote-store.cc
+++ b/third_party/nix/src/libstore/remote-store.cc
@@ -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,
diff --git a/third_party/nix/src/libstore/remote-store.hh b/third_party/nix/src/libstore/remote-store.hh
index a6829226ca..4aad5a5d2f 100644
--- a/third_party/nix/src/libstore/remote-store.hh
+++ b/third_party/nix/src/libstore/remote-store.hh
@@ -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;
diff --git a/third_party/nix/src/libstore/rpc-store.cc b/third_party/nix/src/libstore/rpc-store.cc
index a0b1ef9cfa..2d1278f40c 100644
--- a/third_party/nix/src/libstore/rpc-store.cc
+++ b/third_party/nix/src/libstore/rpc-store.cc
@@ -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,
diff --git a/third_party/nix/src/libstore/rpc-store.hh b/third_party/nix/src/libstore/rpc-store.hh
index 7bfa78a496..4bf417ad1e 100644
--- a/third_party/nix/src/libstore/rpc-store.hh
+++ b/third_party/nix/src/libstore/rpc-store.hh
@@ -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,
diff --git a/third_party/nix/src/libstore/store-api.cc b/third_party/nix/src/libstore/store-api.cc
index ae403b0be6..fc33c7ce57 100644
--- a/third_party/nix/src/libstore/store-api.cc
+++ b/third_party/nix/src/libstore/store-api.cc
@@ -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,
diff --git a/third_party/nix/src/libstore/store-api.hh b/third_party/nix/src/libstore/store-api.hh
index 8d9c6f0861..b1c0b50de3 100644
--- a/third_party/nix/src/libstore/store-api.hh
+++ b/third_party/nix/src/libstore/store-api.hh
@@ -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