From 1cbffe21f3284fbad10b4ca27b0d8381bd554ff3 Mon Sep 17 00:00:00 2001 From: Kane York Date: Mon, 27 Jul 2020 19:57:04 -0700 Subject: chore(3p/nix/hash): prefer StatusOr over throwing constructor 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 Reviewed-by: tazjin Tested-by: BuildkiteCI --- third_party/nix/src/libstore/builtins/fetchurl.cc | 28 +++++++++++++--------- third_party/nix/src/libstore/derivations.cc | 4 +++- third_party/nix/src/libstore/legacy-ssh-store.cc | 7 +++++- third_party/nix/src/libstore/local-store.cc | 10 ++++---- .../nix/src/libstore/nar-info-disk-cache.cc | 7 ++++-- third_party/nix/src/libstore/nar-info.cc | 10 ++++---- third_party/nix/src/libstore/remote-store.cc | 3 ++- third_party/nix/src/libstore/rpc-store.cc | 3 ++- third_party/nix/src/libstore/store-api.cc | 10 +++++--- 9 files changed, 54 insertions(+), 28 deletions(-) (limited to 'third_party/nix/src/libstore') 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(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 #include +#include #include #include #include @@ -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(*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) { -- cgit 1.4.1