From 20799a5151cbb185c1772b8e5160493b2dc2d0e8 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 18:41:33 +0000 Subject: [PATCH 01/21] WIP: Make Hash always store a valid hash type --- src/libstore/build.cc | 2 +- src/libstore/store-api.hh | 3 +- src/libutil/hash.cc | 65 +++++++++++++----------- src/libutil/hash.hh | 17 +++---- src/nix-prefetch-url/nix-prefetch-url.cc | 2 +- src/nix-store/nix-store.cc | 10 ++-- src/nix/add-to-store.cc | 4 +- src/nix/develop.cc | 2 +- 8 files changed, 53 insertions(+), 52 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 82a2ab831..86041cabd 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4997,7 +4997,7 @@ bool Worker::pathContentsGood(const StorePath & path) if (!pathExists(store.printStorePath(path))) res = false; else { - HashResult current = hashPath(*info->narHash.type, store.printStorePath(path)); + HashResult current = hashPath(info->narHash->type, store.printStorePath(path)); Hash nullHash(htSHA256); res = info->narHash == nullHash || info->narHash == current.first; } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a05048290..de9b6a791 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -116,7 +116,8 @@ struct ValidPathInfo { StorePath path; std::optional deriver; - Hash narHash; + // TODO document this + std::optional narHash; StorePathSet references; time_t registrationTime = 0; uint64_t narSize = 0; // 0 = unknown diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index e49eb4569..00bb999cb 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -16,16 +16,19 @@ namespace nix { +static size_t regularHashSize(HashType type) { + switch (type) { + case htMD5: return md5HashSize; + case htSHA1: return sha1HashSize; + case htSHA256: return sha256HashSize; + case htSHA512: return sha512HashSize; + } + abort(); +} void Hash::init() { - if (!type) abort(); - switch (*type) { - case htMD5: hashSize = md5HashSize; break; - case htSHA1: hashSize = sha1HashSize; break; - case htSHA256: hashSize = sha256HashSize; break; - case htSHA512: hashSize = sha512HashSize; break; - } + hashSize = regularHashSize(type); assert(hashSize <= maxHashSize); memset(hash, 0, maxHashSize); } @@ -105,18 +108,11 @@ string printHash16or32(const Hash & hash) } -HashType assertInitHashType(const Hash & h) { - if (h.type) - return *h.type; - else - abort(); -} - std::string Hash::to_string(Base base, bool includeType) const { std::string s; if (base == SRI || includeType) { - s += printHashType(assertInitHashType(*this)); + s += printHashType(type); s += base == SRI ? '-' : ':'; } switch (base) { @@ -137,29 +133,36 @@ std::string Hash::to_string(Base base, bool includeType) const Hash::Hash(std::string_view s, HashType type) : Hash(s, std::optional { type }) { } Hash::Hash(std::string_view s) : Hash(s, std::optional{}) { } -Hash::Hash(std::string_view s, std::optional type) - : type(type) +Hash::Hash(std::string_view s, std::optional optType) { size_t pos = 0; bool isSRI = false; + // Find the : or - separater, and set `isSRI` to the correct value auto sep = s.find(':'); if (sep == string::npos) { sep = s.find('-'); - if (sep != string::npos) { + if (sep != string::npos) isSRI = true; - } else if (! type) - throw BadHash("hash '%s' does not include a type", s); } + // Parse the has type before the separater, if there was one. + std::optional optParsedType; if (sep != string::npos) { - string hts = string(s, 0, sep); - this->type = parseHashType(hts); - if (!this->type) + auto hts = s.substr(0, sep); + auto optParsedType = parseHashType(hts); + if (!optParsedType) throw BadHash("unknown hash type '%s'", hts); - if (type && type != this->type) - throw BadHash("hash '%s' should have type '%s'", s, printHashType(*type)); - pos = sep + 1; + } + + // Either the string or user must provide the type, if they both do they + // must agree. + if (!optParsedType && !optType) { + throw BadHash("hash '%s' does not include a type, nor is the type otherwise known from context.", s); + } else { + this->type = optParsedType ? *optParsedType : *optType; + if (optParsedType && optType && *optParsedType != *optType) + throw BadHash("hash '%s' should have type '%s'", s, printHashType(*optType)); } init(); @@ -214,7 +217,7 @@ Hash::Hash(std::string_view s, std::optional type) } else - throw BadHash("hash '%s' has wrong length for hash type '%s'", s, printHashType(*type)); + throw BadHash("hash '%s' has wrong length for hash type '%s'", s, printHashType(type)); } Hash newHashAllowEmpty(std::string hashStr, std::optional ht) @@ -267,7 +270,7 @@ static void finish(HashType ht, Ctx & ctx, unsigned char * hash) } -Hash hashString(HashType ht, const string & s) +Hash hashString(HashType ht, std::string_view s) { Ctx ctx; Hash hash(ht); @@ -334,7 +337,7 @@ HashResult hashPath( Hash compressHash(const Hash & hash, unsigned int newSize) { - Hash h; + Hash h(hash.type); h.hashSize = newSize; for (unsigned int i = 0; i < hash.hashSize; ++i) h.hash[i % newSize] ^= hash.hash[i]; @@ -342,7 +345,7 @@ Hash compressHash(const Hash & hash, unsigned int newSize) } -std::optional parseHashTypeOpt(const string & s) +std::optional parseHashTypeOpt(std::string_view s) { if (s == "md5") return htMD5; else if (s == "sha1") return htSHA1; @@ -351,7 +354,7 @@ std::optional parseHashTypeOpt(const string & s) else return std::optional {}; } -HashType parseHashType(const string & s) +HashType parseHashType(std::string_view s) { auto opt_h = parseHashTypeOpt(s); if (opt_h) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 0d9916508..b51850ccf 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -25,14 +25,11 @@ enum Base : int { Base64, Base32, Base16, SRI }; struct Hash { - static const unsigned int maxHashSize = 64; - unsigned int hashSize = 0; - unsigned char hash[maxHashSize] = {}; + constexpr static size_t maxHashSize = 64; + size_t hashSize = 0; + uint8_t hash[maxHashSize] = {}; - std::optional type = {}; - - /* Create an unset hash object. */ - Hash() { }; + HashType type; /* Create a zero-filled hash object. */ Hash(HashType type) : type(type) { init(); }; @@ -105,7 +102,7 @@ Hash newHashAllowEmpty(std::string hashStr, std::optional ht); string printHash16or32(const Hash & hash); /* Compute the hash of the given string. */ -Hash hashString(HashType ht, const string & s); +Hash hashString(HashType ht, std::string_view s); /* Compute the hash of the given file. */ Hash hashFile(HashType ht, const Path & path); @@ -121,9 +118,9 @@ HashResult hashPath(HashType ht, const Path & path, Hash compressHash(const Hash & hash, unsigned int newSize); /* Parse a string representing a hash type. */ -HashType parseHashType(const string & s); +HashType parseHashType(std::string_view s); /* Will return nothing on parse error */ -std::optional parseHashTypeOpt(const string & s); +std::optional parseHashTypeOpt(std::string_view s); /* And the reverse. */ string printHashType(HashType ht); diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 40b05a2f3..22410c44c 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -153,7 +153,7 @@ static int _main(int argc, char * * argv) /* If an expected hash is given, the file may already exist in the store. */ - Hash hash, expectedHash(ht); + Hash hash(ht), expectedHash(ht); std::optional storePath; if (args.size() == 2) { expectedHash = Hash(args[1], ht); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 4e02aa2bf..d061317ae 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -372,8 +372,8 @@ static void opQuery(Strings opFlags, Strings opArgs) for (auto & j : maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise)) { auto info = store->queryPathInfo(j); if (query == qHash) { - assert(info->narHash.type == htSHA256); - cout << fmt("%s\n", info->narHash.to_string(Base32, true)); + assert(info->narHash && info->narHash->type == htSHA256); + cout << fmt("%s\n", info->narHash->to_string(Base32, true)); } else if (query == qSize) cout << fmt("%d\n", info->narSize); } @@ -725,7 +725,7 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) auto path = store->followLinksToStorePath(i); printMsg(lvlTalkative, "checking path '%s'...", store->printStorePath(path)); auto info = store->queryPathInfo(path); - HashSink sink(*info->narHash.type); + HashSink sink(info->narHash->type); store->narFromPath(path, sink); auto current = sink.finish(); if (current.first != info->narHash) { @@ -734,7 +734,7 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) .hint = hintfmt( "path '%s' was modified! expected hash '%s', got '%s'", store->printStorePath(path), - info->narHash.to_string(Base32, true), + info->narHash->to_string(Base32, true), current.first.to_string(Base32, true)) }); status = 1; @@ -864,7 +864,7 @@ static void opServe(Strings opFlags, Strings opArgs) out << info->narSize // downloadSize << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 4) - out << (info->narHash ? info->narHash.to_string(Base32, true) : "") << info->ca << info->sigs; + out << (info->narHash ? info->narHash->to_string(Base32, true) : "") << info->ca << info->sigs; } catch (InvalidPath &) { } } diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index f43f774c1..0dda2af38 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -46,9 +46,9 @@ struct CmdAddToStore : MixDryRun, StoreCommand auto narHash = hashString(htSHA256, *sink.s); ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, *namePart)); - info.narHash = narHash; + *info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); + info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, *info.narHash); if (!dryRun) { auto source = StringSource { *sink.s }; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 8b85caf82..4aee9f202 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -139,7 +139,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) .path = shellOutPath, .hash = DerivationOutputHash { .method = FileIngestionMethod::Flat, - .hash = Hash { }, + .hash = Hash { htSHA256 }, }, }); drv.env["out"] = store->printStorePath(shellOutPath); From e7a14118df5b53b07ecf48c8fb1ac712677250b3 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 19 Jun 2020 16:50:28 -0400 Subject: [PATCH 02/21] WIP bug fixing --- src/libexpr/primops.cc | 8 ++++---- src/libexpr/primops/fetchTree.cc | 2 +- src/libstore/binary-cache-store.cc | 6 +++--- src/libstore/build.cc | 10 ++++++---- src/libstore/builtins/fetchurl.cc | 2 +- src/libstore/daemon.cc | 4 ++-- src/libstore/derivations.cc | 2 +- src/libstore/export-import.cc | 4 ++-- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/nar-info.hh | 2 +- src/libstore/references.cc | 6 +++--- src/libstore/references.hh | 3 +-- src/nix/make-content-addressable.cc | 2 +- src/nix/verify.cc | 8 ++++---- 14 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f5fbd3fa6..0dd5624b2 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1122,7 +1122,7 @@ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Valu static void addPath(EvalState & state, const Pos & pos, const string & name, const Path & path_, - Value * filterFun, FileIngestionMethod method, const Hash & expectedHash, Value & v) + Value * filterFun, FileIngestionMethod method, const std::optional expectedHash, Value & v) { const auto path = evalSettings.pureEval && expectedHash ? path_ : @@ -1153,7 +1153,7 @@ static void addPath(EvalState & state, const Pos & pos, const string & name, con std::optional expectedStorePath; if (expectedHash) - expectedStorePath = state.store->makeFixedOutputPath(method, expectedHash, name); + expectedStorePath = state.store->makeFixedOutputPath(method, *expectedHash, name); Path dstPath; if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) { dstPath = state.store->printStorePath(settings.readOnlyMode @@ -1187,7 +1187,7 @@ static void prim_filterSource(EvalState & state, const Pos & pos, Value * * args .nixCode = NixCode { .errPos = pos } }); - addPath(state, pos, std::string(baseNameOf(path)), path, args[0], FileIngestionMethod::Recursive, Hash(), v); + addPath(state, pos, std::string(baseNameOf(path)), path, args[0], FileIngestionMethod::Recursive, std::nullopt, v); } static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value & v) @@ -1197,7 +1197,7 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value string name; Value * filterFun = nullptr; auto method = FileIngestionMethod::Recursive; - Hash expectedHash; + Hash expectedHash(htSHA256); for (auto & attr : *args[0]->attrs) { const string & n(attr.name); diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 9be93710a..a5b836383 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -147,7 +147,7 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v, : hashFile(htSHA256, path); if (hash != *expectedHash) throw Error((unsigned int) 102, "hash mismatch in file downloaded from '%s':\n wanted: %s\n got: %s", - *url, expectedHash->to_string(Base32, true), hash.to_string(Base32, true)); + *url, expectedHash->to_string(Base32, true), hash->to_string(Base32, true)); } if (state.allowedPaths) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 9f52ddafa..98a3eaebb 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -181,7 +181,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource ((1.0 - (double) narCompressed->size() / nar->size()) * 100.0), duration); - narInfo->url = "nar/" + narInfo->fileHash.to_string(Base32, false) + ".nar" + narInfo->url = "nar/" + narInfo->fileHash->to_string(Base32, false) + ".nar" + (compression == "xz" ? ".xz" : compression == "bzip2" ? ".bz2" : compression == "br" ? ".br" : @@ -338,7 +338,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath method for very large paths, but `copyPath' is mainly used for small files. */ StringSink sink; - Hash h; + std::optional h; if (method == FileIngestionMethod::Recursive) { dumpPath(srcPath, sink, filter); h = hashString(hashAlgo, *sink.s); @@ -348,7 +348,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath h = hashString(hashAlgo, s); } - ValidPathInfo info(makeFixedOutputPath(method, h, name)); + ValidPathInfo info(makeFixedOutputPath(method, *h, name)); auto source = StringSource { *sink.s }; addToStore(info, source, repair, CheckSigs, nullptr); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 86041cabd..80351a675 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3730,8 +3730,8 @@ void DerivationGoal::registerOutputs() /* Check the hash. In hash mode, move the path produced by the derivation to its content-addressed location. */ Hash h2 = i.second.hash->method == FileIngestionMethod::Recursive - ? hashPath(*i.second.hash->hash.type, actualPath).first - : hashFile(*i.second.hash->hash.type, actualPath); + ? hashPath(i.second.hash->hash.type, actualPath).first + : hashFile(i.second.hash->hash.type, actualPath); auto dest = worker.store.makeFixedOutputPath(i.second.hash->method, h2, i.second.path.name()); @@ -3777,8 +3777,10 @@ void DerivationGoal::registerOutputs() time. The hash is stored in the database so that we can verify later on whether nobody has messed with the store. */ debug("scanning for references inside '%1%'", path); - HashResult hash; - auto references = worker.store.parseStorePathSet(scanForReferences(actualPath, worker.store.printStorePathSet(referenceablePaths), hash)); + // HashResult hash; + auto pathSetAndHash = scanForReferences(actualPath, worker.store.printStorePathSet(referenceablePaths)); + auto references = worker.store.parseStorePathSet(pathSetAndHash.first); + HashResult hash = pathSetAndHash.second; if (buildMode == bmCheck) { if (!worker.store.isValidPath(worker.store.parseStorePath(path))) continue; diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 1cfe4a46a..f3827684b 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -65,7 +65,7 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/'; auto ht = parseHashTypeOpt(getAttr("outputHashAlgo")); auto h = Hash(getAttr("outputHash"), ht); - fetch(hashedMirror + printHashType(*h.type) + "/" + h.to_string(Base16, false)); + fetch(hashedMirror + printHashType(h.type) + "/" + h.to_string(Base16, false)); return; } catch (Error & e) { debug(e.what()); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index e370e278c..296bfad5d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -314,7 +314,7 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); auto hash = store->queryPathInfo(path)->narHash; logger->stopWork(); - to << hash.to_string(Base16, false); + to << hash->to_string(Base16, false); break; } @@ -646,7 +646,7 @@ static void performOp(TunnelLogger * logger, ref store, if (GET_PROTOCOL_MINOR(clientVersion) >= 17) to << 1; to << (info->deriver ? store->printStorePath(*info->deriver) : "") - << info->narHash.to_string(Base16, false); + << info->narHash->to_string(Base16, false); writeStorePaths(*store, to, info->references); to << info->registrationTime << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 16) { diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index a79b78db6..53bd4da2a 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -9,7 +9,7 @@ namespace nix { std::string DerivationOutputHash::printMethodAlgo() const { - return makeFileIngestionPrefix(method) + printHashType(*hash.type); + return makeFileIngestionPrefix(method) + printHashType(hash.type); } diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index 57b7e9590..9b8cc5c3a 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -55,9 +55,9 @@ void Store::exportPath(const StorePath & path, Sink & sink) filesystem corruption from spreading to other machines. Don't complain if the stored hash is zero (unknown). */ Hash hash = hashAndWriteSink.currentHash(); - if (hash != info->narHash && info->narHash != Hash(*info->narHash.type)) + if (hash != info->narHash && info->narHash != Hash(info->narHash->type)) throw Error("hash of path '%s' has changed from '%s' to '%s'!", - printStorePath(path), info->narHash.to_string(Base32, true), hash.to_string(Base32, true)); + printStorePath(path), info->narHash->to_string(Base32, true), hash.to_string(Base32, true)); hashAndWriteSink << exportMagic diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 45c70fad6..171903980 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -113,7 +113,7 @@ struct LegacySSHStore : public Store if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) { auto s = readString(conn->from); - info->narHash = s.empty() ? Hash() : Hash(s); + info->narHash = s.empty() ? std::optional{} : Hash(s); conn->from >> info->ca; info->sigs = readStrings(conn->from); } diff --git a/src/libstore/nar-info.hh b/src/libstore/nar-info.hh index 373c33427..eff19f0ef 100644 --- a/src/libstore/nar-info.hh +++ b/src/libstore/nar-info.hh @@ -10,7 +10,7 @@ struct NarInfo : ValidPathInfo { std::string url; std::string compression; - Hash fileHash; + std::optional fileHash; uint64_t fileSize = 0; std::string system; diff --git a/src/libstore/references.cc b/src/libstore/references.cc index a10d536a3..8ee8d1ae8 100644 --- a/src/libstore/references.cc +++ b/src/libstore/references.cc @@ -79,8 +79,8 @@ void RefScanSink::operator () (const unsigned char * data, size_t len) } -PathSet scanForReferences(const string & path, - const PathSet & refs, HashResult & hash) +std::pair scanForReferences(const string & path, + const PathSet & refs) { RefScanSink sink; std::map backMap; @@ -114,7 +114,7 @@ PathSet scanForReferences(const string & path, hash = sink.hashSink.finish(); - return found; + return std::pair(found, hash); } diff --git a/src/libstore/references.hh b/src/libstore/references.hh index c38bdd720..598a3203a 100644 --- a/src/libstore/references.hh +++ b/src/libstore/references.hh @@ -5,8 +5,7 @@ namespace nix { -PathSet scanForReferences(const Path & path, const PathSet & refs, - HashResult & hash); +std::pair scanForReferences(const Path & path, const PathSet & refs); struct RewritingSink : Sink { diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index 0ebb8f13b..a712dceef 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -82,7 +82,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON if (hasSelfReference) info.references.insert(info.path); info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); + info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, *info.narHash); if (!json) printInfo("rewrote '%s' to '%s'", pathS, store->printStorePath(info.path)); diff --git a/src/nix/verify.cc b/src/nix/verify.cc index d1aba08e3..c92f894f2 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -88,15 +88,15 @@ struct CmdVerify : StorePathsCommand std::unique_ptr hashSink; if (info->ca == "") - hashSink = std::make_unique(*info->narHash.type); + hashSink = std::make_unique(info->narHash->type); else - hashSink = std::make_unique(*info->narHash.type, std::string(info->path.hashPart())); + hashSink = std::make_unique(info->narHash->type, std::string(info->path.hashPart())); store->narFromPath(info->path, *hashSink); auto hash = hashSink->finish(); - if (hash.first != info->narHash) { + if (hash.first != *info->narHash) { corrupted++; act2.result(resCorruptedPath, store->printStorePath(info->path)); logError({ @@ -104,7 +104,7 @@ struct CmdVerify : StorePathsCommand .hint = hintfmt( "path '%s' was modified! expected hash '%s', got '%s'", store->printStorePath(info->path), - info->narHash.to_string(Base32, true), + info->narHash->to_string(Base32, true), hash.first.to_string(Base32, true)) }); } From cdddf24f2503c8b069a72c4d004ebfa80e8fe041 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 19 Jun 2020 14:54:41 -0600 Subject: [PATCH 03/21] add hintfmt test --- src/libutil/tests/logging.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 4cb54995b..b24975438 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -1,6 +1,7 @@ #include "logging.hh" #include "nixexpr.hh" #include "util.hh" +#include #include @@ -252,4 +253,21 @@ namespace nix { ASSERT_STREQ(str.c_str(), "\x1B[33;1mwarning:\x1B[0m\x1B[34;1m --- warning name --- error-unit-test\x1B[0m\nin file: \x1B[34;1mmyfile.nix (40:13)\x1B[0m\n\nwarning description\n\n 40| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n"); } + /* ---------------------------------------------------------------------------- + * hintfmt + * --------------------------------------------------------------------------*/ + + TEST(hintfmt, withstandsPercentString) { + + const char *teststr = "this is 100%s correct!"; + auto hint = hintfmt(teststr); + + std::ofstream meh("meh.txt"); + meh << hint.str() << std::endl; + + ASSERT_STREQ(hint.str().c_str(), teststr); + } + + + } From db475f9e7e7a35d7d73d39f06633ce04ac1b67fc Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 19 Jun 2020 15:28:13 -0600 Subject: [PATCH 04/21] too few, too many args --- src/libutil/tests/logging.cc | 83 ++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index b24975438..e208efc07 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -70,9 +70,9 @@ namespace nix { testing::internal::CaptureStderr(); logger->logEI({ .level = lvlInfo, - .name = "Info name", - .description = "Info description", - }); + .name = "Info name", + .description = "Info description", + }); auto str = testing::internal::GetCapturedStderr(); ASSERT_STREQ(str.c_str(), "\x1B[32;1minfo:\x1B[0m\x1B[34;1m --- Info name --- error-unit-test\x1B[0m\nInfo description\n"); @@ -86,7 +86,7 @@ namespace nix { logger->logEI({ .level = lvlTalkative, .name = "Talkative name", .description = "Talkative description", - }); + }); auto str = testing::internal::GetCapturedStderr(); ASSERT_STREQ(str.c_str(), "\x1B[32;1mtalk:\x1B[0m\x1B[34;1m --- Talkative name --- error-unit-test\x1B[0m\nTalkative description\n"); @@ -100,7 +100,7 @@ namespace nix { logger->logEI({ .level = lvlChatty, .name = "Chatty name", .description = "Talkative description", - }); + }); auto str = testing::internal::GetCapturedStderr(); ASSERT_STREQ(str.c_str(), "\x1B[32;1mchat:\x1B[0m\x1B[34;1m --- Chatty name --- error-unit-test\x1B[0m\nTalkative description\n"); @@ -114,7 +114,7 @@ namespace nix { logger->logEI({ .level = lvlDebug, .name = "Debug name", .description = "Debug description", - }); + }); auto str = testing::internal::GetCapturedStderr(); ASSERT_STREQ(str.c_str(), "\x1B[33;1mdebug:\x1B[0m\x1B[34;1m --- Debug name --- error-unit-test\x1B[0m\nDebug description\n"); @@ -128,7 +128,7 @@ namespace nix { logger->logEI({ .level = lvlVomit, .name = "Vomit name", .description = "Vomit description", - }); + }); auto str = testing::internal::GetCapturedStderr(); ASSERT_STREQ(str.c_str(), "\x1B[32;1mvomit:\x1B[0m\x1B[34;1m --- Vomit name --- error-unit-test\x1B[0m\nVomit description\n"); @@ -145,7 +145,7 @@ namespace nix { logError({ .name = "name", .description = "error description", - }); + }); auto str = testing::internal::GetCapturedStderr(); ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- name --- error-unit-test\x1B[0m\nerror description\n"); @@ -161,13 +161,13 @@ namespace nix { .name = "error name", .description = "error with code lines", .hint = hintfmt("this hint has %1% templated %2%!!", - "yellow", - "values"), + "yellow", + "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13), - .prevLineOfCode = "previous line of code", - .errLineOfCode = "this is the problem line of code", - .nextLineOfCode = "next line of code", + .errPos = Pos(problem_file, 40, 13), + .prevLineOfCode = "previous line of code", + .errLineOfCode = "this is the problem line of code", + .nextLineOfCode = "next line of code", }}); @@ -184,10 +184,10 @@ namespace nix { .name = "error name", .description = "error without any code lines.", .hint = hintfmt("this hint has %1% templated %2%!!", - "yellow", - "values"), + "yellow", + "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13) + .errPos = Pos(problem_file, 40, 13) }}); auto str = testing::internal::GetCapturedStderr(); @@ -203,7 +203,7 @@ namespace nix { .name = "error name", .hint = hintfmt("hint %1%", "only"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13) + .errPos = Pos(problem_file, 40, 13) }}); auto str = testing::internal::GetCapturedStderr(); @@ -219,10 +219,10 @@ namespace nix { testing::internal::CaptureStderr(); logWarning({ - .name = "name", - .description = "error description", - .hint = hintfmt("there was a %1%", "warning"), - }); + .name = "name", + .description = "error description", + .hint = hintfmt("there was a %1%", "warning"), + }); auto str = testing::internal::GetCapturedStderr(); ASSERT_STREQ(str.c_str(), "\x1B[33;1mwarning:\x1B[0m\x1B[34;1m --- name --- error-unit-test\x1B[0m\nerror description\n\nthere was a \x1B[33;1mwarning\x1B[0m\n"); @@ -239,13 +239,13 @@ namespace nix { .name = "warning name", .description = "warning description", .hint = hintfmt("this hint has %1% templated %2%!!", - "yellow", - "values"), + "yellow", + "values"), .nixCode = NixCode { - .errPos = Pos(problem_file, 40, 13), - .prevLineOfCode = std::nullopt, - .errLineOfCode = "this is the problem line of code", - .nextLineOfCode = std::nullopt + .errPos = Pos(problem_file, 40, 13), + .prevLineOfCode = std::nullopt, + .errLineOfCode = "this is the problem line of code", + .nextLineOfCode = std::nullopt }}); @@ -257,17 +257,28 @@ namespace nix { * hintfmt * --------------------------------------------------------------------------*/ - TEST(hintfmt, withstandsPercentString) { + TEST(hintfmt, percentStringWithoutArgs) { - const char *teststr = "this is 100%s correct!"; - auto hint = hintfmt(teststr); + const char *teststr = "this is 100%s correct!"; + auto hint = hintfmt(teststr); - std::ofstream meh("meh.txt"); - meh << hint.str() << std::endl; + ASSERT_STREQ(hint.str().c_str(), teststr); - ASSERT_STREQ(hint.str().c_str(), teststr); - } + } - + TEST(hintfmt, tooFewArguments) { + ASSERT_STREQ( + hintfmt("only one arg %1% %2%", "fulfilled").str().c_str(), + "only one arg " ANSI_YELLOW "fulfilled" ANSI_NORMAL " "); + + } + + TEST(hintfmt, tooManyArguments) { + + ASSERT_STREQ( + hintfmt("what about this %1% %2%", "%3%", "one", "two").str().c_str(), + "what about this " ANSI_YELLOW "%3%" ANSI_NORMAL " " ANSI_YELLOW "one" ANSI_NORMAL); + + } } From b193aca4ae339fb20184827e28523169d2d5370a Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 19 Jun 2020 15:29:19 -0600 Subject: [PATCH 05/21] escape percents --- src/libutil/fmt.hh | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh index 12ab9c407..ccab1c512 100644 --- a/src/libutil/fmt.hh +++ b/src/libutil/fmt.hh @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include "ansicolor.hh" @@ -103,7 +104,9 @@ class hintformat public: hintformat(const string &format) :fmt(format) { - fmt.exceptions(boost::io::all_error_bits ^ boost::io::too_many_args_bit); + fmt.exceptions(boost::io::all_error_bits ^ + boost::io::too_many_args_bit ^ + boost::io::too_few_args_bit); } hintformat(const hintformat &hf) @@ -136,4 +139,12 @@ inline hintformat hintfmt(const std::string & fs, const Args & ... args) return f; } +inline hintformat hintfmt(std::string fs) +{ + // we won't be receiving any args in this case, so escape all percents. + boost::replace_all(fs, "%", "%%"); + hintformat f(fs); + formatHelper(f); + return f; +} } From 507aa48739f23f9a16c8b7079bfa6fc1806be78e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 18:41:33 +0000 Subject: [PATCH 06/21] WIP: Make Hash always store a valid hash type --- perl/lib/Nix/Store.xs | 4 +- src/libexpr/primops.cc | 8 +-- src/libexpr/primops/fetchTree.cc | 4 +- src/libfetchers/tree-info.cc | 2 +- src/libfetchers/tree-info.hh | 2 +- src/libstore/binary-cache-store.cc | 6 +-- src/libstore/build.cc | 12 +++-- src/libstore/builtins/fetchurl.cc | 2 +- src/libstore/daemon.cc | 4 +- src/libstore/derivations.cc | 2 +- src/libstore/export-import.cc | 4 +- src/libstore/legacy-ssh-store.cc | 4 +- src/libstore/local-store.cc | 26 ++++----- src/libstore/nar-info-disk-cache.cc | 4 +- src/libstore/nar-info.cc | 27 +++++----- src/libstore/nar-info.hh | 2 +- src/libstore/references.cc | 8 +-- src/libstore/references.hh | 3 +- src/libstore/remote-store.cc | 2 +- src/libstore/store-api.cc | 8 +-- src/libstore/store-api.hh | 3 +- src/libutil/hash.cc | 68 +++++++++++++----------- src/libutil/hash.hh | 17 +++--- src/nix-prefetch-url/nix-prefetch-url.cc | 2 +- src/nix-store/nix-store.cc | 10 ++-- src/nix/add-to-store.cc | 4 +- src/nix/develop.cc | 2 +- src/nix/make-content-addressable.cc | 2 +- src/nix/verify.cc | 8 +-- 29 files changed, 126 insertions(+), 124 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 945ed49c7..2a2a0d429 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -80,7 +80,7 @@ SV * queryReferences(char * path) SV * queryPathHash(char * path) PPCODE: try { - auto s = store()->queryPathInfo(store()->parseStorePath(path))->narHash.to_string(Base32, true); + auto s = store()->queryPathInfo(store()->parseStorePath(path))->narHash->to_string(Base32, true); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); } catch (Error & e) { croak("%s", e.what()); @@ -106,7 +106,7 @@ SV * queryPathInfo(char * path, int base32) XPUSHs(&PL_sv_undef); else XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(*info->deriver).c_str(), 0))); - auto s = info->narHash.to_string(base32 ? Base32 : Base16, true); + auto s = info->narHash->to_string(base32 ? Base32 : Base16, true); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); mXPUSHi(info->registrationTime); mXPUSHi(info->narSize); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f5fbd3fa6..0dd5624b2 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1122,7 +1122,7 @@ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Valu static void addPath(EvalState & state, const Pos & pos, const string & name, const Path & path_, - Value * filterFun, FileIngestionMethod method, const Hash & expectedHash, Value & v) + Value * filterFun, FileIngestionMethod method, const std::optional expectedHash, Value & v) { const auto path = evalSettings.pureEval && expectedHash ? path_ : @@ -1153,7 +1153,7 @@ static void addPath(EvalState & state, const Pos & pos, const string & name, con std::optional expectedStorePath; if (expectedHash) - expectedStorePath = state.store->makeFixedOutputPath(method, expectedHash, name); + expectedStorePath = state.store->makeFixedOutputPath(method, *expectedHash, name); Path dstPath; if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) { dstPath = state.store->printStorePath(settings.readOnlyMode @@ -1187,7 +1187,7 @@ static void prim_filterSource(EvalState & state, const Pos & pos, Value * * args .nixCode = NixCode { .errPos = pos } }); - addPath(state, pos, std::string(baseNameOf(path)), path, args[0], FileIngestionMethod::Recursive, Hash(), v); + addPath(state, pos, std::string(baseNameOf(path)), path, args[0], FileIngestionMethod::Recursive, std::nullopt, v); } static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value & v) @@ -1197,7 +1197,7 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value string name; Value * filterFun = nullptr; auto method = FileIngestionMethod::Recursive; - Hash expectedHash; + Hash expectedHash(htSHA256); for (auto & attr : *args[0]->attrs) { const string & n(attr.name); diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 9be93710a..d23d47592 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -23,7 +23,7 @@ void emitTreeAttrs( assert(tree.info.narHash); mkString(*state.allocAttr(v, state.symbols.create("narHash")), - tree.info.narHash.to_string(SRI, true)); + tree.info.narHash->to_string(SRI, true)); if (input->getRev()) { mkString(*state.allocAttr(v, state.symbols.create("rev")), input->getRev()->gitRev()); @@ -147,7 +147,7 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v, : hashFile(htSHA256, path); if (hash != *expectedHash) throw Error((unsigned int) 102, "hash mismatch in file downloaded from '%s':\n wanted: %s\n got: %s", - *url, expectedHash->to_string(Base32, true), hash.to_string(Base32, true)); + *url, expectedHash->to_string(Base32, true), hash->to_string(Base32, true)); } if (state.allowedPaths) diff --git a/src/libfetchers/tree-info.cc b/src/libfetchers/tree-info.cc index b2d8cfc8d..432aa6182 100644 --- a/src/libfetchers/tree-info.cc +++ b/src/libfetchers/tree-info.cc @@ -8,7 +8,7 @@ namespace nix::fetchers { StorePath TreeInfo::computeStorePath(Store & store) const { assert(narHash); - return store.makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, "source"); + return store.makeFixedOutputPath(FileIngestionMethod::Recursive, *narHash, "source"); } } diff --git a/src/libfetchers/tree-info.hh b/src/libfetchers/tree-info.hh index 2c7347281..9d1872097 100644 --- a/src/libfetchers/tree-info.hh +++ b/src/libfetchers/tree-info.hh @@ -11,7 +11,7 @@ namespace nix::fetchers { struct TreeInfo { - Hash narHash; + std::optional narHash; std::optional revCount; std::optional lastModified; diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 9f52ddafa..98a3eaebb 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -181,7 +181,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource ((1.0 - (double) narCompressed->size() / nar->size()) * 100.0), duration); - narInfo->url = "nar/" + narInfo->fileHash.to_string(Base32, false) + ".nar" + narInfo->url = "nar/" + narInfo->fileHash->to_string(Base32, false) + ".nar" + (compression == "xz" ? ".xz" : compression == "bzip2" ? ".bz2" : compression == "br" ? ".br" : @@ -338,7 +338,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath method for very large paths, but `copyPath' is mainly used for small files. */ StringSink sink; - Hash h; + std::optional h; if (method == FileIngestionMethod::Recursive) { dumpPath(srcPath, sink, filter); h = hashString(hashAlgo, *sink.s); @@ -348,7 +348,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath h = hashString(hashAlgo, s); } - ValidPathInfo info(makeFixedOutputPath(method, h, name)); + ValidPathInfo info(makeFixedOutputPath(method, *h, name)); auto source = StringSource { *sink.s }; addToStore(info, source, repair, CheckSigs, nullptr); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 82a2ab831..80351a675 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3730,8 +3730,8 @@ void DerivationGoal::registerOutputs() /* Check the hash. In hash mode, move the path produced by the derivation to its content-addressed location. */ Hash h2 = i.second.hash->method == FileIngestionMethod::Recursive - ? hashPath(*i.second.hash->hash.type, actualPath).first - : hashFile(*i.second.hash->hash.type, actualPath); + ? hashPath(i.second.hash->hash.type, actualPath).first + : hashFile(i.second.hash->hash.type, actualPath); auto dest = worker.store.makeFixedOutputPath(i.second.hash->method, h2, i.second.path.name()); @@ -3777,8 +3777,10 @@ void DerivationGoal::registerOutputs() time. The hash is stored in the database so that we can verify later on whether nobody has messed with the store. */ debug("scanning for references inside '%1%'", path); - HashResult hash; - auto references = worker.store.parseStorePathSet(scanForReferences(actualPath, worker.store.printStorePathSet(referenceablePaths), hash)); + // HashResult hash; + auto pathSetAndHash = scanForReferences(actualPath, worker.store.printStorePathSet(referenceablePaths)); + auto references = worker.store.parseStorePathSet(pathSetAndHash.first); + HashResult hash = pathSetAndHash.second; if (buildMode == bmCheck) { if (!worker.store.isValidPath(worker.store.parseStorePath(path))) continue; @@ -4997,7 +4999,7 @@ bool Worker::pathContentsGood(const StorePath & path) if (!pathExists(store.printStorePath(path))) res = false; else { - HashResult current = hashPath(*info->narHash.type, store.printStorePath(path)); + HashResult current = hashPath(info->narHash->type, store.printStorePath(path)); Hash nullHash(htSHA256); res = info->narHash == nullHash || info->narHash == current.first; } diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 1cfe4a46a..f3827684b 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -65,7 +65,7 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/'; auto ht = parseHashTypeOpt(getAttr("outputHashAlgo")); auto h = Hash(getAttr("outputHash"), ht); - fetch(hashedMirror + printHashType(*h.type) + "/" + h.to_string(Base16, false)); + fetch(hashedMirror + printHashType(h.type) + "/" + h.to_string(Base16, false)); return; } catch (Error & e) { debug(e.what()); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index e370e278c..296bfad5d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -314,7 +314,7 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); auto hash = store->queryPathInfo(path)->narHash; logger->stopWork(); - to << hash.to_string(Base16, false); + to << hash->to_string(Base16, false); break; } @@ -646,7 +646,7 @@ static void performOp(TunnelLogger * logger, ref store, if (GET_PROTOCOL_MINOR(clientVersion) >= 17) to << 1; to << (info->deriver ? store->printStorePath(*info->deriver) : "") - << info->narHash.to_string(Base16, false); + << info->narHash->to_string(Base16, false); writeStorePaths(*store, to, info->references); to << info->registrationTime << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 16) { diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index a79b78db6..53bd4da2a 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -9,7 +9,7 @@ namespace nix { std::string DerivationOutputHash::printMethodAlgo() const { - return makeFileIngestionPrefix(method) + printHashType(*hash.type); + return makeFileIngestionPrefix(method) + printHashType(hash.type); } diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index 57b7e9590..9b8cc5c3a 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -55,9 +55,9 @@ void Store::exportPath(const StorePath & path, Sink & sink) filesystem corruption from spreading to other machines. Don't complain if the stored hash is zero (unknown). */ Hash hash = hashAndWriteSink.currentHash(); - if (hash != info->narHash && info->narHash != Hash(*info->narHash.type)) + if (hash != info->narHash && info->narHash != Hash(info->narHash->type)) throw Error("hash of path '%s' has changed from '%s' to '%s'!", - printStorePath(path), info->narHash.to_string(Base32, true), hash.to_string(Base32, true)); + printStorePath(path), info->narHash->to_string(Base32, true), hash.to_string(Base32, true)); hashAndWriteSink << exportMagic diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 45c70fad6..d3880459c 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -113,7 +113,7 @@ struct LegacySSHStore : public Store if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) { auto s = readString(conn->from); - info->narHash = s.empty() ? Hash() : Hash(s); + info->narHash = s.empty() ? std::optional{} : Hash(s); conn->from >> info->ca; info->sigs = readStrings(conn->from); } @@ -139,7 +139,7 @@ struct LegacySSHStore : public Store << cmdAddToStoreNar << printStorePath(info.path) << (info.deriver ? printStorePath(*info.deriver) : "") - << info.narHash.to_string(Base16, false); + << info.narHash->to_string(Base16, false); writeStorePaths(*this, conn->to, info.references); conn->to << info.registrationTime diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c6b55ff7c..aa2ee5b30 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -586,7 +586,7 @@ uint64_t LocalStore::addValidPath(State & state, state.stmtRegisterValidPath.use() (printStorePath(info.path)) - (info.narHash.to_string(Base16, true)) + (info.narHash->to_string(Base16, true)) (info.registrationTime == 0 ? time(0) : info.registrationTime) (info.deriver ? printStorePath(*info.deriver) : "", (bool) info.deriver) (info.narSize, info.narSize != 0) @@ -686,7 +686,7 @@ void LocalStore::updatePathInfo(State & state, const ValidPathInfo & info) { state.stmtUpdatePathInfo.use() (info.narSize, info.narSize != 0) - (info.narHash.to_string(Base16, true)) + (info.narHash->to_string(Base16, true)) (info.ultimate ? 1 : 0, info.ultimate) (concatStringsSep(" ", info.sigs), !info.sigs.empty()) (info.ca, !info.ca.empty()) @@ -897,7 +897,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) StorePathSet paths; for (auto & i : infos) { - assert(i.narHash.type == htSHA256); + assert(i.narHash && i.narHash->type == htSHA256); if (isValidPath_(*state, i.path)) updatePathInfo(*state, i); else @@ -1010,7 +1010,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, if (hashResult.first != info.narHash) throw Error("hash mismatch importing path '%s';\n wanted: %s\n got: %s", - printStorePath(info.path), info.narHash.to_string(Base32, true), hashResult.first.to_string(Base32, true)); + printStorePath(info.path), info.narHash->to_string(Base32, true), hashResult.first.to_string(Base32, true)); if (hashResult.second != info.narSize) throw Error("size mismatch importing path '%s';\n wanted: %s\n got: %s", @@ -1067,12 +1067,12 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam the path in the database. We may just have computed it above (if called with recursive == true and hashAlgo == sha256); otherwise, compute it here. */ - HashResult hash; - if (method == FileIngestionMethod::Recursive) { - hash.first = hashAlgo == htSHA256 ? h : hashString(htSHA256, dump); - hash.second = dump.size(); - } else - hash = hashPath(htSHA256, realPath); + HashResult hash = method == FileIngestionMethod::Recursive + ? HashResult { + hashAlgo == htSHA256 ? h : hashString(htSHA256, dump), + dump.size(), + } + : hashPath(htSHA256, realPath); optimisePath(realPath); // FIXME: combine with hashPath() @@ -1255,9 +1255,9 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) std::unique_ptr hashSink; if (info->ca == "" || !info->references.count(info->path)) - hashSink = std::make_unique(*info->narHash.type); + hashSink = std::make_unique(info->narHash->type); else - hashSink = std::make_unique(*info->narHash.type, std::string(info->path.hashPart())); + hashSink = std::make_unique(info->narHash->type, std::string(info->path.hashPart())); dumpPath(Store::toRealPath(i), *hashSink); auto current = hashSink->finish(); @@ -1266,7 +1266,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) logError({ .name = "Invalid hash - path modified", .hint = hintfmt("path '%s' was modified! expected hash '%s', got '%s'", - printStorePath(i), info->narHash.to_string(Base32, true), current.first.to_string(Base32, true)) + printStorePath(i), info->narHash->to_string(Base32, true), current.first.to_string(Base32, true)) }); if (repair) repairPath(i); else errors = true; } else { diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 552970248..6036b905e 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -230,9 +230,9 @@ public: (std::string(info->path.name())) (narInfo ? narInfo->url : "", narInfo != 0) (narInfo ? narInfo->compression : "", narInfo != 0) - (narInfo && narInfo->fileHash ? narInfo->fileHash.to_string(Base32, true) : "", narInfo && narInfo->fileHash) + (narInfo && narInfo->fileHash ? narInfo->fileHash->to_string(Base32, true) : "", narInfo && narInfo->fileHash) (narInfo ? narInfo->fileSize : 0, narInfo != 0 && narInfo->fileSize) - (info->narHash.to_string(Base32, true)) + (info->narHash->to_string(Base32, true)) (info->narSize) (concatStringsSep(" ", info->shortRefs())) (info->deriver ? std::string(info->deriver->to_string()) : "", (bool) info->deriver) diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index bb4448c90..fb84b0410 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -7,15 +7,14 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & : ValidPathInfo(StorePath(StorePath::dummy)) // FIXME: hack { auto corrupt = [&]() { - throw Error("NAR info file '%1%' is corrupt", whence); + return Error("NAR info file '%1%' is corrupt", whence); }; auto parseHashField = [&](const string & s) { try { return Hash(s); } catch (BadHash &) { - corrupt(); - return Hash(); // never reached + throw corrupt(); } }; @@ -25,12 +24,12 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & while (pos < s.size()) { size_t colon = s.find(':', pos); - if (colon == std::string::npos) corrupt(); + if (colon == std::string::npos) throw corrupt(); std::string name(s, pos, colon - pos); size_t eol = s.find('\n', colon + 2); - if (eol == std::string::npos) corrupt(); + if (eol == std::string::npos) throw corrupt(); std::string value(s, colon + 2, eol - colon - 2); @@ -45,16 +44,16 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & else if (name == "FileHash") fileHash = parseHashField(value); else if (name == "FileSize") { - if (!string2Int(value, fileSize)) corrupt(); + if (!string2Int(value, fileSize)) throw corrupt(); } else if (name == "NarHash") narHash = parseHashField(value); else if (name == "NarSize") { - if (!string2Int(value, narSize)) corrupt(); + if (!string2Int(value, narSize)) throw corrupt(); } else if (name == "References") { auto refs = tokenizeString(value, " "); - if (!references.empty()) corrupt(); + if (!references.empty()) throw corrupt(); for (auto & r : refs) references.insert(StorePath(r)); } @@ -67,7 +66,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & else if (name == "Sig") sigs.insert(value); else if (name == "CA") { - if (!ca.empty()) corrupt(); + if (!ca.empty()) throw corrupt(); ca = value; } @@ -76,7 +75,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & if (compression == "") compression = "bzip2"; - if (!havePath || url.empty() || narSize == 0 || !narHash) corrupt(); + if (!havePath || url.empty() || narSize == 0 || !narHash) throw corrupt(); } std::string NarInfo::to_string(const Store & store) const @@ -86,11 +85,11 @@ std::string NarInfo::to_string(const Store & store) const res += "URL: " + url + "\n"; assert(compression != ""); res += "Compression: " + compression + "\n"; - assert(fileHash.type == htSHA256); - res += "FileHash: " + fileHash.to_string(Base32, true) + "\n"; + assert(fileHash && fileHash->type == htSHA256); + res += "FileHash: " + fileHash->to_string(Base32, true) + "\n"; res += "FileSize: " + std::to_string(fileSize) + "\n"; - assert(narHash.type == htSHA256); - res += "NarHash: " + narHash.to_string(Base32, true) + "\n"; + assert(narHash && narHash->type == htSHA256); + res += "NarHash: " + narHash->to_string(Base32, true) + "\n"; res += "NarSize: " + std::to_string(narSize) + "\n"; res += "References: " + concatStringsSep(" ", shortRefs()) + "\n"; diff --git a/src/libstore/nar-info.hh b/src/libstore/nar-info.hh index 373c33427..eff19f0ef 100644 --- a/src/libstore/nar-info.hh +++ b/src/libstore/nar-info.hh @@ -10,7 +10,7 @@ struct NarInfo : ValidPathInfo { std::string url; std::string compression; - Hash fileHash; + std::optional fileHash; uint64_t fileSize = 0; std::string system; diff --git a/src/libstore/references.cc b/src/libstore/references.cc index a10d536a3..4733bc388 100644 --- a/src/libstore/references.cc +++ b/src/libstore/references.cc @@ -79,8 +79,8 @@ void RefScanSink::operator () (const unsigned char * data, size_t len) } -PathSet scanForReferences(const string & path, - const PathSet & refs, HashResult & hash) +std::pair scanForReferences(const string & path, + const PathSet & refs) { RefScanSink sink; std::map backMap; @@ -112,9 +112,9 @@ PathSet scanForReferences(const string & path, found.insert(j->second); } - hash = sink.hashSink.finish(); + auto hash = sink.hashSink.finish(); - return found; + return std::pair(found, hash); } diff --git a/src/libstore/references.hh b/src/libstore/references.hh index c38bdd720..598a3203a 100644 --- a/src/libstore/references.hh +++ b/src/libstore/references.hh @@ -5,8 +5,7 @@ namespace nix { -PathSet scanForReferences(const Path & path, const PathSet & refs, - HashResult & hash); +std::pair scanForReferences(const Path & path, const PathSet & refs); struct RewritingSink : Sink { diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index f5f2ab7fd..c206a4538 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -462,7 +462,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, conn->to << wopAddToStoreNar << printStorePath(info.path) << (info.deriver ? printStorePath(*info.deriver) : "") - << info.narHash.to_string(Base16, false); + << info.narHash->to_string(Base16, false); writeStorePaths(*this, conn->to, info.references); conn->to << info.registrationTime << info.narSize << info.ultimate << info.sigs << info.ca diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 982fc22b6..c74aeec48 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -430,7 +430,7 @@ string Store::makeValidityRegistration(const StorePathSet & paths, auto info = queryPathInfo(i); if (showHash) { - s += info->narHash.to_string(Base16, false) + "\n"; + s += info->narHash->to_string(Base16, false) + "\n"; s += (format("%1%\n") % info->narSize).str(); } @@ -462,7 +462,7 @@ void Store::pathInfoToJSON(JSONPlaceholder & jsonOut, const StorePathSet & store auto info = queryPathInfo(storePath); jsonPath - .attr("narHash", info->narHash.to_string(hashBase, true)) + .attr("narHash", info->narHash->to_string(hashBase, true)) .attr("narSize", info->narSize); { @@ -505,7 +505,7 @@ void Store::pathInfoToJSON(JSONPlaceholder & jsonOut, const StorePathSet & store if (!narInfo->url.empty()) jsonPath.attr("url", narInfo->url); if (narInfo->fileHash) - jsonPath.attr("downloadHash", narInfo->fileHash.to_string(Base32, true)); + jsonPath.attr("downloadHash", narInfo->fileHash->to_string(Base32, true)); if (narInfo->fileSize) jsonPath.attr("downloadSize", narInfo->fileSize); if (showClosureSize) @@ -746,7 +746,7 @@ std::string ValidPathInfo::fingerprint(const Store & store) const store.printStorePath(path)); return "1;" + store.printStorePath(path) + ";" - + narHash.to_string(Base32, true) + ";" + + narHash->to_string(Base32, true) + ";" + std::to_string(narSize) + ";" + concatStringsSep(",", store.printStorePathSet(references)); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a05048290..de9b6a791 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -116,7 +116,8 @@ struct ValidPathInfo { StorePath path; std::optional deriver; - Hash narHash; + // TODO document this + std::optional narHash; StorePathSet references; time_t registrationTime = 0; uint64_t narSize = 0; // 0 = unknown diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index e49eb4569..35a449530 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -16,16 +16,19 @@ namespace nix { +static size_t regularHashSize(HashType type) { + switch (type) { + case htMD5: return md5HashSize; + case htSHA1: return sha1HashSize; + case htSHA256: return sha256HashSize; + case htSHA512: return sha512HashSize; + } + abort(); +} void Hash::init() { - if (!type) abort(); - switch (*type) { - case htMD5: hashSize = md5HashSize; break; - case htSHA1: hashSize = sha1HashSize; break; - case htSHA256: hashSize = sha256HashSize; break; - case htSHA512: hashSize = sha512HashSize; break; - } + hashSize = regularHashSize(type); assert(hashSize <= maxHashSize); memset(hash, 0, maxHashSize); } @@ -105,18 +108,11 @@ string printHash16or32(const Hash & hash) } -HashType assertInitHashType(const Hash & h) { - if (h.type) - return *h.type; - else - abort(); -} - std::string Hash::to_string(Base base, bool includeType) const { std::string s; if (base == SRI || includeType) { - s += printHashType(assertInitHashType(*this)); + s += printHashType(type); s += base == SRI ? '-' : ':'; } switch (base) { @@ -137,31 +133,39 @@ std::string Hash::to_string(Base base, bool includeType) const Hash::Hash(std::string_view s, HashType type) : Hash(s, std::optional { type }) { } Hash::Hash(std::string_view s) : Hash(s, std::optional{}) { } -Hash::Hash(std::string_view s, std::optional type) - : type(type) +Hash::Hash(std::string_view s, std::optional optType) { size_t pos = 0; bool isSRI = false; + // Find the : or - separater, and set `isSRI` to the correct value auto sep = s.find(':'); - if (sep == string::npos) { + if (sep == std::string_view::npos) { sep = s.find('-'); - if (sep != string::npos) { + if (sep != std::string_view::npos) isSRI = true; - } else if (! type) - throw BadHash("hash '%s' does not include a type", s); } - if (sep != string::npos) { - string hts = string(s, 0, sep); - this->type = parseHashType(hts); - if (!this->type) + // Parse the has type before the separater, if there was one. + std::optional optParsedType; + if (sep != std::string_view::npos) { + auto hts = s.substr(0, sep); + auto optParsedType = parseHashTypeOpt(hts); + if (!optParsedType) throw BadHash("unknown hash type '%s'", hts); - if (type && type != this->type) - throw BadHash("hash '%s' should have type '%s'", s, printHashType(*type)); pos = sep + 1; } + // Either the string or user must provide the type, if they both do they + // must agree. + if (!optParsedType && !optType) { + throw BadHash("hash '%s' does not include a type, nor is the type otherwise known from context.", s); + } else { + this->type = optParsedType ? *optParsedType : *optType; + if (optParsedType && optType && *optParsedType != *optType) + throw BadHash("hash '%s' should have type '%s'", s, printHashType(*optType)); + } + init(); size_t size = s.size() - pos; @@ -214,7 +218,7 @@ Hash::Hash(std::string_view s, std::optional type) } else - throw BadHash("hash '%s' has wrong length for hash type '%s'", s, printHashType(*type)); + throw BadHash("hash '%s' has wrong length for hash type '%s'", s, printHashType(type)); } Hash newHashAllowEmpty(std::string hashStr, std::optional ht) @@ -267,7 +271,7 @@ static void finish(HashType ht, Ctx & ctx, unsigned char * hash) } -Hash hashString(HashType ht, const string & s) +Hash hashString(HashType ht, std::string_view s) { Ctx ctx; Hash hash(ht); @@ -334,7 +338,7 @@ HashResult hashPath( Hash compressHash(const Hash & hash, unsigned int newSize) { - Hash h; + Hash h(hash.type); h.hashSize = newSize; for (unsigned int i = 0; i < hash.hashSize; ++i) h.hash[i % newSize] ^= hash.hash[i]; @@ -342,7 +346,7 @@ Hash compressHash(const Hash & hash, unsigned int newSize) } -std::optional parseHashTypeOpt(const string & s) +std::optional parseHashTypeOpt(std::string_view s) { if (s == "md5") return htMD5; else if (s == "sha1") return htSHA1; @@ -351,7 +355,7 @@ std::optional parseHashTypeOpt(const string & s) else return std::optional {}; } -HashType parseHashType(const string & s) +HashType parseHashType(std::string_view s) { auto opt_h = parseHashTypeOpt(s); if (opt_h) diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 0d9916508..b51850ccf 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -25,14 +25,11 @@ enum Base : int { Base64, Base32, Base16, SRI }; struct Hash { - static const unsigned int maxHashSize = 64; - unsigned int hashSize = 0; - unsigned char hash[maxHashSize] = {}; + constexpr static size_t maxHashSize = 64; + size_t hashSize = 0; + uint8_t hash[maxHashSize] = {}; - std::optional type = {}; - - /* Create an unset hash object. */ - Hash() { }; + HashType type; /* Create a zero-filled hash object. */ Hash(HashType type) : type(type) { init(); }; @@ -105,7 +102,7 @@ Hash newHashAllowEmpty(std::string hashStr, std::optional ht); string printHash16or32(const Hash & hash); /* Compute the hash of the given string. */ -Hash hashString(HashType ht, const string & s); +Hash hashString(HashType ht, std::string_view s); /* Compute the hash of the given file. */ Hash hashFile(HashType ht, const Path & path); @@ -121,9 +118,9 @@ HashResult hashPath(HashType ht, const Path & path, Hash compressHash(const Hash & hash, unsigned int newSize); /* Parse a string representing a hash type. */ -HashType parseHashType(const string & s); +HashType parseHashType(std::string_view s); /* Will return nothing on parse error */ -std::optional parseHashTypeOpt(const string & s); +std::optional parseHashTypeOpt(std::string_view s); /* And the reverse. */ string printHashType(HashType ht); diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 40b05a2f3..22410c44c 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -153,7 +153,7 @@ static int _main(int argc, char * * argv) /* If an expected hash is given, the file may already exist in the store. */ - Hash hash, expectedHash(ht); + Hash hash(ht), expectedHash(ht); std::optional storePath; if (args.size() == 2) { expectedHash = Hash(args[1], ht); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 4e02aa2bf..d061317ae 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -372,8 +372,8 @@ static void opQuery(Strings opFlags, Strings opArgs) for (auto & j : maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise)) { auto info = store->queryPathInfo(j); if (query == qHash) { - assert(info->narHash.type == htSHA256); - cout << fmt("%s\n", info->narHash.to_string(Base32, true)); + assert(info->narHash && info->narHash->type == htSHA256); + cout << fmt("%s\n", info->narHash->to_string(Base32, true)); } else if (query == qSize) cout << fmt("%d\n", info->narSize); } @@ -725,7 +725,7 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) auto path = store->followLinksToStorePath(i); printMsg(lvlTalkative, "checking path '%s'...", store->printStorePath(path)); auto info = store->queryPathInfo(path); - HashSink sink(*info->narHash.type); + HashSink sink(info->narHash->type); store->narFromPath(path, sink); auto current = sink.finish(); if (current.first != info->narHash) { @@ -734,7 +734,7 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) .hint = hintfmt( "path '%s' was modified! expected hash '%s', got '%s'", store->printStorePath(path), - info->narHash.to_string(Base32, true), + info->narHash->to_string(Base32, true), current.first.to_string(Base32, true)) }); status = 1; @@ -864,7 +864,7 @@ static void opServe(Strings opFlags, Strings opArgs) out << info->narSize // downloadSize << info->narSize; if (GET_PROTOCOL_MINOR(clientVersion) >= 4) - out << (info->narHash ? info->narHash.to_string(Base32, true) : "") << info->ca << info->sigs; + out << (info->narHash ? info->narHash->to_string(Base32, true) : "") << info->ca << info->sigs; } catch (InvalidPath &) { } } diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index f43f774c1..0dda2af38 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -46,9 +46,9 @@ struct CmdAddToStore : MixDryRun, StoreCommand auto narHash = hashString(htSHA256, *sink.s); ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, *namePart)); - info.narHash = narHash; + *info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); + info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, *info.narHash); if (!dryRun) { auto source = StringSource { *sink.s }; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 8b85caf82..4aee9f202 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -139,7 +139,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) .path = shellOutPath, .hash = DerivationOutputHash { .method = FileIngestionMethod::Flat, - .hash = Hash { }, + .hash = Hash { htSHA256 }, }, }); drv.env["out"] = store->printStorePath(shellOutPath); diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index 0ebb8f13b..a712dceef 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -82,7 +82,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON if (hasSelfReference) info.references.insert(info.path); info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); + info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, *info.narHash); if (!json) printInfo("rewrote '%s' to '%s'", pathS, store->printStorePath(info.path)); diff --git a/src/nix/verify.cc b/src/nix/verify.cc index d1aba08e3..c92f894f2 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -88,15 +88,15 @@ struct CmdVerify : StorePathsCommand std::unique_ptr hashSink; if (info->ca == "") - hashSink = std::make_unique(*info->narHash.type); + hashSink = std::make_unique(info->narHash->type); else - hashSink = std::make_unique(*info->narHash.type, std::string(info->path.hashPart())); + hashSink = std::make_unique(info->narHash->type, std::string(info->path.hashPart())); store->narFromPath(info->path, *hashSink); auto hash = hashSink->finish(); - if (hash.first != info->narHash) { + if (hash.first != *info->narHash) { corrupted++; act2.result(resCorruptedPath, store->printStorePath(info->path)); logError({ @@ -104,7 +104,7 @@ struct CmdVerify : StorePathsCommand .hint = hintfmt( "path '%s' was modified! expected hash '%s', got '%s'", store->printStorePath(info->path), - info->narHash.to_string(Base32, true), + info->narHash->to_string(Base32, true), hash.first.to_string(Base32, true)) }); } From 397dbe114e78682a370832e31269147b9d6ac6fa Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 19 Jun 2020 15:57:19 -0600 Subject: [PATCH 07/21] remove formathelper --- src/libutil/fmt.hh | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh index ccab1c512..789e500ef 100644 --- a/src/libutil/fmt.hh +++ b/src/libutil/fmt.hh @@ -144,7 +144,6 @@ inline hintformat hintfmt(std::string fs) // we won't be receiving any args in this case, so escape all percents. boost::replace_all(fs, "%", "%%"); hintformat f(fs); - formatHelper(f); return f; } } From 0309488a66ffc4e6672772e37bcec2f92019efc0 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 19 Jun 2020 16:46:49 -0600 Subject: [PATCH 08/21] fmt -> hintfmt test --- src/libutil/tests/logging.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index e208efc07..104f3e02c 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -266,6 +266,14 @@ namespace nix { } + TEST(hintfmt, fmtToHintfmt) { + + auto hint = hintfmt(fmt("the color of this this text is %1%", "not yellow")); + + ASSERT_STREQ(hint.str().c_str(), "the color of this this text is not yellow"); + + } + TEST(hintfmt, tooFewArguments) { ASSERT_STREQ( From be4f444175568fe48fdaf58387820a75bb16aeaa Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 19 Jun 2020 16:58:12 -0600 Subject: [PATCH 09/21] tidying up --- src/libutil/tests/logging.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 104f3e02c..31e3de697 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -260,17 +260,18 @@ namespace nix { TEST(hintfmt, percentStringWithoutArgs) { const char *teststr = "this is 100%s correct!"; - auto hint = hintfmt(teststr); - ASSERT_STREQ(hint.str().c_str(), teststr); + ASSERT_STREQ( + hintfmt(teststr).str().c_str(), + teststr); } TEST(hintfmt, fmtToHintfmt) { - auto hint = hintfmt(fmt("the color of this this text is %1%", "not yellow")); - - ASSERT_STREQ(hint.str().c_str(), "the color of this this text is not yellow"); + ASSERT_STREQ( + hintfmt(fmt("the color of this this text is %1%", "not yellow")).str().c_str(), + "the color of this this text is not yellow"); } From 73ac003b377aa4750de057bba2de949295524a9c Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 22 Jun 2020 10:03:19 -0400 Subject: [PATCH 10/21] More bug fixing --- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/local-store.cc | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 171903980..d3880459c 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -139,7 +139,7 @@ struct LegacySSHStore : public Store << cmdAddToStoreNar << printStorePath(info.path) << (info.deriver ? printStorePath(*info.deriver) : "") - << info.narHash.to_string(Base16, false); + << info.narHash->to_string(Base16, false); writeStorePaths(*this, conn->to, info.references); conn->to << info.registrationTime diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c6b55ff7c..e276576e1 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -586,7 +586,7 @@ uint64_t LocalStore::addValidPath(State & state, state.stmtRegisterValidPath.use() (printStorePath(info.path)) - (info.narHash.to_string(Base16, true)) + (info.narHash->to_string(Base16, true)) (info.registrationTime == 0 ? time(0) : info.registrationTime) (info.deriver ? printStorePath(*info.deriver) : "", (bool) info.deriver) (info.narSize, info.narSize != 0) @@ -686,7 +686,7 @@ void LocalStore::updatePathInfo(State & state, const ValidPathInfo & info) { state.stmtUpdatePathInfo.use() (info.narSize, info.narSize != 0) - (info.narHash.to_string(Base16, true)) + (info.narHash->to_string(Base16, true)) (info.ultimate ? 1 : 0, info.ultimate) (concatStringsSep(" ", info.sigs), !info.sigs.empty()) (info.ca, !info.ca.empty()) @@ -897,7 +897,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) StorePathSet paths; for (auto & i : infos) { - assert(i.narHash.type == htSHA256); + assert(i.narHash->type == htSHA256); if (isValidPath_(*state, i.path)) updatePathInfo(*state, i); else @@ -1010,7 +1010,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, if (hashResult.first != info.narHash) throw Error("hash mismatch importing path '%s';\n wanted: %s\n got: %s", - printStorePath(info.path), info.narHash.to_string(Base32, true), hashResult.first.to_string(Base32, true)); + printStorePath(info.path), info.narHash->to_string(Base32, true), hashResult.first.to_string(Base32, true)); if (hashResult.second != info.narSize) throw Error("size mismatch importing path '%s';\n wanted: %s\n got: %s", From f4a5913125cb6a8ccfba556a0d75b19c2a5cfa37 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 22 Jun 2020 15:17:20 +0000 Subject: [PATCH 11/21] hashed-mirrors: Use parsed derivation output rather than reconstructing it Now the derivation outputs are parsed up front, we can avoid a reparse by doing it. Also, this just feels a bit better as the `output*` env vars are more of a `libnixexpr` interface than `libnixstore` interface: ultimately, it's the derivation outputs that decide whether the derivation is fixed-output. Yes, hashed mirrors might go away with #3689, but this bit of code would be moved rather than deleted, so it's worth doing a cleanup anyways I think. --- src/libstore/builtins/fetchurl.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 1cfe4a46a..e630cf6f1 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -58,13 +58,16 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) } }; + /* We always have one output, and if it's a fixed-output derivation (as + checked below) it must be the only output */ + auto & output = drv.outputs.begin()->second; + /* Try the hashed mirrors first. */ - if (getAttr("outputHashMode") == "flat") + if (output.hash && output.hash->method == FileIngestionMethod::Flat) for (auto hashedMirror : settings.hashedMirrors.get()) try { if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/'; - auto ht = parseHashTypeOpt(getAttr("outputHashAlgo")); - auto h = Hash(getAttr("outputHash"), ht); + auto & h = output.hash->hash; fetch(hashedMirror + printHashType(*h.type) + "/" + h.to_string(Base16, false)); return; } catch (Error & e) { From 28b079067f4248c81b8946c459435c7a91dc2971 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Mon, 22 Jun 2020 10:00:37 -0600 Subject: [PATCH 12/21] Update src/libutil/fmt.hh Co-authored-by: John Ericson --- src/libutil/fmt.hh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh index 789e500ef..dc7fe8056 100644 --- a/src/libutil/fmt.hh +++ b/src/libutil/fmt.hh @@ -139,11 +139,9 @@ inline hintformat hintfmt(const std::string & fs, const Args & ... args) return f; } -inline hintformat hintfmt(std::string fs) +inline hintformat hintfmt(std::string plain_string) { - // we won't be receiving any args in this case, so escape all percents. - boost::replace_all(fs, "%", "%%"); - hintformat f(fs); - return f; + // we won't be receiving any args in this case, so just print the original string + return hintfmt("%s", plain_string); } } From 9d1cb0c5e64db3e34896ac43de978f132860f894 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Mon, 22 Jun 2020 11:32:20 -0600 Subject: [PATCH 13/21] with normaltxt, elide yellow color code instead of canceling it; use normaltxt on plain_string hintfmt --- src/libutil/fmt.hh | 9 ++++++++- src/libutil/tests/logging.cc | 5 ++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh index dc7fe8056..a39de041f 100644 --- a/src/libutil/fmt.hh +++ b/src/libutil/fmt.hh @@ -120,6 +120,13 @@ public: return *this; } + template + hintformat& operator%(const normaltxt &value) + { + fmt % value.value; + return *this; + } + std::string str() const { return fmt.str(); @@ -142,6 +149,6 @@ inline hintformat hintfmt(const std::string & fs, const Args & ... args) inline hintformat hintfmt(std::string plain_string) { // we won't be receiving any args in this case, so just print the original string - return hintfmt("%s", plain_string); + return hintfmt("%s", normaltxt(plain_string)); } } diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index 31e3de697..6a6fb4ac3 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -43,7 +43,7 @@ namespace nix { logger->logEI(ei); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- TestError --- error-unit-test\x1B[0m\n\x1B[33;1m\x1B[0minitial error\x1B[0m; subsequent error message.\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- TestError --- error-unit-test\x1B[0m\ninitial error; subsequent error message.\n"); } } @@ -61,8 +61,7 @@ namespace nix { logError(e.info()); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\n\x1B[33;1m\x1B[0mstatting file\x1B[0m: \x1B[33;1mBad file descriptor\x1B[0m\n"); - + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n"); } } From ec3a85730742497b29782c0b545911eb78d3c65e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 23 Jun 2020 18:19:40 +0000 Subject: [PATCH 14/21] Fix and clean up hash parser --- src/libutil/hash.cc | 62 ++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index f44210737..70a18f9a0 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -133,68 +133,66 @@ std::string Hash::to_string(Base base, bool includeType) const Hash::Hash(std::string_view s, HashType type) : Hash(s, std::optional { type }) { } Hash::Hash(std::string_view s) : Hash(s, std::optional{}) { } -Hash::Hash(std::string_view s, std::optional optType) +Hash::Hash(std::string_view original, std::optional optType) { + auto rest = original; + size_t pos = 0; bool isSRI = false; - // Find the : or - separater, and set `isSRI` to the correct value - auto sep = s.find(':'); - if (sep == std::string_view::npos) { - sep = s.find('-'); - if (sep != std::string_view::npos) - isSRI = true; - } - // Parse the has type before the separater, if there was one. std::optional optParsedType; - if (sep != std::string_view::npos) { - auto hts = s.substr(0, sep); - auto optParsedType = parseHashTypeOpt(hts); - if (!optParsedType) - throw BadHash("unknown hash type '%s'", hts); - pos = sep + 1; + { + auto sep = rest.find(':'); + if (sep == std::string_view::npos) { + sep = rest.find('-'); + if (sep != std::string_view::npos) + isSRI = true; + } + if (sep != std::string_view::npos) { + auto hashRaw = rest.substr(0, sep); + optParsedType = parseHashType(hashRaw); + rest = rest.substr(sep + 1); + } } // Either the string or user must provide the type, if they both do they // must agree. if (!optParsedType && !optType) { - throw BadHash("hash '%s' does not include a type, nor is the type otherwise known from context.", s); + throw BadHash("hash '%s' does not include a type, nor is the type otherwise known from context.", rest); } else { this->type = optParsedType ? *optParsedType : *optType; if (optParsedType && optType && *optParsedType != *optType) - throw BadHash("hash '%s' should have type '%s'", s, printHashType(*optType)); + throw BadHash("hash '%s' should have type '%s'", original, printHashType(*optType)); } init(); - size_t size = s.size() - pos; - - if (!isSRI && size == base16Len()) { + if (!isSRI && rest.size() == base16Len()) { auto parseHexDigit = [&](char c) { if (c >= '0' && c <= '9') return c - '0'; if (c >= 'A' && c <= 'F') return c - 'A' + 10; if (c >= 'a' && c <= 'f') return c - 'a' + 10; - throw BadHash("invalid base-16 hash '%s'", s); + throw BadHash("invalid base-16 hash '%s'", original); }; for (unsigned int i = 0; i < hashSize; i++) { hash[i] = - parseHexDigit(s[pos + i * 2]) << 4 - | parseHexDigit(s[pos + i * 2 + 1]); + parseHexDigit(rest[pos + i * 2]) << 4 + | parseHexDigit(rest[pos + i * 2 + 1]); } } - else if (!isSRI && size == base32Len()) { + else if (!isSRI && rest.size() == base32Len()) { - for (unsigned int n = 0; n < size; ++n) { - char c = s[pos + size - n - 1]; + for (unsigned int n = 0; n < rest.size(); ++n) { + char c = rest[rest.size() - n - 1]; unsigned char digit; for (digit = 0; digit < base32Chars.size(); ++digit) /* !!! slow */ if (base32Chars[digit] == c) break; if (digit >= 32) - throw BadHash("invalid base-32 hash '%s'", s); + throw BadHash("invalid base-32 hash '%s'", original); unsigned int b = n * 5; unsigned int i = b / 8; unsigned int j = b % 8; @@ -204,21 +202,21 @@ Hash::Hash(std::string_view s, std::optional optType) hash[i + 1] |= digit >> (8 - j); } else { if (digit >> (8 - j)) - throw BadHash("invalid base-32 hash '%s'", s); + throw BadHash("invalid base-32 hash '%s'", original); } } } - else if (isSRI || size == base64Len()) { - auto d = base64Decode(s.substr(pos)); + else if (isSRI || rest.size() == base64Len()) { + auto d = base64Decode(rest); if (d.size() != hashSize) - throw BadHash("invalid %s hash '%s'", isSRI ? "SRI" : "base-64", s); + throw BadHash("invalid %s hash '%s'", isSRI ? "SRI" : "base-64", original); assert(hashSize); memcpy(hash, d.data(), hashSize); } else - throw BadHash("hash '%s' has wrong length for hash type '%s'", s, printHashType(type)); + throw BadHash("hash '%s' has wrong length for hash type '%s'", rest, printHashType(this->type)); } Hash newHashAllowEmpty(std::string hashStr, std::optional ht) From 8d51d38e4ccf2ca0fa0b0471b32531fe287e38a4 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 23 Jun 2020 17:16:20 -0400 Subject: [PATCH 15/21] Fix test suite --- src/nix/add-to-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index de68e9501..ad1f9e91f 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -46,7 +46,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand auto narHash = hashString(htSHA256, *sink.s); ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, *namePart)); - *info.narHash = narHash; + info.narHash = narHash; info.narSize = sink.s->size(); info.ca = std::optional { FixedOutputHash { .method = FileIngestionMethod::Recursive, From 3685f4eec650be473226b9c1623a28ab07d1f918 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Tue, 23 Jun 2020 22:51:09 +0100 Subject: [PATCH 16/21] docs/installer: add correct curl flags also see https://nixos.org/download.html --- doc/manual/installation/installing-binary.xml | 6 +++--- scripts/install-multi-user.sh | 4 ++-- scripts/install-nix-from-closure.sh | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index d25c46b85..64c7a37fb 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -97,7 +97,7 @@ $ rm -rf /nix installation on your system: - sh <(curl https://nixos.org/nix/install) --daemon + sh <(curl -L https://nixos.org/nix/install) --daemon The multi-user installation of Nix will create build users between @@ -178,7 +178,7 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist is a bit of a misnomer). To use this approach, just install Nix with: - $ sh <(curl https://nixos.org/nix/install) --darwin-use-unencrypted-nix-store-volume + $ sh <(curl -L https://nixos.org/nix/install) --darwin-use-unencrypted-nix-store-volume If you don't like the sound of this, you'll want to weigh the @@ -429,7 +429,7 @@ LABEL=Nix\040Store /nix apfs rw,nobrowse NixOS.org installation script: - sh <(curl https://nixos.org/nix/install) + sh <(curl -L https://nixos.org/nix/install) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 157e8ddb4..00c9d540b 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -526,7 +526,7 @@ This script is going to call sudo a lot. Normally, it would show you exactly what commands it is running and why. However, the script is run in a headless fashion, like this: - $ curl https://nixos.org/nix/install | sh + $ curl -L https://nixos.org/nix/install | sh or maybe in a CI pipeline. Because of that, we're going to skip the verbose output in the interest of brevity. @@ -534,7 +534,7 @@ verbose output in the interest of brevity. If you would like to see the output, try like this: - $ curl -o install-nix https://nixos.org/nix/install + $ curl -L -o install-nix https://nixos.org/nix/install $ sh ./install-nix EOF diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 826ca8b8c..5824c2217 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -113,7 +113,7 @@ if [ "$(uname -s)" = "Darwin" ]; then ( echo "" echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." - echo "Use sh <(curl https://nixos.org/nix/install) --darwin-use-unencrypted-nix-store-volume or run the preparation steps manually." + echo "Use sh <(curl -L https://nixos.org/nix/install) --darwin-use-unencrypted-nix-store-volume or run the preparation steps manually." echo "See https://nixos.org/nix/manual/#sect-macos-installation" echo "" ) >&2 From 3a642187c33ed46d952d3a50a83b2576b704fab7 Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Thu, 25 Jun 2020 12:03:26 +0200 Subject: [PATCH 17/21] Fall back to copyPath if link fails with EPERM BeeGFS doesn't allow hard-links and returns EPERM, so we fall back to copyPath. See https://github.com/NixOS/nix/issues/3748 --- src/libstore/build.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0c25897f8..9ac5fd923 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1950,8 +1950,11 @@ void linkOrCopy(const Path & from, const Path & to) /* Hard-linking fails if we exceed the maximum link count on a file (e.g. 32000 of ext3), which is quite possible after a 'nix-store --optimise'. FIXME: actually, why don't we just - bind-mount in this case? */ - if (errno != EMLINK) + bind-mount in this case? + + It can also fail with EPERM in BeegFS v7 and earlier versions + which don't allow hard-links to other directories */ + if (errno != EMLINK && errno != EPERM) throw SysError("linking '%s' to '%s'", to, from); copyPath(from, to); } From de2641ae99965ecf88e6f72e3969c0c7b7e7020a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 25 Jun 2020 15:50:30 +0200 Subject: [PATCH 18/21] Fix empty std::optional dereference in writeDerivation() https://hydra.nixos.org/build/123017579 --- src/libstore/derivations.cc | 17 +++++++++++------ src/libutil/hash.cc | 27 ++++++++++++++------------- src/libutil/hash.hh | 2 +- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 6c49075ba..42551ef6b 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -404,7 +404,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) { auto path = store.parseStorePath(readString(in)); auto hashAlgo = readString(in); - const auto hash = readString(in); + auto hash = readString(in); std::optional fsh; if (hashAlgo != "") { @@ -413,7 +413,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) method = FileIngestionMethod::Recursive; hashAlgo = string(hashAlgo, 2); } - const HashType hashType = parseHashType(hashAlgo); + auto hashType = parseHashType(hashAlgo); fsh = FixedOutputHash { .method = std::move(method), .hash = Hash(hash, hashType), @@ -463,11 +463,16 @@ Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv) void writeDerivation(Sink & out, const Store & store, const BasicDerivation & drv) { out << drv.outputs.size(); - for (auto & i : drv.outputs) + for (auto & i : drv.outputs) { out << i.first - << store.printStorePath(i.second.path) - << i.second.hash->printMethodAlgo() - << i.second.hash->hash.to_string(Base16, false); + << store.printStorePath(i.second.path); + if (i.second.hash) { + out << i.second.hash->printMethodAlgo() + << i.second.hash->hash.to_string(Base16, false); + } else { + out << "" << ""; + } + } writeStorePaths(store, out, drv.inputSrcs); out << drv.platform << drv.builder << drv.args; out << drv.env.size(); diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index c8fcdfed0..01fae3044 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -19,7 +19,7 @@ namespace nix { void Hash::init() { - if (!type) abort(); + assert(type); switch (*type) { case htMD5: hashSize = md5HashSize; break; case htSHA1: hashSize = sha1HashSize; break; @@ -101,15 +101,15 @@ static string printHash32(const Hash & hash) string printHash16or32(const Hash & hash) { + assert(hash.type); return hash.to_string(hash.type == htMD5 ? Base16 : Base32, false); } -HashType assertInitHashType(const Hash & h) { - if (h.type) - return *h.type; - else - abort(); +HashType assertInitHashType(const Hash & h) +{ + assert(h.type); + return *h.type; } std::string Hash::to_string(Base base, bool includeType) const @@ -363,14 +363,15 @@ HashType parseHashType(const string & s) string printHashType(HashType ht) { switch (ht) { - case htMD5: return "md5"; break; - case htSHA1: return "sha1"; break; - case htSHA256: return "sha256"; break; - case htSHA512: return "sha512"; break; + case htMD5: return "md5"; + case htSHA1: return "sha1"; + case htSHA256: return "sha256"; + case htSHA512: return "sha512"; + default: + // illegal hash type enum value internally, as opposed to external input + // which should be validated with nice error message. + abort(); } - // illegal hash type enum value internally, as opposed to external input - // which should be validated with nice error message. - abort(); } } diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 0d9916508..23259dced 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -10,7 +10,7 @@ namespace nix { MakeError(BadHash, Error); -enum HashType : char { htMD5, htSHA1, htSHA256, htSHA512 }; +enum HashType : char { htMD5 = 42, htSHA1, htSHA256, htSHA512 }; const int md5HashSize = 16; From b7ccf7ae2af3d7eaf3696358ecbbce5a2bcfe652 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 25 Jun 2020 18:26:34 +0200 Subject: [PATCH 19/21] build-remote.sh: Test LegacySSHStore --- tests/build-hook.nix | 32 ++++++++++++++++++++++++-------- tests/build-remote.sh | 33 ++++++++++++++++++++------------- tests/common.sh.in | 1 + tests/post-hook.sh | 2 ++ tests/recursive.sh | 2 ++ tests/structured-attrs.sh | 2 ++ 6 files changed, 51 insertions(+), 21 deletions(-) diff --git a/tests/build-hook.nix b/tests/build-hook.nix index 8c5ca8cd3..a19c10dde 100644 --- a/tests/build-hook.nix +++ b/tests/build-hook.nix @@ -1,23 +1,39 @@ +{ busybox }: + with import ./config.nix; let + mkDerivation = args: + derivation ({ + inherit system; + builder = busybox; + args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")]; + } // removeAttrs args ["builder" "meta"]) + // { meta = args.meta or {}; }; + input1 = mkDerivation { - name = "build-hook-input-1"; - buildCommand = "mkdir $out; echo FOO > $out/foo"; + shell = busybox; + name = "build-remote-input-1"; + buildCommand = "echo FOO > $out"; requiredSystemFeatures = ["foo"]; }; input2 = mkDerivation { - name = "build-hook-input-2"; - buildCommand = "mkdir $out; echo BAR > $out/bar"; + shell = busybox; + name = "build-remote-input-2"; + buildCommand = "echo BAR > $out"; }; in mkDerivation { - name = "build-hook"; - builder = ./dependencies.builder0.sh; - input1 = " " + input1 + "/."; - input2 = " ${input2}/."; + shell = busybox; + name = "build-remote"; + buildCommand = + '' + read x < ${input1} + read y < ${input2} + echo $x$y > $out + ''; } diff --git a/tests/build-remote.sh b/tests/build-remote.sh index a550f4460..4dfb753e1 100644 --- a/tests/build-remote.sh +++ b/tests/build-remote.sh @@ -3,22 +3,29 @@ source common.sh clearStore if ! canUseSandbox; then exit; fi -if [[ ! $SHELL =~ /nix/store ]]; then exit; fi +if ! [[ $busybox =~ busybox ]]; then exit; fi -chmod -R u+w $TEST_ROOT/store0 || true -chmod -R u+w $TEST_ROOT/store1 || true -rm -rf $TEST_ROOT/store0 $TEST_ROOT/store1 +chmod -R u+w $TEST_ROOT/machine0 || true +chmod -R u+w $TEST_ROOT/machine1 || true +chmod -R u+w $TEST_ROOT/machine2 || true +rm -rf $TEST_ROOT/machine0 $TEST_ROOT/machine1 $TEST_ROOT/machine2 +rm -f $TEST_ROOT/result -nix build -f build-hook.nix -o $TEST_ROOT/result --max-jobs 0 \ - --sandbox-paths /nix/store --sandbox-build-dir /build-tmp \ - --builders "$TEST_ROOT/store0; $TEST_ROOT/store1 - - 1 1 foo" \ +unset NIX_STORE_DIR +unset NIX_STATE_DIR + +# Note: ssh://localhost bypasses ssh, directly invoking nix-store as a +# child process. This allows us to test LegacySSHStore::buildDerivation(). +nix build -L -v -f build-hook.nix -o $TEST_ROOT/result --max-jobs 0 \ + --arg busybox $busybox \ + --store $TEST_ROOT/machine0 \ + --builders "ssh://localhost?remote-store=$TEST_ROOT/machine1; $TEST_ROOT/machine2 - - 1 1 foo" \ --system-features foo -outPath=$TEST_ROOT/result +outPath=$(readlink -f $TEST_ROOT/result) -cat $outPath/foobar | grep FOOBAR +cat $TEST_ROOT/machine0/$outPath | grep FOOBAR -# Ensure that input1 was built on store1 due to the required feature. -p=$(readlink -f $outPath/input-2) -(! nix path-info --store $TEST_ROOT/store0 --all | grep builder-build-hook-input-1.sh) -nix path-info --store $TEST_ROOT/store1 --all | grep builder-build-hook-input-1.sh +# Ensure that input1 was built on store2 due to the required feature. +(! nix path-info --store $TEST_ROOT/machine1 --all | grep builder-build-remote-input-1.sh) +nix path-info --store $TEST_ROOT/machine2 --all | grep builder-build-remote-input-1.sh diff --git a/tests/common.sh.in b/tests/common.sh.in index dd7e61822..73fe77345 100644 --- a/tests/common.sh.in +++ b/tests/common.sh.in @@ -35,6 +35,7 @@ export xmllint="@xmllint@" export SHELL="@bash@" export PAGER=cat export HAVE_SODIUM="@HAVE_SODIUM@" +export busybox="@sandbox_shell@" export version=@PACKAGE_VERSION@ export system=@system@ diff --git a/tests/post-hook.sh b/tests/post-hook.sh index a02657215..aa3e6a574 100644 --- a/tests/post-hook.sh +++ b/tests/post-hook.sh @@ -2,6 +2,8 @@ source common.sh clearStore +rm -f $TEST_ROOT/result + export REMOTE_STORE=$TEST_ROOT/remote_store # Build the dependencies and push them to the remote store diff --git a/tests/recursive.sh b/tests/recursive.sh index 394ae5ddb..2d4f83895 100644 --- a/tests/recursive.sh +++ b/tests/recursive.sh @@ -5,6 +5,8 @@ if [[ $(uname) != Linux ]]; then exit; fi clearStore +rm -f $TEST_ROOT/result + export unreachable=$(nix add-to-store ./recursive.sh) nix --experimental-features 'nix-command recursive-nix' build -o $TEST_ROOT/result -L '( diff --git a/tests/structured-attrs.sh b/tests/structured-attrs.sh index 646bdb876..dcfe6d580 100644 --- a/tests/structured-attrs.sh +++ b/tests/structured-attrs.sh @@ -2,6 +2,8 @@ source common.sh clearStore +rm -f $TEST_ROOT/result + nix-build structured-attrs.nix -A all -o $TEST_ROOT/result [[ $(cat $TEST_ROOT/result/foo) = bar ]] From 7af734bac138e80a2e689c140127443366e566d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 27 Jun 2020 20:35:33 +0100 Subject: [PATCH 20/21] dependabot: automatically keep github actions up-to-date --- .github/dependabot.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 000000000..5ace4600a --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,6 @@ +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" From 9937f4ed37a1120306103c6df16b48388aaeb896 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 28 Jun 2020 06:02:57 +0000 Subject: [PATCH 21/21] Bump cachix/install-nix-action from v8 to v10 Bumps [cachix/install-nix-action](https://github.com/cachix/install-nix-action) from v8 to v10. - [Release notes](https://github.com/cachix/install-nix-action/releases) - [Commits](https://github.com/cachix/install-nix-action/compare/v8...63cf434de4e4292c6960639d56c5dd550e789d77) Signed-off-by: dependabot[bot] --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7feefc855..7755466a0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,5 +10,5 @@ jobs: runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v2 - - uses: cachix/install-nix-action@v8 + - uses: cachix/install-nix-action@v10 - run: nix-build release.nix --arg nix '{ outPath = ./.; revCount = 123; shortRev = "abcdefgh"; }' --arg systems '[ builtins.currentSystem ]' -A installerScript -A perlBindings