chore(3p/nix/hash): prefer StatusOr over throwing constructor r/1515

The use of `unwrap_throw` can be used as a later grep target.

Change-Id: I8c54ed90c4289f07aecb8a1393dd10204c8bce4e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1493
Reviewed-by: glittershark <grfn@gws.fyi>
Reviewed-by: tazjin <mail@tazj.in>
Tested-by: BuildkiteCI

15 files changed, 97 insertions, 49 deletions

diff --git a/third_party/nix/src/libexpr/primops.cc b/third_party/nix/src/libexpr/primops.cc
index c754f0392b89..8e916163b4ec 100644
--- a/third_party/nix/src/libexpr/primops.cc
+++ b/third_party/nix/src/libexpr/primops.cc
@@ -704,7 +704,8 @@ static void prim_derivationStrict(EvalState& state, const Pos& pos,
 
     HashType ht =
         outputHashAlgo.empty() ? htUnknown : parseHashType(outputHashAlgo);
-    Hash h(*outputHash, ht);
+    auto hash_ = Hash::deserialize(*outputHash, ht);
+    auto h = Hash::unwrap_throw(hash_);
 
     Path outPath =
         state.store->makeFixedOutputPath(outputHashRecursive, h, drvName);
@@ -1144,9 +1145,10 @@ static void prim_path(EvalState& state, const Pos& pos, Value** args,
     } else if (n == "recursive") {
       recursive = state.forceBool(*attr.second.value, *attr.second.pos);
     } else if (n == "sha256") {
-      expectedHash =
-          Hash(state.forceStringNoCtx(*attr.second.value, *attr.second.pos),
-               htSHA256);
+      auto hash_ = Hash::deserialize(
+          state.forceStringNoCtx(*attr.second.value, *attr.second.pos),
+          htSHA256);
+      expectedHash = Hash::unwrap_throw(hash_);
     } else {
       throw EvalError(
           format("unsupported argument '%1%' to 'addPath', at %2%") %
@@ -2077,9 +2079,10 @@ void fetch(EvalState& state, const Pos& pos, Value** args, Value& v,
         request.uri =
             state.forceStringNoCtx(*attr.second.value, *attr.second.pos);
       } else if (n == "sha256") {
-        request.expectedHash =
-            Hash(state.forceStringNoCtx(*attr.second.value, *attr.second.pos),
-                 htSHA256);
+        auto hash_ = Hash::deserialize(
+            state.forceStringNoCtx(*attr.second.value, *attr.second.pos),
+            htSHA256);
+        request.expectedHash = Hash::unwrap_throw(hash_);
       } else if (n == "name") {
         request.name =
             state.forceStringNoCtx(*attr.second.value, *attr.second.pos);
diff --git a/third_party/nix/src/libstore/builtins/fetchurl.cc b/third_party/nix/src/libstore/builtins/fetchurl.cc
index f7857b543d55..961d08142347 100644
--- a/third_party/nix/src/libstore/builtins/fetchurl.cc
+++ b/third_party/nix/src/libstore/builtins/fetchurl.cc
@@ -64,19 +64,25 @@ void builtinFetchurl(const BasicDerivation& drv, const std::string& netrcData) {
 
   /* Try the hashed mirrors first. */
   if (getAttr("outputHashMode") == "flat") {
-    for (auto hashedMirror : settings.hashedMirrors.get()) {
-      try {
-        if (!absl::EndsWith(hashedMirror, "/")) {
-          hashedMirror += '/';
+    auto hash_ = Hash::deserialize(getAttr("outputHash"),
+                                   parseHashType(getAttr("outputHashAlgo")));
+    if (hash_.ok()) {
+      auto h = *hash_;
+      for (auto hashedMirror : settings.hashedMirrors.get()) {
+        try {
+          if (!absl::EndsWith(hashedMirror, "/")) {
+            hashedMirror += '/';
+          }
+          fetch(hashedMirror + printHashType(h.type) + "/" +
+                h.to_string(Base16, false));
+          return;
+        } catch (Error& e) {
+          LOG(ERROR) << e.what();
         }
-        auto ht = parseHashType(getAttr("outputHashAlgo"));
-        auto h = Hash(getAttr("outputHash"), ht);
-        fetch(hashedMirror + printHashType(h.type) + "/" +
-              h.to_string(Base16, false));
-        return;
-      } catch (Error& e) {
-        LOG(ERROR) << e.what();
       }
+    } else {
+      LOG(ERROR) << "checking mirrors for '" << mainUrl
+                 << "': " << hash_.status().ToString();
     }
   }
 
diff --git a/third_party/nix/src/libstore/derivations.cc b/third_party/nix/src/libstore/derivations.cc
index 208e7e981c4a..243aa0a242f7 100644
--- a/third_party/nix/src/libstore/derivations.cc
+++ b/third_party/nix/src/libstore/derivations.cc
@@ -13,6 +13,7 @@
 
 namespace nix {
 
+// TODO(#statusor): looks like easy absl::Status conversion
 void DerivationOutput::parseHashInfo(bool& recursive, Hash& hash) const {
   recursive = false;
   std::string algo = hashAlgo;
@@ -27,7 +28,8 @@ void DerivationOutput::parseHashInfo(bool& recursive, Hash& hash) const {
     throw Error(format("unknown hash algorithm '%1%'") % algo);
   }
 
-  hash = Hash(this->hash, hashType);
+  auto hash_ = Hash::deserialize(this->hash, hashType);
+  hash = Hash::unwrap_throw(hash_);
 }
 
 BasicDerivation BasicDerivation::from_proto(
diff --git a/third_party/nix/src/libstore/legacy-ssh-store.cc b/third_party/nix/src/libstore/legacy-ssh-store.cc
index abff734efc48..8ae40d0da95c 100644
--- a/third_party/nix/src/libstore/legacy-ssh-store.cc
+++ b/third_party/nix/src/libstore/legacy-ssh-store.cc
@@ -115,7 +115,12 @@ struct LegacySSHStore : public Store {
 
       if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) {
         auto s = readString(conn->from);
-        info->narHash = s.empty() ? Hash() : Hash(s);
+        if (s.empty()) {
+          info->narHash = Hash();
+        } else {
+          auto hash_ = Hash::deserialize(s);
+          info->narHash = Hash::unwrap_throw(hash_);
+        }
         conn->from >> info->ca;
         info->sigs = readStrings<StringSet>(conn->from);
       }
diff --git a/third_party/nix/src/libstore/local-store.cc b/third_party/nix/src/libstore/local-store.cc
index c513e3ac4e9d..14ba580654ab 100644
--- a/third_party/nix/src/libstore/local-store.cc
+++ b/third_party/nix/src/libstore/local-store.cc
@@ -8,6 +8,7 @@
 #include <iostream>
 
 #include <absl/strings/numbers.h>
+#include <absl/strings/str_cat.h>
 #include <absl/strings/str_split.h>
 #include <fcntl.h>
 #include <glog/logging.h>
@@ -681,11 +682,12 @@ void LocalStore::queryPathInfoUncached(
 
       info->id = useQueryPathInfo.getInt(0);
 
-      try {
-        info->narHash = Hash(useQueryPathInfo.getStr(1));
-      } catch (BadHash& e) {
-        throw Error("in valid-path entry for '%s': %s", path, e.what());
+      auto hash_ = Hash::deserialize(useQueryPathInfo.getStr(1));
+      if (!hash_.ok()) {
+        throw Error(absl::StrCat("in valid-path entry for '", path,
+                                 "': ", hash_.status().ToString()));
       }
+      info->narHash = *hash_;
 
       info->registrationTime = useQueryPathInfo.getInt(2);
 
diff --git a/third_party/nix/src/libstore/nar-info-disk-cache.cc b/third_party/nix/src/libstore/nar-info-disk-cache.cc
index 8f723a810906..e4c4dc882ee6 100644
--- a/third_party/nix/src/libstore/nar-info-disk-cache.cc
+++ b/third_party/nix/src/libstore/nar-info-disk-cache.cc
@@ -226,10 +226,13 @@ class NarInfoDiskCacheImpl final : public NarInfoDiskCache {
           narInfo->url = queryNAR.getStr(2);
           narInfo->compression = queryNAR.getStr(3);
           if (!queryNAR.isNull(4)) {
-            narInfo->fileHash = Hash(queryNAR.getStr(4));
+            auto hash_ = Hash::deserialize(queryNAR.getStr(4));
+            // TODO(#statusor): does this throw mess with retrySQLite?
+            narInfo->fileHash = Hash::unwrap_throw(hash_);
           }
           narInfo->fileSize = queryNAR.getInt(5);
-          narInfo->narHash = Hash(queryNAR.getStr(6));
+          auto hash_ = Hash::deserialize(queryNAR.getStr(6));
+          narInfo->narHash = Hash::unwrap_throw(hash_);
           narInfo->narSize = queryNAR.getInt(7);
           for (auto r : absl::StrSplit(queryNAR.getStr(8), absl::ByChar(' '))) {
             narInfo->references.insert(absl::StrCat(cache.storeDir, "/", r));
diff --git a/third_party/nix/src/libstore/nar-info.cc b/third_party/nix/src/libstore/nar-info.cc
index aa764f4a16a0..ec9f882f4fb4 100644
--- a/third_party/nix/src/libstore/nar-info.cc
+++ b/third_party/nix/src/libstore/nar-info.cc
@@ -14,11 +14,13 @@ NarInfo::NarInfo(const Store& store, const std::string& s,
   };
 
   auto parseHashField = [&](const std::string& s) {
-    try {
-      return Hash(s);
-    } catch (BadHash&) {
+    auto hash_ = Hash::deserialize(s);
+    if (hash_.ok()) {
+      return *hash_;
+    } else {
+      // TODO(#statusor): return an actual error
       corrupt();
-      return Hash();  // never reached
+      return Hash();
     }
   };
 
diff --git a/third_party/nix/src/libstore/remote-store.cc b/third_party/nix/src/libstore/remote-store.cc
index b2e660dc2af9..8d278d500dd5 100644
--- a/third_party/nix/src/libstore/remote-store.cc
+++ b/third_party/nix/src/libstore/remote-store.cc
@@ -359,7 +359,8 @@ void RemoteStore::queryPathInfoUncached(
       if (!info->deriver.empty()) {
         assertStorePath(info->deriver);
       }
-      info->narHash = Hash(readString(conn->from), htSHA256);
+      auto hash_ = Hash::deserialize(readString(conn->from), htSHA256);
+      info->narHash = Hash::unwrap_throw(hash_);
       info->references = readStorePaths<PathSet>(*this, conn->from);
       conn->from >> info->registrationTime >> info->narSize;
       if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) {
diff --git a/third_party/nix/src/libstore/rpc-store.cc b/third_party/nix/src/libstore/rpc-store.cc
index 6d31f1aa81d3..e2eb240f8b8a 100644
--- a/third_party/nix/src/libstore/rpc-store.cc
+++ b/third_party/nix/src/libstore/rpc-store.cc
@@ -100,7 +100,8 @@ void RpcStore::queryPathInfoUncached(
     if (!info->deriver.empty()) {
       assertStorePath(info->deriver);
     }
-    info->narHash = Hash(path_info.nar_hash(), htSHA256);
+    auto hash_ = Hash::deserialize(path_info.nar_hash(), htSHA256);
+    info->narHash = Hash::unwrap_throw(hash_);
     info->references.insert(path_info.references().begin(),
                             path_info.references().end());
     info->registrationTime =
diff --git a/third_party/nix/src/libstore/store-api.cc b/third_party/nix/src/libstore/store-api.cc
index f6fef4b912f2..a68e6e8fc812 100644
--- a/third_party/nix/src/libstore/store-api.cc
+++ b/third_party/nix/src/libstore/store-api.cc
@@ -774,7 +774,8 @@ ValidPathInfo decodeValidPathInfo(std::istream& str, bool hashGiven) {
   if (hashGiven) {
     std::string s;
     getline(str, s);
-    info.narHash = Hash(s, htSHA256);
+    auto hash_ = Hash::deserialize(s, htSHA256);
+    info.narHash = Hash::unwrap_throw(hash_);
     getline(str, s);
     if (!absl::SimpleAtoi(s, &info.narSize)) {
       throw Error("number expected");
@@ -829,7 +830,8 @@ bool ValidPathInfo::isContentAddressed(const Store& store) const {
   };
 
   if (absl::StartsWith(ca, "text:")) {
-    Hash hash(std::string(ca, 5));
+    auto hash_ = Hash::deserialize(std::string_view(ca).substr(5));
+    Hash hash = Hash::unwrap_throw(hash_);
     if (store.makeTextPath(storePathToName(path), hash, references) == path) {
       return true;
     }
@@ -839,7 +841,9 @@ bool ValidPathInfo::isContentAddressed(const Store& store) const {
 
   else if (absl::StartsWith(ca, "fixed:")) {
     bool recursive = ca.compare(6, 2, "r:") == 0;
-    Hash hash(std::string(ca, recursive ? 8 : 6));
+    auto hash_ =
+        Hash::deserialize(std::string_view(ca).substr(recursive ? 8 : 6));
+    Hash hash = Hash::unwrap_throw(hash_);
     if (references.empty() &&
         store.makeFixedOutputPath(recursive, hash, storePathToName(path)) ==
             path) {
diff --git a/third_party/nix/src/libutil/hash.cc b/third_party/nix/src/libutil/hash.cc
index 4c6eef0e6d9b..44fc4323b337 100644
--- a/third_party/nix/src/libutil/hash.cc
+++ b/third_party/nix/src/libutil/hash.cc
@@ -177,16 +177,12 @@ std::string Hash::to_string(Base base, bool includeType) const {
   return s;
 }
 
-Hash::Hash(const std::string& s, HashType type) : type(type) {
+Hash::Hash(std::string_view s, HashType type) : type(type) {
   absl::StatusOr<Hash> result = deserialize(s, type);
-  if (result.ok()) {
-    *this = *result;
-  } else {
-    throw BadHash(result.status().message());
-  }
+  *this = unwrap_throw(result);
 }
 
-absl::StatusOr<Hash> Hash::deserialize(const std::string& s, HashType type) {
+absl::StatusOr<Hash> Hash::deserialize(std::string_view s, HashType type) {
   size_t pos = 0;
   bool isSRI = false;
 
@@ -280,6 +276,14 @@ absl::StatusOr<Hash> Hash::deserialize(const std::string& s, HashType type) {
   return dest;
 }
 
+Hash Hash::unwrap_throw(absl::StatusOr<Hash> hash) {
+  if (hash.ok()) {
+    return *hash;
+  } else {
+    throw BadHash(hash.status().message());
+  }
+}
+
 namespace hash {
 
 union Ctx {
diff --git a/third_party/nix/src/libutil/hash.hh b/third_party/nix/src/libutil/hash.hh
index 208615f67bc8..4ad4ef6ada02 100644
--- a/third_party/nix/src/libutil/hash.hh
+++ b/third_party/nix/src/libutil/hash.hh
@@ -36,12 +36,15 @@ struct Hash {
      Subresource Integrity hash expression). If the 'type' argument
      is htUnknown, then the hash type must be specified in the
      string. */
-  Hash(const std::string& s, HashType type = htUnknown);
+  Hash(std::string_view s, HashType type = htUnknown);
 
   /* Status-returning version of above constructor */
-  static absl::StatusOr<Hash> deserialize(const std::string& s,
+  static absl::StatusOr<Hash> deserialize(std::string_view s,
                                           HashType type = htUnknown);
 
+  // Legacy unwrapper for StatusOr. Throws BadHash.
+  static Hash unwrap_throw(absl::StatusOr<Hash> hash) noexcept(false);
+
   void init();
 
   /* Check whether a hash is set. */
diff --git a/third_party/nix/src/nix-prefetch-url/nix-prefetch-url.cc b/third_party/nix/src/nix-prefetch-url/nix-prefetch-url.cc
index 0e5b2ec8a537..644df1dba238 100644
--- a/third_party/nix/src/nix-prefetch-url/nix-prefetch-url.cc
+++ b/third_party/nix/src/nix-prefetch-url/nix-prefetch-url.cc
@@ -167,7 +167,8 @@ static int _main(int argc, char** argv) {
     Hash expectedHash(ht);
     Path storePath;
     if (args.size() == 2) {
-      expectedHash = Hash(args[1], ht);
+      auto expectedHash_ = Hash::deserialize(args[1], ht);
+      expectedHash = Hash::unwrap_throw(expectedHash);
       storePath = store->makeFixedOutputPath(unpack, expectedHash, name);
       if (store->isValidPath(storePath)) {
         hash = expectedHash;
diff --git a/third_party/nix/src/nix-store/nix-store.cc b/third_party/nix/src/nix-store/nix-store.cc
index 2183d053e577..9f37bfbcac99 100644
--- a/third_party/nix/src/nix-store/nix-store.cc
+++ b/third_party/nix/src/nix-store/nix-store.cc
@@ -256,8 +256,11 @@ static void opPrintFixedPath(Strings opFlags, Strings opArgs) {
   std::string hash = *i++;
   std::string name = *i++;
 
-  cout << format("%1%\n") %
-              store->makeFixedOutputPath(recursive, Hash(hash, hashAlgo), name);
+  auto hash_ = Hash::deserialize(hash, hashAlgo);
+  Hash::unwrap_throw(hash_);
+
+  cout << absl::StrCat(store->makeFixedOutputPath(recursive, *hash_, name),
+                       "\n");
 }
 
 static PathSet maybeUseOutputs(const Path& storePath, bool useOutput,
@@ -1116,7 +1119,8 @@ static void opServe(Strings opFlags, Strings opArgs) {
         if (!info.deriver.empty()) {
           store->assertStorePath(info.deriver);
         }
-        info.narHash = Hash(readString(in), htSHA256);
+        auto hash_ = Hash::deserialize(readString(in), htSHA256);
+        info.narHash = Hash::unwrap_throw(hash_);
         info.references = readStorePaths<PathSet>(*store, in);
         in >> info.registrationTime >> info.narSize >> info.ultimate;
         info.sigs = readStrings<StringSet>(in);
diff --git a/third_party/nix/src/nix/hash.cc b/third_party/nix/src/nix/hash.cc
index 521f97d53a09..24529c67ce8c 100644
--- a/third_party/nix/src/nix/hash.cc
+++ b/third_party/nix/src/nix/hash.cc
@@ -74,7 +74,14 @@ struct CmdToBase final : Command {
 
   void run() override {
     for (const auto& s : args) {
-      std::cout << fmt("%s\n", Hash(s, ht).to_string(base, base == SRI));
+      auto hash_ = Hash::deserialize(s, ht);
+      if (hash_.ok()) {
+        std::cout << hash_->to_string(base, base == SRI) << "\n";
+      } else {
+        std::cerr << "failed to parse: " << hash_.status().ToString() << "\n";
+        // create a matching blank line, for scripting
+        std::cout << "\n";
+      }
     }
   }
 };