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 <grfn@gws.fyi>
Reviewed-by: tazjin <mail@tazj.in>
Tested-by: BuildkiteCI
This commit is contained in:
Kane York 2020-07-27 19:57:04 -07:00 committed by kanepyork
parent 2a292c71f4
commit 1cbffe21f3
15 changed files with 97 additions and 49 deletions

View file

@ -704,7 +704,8 @@ static void prim_derivationStrict(EvalState& state, const Pos& pos,
HashType ht = HashType ht =
outputHashAlgo.empty() ? htUnknown : parseHashType(outputHashAlgo); outputHashAlgo.empty() ? htUnknown : parseHashType(outputHashAlgo);
Hash h(*outputHash, ht); auto hash_ = Hash::deserialize(*outputHash, ht);
auto h = Hash::unwrap_throw(hash_);
Path outPath = Path outPath =
state.store->makeFixedOutputPath(outputHashRecursive, h, drvName); 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") { } else if (n == "recursive") {
recursive = state.forceBool(*attr.second.value, *attr.second.pos); recursive = state.forceBool(*attr.second.value, *attr.second.pos);
} else if (n == "sha256") { } else if (n == "sha256") {
expectedHash = auto hash_ = Hash::deserialize(
Hash(state.forceStringNoCtx(*attr.second.value, *attr.second.pos), state.forceStringNoCtx(*attr.second.value, *attr.second.pos),
htSHA256); htSHA256);
expectedHash = Hash::unwrap_throw(hash_);
} else { } else {
throw EvalError( throw EvalError(
format("unsupported argument '%1%' to 'addPath', at %2%") % 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 = request.uri =
state.forceStringNoCtx(*attr.second.value, *attr.second.pos); state.forceStringNoCtx(*attr.second.value, *attr.second.pos);
} else if (n == "sha256") { } else if (n == "sha256") {
request.expectedHash = auto hash_ = Hash::deserialize(
Hash(state.forceStringNoCtx(*attr.second.value, *attr.second.pos), state.forceStringNoCtx(*attr.second.value, *attr.second.pos),
htSHA256); htSHA256);
request.expectedHash = Hash::unwrap_throw(hash_);
} else if (n == "name") { } else if (n == "name") {
request.name = request.name =
state.forceStringNoCtx(*attr.second.value, *attr.second.pos); state.forceStringNoCtx(*attr.second.value, *attr.second.pos);

View file

@ -64,13 +64,15 @@ void builtinFetchurl(const BasicDerivation& drv, const std::string& netrcData) {
/* Try the hashed mirrors first. */ /* Try the hashed mirrors first. */
if (getAttr("outputHashMode") == "flat") { if (getAttr("outputHashMode") == "flat") {
auto hash_ = Hash::deserialize(getAttr("outputHash"),
parseHashType(getAttr("outputHashAlgo")));
if (hash_.ok()) {
auto h = *hash_;
for (auto hashedMirror : settings.hashedMirrors.get()) { for (auto hashedMirror : settings.hashedMirrors.get()) {
try { try {
if (!absl::EndsWith(hashedMirror, "/")) { if (!absl::EndsWith(hashedMirror, "/")) {
hashedMirror += '/'; hashedMirror += '/';
} }
auto ht = parseHashType(getAttr("outputHashAlgo"));
auto h = Hash(getAttr("outputHash"), ht);
fetch(hashedMirror + printHashType(h.type) + "/" + fetch(hashedMirror + printHashType(h.type) + "/" +
h.to_string(Base16, false)); h.to_string(Base16, false));
return; return;
@ -78,6 +80,10 @@ void builtinFetchurl(const BasicDerivation& drv, const std::string& netrcData) {
LOG(ERROR) << e.what(); LOG(ERROR) << e.what();
} }
} }
} else {
LOG(ERROR) << "checking mirrors for '" << mainUrl
<< "': " << hash_.status().ToString();
}
} }
/* Otherwise try the specified URL. */ /* Otherwise try the specified URL. */

View file

@ -13,6 +13,7 @@
namespace nix { namespace nix {
// TODO(#statusor): looks like easy absl::Status conversion
void DerivationOutput::parseHashInfo(bool& recursive, Hash& hash) const { void DerivationOutput::parseHashInfo(bool& recursive, Hash& hash) const {
recursive = false; recursive = false;
std::string algo = hashAlgo; std::string algo = hashAlgo;
@ -27,7 +28,8 @@ void DerivationOutput::parseHashInfo(bool& recursive, Hash& hash) const {
throw Error(format("unknown hash algorithm '%1%'") % algo); 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( BasicDerivation BasicDerivation::from_proto(

View file

@ -115,7 +115,12 @@ struct LegacySSHStore : public Store {
if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) { if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) {
auto s = readString(conn->from); 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; conn->from >> info->ca;
info->sigs = readStrings<StringSet>(conn->from); info->sigs = readStrings<StringSet>(conn->from);
} }

View file

@ -8,6 +8,7 @@
#include <iostream> #include <iostream>
#include <absl/strings/numbers.h> #include <absl/strings/numbers.h>
#include <absl/strings/str_cat.h>
#include <absl/strings/str_split.h> #include <absl/strings/str_split.h>
#include <fcntl.h> #include <fcntl.h>
#include <glog/logging.h> #include <glog/logging.h>
@ -681,11 +682,12 @@ void LocalStore::queryPathInfoUncached(
info->id = useQueryPathInfo.getInt(0); info->id = useQueryPathInfo.getInt(0);
try { auto hash_ = Hash::deserialize(useQueryPathInfo.getStr(1));
info->narHash = Hash(useQueryPathInfo.getStr(1)); if (!hash_.ok()) {
} catch (BadHash& e) { throw Error(absl::StrCat("in valid-path entry for '", path,
throw Error("in valid-path entry for '%s': %s", path, e.what()); "': ", hash_.status().ToString()));
} }
info->narHash = *hash_;
info->registrationTime = useQueryPathInfo.getInt(2); info->registrationTime = useQueryPathInfo.getInt(2);

View file

@ -226,10 +226,13 @@ class NarInfoDiskCacheImpl final : public NarInfoDiskCache {
narInfo->url = queryNAR.getStr(2); narInfo->url = queryNAR.getStr(2);
narInfo->compression = queryNAR.getStr(3); narInfo->compression = queryNAR.getStr(3);
if (!queryNAR.isNull(4)) { 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->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); narInfo->narSize = queryNAR.getInt(7);
for (auto r : absl::StrSplit(queryNAR.getStr(8), absl::ByChar(' '))) { for (auto r : absl::StrSplit(queryNAR.getStr(8), absl::ByChar(' '))) {
narInfo->references.insert(absl::StrCat(cache.storeDir, "/", r)); narInfo->references.insert(absl::StrCat(cache.storeDir, "/", r));

View file

@ -14,11 +14,13 @@ NarInfo::NarInfo(const Store& store, const std::string& s,
}; };
auto parseHashField = [&](const std::string& s) { auto parseHashField = [&](const std::string& s) {
try { auto hash_ = Hash::deserialize(s);
return Hash(s); if (hash_.ok()) {
} catch (BadHash&) { return *hash_;
} else {
// TODO(#statusor): return an actual error
corrupt(); corrupt();
return Hash(); // never reached return Hash();
} }
}; };

View file

@ -359,7 +359,8 @@ void RemoteStore::queryPathInfoUncached(
if (!info->deriver.empty()) { if (!info->deriver.empty()) {
assertStorePath(info->deriver); 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); info->references = readStorePaths<PathSet>(*this, conn->from);
conn->from >> info->registrationTime >> info->narSize; conn->from >> info->registrationTime >> info->narSize;
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) {

View file

@ -100,7 +100,8 @@ void RpcStore::queryPathInfoUncached(
if (!info->deriver.empty()) { if (!info->deriver.empty()) {
assertStorePath(info->deriver); 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(), info->references.insert(path_info.references().begin(),
path_info.references().end()); path_info.references().end());
info->registrationTime = info->registrationTime =

View file

@ -774,7 +774,8 @@ ValidPathInfo decodeValidPathInfo(std::istream& str, bool hashGiven) {
if (hashGiven) { if (hashGiven) {
std::string s; std::string s;
getline(str, s); getline(str, s);
info.narHash = Hash(s, htSHA256); auto hash_ = Hash::deserialize(s, htSHA256);
info.narHash = Hash::unwrap_throw(hash_);
getline(str, s); getline(str, s);
if (!absl::SimpleAtoi(s, &info.narSize)) { if (!absl::SimpleAtoi(s, &info.narSize)) {
throw Error("number expected"); throw Error("number expected");
@ -829,7 +830,8 @@ bool ValidPathInfo::isContentAddressed(const Store& store) const {
}; };
if (absl::StartsWith(ca, "text:")) { 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) { if (store.makeTextPath(storePathToName(path), hash, references) == path) {
return true; return true;
} }
@ -839,7 +841,9 @@ bool ValidPathInfo::isContentAddressed(const Store& store) const {
else if (absl::StartsWith(ca, "fixed:")) { else if (absl::StartsWith(ca, "fixed:")) {
bool recursive = ca.compare(6, 2, "r:") == 0; 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() && if (references.empty() &&
store.makeFixedOutputPath(recursive, hash, storePathToName(path)) == store.makeFixedOutputPath(recursive, hash, storePathToName(path)) ==
path) { path) {

View file

@ -177,16 +177,12 @@ std::string Hash::to_string(Base base, bool includeType) const {
return s; 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); absl::StatusOr<Hash> result = deserialize(s, type);
if (result.ok()) { *this = unwrap_throw(result);
*this = *result;
} else {
throw BadHash(result.status().message());
}
} }
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; size_t pos = 0;
bool isSRI = false; bool isSRI = false;
@ -280,6 +276,14 @@ absl::StatusOr<Hash> Hash::deserialize(const std::string& s, HashType type) {
return dest; return dest;
} }
Hash Hash::unwrap_throw(absl::StatusOr<Hash> hash) {
if (hash.ok()) {
return *hash;
} else {
throw BadHash(hash.status().message());
}
}
namespace hash { namespace hash {
union Ctx { union Ctx {

View file

@ -36,12 +36,15 @@ struct Hash {
Subresource Integrity hash expression). If the 'type' argument Subresource Integrity hash expression). If the 'type' argument
is htUnknown, then the hash type must be specified in the is htUnknown, then the hash type must be specified in the
string. */ string. */
Hash(const std::string& s, HashType type = htUnknown); Hash(std::string_view s, HashType type = htUnknown);
/* Status-returning version of above constructor */ /* 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); HashType type = htUnknown);
// Legacy unwrapper for StatusOr. Throws BadHash.
static Hash unwrap_throw(absl::StatusOr<Hash> hash) noexcept(false);
void init(); void init();
/* Check whether a hash is set. */ /* Check whether a hash is set. */

View file

@ -167,7 +167,8 @@ static int _main(int argc, char** argv) {
Hash expectedHash(ht); Hash expectedHash(ht);
Path storePath; Path storePath;
if (args.size() == 2) { 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); storePath = store->makeFixedOutputPath(unpack, expectedHash, name);
if (store->isValidPath(storePath)) { if (store->isValidPath(storePath)) {
hash = expectedHash; hash = expectedHash;

View file

@ -256,8 +256,11 @@ static void opPrintFixedPath(Strings opFlags, Strings opArgs) {
std::string hash = *i++; std::string hash = *i++;
std::string name = *i++; std::string name = *i++;
cout << format("%1%\n") % auto hash_ = Hash::deserialize(hash, hashAlgo);
store->makeFixedOutputPath(recursive, Hash(hash, hashAlgo), name); Hash::unwrap_throw(hash_);
cout << absl::StrCat(store->makeFixedOutputPath(recursive, *hash_, name),
"\n");
} }
static PathSet maybeUseOutputs(const Path& storePath, bool useOutput, static PathSet maybeUseOutputs(const Path& storePath, bool useOutput,
@ -1116,7 +1119,8 @@ static void opServe(Strings opFlags, Strings opArgs) {
if (!info.deriver.empty()) { if (!info.deriver.empty()) {
store->assertStorePath(info.deriver); 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); info.references = readStorePaths<PathSet>(*store, in);
in >> info.registrationTime >> info.narSize >> info.ultimate; in >> info.registrationTime >> info.narSize >> info.ultimate;
info.sigs = readStrings<StringSet>(in); info.sigs = readStrings<StringSet>(in);

View file

@ -74,7 +74,14 @@ struct CmdToBase final : Command {
void run() override { void run() override {
for (const auto& s : args) { 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";
}
} }
} }
}; };