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/libexpr/primops.cc | 17 +++++++------ 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 +++++--- third_party/nix/src/libutil/hash.cc | 18 ++++++++------ third_party/nix/src/libutil/hash.hh | 7 ++++-- .../nix/src/nix-prefetch-url/nix-prefetch-url.cc | 3 ++- third_party/nix/src/nix-store/nix-store.cc | 10 +++++--- third_party/nix/src/nix/hash.cc | 9 ++++++- 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(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) { 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 result = deserialize(s, type); - if (result.ok()) { - *this = *result; - } else { - throw BadHash(result.status().message()); - } + *this = unwrap_throw(result); } -absl::StatusOr Hash::deserialize(const std::string& s, HashType type) { +absl::StatusOr Hash::deserialize(std::string_view s, HashType type) { size_t pos = 0; bool isSRI = false; @@ -280,6 +276,14 @@ absl::StatusOr Hash::deserialize(const std::string& s, HashType type) { return dest; } +Hash Hash::unwrap_throw(absl::StatusOr 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 deserialize(const std::string& s, + static absl::StatusOr deserialize(std::string_view s, HashType type = htUnknown); + // Legacy unwrapper for StatusOr. Throws BadHash. + static Hash unwrap_throw(absl::StatusOr 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(*store, in); in >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(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"; + } } } }; -- cgit 1.4.1