From 20799a5151cbb185c1772b8e5160493b2dc2d0e8 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 18:41:33 +0000 Subject: [PATCH 01/29] 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/29] 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 507aa48739f23f9a16c8b7079bfa6fc1806be78e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 Jun 2020 18:41:33 +0000 Subject: [PATCH 03/29] 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 73ac003b377aa4750de057bba2de949295524a9c Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Mon, 22 Jun 2020 10:03:19 -0400 Subject: [PATCH 04/29] 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 ec3a85730742497b29782c0b545911eb78d3c65e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 23 Jun 2020 18:19:40 +0000 Subject: [PATCH 05/29] 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 06/29] 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 1be279af2622b196cc4630c48254adc96071c7e9 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 2 Jul 2020 21:44:18 +0000 Subject: [PATCH 07/29] Fix Narinfo corruption detection bug The aim of this check was just to ensure each key occurs once. --- src/libstore/nar-info.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index c403d4bec..ca471463c 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -66,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 (!value.empty()) throw corrupt(); + if (ca) throw corrupt(); // FIXME: allow blank ca or require skipping field? ca = parseContentAddressOpt(value); } From af95a7c16b0fc0b033a7191f686fe98b2015162f Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 8 Jul 2020 15:38:01 -0400 Subject: [PATCH 08/29] Add name to BasicDerivation We always have a name for BasicDerivation, since we have a derivation store path that has a name. --- src/libexpr/primops.cc | 6 ++++-- src/libstore/daemon.cc | 2 +- src/libstore/derivations.cc | 16 ++++++++++------ src/libstore/derivations.hh | 5 +++-- src/nix-store/nix-store.cc | 4 ++-- src/nix/repl.cc | 2 +- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index dec917b38..da1937e6b 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -102,9 +102,11 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args Path realPath = state.checkSourcePath(state.toRealPath(path, context)); + StorePath storePath = state.store->parseStorePath(path); + // FIXME - if (state.store->isStorePath(path) && state.store->isValidPath(state.store->parseStorePath(path)) && isDerivation(path)) { - Derivation drv = readDerivation(*state.store, realPath); + if (state.store->isStorePath(path) && state.store->isValidPath(storePath) && isDerivation(path)) { + Derivation drv = readDerivation(*state.store, realPath, std::string(storePath.name())); Value & w = *state.allocValue(); state.mkAttrs(w, 3 + drv.outputs.size()); Value * v2 = state.allocAttr(w, state.sDrvPath); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index ebc4d0285..4cf67064a 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -474,7 +474,7 @@ static void performOp(TunnelLogger * logger, ref store, case wopBuildDerivation: { auto drvPath = store->parseStorePath(readString(from)); BasicDerivation drv; - readDerivation(from, *store, drv); + readDerivation(from, *store, drv, std::string(drvPath.name())); BuildMode buildMode = (BuildMode) readInt(from); logger->startWork(); if (!trusted) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 42551ef6b..bdc6f4f68 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -129,9 +129,11 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream } -static Derivation parseDerivation(const Store & store, const string & s) +static Derivation parseDerivation(const Store & store, const string & s, string name) { Derivation drv; + drv.name = name; + istringstream_nocopy str(s); expect(str, "Derive(["); @@ -175,10 +177,10 @@ static Derivation parseDerivation(const Store & store, const string & s) } -Derivation readDerivation(const Store & store, const Path & drvPath) +Derivation readDerivation(const Store & store, const Path & drvPath, std::string name) { try { - return parseDerivation(store, readFile(drvPath)); + return parseDerivation(store, readFile(drvPath), name); } catch (FormatError & e) { throw Error("error parsing derivation '%1%': %2%", drvPath, e.msg()); } @@ -196,7 +198,7 @@ Derivation Store::readDerivation(const StorePath & drvPath) { auto accessor = getFSAccessor(); try { - return parseDerivation(*this, accessor->readFile(printStorePath(drvPath))); + return parseDerivation(*this, accessor->readFile(printStorePath(drvPath)), std::string(drvPath.name())); } catch (FormatError & e) { throw Error("error parsing derivation '%s': %s", printStorePath(drvPath), e.msg()); } @@ -369,7 +371,7 @@ Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutput if (h == drvHashes.end()) { assert(store.isValidPath(i.first)); h = drvHashes.insert_or_assign(i.first, hashDerivationModulo(store, - store.readDerivation(i.first), false)).first; + store.readDerivation(i.first), false)).first; } inputs2.insert_or_assign(h->second.to_string(Base16, false), i.second); } @@ -435,8 +437,10 @@ StringSet BasicDerivation::outputNames() const } -Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv) +Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, string name) { + drv.name = name; + drv.outputs.clear(); auto nr = readNum(in); for (size_t n = 0; n < nr; n++) { diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 68c53c1ff..f9d328196 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -35,6 +35,7 @@ struct BasicDerivation Path builder; Strings args; StringPairs env; + string name; BasicDerivation() { } virtual ~BasicDerivation() { }; @@ -76,7 +77,7 @@ StorePath writeDerivation(ref store, const Derivation & drv, std::string_view name, RepairFlag repair = NoRepair); /* Read a derivation from a file. */ -Derivation readDerivation(const Store & store, const Path & drvPath); +Derivation readDerivation(const Store & store, const Path & drvPath, string name); // FIXME: remove bool isDerivation(const string & fileName); @@ -93,7 +94,7 @@ bool wantOutput(const string & output, const std::set & wanted); struct Source; struct Sink; -Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv); +Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, string name); void writeDerivation(Sink & out, const Store & store, const BasicDerivation & drv); std::string hashPlaceholder(const std::string & outputName); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 7d81bf54f..9a8cf0f2f 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -914,9 +914,9 @@ static void opServe(Strings opFlags, Strings opArgs) if (!writeAllowed) throw Error("building paths is not allowed"); - auto drvPath = store->parseStorePath(readString(in)); // informational only + auto drvPath = store->parseStorePath(readString(in)); BasicDerivation drv; - readDerivation(in, *store, drv); + readDerivation(in, *store, drv, std::string(drvPath.name())); getBuildSettings(); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index fdacf604b..a3d85ed31 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -483,7 +483,7 @@ bool NixRepl::processLine(string line) but doing it in a child makes it easier to recover from problems / SIGINT. */ if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPath}) == 0) { - auto drv = readDerivation(*state->store, drvPath); + auto drv = readDerivation(*state->store, drvPath, std::string(state->store->parseStorePath(drvPath).name())); std::cout << std::endl << "this derivation produced the following outputs:" << std::endl; for (auto & i : drv.outputs) std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.path)); From 8e0d0689be797f9e42f9b43b06f50c1af7f20b4a Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 8 Jul 2020 19:11:39 -0400 Subject: [PATCH 09/29] Only store hash of fixed derivation output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit we don’t need a full storepath for a fixedoutput derivation. So just putting the ingestion method + the hash is sufficient. --- perl/lib/Nix/Store.xs | 2 +- src/libexpr/get-drvs.cc | 2 +- src/libexpr/primops.cc | 27 ++++---- src/libstore/build.cc | 76 ++++++++++++----------- src/libstore/builtins/fetchurl.cc | 21 ++++--- src/libstore/derivations.cc | 100 +++++++++++++++++++----------- src/libstore/derivations.hh | 18 ++++-- src/libstore/local-store.cc | 11 +--- src/libstore/misc.cc | 4 +- src/nix-env/nix-env.cc | 2 +- src/nix-store/nix-store.cc | 6 +- src/nix/develop.cc | 4 +- src/nix/repl.cc | 2 +- src/nix/show-derivation.cc | 8 +-- 14 files changed, 162 insertions(+), 121 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 945ed49c7..6dba07549 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -304,7 +304,7 @@ SV * derivationFromPath(char * drvPath) HV * outputs = newHV(); for (auto & i : drv.outputs) - hv_store(outputs, i.first.c_str(), i.first.size(), newSVpv(store()->printStorePath(i.second.path).c_str(), 0), 0); + hv_store(outputs, i.first.c_str(), i.first.size(), newSVpv(store()->printStorePath(i.second.path(store(), drv.name)).c_str(), 0), 0); hv_stores(hash, "outputs", newRV((SV *) outputs)); AV * inputDrvs = newAV(); diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 9055f59a1..5d6e39aa0 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -39,7 +39,7 @@ DrvInfo::DrvInfo(EvalState & state, ref store, const std::string & drvPat if (i == drv.outputs.end()) throw Error("derivation '%s' does not have output '%s'", store->printStorePath(drvPath), outputName); - outPath = store->printStorePath(i->second.path); + outPath = store->printStorePath(i->second.path(*store, drv.name)); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index da1937e6b..9b613eef9 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -64,7 +64,7 @@ void EvalState::realiseContext(const PathSet & context) DerivationOutputs::iterator i = drv.outputs.find(outputName); if (i == drv.outputs.end()) throw Error("derivation '%s' does not have an output named '%s'", ctxS, outputName); - allowedPaths->insert(store->printStorePath(i->second.path)); + allowedPaths->insert(store->printStorePath(i->second.path(*store, drv.name))); } } } @@ -120,7 +120,7 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args for (const auto & o : drv.outputs) { v2 = state.allocAttr(w, state.symbols.create(o.first)); - mkString(*v2, state.store->printStorePath(o.second.path), {"!" + o.first + "!" + path}); + mkString(*v2, state.store->printStorePath(o.second.path(*state.store, drv.name)), {"!" + o.first + "!" + path}); outputsVal->listElems()[outputs_index] = state.allocValue(); mkString(*(outputsVal->listElems()[outputs_index++]), o.first); } @@ -778,11 +778,12 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName); if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign("out", DerivationOutput { - .path = std::move(outPath), - .hash = FixedOutputHash { - .method = ingestionMethod, - .hash = std::move(h), - }, + .output = DerivationOutputFixed { + .hash = FixedOutputHash { + .method = ingestionMethod, + .hash = std::move(h), + }, + }, }); } @@ -797,8 +798,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (!jsonObject) drv.env[i] = ""; drv.outputs.insert_or_assign(i, DerivationOutput { - .path = StorePath::dummy, - .hash = std::optional {}, + .output = DerivationOutputIntensional { + .path = StorePath::dummy, + }, }); } @@ -809,8 +811,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (!jsonObject) drv.env[i] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign(i, DerivationOutput { - .path = std::move(outPath), - .hash = std::optional(), + .output = DerivationOutputIntensional { + .path = std::move(outPath), + }, }); } } @@ -831,7 +834,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * mkString(*state.allocAttr(v, state.sDrvPath), drvPathS, {"=" + drvPathS}); for (auto & i : drv.outputs) { mkString(*state.allocAttr(v, state.symbols.create(i.first)), - state.store->printStorePath(i.second.path), {"!" + i.first + "!" + drvPathS}); + state.store->printStorePath(i.second.path(*state.store, drv.name)), {"!" + i.first + "!" + drvPathS}); } v.attrs->sort(); } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0ef2f288f..bf7d4c2f9 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1047,7 +1047,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation { this->drv = std::make_unique(BasicDerivation(drv)); state = &DerivationGoal::haveDerivation; - name = fmt("building of %s", worker.store.showPaths(drv.outputPaths())); + name = fmt("building of %s", worker.store.showPaths(drv.outputPaths(worker.store))); trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); @@ -1182,7 +1182,7 @@ void DerivationGoal::haveDerivation() retrySubstitution = false; for (auto & i : drv->outputs) - worker.store.addTempRoot(i.second.path); + worker.store.addTempRoot(i.second.path(worker.store, drv->name)); /* Check what outputs paths are not already valid. */ auto invalidOutputs = checkPathValidity(false, buildMode == bmRepair); @@ -1290,12 +1290,12 @@ void DerivationGoal::repairClosure() StorePathSet outputClosure; for (auto & i : drv->outputs) { if (!wantOutput(i.first, wantedOutputs)) continue; - worker.store.computeFSClosure(i.second.path, outputClosure); + worker.store.computeFSClosure(i.second.path(worker.store, drv->name), outputClosure); } /* Filter out our own outputs (which we have already checked). */ for (auto & i : drv->outputs) - outputClosure.erase(i.second.path); + outputClosure.erase(i.second.path(worker.store, drv->name)); /* Get all dependencies of this derivation so that we know which derivation is responsible for which path in the output @@ -1307,7 +1307,7 @@ void DerivationGoal::repairClosure() if (i.isDerivation()) { Derivation drv = worker.store.derivationFromPath(i); for (auto & j : drv.outputs) - outputsToDrv.insert_or_assign(j.second.path, i); + outputsToDrv.insert_or_assign(j.second.path(worker.store, drv.name), i); } /* Check each path (slow!). */ @@ -1379,7 +1379,7 @@ void DerivationGoal::inputsRealised() for (auto & j : i.second) { auto k = inDrv.outputs.find(j); if (k != inDrv.outputs.end()) - worker.store.computeFSClosure(k->second.path, inputPaths); + worker.store.computeFSClosure(k->second.path(worker.store, inDrv.name), inputPaths); else throw Error( "derivation '%s' requires non-existent output '%s' from input derivation '%s'", @@ -1432,7 +1432,7 @@ void DerivationGoal::tryToBuild() goal can start a build, and if not, the main loop will sleep a few seconds and then retry this goal. */ PathSet lockFiles; - for (auto & outPath : drv->outputPaths()) + for (auto & outPath : drv->outputPaths(worker.store)) lockFiles.insert(worker.store.Store::toRealPath(outPath)); if (!outputLocks.lockPaths(lockFiles, "", false)) { @@ -1460,16 +1460,16 @@ void DerivationGoal::tryToBuild() return; } - missingPaths = drv->outputPaths(); + missingPaths = drv->outputPaths(worker.store); if (buildMode != bmCheck) for (auto & i : validPaths) missingPaths.erase(i); /* If any of the outputs already exist but are not valid, delete them. */ for (auto & i : drv->outputs) { - if (worker.store.isValidPath(i.second.path)) continue; - debug("removing invalid path '%s'", worker.store.printStorePath(i.second.path)); - deletePath(worker.store.Store::toRealPath(i.second.path)); + if (worker.store.isValidPath(i.second.path(worker.store, drv->name))) continue; + debug("removing invalid path '%s'", worker.store.printStorePath(i.second.path(worker.store, drv->name))); + deletePath(worker.store.Store::toRealPath(i.second.path(worker.store, drv->name))); } /* Don't do a remote build if the derivation has the attribute @@ -1692,7 +1692,7 @@ void DerivationGoal::buildDone() fmt("running post-build-hook '%s'", settings.postBuildHook), Logger::Fields{worker.store.printStorePath(drvPath)}); PushActivity pact(act.id); - auto outputPaths = drv->outputPaths(); + auto outputPaths = drv->outputPaths(worker.store); std::map hookEnvironment = getEnv(); hookEnvironment.emplace("DRV_PATH", worker.store.printStorePath(drvPath)); @@ -1920,7 +1920,7 @@ StorePathSet DerivationGoal::exportReferences(const StorePathSet & storePaths) if (j.isDerivation()) { Derivation drv = worker.store.derivationFromPath(j); for (auto & k : drv.outputs) - worker.store.computeFSClosure(k.second.path, paths); + worker.store.computeFSClosure(k.second.path(worker.store, drv.name), paths); } } @@ -2015,7 +2015,7 @@ void DerivationGoal::startBuilder() /* Substitute output placeholders with the actual output paths. */ for (auto & output : drv->outputs) - inputRewrites[hashPlaceholder(output.first)] = worker.store.printStorePath(output.second.path); + inputRewrites[hashPlaceholder(output.first)] = worker.store.printStorePath(output.second.path(worker.store, drv->name)); /* Construct the environment passed to the builder. */ initEnv(); @@ -2200,7 +2200,7 @@ void DerivationGoal::startBuilder() (typically the dependencies of /bin/sh). Throw them out. */ for (auto & i : drv->outputs) - dirsInChroot.erase(worker.store.printStorePath(i.second.path)); + dirsInChroot.erase(worker.store.printStorePath(i.second.path(drv->name))); #elif __APPLE__ /* We don't really have any parent prep work to do (yet?) @@ -2613,7 +2613,7 @@ void DerivationGoal::writeStructuredAttrs() /* Add an "outputs" object containing the output paths. */ nlohmann::json outputs; for (auto & i : drv->outputs) - outputs[i.first] = rewriteStrings(worker.store.printStorePath(i.second.path), inputRewrites); + outputs[i.first] = rewriteStrings(worker.store.printStorePath(i.second.path(worker.store, drv->name)), inputRewrites); json["outputs"] = outputs; /* Handle exportReferencesGraph. */ @@ -2818,7 +2818,7 @@ struct RestrictedStore : public LocalFSStore auto drv = derivationFromPath(path.path); for (auto & output : drv.outputs) if (wantOutput(output.first, path.outputs)) - newPaths.insert(output.second.path); + newPaths.insert(output.second.path(*this, drv.name)); } else if (!goal.isAllowed(path.path)) throw InvalidPath("cannot build unknown path '%s' in recursive Nix", printStorePath(path.path)); } @@ -3580,7 +3580,7 @@ StorePathSet parseReferenceSpecifiers(Store & store, const BasicDerivation & drv if (store.isStorePath(i)) result.insert(store.parseStorePath(i)); else if (drv.outputs.count(i)) - result.insert(drv.outputs.find(i)->second.path); + result.insert(drv.outputs.find(i)->second.path(store, drv.name)); else throw BuildError("derivation contains an illegal reference specifier '%s'", i); } return result; @@ -3618,7 +3618,7 @@ void DerivationGoal::registerOutputs() if (hook) { bool allValid = true; for (auto & i : drv->outputs) - if (!worker.store.isValidPath(i.second.path)) allValid = false; + if (!worker.store.isValidPath(i.second.path(worker.store, drv->name))) allValid = false; if (allValid) return; } @@ -3639,23 +3639,23 @@ void DerivationGoal::registerOutputs() Nix calls. */ StorePathSet referenceablePaths; for (auto & p : inputPaths) referenceablePaths.insert(p); - for (auto & i : drv->outputs) referenceablePaths.insert(i.second.path); + for (auto & i : drv->outputs) referenceablePaths.insert(i.second.path(worker.store, drv->name)); for (auto & p : addedPaths) referenceablePaths.insert(p); /* Check whether the output paths were created, and grep each output path to determine what other paths it references. Also make all output paths read-only. */ for (auto & i : drv->outputs) { - auto path = worker.store.printStorePath(i.second.path); - if (!missingPaths.count(i.second.path)) continue; + auto path = worker.store.printStorePath(i.second.path(worker.store, drv->name)); + if (!missingPaths.count(i.second.path(worker.store, drv->name))) continue; Path actualPath = path; if (needsHashRewrite()) { - auto r = redirectedOutputs.find(i.second.path); + auto r = redirectedOutputs.find(i.second.path(worker.store, drv->name)); if (r != redirectedOutputs.end()) { auto redirected = worker.store.Store::toRealPath(r->second); if (buildMode == bmRepair - && redirectedBadOutputs.count(i.second.path) + && redirectedBadOutputs.count(i.second.path(worker.store, drv->name)) && pathExists(redirected)) replaceValidPath(path, redirected); if (buildMode == bmCheck) @@ -3724,7 +3724,9 @@ void DerivationGoal::registerOutputs() if (fixedOutput) { - if (i.second.hash->method == FileIngestionMethod::Flat) { + FixedOutputHash outputHash = std::get(i.second.output).hash; + + if (outputHash.method == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) throw BuildError( @@ -3735,13 +3737,13 @@ 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); + Hash h2 = outputHash.method == FileIngestionMethod::Recursive + ? hashPath(*outputHash.hash.type, actualPath).first + : hashFile(*outputHash.hash.type, actualPath); - auto dest = worker.store.makeFixedOutputPath(i.second.hash->method, h2, i.second.path.name()); + auto dest = worker.store.makeFixedOutputPath(outputHash.method, h2, i.second.path(worker.store, drv->name).name()); - if (i.second.hash->hash != h2) { + if (outputHash.hash != h2) { /* Throw an error after registering the path as valid. */ @@ -3749,7 +3751,7 @@ void DerivationGoal::registerOutputs() delayedException = std::make_exception_ptr( BuildError("hash mismatch in fixed-output derivation '%s':\n wanted: %s\n got: %s", worker.store.printStorePath(dest), - i.second.hash->hash.to_string(SRI, true), + outputHash.hash.to_string(SRI, true), h2.to_string(SRI, true))); Path actualDest = worker.store.Store::toRealPath(dest); @@ -3771,7 +3773,7 @@ void DerivationGoal::registerOutputs() assert(worker.store.parseStorePath(path) == dest); ca = FixedOutputHash { - .method = i.second.hash->method, + .method = outputHash.method, .hash = h2, }; } @@ -3895,7 +3897,7 @@ void DerivationGoal::registerOutputs() /* If this is the first round of several, then move the output out of the way. */ if (nrRounds > 1 && curRound == 1 && curRound < nrRounds && keepPreviousRound) { for (auto & i : drv->outputs) { - auto path = worker.store.printStorePath(i.second.path); + auto path = worker.store.printStorePath(i.second.path(worker.store, drv->name)); Path prev = path + checkSuffix; deletePath(prev); Path dst = path + checkSuffix; @@ -3913,7 +3915,7 @@ void DerivationGoal::registerOutputs() if the result was not determistic? */ if (curRound == nrRounds) { for (auto & i : drv->outputs) { - Path prev = worker.store.printStorePath(i.second.path) + checkSuffix; + Path prev = worker.store.printStorePath(i.second.path(worker.store, drv->name)) + checkSuffix; deletePath(prev); } } @@ -4214,9 +4216,9 @@ StorePathSet DerivationGoal::checkPathValidity(bool returnValid, bool checkHash) for (auto & i : drv->outputs) { if (!wantOutput(i.first, wantedOutputs)) continue; bool good = - worker.store.isValidPath(i.second.path) && - (!checkHash || worker.pathContentsGood(i.second.path)); - if (good == returnValid) result.insert(i.second.path); + worker.store.isValidPath(i.second.path(worker.store, drv->name)) && + (!checkHash || worker.pathContentsGood(i.second.path(worker.store, drv->name))); + if (good == returnValid) result.insert(i.second.path(worker.store, drv->name)); } return result; } diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index e630cf6f1..017a633b5 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -63,16 +63,19 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData) auto & output = drv.outputs.begin()->second; /* Try the hashed mirrors first. */ - if (output.hash && output.hash->method == FileIngestionMethod::Flat) - for (auto hashedMirror : settings.hashedMirrors.get()) - try { - if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/'; - auto & h = output.hash->hash; - fetch(hashedMirror + printHashType(*h.type) + "/" + h.to_string(Base16, false)); - return; - } catch (Error & e) { - debug(e.what()); + if (auto hash = std::get_if(&output.output)) { + if (hash->hash.method == FileIngestionMethod::Flat) { + for (auto hashedMirror : settings.hashedMirrors.get()) { + try { + if (!hasSuffix(hashedMirror, "/")) hashedMirror += '/'; + fetch(hashedMirror + printHashType(*hash->hash.hash.type) + "/" + hash->hash.hash.to_string(Base16, false)); + return; + } catch (Error & e) { + debug(e.what()); + } } + } + } /* Otherwise try the specified URL. */ fetch(mainUrl); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index bdc6f4f68..ffeafea87 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -8,12 +8,28 @@ namespace nix { -const StorePath & BasicDerivation::findOutput(const string & id) const +// FIXME Put this somewhere? +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; + +StorePath DerivationOutput::path(const Store & store, string drvName) const +{ + return std::visit(overloaded { + [](DerivationOutputIntensional doi) { + return doi.path; + }, + [&](DerivationOutputFixed dof) { + return store.makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName); + } + }, output); +} + +const StorePath & BasicDerivation::findOutput(const Store & store, const string & id) const { auto i = outputs.find(id); if (i == outputs.end()) throw Error("derivation has no output '%s'", id); - return i->second.path; + return i->second.path(store, name); } @@ -108,7 +124,6 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream expect(str, ","); const auto hash = parseString(str); expect(str, ")"); - std::optional fsh; if (hashAlgo != "") { auto method = FileIngestionMethod::Flat; if (string(hashAlgo, 0, 2) == "r:") { @@ -116,16 +131,21 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream hashAlgo = string(hashAlgo, 2); } const HashType hashType = parseHashType(hashAlgo); - fsh = FixedOutputHash { - .method = std::move(method), - .hash = Hash(hash, hashType), - }; - } - return DerivationOutput { - .path = std::move(path), - .hash = std::move(fsh), - }; + return DerivationOutput { + .output = DerivationOutputFixed { + .hash = FixedOutputHash { + .method = std::move(method), + .hash = Hash(hash, hashType), + }, + } + }; + } else + return DerivationOutput { + .output = DerivationOutputIntensional { + .path = std::move(path), + } + }; } @@ -266,10 +286,14 @@ string Derivation::unparse(const Store & store, bool maskOutputs, for (auto & i : outputs) { if (first) first = false; else s += ','; s += '('; printUnquotedString(s, i.first); - s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(i.second.path)); - s += ','; printUnquotedString(s, i.second.hash ? i.second.hash->printMethodAlgo() : ""); - s += ','; printUnquotedString(s, - i.second.hash ? i.second.hash->hash.to_string(Base16, false) : ""); + s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(i.second.path(store, name))); + if (auto hash = std::get_if(&i.second.output)) { + s += ','; printUnquotedString(s, hash->hash.printMethodAlgo()); + s += ','; printUnquotedString(s, hash->hash.hash.to_string(Base16, false)); + } else { + s += ','; printUnquotedString(s, ""); + s += ','; printUnquotedString(s, ""); + } s += ')'; } @@ -325,7 +349,7 @@ bool BasicDerivation::isFixedOutput() const { return outputs.size() == 1 && outputs.begin()->first == "out" && - outputs.begin()->second.hash; + std::holds_alternative(outputs.begin()->second.output); } @@ -357,10 +381,11 @@ Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutput /* Return a fixed hash for fixed-output derivations. */ if (drv.isFixedOutput()) { DerivationOutputs::const_iterator i = drv.outputs.begin(); + auto hash = std::get(i->second.output); return hashString(htSHA256, "fixed:out:" - + i->second.hash->printMethodAlgo() + ":" - + i->second.hash->hash.to_string(Base16, false) + ":" - + store.printStorePath(i->second.path)); + + hash.hash.printMethodAlgo() + ":" + + hash.hash.hash.to_string(Base16, false) + ":" + + store.printStorePath(i->second.path(store, drv.name))); } /* For other derivations, replace the inputs paths with recursive @@ -394,11 +419,11 @@ bool wantOutput(const string & output, const std::set & wanted) } -StorePathSet BasicDerivation::outputPaths() const +StorePathSet BasicDerivation::outputPaths(const Store & store) const { StorePathSet paths; for (auto & i : outputs) - paths.insert(i.second.path); + paths.insert(i.second.path(store, name)); return paths; } @@ -408,7 +433,6 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) auto hashAlgo = readString(in); auto hash = readString(in); - std::optional fsh; if (hashAlgo != "") { auto method = FileIngestionMethod::Flat; if (string(hashAlgo, 0, 2) == "r:") { @@ -416,16 +440,20 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) hashAlgo = string(hashAlgo, 2); } auto hashType = parseHashType(hashAlgo); - fsh = FixedOutputHash { - .method = std::move(method), - .hash = Hash(hash, hashType), + return DerivationOutput { + .output = DerivationOutputFixed { + .hash = FixedOutputHash { + .method = std::move(method), + .hash = Hash(hash, hashType), + }, + } + }; + } else + return DerivationOutput { + .output = DerivationOutputIntensional { + .path = std::move(path), + } }; - } - - return DerivationOutput { - .path = std::move(path), - .hash = std::move(fsh), - }; } StringSet BasicDerivation::outputNames() const @@ -469,10 +497,10 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr out << drv.outputs.size(); for (auto & i : drv.outputs) { out << i.first - << store.printStorePath(i.second.path); - if (i.second.hash) { - out << i.second.hash->printMethodAlgo() - << i.second.hash->hash.to_string(Base16, false); + << store.printStorePath(i.second.path(store, drv.name)); + if (auto hash = std::get_if(&i.second.output)) { + out << hash->hash.printMethodAlgo() + << hash->hash.hash.to_string(Base16, false); } else { out << "" << ""; } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index f9d328196..d8cee0f34 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -13,10 +13,20 @@ namespace nix { /* Abstract syntax of derivations. */ -struct DerivationOutput +struct DerivationOutputIntensional { StorePath path; - std::optional hash; /* hash used for expected hash computation */ +}; + +struct DerivationOutputFixed +{ + FixedOutputHash hash; /* hash used for expected hash computation */ +}; + +struct DerivationOutput +{ + std::variant output; + StorePath path(const Store & store, string drvName) const; }; typedef std::map DerivationOutputs; @@ -42,7 +52,7 @@ struct BasicDerivation /* Return the path corresponding to the output identifier `id' in the given derivation. */ - const StorePath & findOutput(const std::string & id) const; + const StorePath & findOutput(const Store & store, const std::string & id) const; bool isBuiltin() const; @@ -50,7 +60,7 @@ struct BasicDerivation bool isFixedOutput() const; /* Return the output paths of a derivation. */ - StorePathSet outputPaths() const; + StorePathSet outputPaths(const Store & store) const; /* Return the output names of a derivation. */ StringSet outputNames() const; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index eed225349..7ecaa8743 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -560,19 +560,12 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat DerivationOutputs::const_iterator out = drv.outputs.find("out"); if (out == drv.outputs.end()) throw Error("derivation '%s' does not have an output named 'out'", printStorePath(drvPath)); - - check( - makeFixedOutputPath( - out->second.hash->method, - out->second.hash->hash, - drvName), - out->second.path, "out"); } else { Hash h = hashDerivationModulo(*this, drv, true); for (auto & i : drv.outputs) - check(makeOutputPath(i.first, h, drvName), i.second.path, i.first); + check(makeOutputPath(i.first, h, drvName), i.second.path(*this, drv.name), i.first); } } @@ -614,7 +607,7 @@ uint64_t LocalStore::addValidPath(State & state, state.stmtAddDerivationOutput.use() (id) (i.first) - (printStorePath(i.second.path)) + (printStorePath(i.second.path(*this, drv.name))) .exec(); } } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index e68edb38c..c4d22a634 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -198,8 +198,8 @@ void Store::queryMissing(const std::vector & targets, PathSet invalid; for (auto & j : drv->outputs) if (wantOutput(j.first, path.outputs) - && !isValidPath(j.second.path)) - invalid.insert(printStorePath(j.second.path)); + && !isValidPath(j.second.path(*this, drv->name))) + invalid.insert(printStorePath(j.second.path(*this, drv->name))); if (invalid.empty()) return; if (settings.useSubstitutes && parsedDrv.substitutesAllowed()) { diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index c992b7d74..a42859fd0 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -381,7 +381,7 @@ static void queryInstSources(EvalState & state, if (path.isDerivation()) { elem.setDrvPath(state.store->printStorePath(path)); - elem.setOutPath(state.store->printStorePath(state.store->derivationFromPath(path).findOutput("out"))); + elem.setOutPath(state.store->printStorePath(state.store->derivationFromPath(path).findOutput(*state.store, "out"))); if (name.size() >= drvExtension.size() && string(name, name.size() - drvExtension.size()) == drvExtension) name = string(name, 0, name.size() - drvExtension.size()); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 9a8cf0f2f..2a2f16ac1 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -77,7 +77,7 @@ static PathSet realisePath(StorePathWithOutputs path, bool build = true) if (i == drv.outputs.end()) throw Error("derivation '%s' does not have an output named '%s'", store2->printStorePath(path.path), j); - auto outPath = store2->printStorePath(i->second.path); + auto outPath = store2->printStorePath(i->second.path(*store, drv.name)); if (store2) { if (gcRoot == "") printGCWarning(); @@ -219,7 +219,7 @@ static StorePathSet maybeUseOutputs(const StorePath & storePath, bool useOutput, auto drv = store->derivationFromPath(storePath); StorePathSet outputs; for (auto & i : drv.outputs) - outputs.insert(i.second.path); + outputs.insert(i.second.path(*store, drv.name)); return outputs; } else return {storePath}; @@ -313,7 +313,7 @@ static void opQuery(Strings opFlags, Strings opArgs) if (forceRealise) realisePath({i2}); Derivation drv = store->derivationFromPath(i2); for (auto & j : drv.outputs) - cout << fmt("%1%\n", store->printStorePath(j.second.path)); + cout << fmt("%1%\n", store->printStorePath(j.second.path(*store, drv.name))); } break; } diff --git a/src/nix/develop.cc b/src/nix/develop.cc index eb93f56fc..929fae5c4 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -135,7 +135,9 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) drv.inputSrcs.insert(std::move(getEnvShPath)); Hash h = hashDerivationModulo(*store, drv, true); auto shellOutPath = store->makeOutputPath("out", h, drvName); - drv.outputs.insert_or_assign("out", DerivationOutput { .path = shellOutPath }); + drv.outputs.insert_or_assign("out", DerivationOutput { .output = DerivationOutputIntensional { + .path = shellOutPath + } }); drv.env["out"] = store->printStorePath(shellOutPath); auto shellDrvPath2 = writeDerivation(store, drv, drvName); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index a3d85ed31..bd72a3b19 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -486,7 +486,7 @@ bool NixRepl::processLine(string line) auto drv = readDerivation(*state->store, drvPath, std::string(state->store->parseStorePath(drvPath).name())); std::cout << std::endl << "this derivation produced the following outputs:" << std::endl; for (auto & i : drv.outputs) - std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.path)); + std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.path(*state->store, drv.name))); } } else if (command == ":i") { runProgram(settings.nixBinDir + "/nix-env", Strings{"-i", drvPath}); diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 5d77cfdca..a868023d4 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -69,10 +69,10 @@ struct CmdShowDerivation : InstallablesCommand auto outputsObj(drvObj.object("outputs")); for (auto & output : drv.outputs) { auto outputObj(outputsObj.object(output.first)); - outputObj.attr("path", store->printStorePath(output.second.path)); - if (output.second.hash) { - outputObj.attr("hashAlgo", output.second.hash->printMethodAlgo()); - outputObj.attr("hash", output.second.hash->hash.to_string(Base16, false)); + outputObj.attr("path", store->printStorePath(output.second.path(*store, drv.name))); + if (auto hash = std::get_if(&output.second.output)) { + outputObj.attr("hashAlgo", hash->hash.printMethodAlgo()); + outputObj.attr("hash", hash->hash.hash.to_string(Base16, false)); } } } From 06a4e15478920bcf8d39c36abc1a73a1e83bc9b7 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 8 Jul 2020 19:27:51 -0400 Subject: [PATCH 10/29] Fix build.cc on linux --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index bf7d4c2f9..9a53d9df7 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2200,7 +2200,7 @@ void DerivationGoal::startBuilder() (typically the dependencies of /bin/sh). Throw them out. */ for (auto & i : drv->outputs) - dirsInChroot.erase(worker.store.printStorePath(i.second.path(drv->name))); + dirsInChroot.erase(worker.store.printStorePath(i.second.path(worker.store, drv->name))); #elif __APPLE__ /* We don't really have any parent prep work to do (yet?) From a7884970c57f821e274fba20473d6fcce95ff6c8 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Thu, 9 Jul 2020 11:37:18 -0400 Subject: [PATCH 11/29] Fix DerivationOutputExtensional name --- src/libexpr/primops.cc | 4 ++-- src/libstore/derivations.cc | 6 +++--- src/libstore/derivations.hh | 4 ++-- src/nix/develop.cc | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 9b613eef9..bd7ec628c 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -798,7 +798,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (!jsonObject) drv.env[i] = ""; drv.outputs.insert_or_assign(i, DerivationOutput { - .output = DerivationOutputIntensional { + .output = DerivationOutputExtensional { .path = StorePath::dummy, }, }); @@ -811,7 +811,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (!jsonObject) drv.env[i] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign(i, DerivationOutput { - .output = DerivationOutputIntensional { + .output = DerivationOutputExtensional { .path = std::move(outPath), }, }); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index ffeafea87..86f55342d 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -15,7 +15,7 @@ template overloaded(Ts...) -> overloaded; StorePath DerivationOutput::path(const Store & store, string drvName) const { return std::visit(overloaded { - [](DerivationOutputIntensional doi) { + [](DerivationOutputExtensional doi) { return doi.path; }, [&](DerivationOutputFixed dof) { @@ -142,7 +142,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream }; } else return DerivationOutput { - .output = DerivationOutputIntensional { + .output = DerivationOutputExtensional { .path = std::move(path), } }; @@ -450,7 +450,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) }; } else return DerivationOutput { - .output = DerivationOutputIntensional { + .output = DerivationOutputExtensional { .path = std::move(path), } }; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index d8cee0f34..2515188ec 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -13,7 +13,7 @@ namespace nix { /* Abstract syntax of derivations. */ -struct DerivationOutputIntensional +struct DerivationOutputExtensional { StorePath path; }; @@ -25,7 +25,7 @@ struct DerivationOutputFixed struct DerivationOutput { - std::variant output; + std::variant output; StorePath path(const Store & store, string drvName) const; }; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 929fae5c4..e03d2e057 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -135,7 +135,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) drv.inputSrcs.insert(std::move(getEnvShPath)); Hash h = hashDerivationModulo(*store, drv, true); auto shellOutPath = store->makeOutputPath("out", h, drvName); - drv.outputs.insert_or_assign("out", DerivationOutput { .output = DerivationOutputIntensional { + drv.outputs.insert_or_assign("out", DerivationOutput { .output = DerivationOutputExtensional { .path = shellOutPath } }); drv.env["out"] = store->printStorePath(shellOutPath); From abea26a9683d58e0bee43d9ededeb710f3617603 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 01:57:06 +0000 Subject: [PATCH 12/29] BasicDerivation::findOutput cannot return reference anymore --- src/libstore/derivations.cc | 2 +- src/libstore/derivations.hh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 86f55342d..bde147502 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -24,7 +24,7 @@ StorePath DerivationOutput::path(const Store & store, string drvName) const }, output); } -const StorePath & BasicDerivation::findOutput(const Store & store, const string & id) const +const StorePath BasicDerivation::findOutput(const Store & store, const string & id) const { auto i = outputs.find(id); if (i == outputs.end()) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 2515188ec..4694ec0dd 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -52,7 +52,7 @@ struct BasicDerivation /* Return the path corresponding to the output identifier `id' in the given derivation. */ - const StorePath & findOutput(const Store & store, const std::string & id) const; + const StorePath findOutput(const Store & store, const std::string & id) const; bool isBuiltin() const; From 1c9bec226f9ff1a9586bfefcd0c3b66ed989b2d3 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 02:38:03 +0000 Subject: [PATCH 13/29] Don't improperly assume path is store path --- src/libexpr/primops.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index bd7ec628c..4367f9b22 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -102,10 +102,17 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args Path realPath = state.checkSourcePath(state.toRealPath(path, context)); - StorePath storePath = state.store->parseStorePath(path); - // FIXME - if (state.store->isStorePath(path) && state.store->isValidPath(storePath) && isDerivation(path)) { + auto isValidDerivationInStore = [&]() -> std::optional { + if (!state.store->isStorePath(path)) + return std::nullopt; + auto storePath = state.store->parseStorePath(path); + if (!(state.store->isValidPath(storePath) && isDerivation(path))) + return std::nullopt; + return storePath; + }; + if (auto optStorePath = isValidDerivationInStore()) { + auto storePath = *optStorePath; Derivation drv = readDerivation(*state.store, realPath, std::string(storePath.name())); Value & w = *state.allocValue(); state.mkAttrs(w, 3 + drv.outputs.size()); From 13ec627e0a46d26b5de227e193a05b51af7f55e5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 03:03:12 +0000 Subject: [PATCH 14/29] Set derivation name in dervationStrict --- src/libexpr/primops.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 4367f9b22..14a7deed5 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -591,6 +591,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * /* Build the derivation expression by processing the attributes. */ Derivation drv; + drv.name = drvName; PathSet context; From 5d0b75e5b6a1c70203257fecc25743e5a1e009cb Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 15:02:36 +0000 Subject: [PATCH 15/29] std::string_view for new derivation name parameters --- src/libstore/derivations.cc | 8 ++++---- src/libstore/derivations.hh | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index bde147502..97fd21885 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -12,7 +12,7 @@ namespace nix { template struct overloaded : Ts... { using Ts::operator()...; }; template overloaded(Ts...) -> overloaded; -StorePath DerivationOutput::path(const Store & store, string drvName) const +StorePath DerivationOutput::path(const Store & store, std::string_view drvName) const { return std::visit(overloaded { [](DerivationOutputExtensional doi) { @@ -149,7 +149,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream } -static Derivation parseDerivation(const Store & store, const string & s, string name) +static Derivation parseDerivation(const Store & store, const string & s, std::string_view name) { Derivation drv; drv.name = name; @@ -197,7 +197,7 @@ static Derivation parseDerivation(const Store & store, const string & s, string } -Derivation readDerivation(const Store & store, const Path & drvPath, std::string name) +Derivation readDerivation(const Store & store, const Path & drvPath, std::string_view name) { try { return parseDerivation(store, readFile(drvPath), name); @@ -465,7 +465,7 @@ StringSet BasicDerivation::outputNames() const } -Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, string name) +Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, std::string_view name) { drv.name = name; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 4694ec0dd..81c196ff7 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -26,7 +26,7 @@ struct DerivationOutputFixed struct DerivationOutput { std::variant output; - StorePath path(const Store & store, string drvName) const; + StorePath path(const Store & store, std::string_view drvName) const; }; typedef std::map DerivationOutputs; @@ -45,7 +45,7 @@ struct BasicDerivation Path builder; Strings args; StringPairs env; - string name; + std::string name; BasicDerivation() { } virtual ~BasicDerivation() { }; @@ -87,7 +87,7 @@ StorePath writeDerivation(ref store, const Derivation & drv, std::string_view name, RepairFlag repair = NoRepair); /* Read a derivation from a file. */ -Derivation readDerivation(const Store & store, const Path & drvPath, string name); +Derivation readDerivation(const Store & store, const Path & drvPath, std::string_view name); // FIXME: remove bool isDerivation(const string & fileName); @@ -104,7 +104,7 @@ bool wantOutput(const string & output, const std::set & wanted); struct Source; struct Sink; -Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, string name); +Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, std::string_view name); void writeDerivation(Sink & out, const Store & store, const BasicDerivation & drv); std::string hashPlaceholder(const std::string & outputName); From 18152406ce9db9491385f2e67da6f7f78e8d98d5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 15:26:30 +0000 Subject: [PATCH 16/29] String .drv suffix to create derivation name --- src/libexpr/primops.cc | 2 +- src/libstore/daemon.cc | 2 +- src/libstore/derivations.cc | 11 ++++++++++- src/libstore/derivations.hh | 2 ++ src/nix-store/nix-store.cc | 2 +- src/nix/repl.cc | 2 +- 6 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 14a7deed5..21bd4fb52 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -113,7 +113,7 @@ static void prim_scopedImport(EvalState & state, const Pos & pos, Value * * args }; if (auto optStorePath = isValidDerivationInStore()) { auto storePath = *optStorePath; - Derivation drv = readDerivation(*state.store, realPath, std::string(storePath.name())); + Derivation drv = readDerivation(*state.store, realPath, Derivation::nameFromPath(storePath)); Value & w = *state.allocValue(); state.mkAttrs(w, 3 + drv.outputs.size()); Value * v2 = state.allocAttr(w, state.sDrvPath); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 4cf67064a..2ff53c964 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -474,7 +474,7 @@ static void performOp(TunnelLogger * logger, ref store, case wopBuildDerivation: { auto drvPath = store->parseStorePath(readString(from)); BasicDerivation drv; - readDerivation(from, *store, drv, std::string(drvPath.name())); + readDerivation(from, *store, drv, Derivation::nameFromPath(drvPath)); BuildMode buildMode = (BuildMode) readInt(from); logger->startWork(); if (!trusted) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 97fd21885..a85a70c0a 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -218,7 +218,7 @@ Derivation Store::readDerivation(const StorePath & drvPath) { auto accessor = getFSAccessor(); try { - return parseDerivation(*this, accessor->readFile(printStorePath(drvPath)), std::string(drvPath.name())); + return parseDerivation(*this, accessor->readFile(printStorePath(drvPath)), Derivation::nameFromPath(drvPath)); } catch (FormatError & e) { throw Error("error parsing derivation '%s': %s", printStorePath(drvPath), e.msg()); } @@ -465,6 +465,15 @@ StringSet BasicDerivation::outputNames() const } +std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath) { + auto nameWithSuffix = drvPath.name(); + constexpr std::string_view extension = ".drv"; + assert(hasSuffix(nameWithSuffix, extension)); + nameWithSuffix.remove_suffix(extension.size()); + return nameWithSuffix; +} + + Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, std::string_view name) { drv.name = name; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 81c196ff7..6d3b54d3f 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -64,6 +64,8 @@ struct BasicDerivation /* Return the output names of a derivation. */ StringSet outputNames() const; + + static std::string_view nameFromPath(const StorePath & storePath); }; struct Derivation : BasicDerivation diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 39a47e19a..9605e4b32 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -916,7 +916,7 @@ static void opServe(Strings opFlags, Strings opArgs) auto drvPath = store->parseStorePath(readString(in)); BasicDerivation drv; - readDerivation(in, *store, drv, std::string(drvPath.name())); + readDerivation(in, *store, drv, Derivation::nameFromPath(drvPath)); getBuildSettings(); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index bd72a3b19..74f0b3ec8 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -483,7 +483,7 @@ bool NixRepl::processLine(string line) but doing it in a child makes it easier to recover from problems / SIGINT. */ if (runProgram(settings.nixBinDir + "/nix", Strings{"build", "--no-link", drvPath}) == 0) { - auto drv = readDerivation(*state->store, drvPath, std::string(state->store->parseStorePath(drvPath).name())); + auto drv = readDerivation(*state->store, drvPath, Derivation::nameFromPath(state->store->parseStorePath(drvPath))); std::cout << std::endl << "this derivation produced the following outputs:" << std::endl; for (auto & i : drv.outputs) std::cout << fmt(" %s -> %s\n", i.first, state->store->printStorePath(i.second.path(*state->store, drv.name))); From a8d4707107c39d0258fef17b531ac031dd16ec88 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 15:54:12 +0000 Subject: [PATCH 17/29] Undo erroneous indentation change --- src/libstore/derivations.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index a85a70c0a..f41edd81d 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -396,7 +396,7 @@ Hash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutput if (h == drvHashes.end()) { assert(store.isValidPath(i.first)); h = drvHashes.insert_or_assign(i.first, hashDerivationModulo(store, - store.readDerivation(i.first), false)).first; + store.readDerivation(i.first), false)).first; } inputs2.insert_or_assign(h->second.to_string(Base16, false), i.second); } From 503b4256903b2cf414a6d697d19be40db299e2c6 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 15:56:20 +0000 Subject: [PATCH 18/29] DerivationOutputExtensional -> DerivationOutputInputAddressed Thanks @regnat for the great name. --- src/libexpr/primops.cc | 4 ++-- src/libstore/derivations.cc | 6 +++--- src/libstore/derivations.hh | 4 ++-- src/nix/develop.cc | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 21bd4fb52..5a9c17814 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -806,7 +806,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (!jsonObject) drv.env[i] = ""; drv.outputs.insert_or_assign(i, DerivationOutput { - .output = DerivationOutputExtensional { + .output = DerivationOutputInputAddressed { .path = StorePath::dummy, }, }); @@ -819,7 +819,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * if (!jsonObject) drv.env[i] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign(i, DerivationOutput { - .output = DerivationOutputExtensional { + .output = DerivationOutputInputAddressed { .path = std::move(outPath), }, }); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index f41edd81d..d267468af 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -15,7 +15,7 @@ template overloaded(Ts...) -> overloaded; StorePath DerivationOutput::path(const Store & store, std::string_view drvName) const { return std::visit(overloaded { - [](DerivationOutputExtensional doi) { + [](DerivationOutputInputAddressed doi) { return doi.path; }, [&](DerivationOutputFixed dof) { @@ -142,7 +142,7 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream }; } else return DerivationOutput { - .output = DerivationOutputExtensional { + .output = DerivationOutputInputAddressed { .path = std::move(path), } }; @@ -450,7 +450,7 @@ static DerivationOutput readDerivationOutput(Source & in, const Store & store) }; } else return DerivationOutput { - .output = DerivationOutputExtensional { + .output = DerivationOutputInputAddressed { .path = std::move(path), } }; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 6d3b54d3f..fd8828373 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -13,7 +13,7 @@ namespace nix { /* Abstract syntax of derivations. */ -struct DerivationOutputExtensional +struct DerivationOutputInputAddressed { StorePath path; }; @@ -25,7 +25,7 @@ struct DerivationOutputFixed struct DerivationOutput { - std::variant output; + std::variant output; StorePath path(const Store & store, std::string_view drvName) const; }; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index e03d2e057..654f62f38 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -135,7 +135,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) drv.inputSrcs.insert(std::move(getEnvShPath)); Hash h = hashDerivationModulo(*store, drv, true); auto shellOutPath = store->makeOutputPath("out", h, drvName); - drv.outputs.insert_or_assign("out", DerivationOutput { .output = DerivationOutputExtensional { + drv.outputs.insert_or_assign("out", DerivationOutput { .output = DerivationOutputInputAddressed { .path = shellOutPath } }); drv.env["out"] = store->printStorePath(shellOutPath); From 886c91dfcc462d157dc2ce5265800e98d1bc45dd Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 12 Jul 2020 18:26:10 +0000 Subject: [PATCH 19/29] Try to fix perl bindings --- perl/lib/Nix/Store.xs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 6dba07549..522de5802 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -304,7 +304,10 @@ SV * derivationFromPath(char * drvPath) HV * outputs = newHV(); for (auto & i : drv.outputs) - hv_store(outputs, i.first.c_str(), i.first.size(), newSVpv(store()->printStorePath(i.second.path(store(), drv.name)).c_str(), 0), 0); + hv_store( + outputs, i.first.c_str(), i.first.size(), + newSVpv(store()->printStorePath(i.second.path(*store(), drv.name)).c_str(), 0), + 0); hv_stores(hash, "outputs", newRV((SV *) outputs)); AV * inputDrvs = newAV(); From 2292814049256980c6e809ab364ebe0da3c9d76a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 24 Jul 2020 11:19:17 +0200 Subject: [PATCH 20/29] createUnixDomainSocket(): Fix off-by-one error in copying the socket path Reported by Kane York. --- src/libutil/util.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 93798a765..a0a8ff4d3 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1581,7 +1581,7 @@ AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode) struct sockaddr_un addr; addr.sun_family = AF_UNIX; - if (path.size() >= sizeof(addr.sun_path)) + if (path.size() + 1 >= sizeof(addr.sun_path)) throw Error("socket path '%1%' is too long", path); strcpy(addr.sun_path, path.c_str()); From 72f8771094d575e924846f16e5c60742eea9420b Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Sat, 25 Jul 2020 18:02:42 -0500 Subject: [PATCH 21/29] Allow PRECOMPILE_HEADERS in cross-compilation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In cross, CXX will look like aarch64-unknown-linux-gnu-g++. We could run some command to check what kind of compiler it is, but for now we can just check if g++ is anywhere in the string. I couldn’t find any "ends with" for makefile, so it can be anywhere in CXX. --- mk/precompiled-headers.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mk/precompiled-headers.mk b/mk/precompiled-headers.mk index 1c0452dc2..500c99e4a 100644 --- a/mk/precompiled-headers.mk +++ b/mk/precompiled-headers.mk @@ -21,13 +21,13 @@ clean-files += $(GCH) $(PCH) ifeq ($(PRECOMPILE_HEADERS), 1) - ifeq ($(CXX), g++) + ifeq ($(findstring g++,$(CXX)), g++) GLOBAL_CXXFLAGS_PCH += -include $(buildprefix)precompiled-headers.h -Winvalid-pch GLOBAL_ORDER_AFTER += $(GCH) - else ifeq ($(CXX), clang++) + else ifeq ($(findstring clang++,$(CXX)), clang++) GLOBAL_CXXFLAGS_PCH += -include-pch $(PCH) -Winvalid-pch From f74243846512ffabf082985bca395890c97643e0 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Wed, 29 Apr 2020 22:39:58 +0200 Subject: [PATCH 22/29] Merge legacy `fetchGit`-builtin with the generic `fetchTree`-function The original idea was to implement a git-fetcher in Nix's core that supports content hashes[1]. In #3549[2] it has been suggested to actually use `fetchTree` for this since it's a fairly generic wrapper over the new fetcher-API[3] and already supports content-hashes. This patch implements a new git-fetcher based on `fetchTree` by incorporating the following changes: * Removed the original `fetchGit`-implementation and replaced it with an alias on the `fetchTree` implementation. * Ensured that the `git`-fetcher from `libfetchers` always computes a content-hash and returns an "empty" revision on dirty trees (the latter one is needed to retain backwards-compatibility). * The hash-mismatch error in the fetcher-API exits with code 102 as it usually happens whenever a hash-mismatch is detected by Nix. * Removed the `flakes`-feature-flag: I didn't see a reason why this API is so tightly coupled to the flakes-API and at least `fetchGit` should remain usable without any feature-flags. * It's only possible to specify a `narHash` for a `git`-tree if either a `ref` or a `rev` is given[4]. * It's now possible to specify an URL without a protocol. If it's missing, `file://` is automatically added as it was the case in the original `fetchGit`-implementation. [1] https://github.com/NixOS/nix/pull/3216 [2] https://github.com/NixOS/nix/pull/3549#issuecomment-625194383 [3] https://github.com/NixOS/nix/pull/3459 [4] https://github.com/NixOS/nix/pull/3216#issuecomment-553956703 --- src/libexpr/primops/fetchGit.cc | 91 -------------------------------- src/libexpr/primops/fetchTree.cc | 62 +++++++++++++++++++--- src/libfetchers/fetchers.cc | 2 +- tests/fetchGit.sh | 12 +++-- tests/tarball.sh | 11 ++-- 5 files changed, 73 insertions(+), 105 deletions(-) delete mode 100644 src/libexpr/primops/fetchGit.cc diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc deleted file mode 100644 index 5013e74f0..000000000 --- a/src/libexpr/primops/fetchGit.cc +++ /dev/null @@ -1,91 +0,0 @@ -#include "primops.hh" -#include "eval-inline.hh" -#include "store-api.hh" -#include "hash.hh" -#include "fetchers.hh" -#include "url.hh" - -namespace nix { - -static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Value & v) -{ - std::string url; - std::optional ref; - std::optional rev; - std::string name = "source"; - bool fetchSubmodules = false; - PathSet context; - - state.forceValue(*args[0]); - - if (args[0]->type == tAttrs) { - - state.forceAttrs(*args[0], pos); - - for (auto & attr : *args[0]->attrs) { - string n(attr.name); - if (n == "url") - url = state.coerceToString(*attr.pos, *attr.value, context, false, false); - else if (n == "ref") - ref = state.forceStringNoCtx(*attr.value, *attr.pos); - else if (n == "rev") - rev = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1); - else if (n == "name") - name = state.forceStringNoCtx(*attr.value, *attr.pos); - else if (n == "submodules") - fetchSubmodules = state.forceBool(*attr.value, *attr.pos); - else - throw EvalError({ - .hint = hintfmt("unsupported argument '%s' to 'fetchGit'", attr.name), - .errPos = *attr.pos - }); - } - - if (url.empty()) - throw EvalError({ - .hint = hintfmt("'url' argument required"), - .errPos = pos - }); - - } else - url = state.coerceToString(pos, *args[0], context, false, false); - - // FIXME: git externals probably can be used to bypass the URI - // whitelist. Ah well. - state.checkURI(url); - - if (evalSettings.pureEval && !rev) - throw Error("in pure evaluation mode, 'fetchGit' requires a Git revision"); - - fetchers::Attrs attrs; - attrs.insert_or_assign("type", "git"); - attrs.insert_or_assign("url", url.find("://") != std::string::npos ? url : "file://" + url); - if (ref) attrs.insert_or_assign("ref", *ref); - if (rev) attrs.insert_or_assign("rev", rev->gitRev()); - if (fetchSubmodules) attrs.insert_or_assign("submodules", fetchers::Explicit{true}); - auto input = fetchers::Input::fromAttrs(std::move(attrs)); - - // FIXME: use name? - auto [tree, input2] = input.fetch(state.store); - - state.mkAttrs(v, 8); - auto storePath = state.store->printStorePath(tree.storePath); - mkString(*state.allocAttr(v, state.sOutPath), storePath, PathSet({storePath})); - // Backward compatibility: set 'rev' to - // 0000000000000000000000000000000000000000 for a dirty tree. - auto rev2 = input2.getRev().value_or(Hash(htSHA1)); - mkString(*state.allocAttr(v, state.symbols.create("rev")), rev2.gitRev()); - mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev2.gitShortRev()); - // Backward compatibility: set 'revCount' to 0 for a dirty tree. - mkInt(*state.allocAttr(v, state.symbols.create("revCount")), - input2.getRevCount().value_or(0)); - mkBool(*state.allocAttr(v, state.symbols.create("submodules")), fetchSubmodules); - v.attrs->sort(); - - if (state.allowedPaths) - state.allowedPaths->insert(tree.actualPath); -} - -static RegisterPrimOp r("fetchGit", 1, prim_fetchGit); - -} diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 5f480d919..887d2d14f 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -36,6 +36,9 @@ void emitTreeAttrs( mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev->gitShortRev()); } + if (input.getType() == "git") + mkBool(*state.allocAttr(v, state.symbols.create("submodules")), maybeGetBoolAttr(input.attrs, "submodules").value_or(false)); + if (auto revCount = input.getRevCount()) mkInt(*state.allocAttr(v, state.symbols.create("revCount")), *revCount); @@ -48,10 +51,25 @@ void emitTreeAttrs( v.attrs->sort(); } -static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v) +std::string fixURI(std::string uri, EvalState &state) { - settings.requireExperimentalFeature("flakes"); + state.checkURI(uri); + return uri.find("://") != std::string::npos ? uri : "file://" + uri; +} +void addURI(EvalState &state, fetchers::Attrs &attrs, Symbol name, std::string v) +{ + string n(name); + attrs.emplace(name, n == "url" ? fixURI(v, state) : v); +} + +static void fetchTree( + EvalState &state, + const Pos &pos, + Value **args, + Value &v, + const std::optional type +) { fetchers::Input input; PathSet context; @@ -64,8 +82,15 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V for (auto & attr : *args[0]->attrs) { state.forceValue(*attr.value); - if (attr.value->type == tString) - attrs.emplace(attr.name, attr.value->string.s); + if (attr.value->type == tPath || attr.value->type == tString) + addURI( + state, + attrs, + attr.name, + state.coerceToString(*attr.pos, *attr.value, context, false, false) + ); + else if (attr.value->type == tString) + addURI(state, attrs, attr.name, attr.value->string.s); else if (attr.value->type == tBool) attrs.emplace(attr.name, fetchers::Explicit{attr.value->boolean}); else if (attr.value->type == tInt) @@ -75,6 +100,9 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V attr.name, showType(*attr.value)); } + if (type) + attrs.emplace("type", type.value()); + if (!attrs.count("type")) throw Error({ .hint = hintfmt("attribute 'type' is missing in call to 'fetchTree'"), @@ -82,8 +110,18 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V }); input = fetchers::Input::fromAttrs(std::move(attrs)); - } else - input = fetchers::Input::fromURL(state.coerceToString(pos, *args[0], context, false, false)); + } else { + auto url = fixURI(state.coerceToString(pos, *args[0], context, false, false), state); + + if (type == "git") { + fetchers::Attrs attrs; + attrs.emplace("type", "git"); + attrs.emplace("url", url); + input = fetchers::Input::fromAttrs(std::move(attrs)); + } else { + input = fetchers::Input::fromURL(url); + } + } if (!evalSettings.pureEval && !input.isDirect()) input = lookupInRegistries(state.store, input).first; @@ -99,6 +137,12 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V emitTreeAttrs(state, tree, input2, v); } +static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + settings.requireExperimentalFeature("flakes"); + fetchTree(state, pos, args, v, std::nullopt); +} + static RegisterPrimOp r("fetchTree", 1, prim_fetchTree); static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v, @@ -178,7 +222,13 @@ static void prim_fetchTarball(EvalState & state, const Pos & pos, Value * * args fetch(state, pos, args, v, "fetchTarball", true, "source"); } +static void prim_fetchGit(EvalState &state, const Pos &pos, Value **args, Value &v) +{ + fetchTree(state, pos, args, v, "git"); +} + static RegisterPrimOp r2("__fetchurl", 1, prim_fetchurl); static RegisterPrimOp r3("fetchTarball", 1, prim_fetchTarball); +static RegisterPrimOp r4("fetchGit", 1, prim_fetchGit); } diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index c3ee9bf43..28db8aa9c 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -134,7 +134,7 @@ std::pair Input::fetch(ref store) const if (auto prevNarHash = getNarHash()) { if (narHash != *prevNarHash) - throw Error("NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'", + throw Error((unsigned int) 102, "NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'", to_string(), tree.actualPath, prevNarHash->to_string(SRI, true), narHash->to_string(SRI, true)); } diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index 9faa5d9f6..1b9364e60 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -32,6 +32,8 @@ rev2=$(git -C $repo rev-parse HEAD) # Fetch a worktree unset _NIX_FORCE_HTTP path0=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$TEST_ROOT/worktree).outPath") +path0_=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; url = file://$TEST_ROOT/worktree; }).outPath") +[[ $path0 = $path0_ ]] export _NIX_FORCE_HTTP=1 [[ $(tail -n 1 $path0/hello) = "hello" ]] @@ -87,8 +89,6 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [ ! -e $path2/.git ] [[ $(cat $path2/dir1/foo) = foo ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] - # ... unless we're using an explicit ref or rev. path3=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"master\"; }).outPath") [[ $path = $path3 ]] @@ -102,6 +102,12 @@ git -C $repo commit -m 'Bla3' -a path4=$(nix eval --impure --refresh --raw --expr "(builtins.fetchGit file://$repo).outPath") [[ $path2 = $path4 ]] +nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-B5yIPHhEm0eysJKEsO7nqxprh9vcblFxpJG11gXJus1=\"; }).outPath" || status=$? +[[ "$status" = "102" ]] + +path5=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-Hr8g6AqANb3xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath") +[[ $path = $path5 ]] + # tarball-ttl should be ignored if we specify a rev echo delft > $repo/hello git -C $repo add hello @@ -123,7 +129,7 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath # Using local path with branch other than 'master' should work when clean or dirty path3=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") # (check dirty-tree handling was used) -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] +[[ $(nix eval --impure --expr "(builtins.fetchGit $repo).rev or null") = null ]] # Committing shouldn't change store path, or switch to using 'master' git -C $repo commit -m 'Bla5' -a diff --git a/tests/tarball.sh b/tests/tarball.sh index b3ec16d40..88a1a07a0 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -27,10 +27,13 @@ test_tarball() { nix-build -o $TEST_ROOT/result -E "import (fetchTarball file://$tarball)" - nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree file://$tarball)" - nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; })" - nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })" - nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"sha256-xdKv2pq/IiwLSnBBJXW8hNowI4MrdZfW+SYqDQs7Tzc=\"; })" 2>&1 | grep 'NAR hash mismatch in input' + nix-build -o $TEST_ROOT/result -E "import (fetchTree file://$tarball)" + nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; })" + nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })" + nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"sha256-xdKv2pq/IiwLSnBBJXW8hNowI4MrdZfW+SYqDQs7Tzc=\"; })" 2>&1 | grep 'NAR hash mismatch in input' + + nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" >&2 + nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" 2>&1 | grep 'true' nix-instantiate --eval -E '1 + 2' -I fnord=file://no-such-tarball.tar$ext nix-instantiate --eval -E 'with ; 1 + 2' -I fnord=file://no-such-tarball$ext From 189e6f5e1d949f50ab0b6e5acd25e230d206692d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 24 Jul 2020 20:51:16 +0200 Subject: [PATCH 23/29] Bump version to 3.0 Since there are some incompatible changes, it's better to bump the major version number. --- .version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.version b/.version index 7208c2182..f398a2061 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -2.4 \ No newline at end of file +3.0 \ No newline at end of file From b8eea7e81af53905be7845dffc6d0a83ea8edc97 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 28 Jul 2020 13:27:56 +0200 Subject: [PATCH 24/29] Remove putBytes istream->tellg() returns -1 so we can't get the number of bytes written. Fixes 'uploaded 's3://nix-cache/nar/00819r9lp5kajr6baxfw5dhhc0cx8ndxaz43qmd2f0gn1hk1ynlp.nar.xz' (-1 bytes) in 11620 ms' messages. --- src/libstore/s3-binary-cache-store.cc | 7 ++----- src/libstore/s3-binary-cache-store.hh | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 1b7dff085..67935f3ba 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -343,13 +343,10 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore std::chrono::duration_cast(now2 - now1) .count(); - auto size = istream->tellg(); - - printInfo("uploaded 's3://%s/%s' (%d bytes) in %d ms", - bucketName, path, size, duration); + printInfo("uploaded 's3://%s/%s' in %d ms", + bucketName, path, duration); stats.putTimeMs += duration; - stats.putBytes += size; stats.put++; } diff --git a/src/libstore/s3-binary-cache-store.hh b/src/libstore/s3-binary-cache-store.hh index 4d43fe4d2..b2b75d498 100644 --- a/src/libstore/s3-binary-cache-store.hh +++ b/src/libstore/s3-binary-cache-store.hh @@ -19,7 +19,6 @@ public: struct Stats { std::atomic put{0}; - std::atomic putBytes{0}; std::atomic putTimeMs{0}; std::atomic get{0}; std::atomic getBytes{0}; From e4940e90f399c5cde5f44f099ca5cdf5340d22b8 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 28 Jul 2020 22:46:39 +0200 Subject: [PATCH 25/29] Restore backwards-compat for current `builtins.fetchGit` If a repo is dirty, it used to return a `rev` object with an "empty" sha1 (0000000000000000000000000000000000000000). Please note that this only applies for `builtins.fetchGit` and *not* for `builtins.fetchTree{ type = "git"; }`. --- src/libexpr/flake/flake.hh | 2 +- src/libexpr/primops/fetchTree.cc | 17 +++++++++++++---- tests/fetchGit.sh | 4 +++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh index 77f3abdeb..c2bb2888b 100644 --- a/src/libexpr/flake/flake.hh +++ b/src/libexpr/flake/flake.hh @@ -106,6 +106,6 @@ void emitTreeAttrs( EvalState & state, const fetchers::Tree & tree, const fetchers::Input & input, - Value & v); + Value & v, bool emptyRevFallback = false); } diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 887d2d14f..cddcf0e59 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -14,7 +14,8 @@ void emitTreeAttrs( EvalState & state, const fetchers::Tree & tree, const fetchers::Input & input, - Value & v) + Value & v, + bool emptyRevFallback) { assert(input.isImmutable()); @@ -34,6 +35,11 @@ void emitTreeAttrs( if (auto rev = input.getRev()) { mkString(*state.allocAttr(v, state.symbols.create("rev")), rev->gitRev()); mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev->gitShortRev()); + } else if (emptyRevFallback) { + // Backwards compat for `builtins.fetchGit`: dirty repos return an empty sha1 as rev + auto emptyHash = Hash(htSHA1); + mkString(*state.allocAttr(v, state.symbols.create("rev")), emptyHash.gitRev()); + mkString(*state.allocAttr(v, state.symbols.create("shortRev")), emptyHash.gitRev()); } if (input.getType() == "git") @@ -41,6 +47,8 @@ void emitTreeAttrs( if (auto revCount = input.getRevCount()) mkInt(*state.allocAttr(v, state.symbols.create("revCount")), *revCount); + else if (emptyRevFallback) + mkInt(*state.allocAttr(v, state.symbols.create("revCount")), 0); if (auto lastModified = input.getLastModified()) { mkInt(*state.allocAttr(v, state.symbols.create("lastModified")), *lastModified); @@ -68,7 +76,8 @@ static void fetchTree( const Pos &pos, Value **args, Value &v, - const std::optional type + const std::optional type, + bool emptyRevFallback = false ) { fetchers::Input input; PathSet context; @@ -134,7 +143,7 @@ static void fetchTree( if (state.allowedPaths) state.allowedPaths->insert(tree.actualPath); - emitTreeAttrs(state, tree, input2, v); + emitTreeAttrs(state, tree, input2, v, emptyRevFallback); } static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v) @@ -224,7 +233,7 @@ static void prim_fetchTarball(EvalState & state, const Pos & pos, Value * * args static void prim_fetchGit(EvalState &state, const Pos &pos, Value **args, Value &v) { - fetchTree(state, pos, args, v, "git"); + fetchTree(state, pos, args, v, "git", true); } static RegisterPrimOp r2("__fetchurl", 1, prim_fetchurl); diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index 1b9364e60..cedd796f7 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -89,6 +89,8 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [ ! -e $path2/.git ] [[ $(cat $path2/dir1/foo) = foo ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] + # ... unless we're using an explicit ref or rev. path3=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"master\"; }).outPath") [[ $path = $path3 ]] @@ -129,7 +131,7 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath # Using local path with branch other than 'master' should work when clean or dirty path3=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") # (check dirty-tree handling was used) -[[ $(nix eval --impure --expr "(builtins.fetchGit $repo).rev or null") = null ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] # Committing shouldn't change store path, or switch to using 'master' git -C $repo commit -m 'Bla5' -a From c159f48a39835d5b2fe7c1ddd4467bc093ee251f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 29 Jul 2020 00:24:55 +0200 Subject: [PATCH 26/29] Cleanup --- src/libstore/store-api.cc | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b6e6952b7..c804399d2 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -729,9 +729,9 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st { auto valid = dstStore->queryValidPaths(storePaths, substitute); - PathSet missing; + StorePathSet missing; for (auto & path : storePaths) - if (!valid.count(path)) missing.insert(srcStore->printStorePath(path)); + if (!valid.count(path)) missing.insert(path); if (missing.empty()) return; @@ -748,29 +748,27 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st ThreadPool pool; - processGraph(pool, - PathSet(missing.begin(), missing.end()), + processGraph(pool, + StorePathSet(missing.begin(), missing.end()), - [&](const Path & storePath) { - if (dstStore->isValidPath(dstStore->parseStorePath(storePath))) { + [&](const StorePath & storePath) { + if (dstStore->isValidPath(storePath)) { nrDone++; showProgress(); - return PathSet(); + return StorePathSet(); } - auto info = srcStore->queryPathInfo(srcStore->parseStorePath(storePath)); + auto info = srcStore->queryPathInfo(storePath); bytesExpected += info->narSize; act.setExpected(actCopyPath, bytesExpected); - return srcStore->printStorePathSet(info->references); + return info->references; }, - [&](const Path & storePathS) { + [&](const StorePath & storePath) { checkInterrupt(); - auto storePath = dstStore->parseStorePath(storePathS); - if (!dstStore->isValidPath(storePath)) { MaintainCount mc(nrRunning); showProgress(); @@ -780,7 +778,7 @@ void copyPaths(ref srcStore, ref dstStore, const StorePathSet & st nrFailed++; if (!settings.keepGoing) throw e; - logger->log(lvlError, fmt("could not copy %s: %s", storePathS, e.what())); + logger->log(lvlError, fmt("could not copy %s: %s", dstStore->printStorePath(storePath), e.what())); showProgress(); return; } From 4c0077a07d2ee5362bd8ffb0ea9ee053c759303d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 29 Jul 2020 00:48:39 +0200 Subject: [PATCH 27/29] Fix RemoteStore::addToStore() latency Since 6185d25e523a3cd223dd6f6aca10cf6ff15b4823, this was very latency-bound since it required a round-trip for every 32 KiB. So for example copying a 514 MiB closure over a virtual ethernet device with a articial delay of just 1 ms took 343s. Now it takes 2.7s. Fixes #3372. --- src/libstore/daemon.cc | 90 +++++++++++++++++++++++++++------ src/libstore/remote-store.cc | 81 +++++++++++++++++++++++++++-- src/libstore/worker-protocol.hh | 2 +- 3 files changed, 154 insertions(+), 19 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 503e04f92..478ae39ca 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -86,7 +86,7 @@ struct TunnelLogger : public Logger } /* startWork() means that we're starting an operation for which we - want to send out stderr to the client. */ + want to send out stderr to the client. */ void startWork() { auto state(state_.lock()); @@ -703,24 +703,84 @@ static void performOp(TunnelLogger * logger, ref store, if (!trusted) info.ultimate = false; - std::unique_ptr source; - if (GET_PROTOCOL_MINOR(clientVersion) >= 21) - source = std::make_unique(from, to); - else { - StringSink saved; - TeeSource tee { from, saved }; - ParseSink ether; - parseDump(ether, tee); - source = std::make_unique(std::move(*saved.s)); + if (GET_PROTOCOL_MINOR(clientVersion) >= 23) { + + struct FramedSource : Source + { + Source & from; + bool eof = false; + std::vector pending; + size_t pos = 0; + + FramedSource(Source & from) : from(from) + { } + + ~FramedSource() + { + if (!eof) { + while (true) { + auto n = readInt(from); + if (!n) break; + std::vector data(n); + from(data.data(), n); + } + } + } + + size_t read(unsigned char * data, size_t len) override + { + if (eof) throw EndOfFile("reached end of FramedSource"); + + if (pos >= pending.size()) { + size_t len = readInt(from); + if (!len) { + eof = true; + return 0; + } + pending = std::vector(len); + pos = 0; + from(pending.data(), len); + } + + auto n = std::min(len, pending.size() - pos); + memcpy(data, pending.data() + pos, n); + pos += n; + return n; + } + }; + + logger->startWork(); + + { + FramedSource source(from); + store->addToStore(info, source, (RepairFlag) repair, + dontCheckSigs ? NoCheckSigs : CheckSigs); + } + + logger->stopWork(); } - logger->startWork(); + else { + std::unique_ptr source; + if (GET_PROTOCOL_MINOR(clientVersion) >= 21) + source = std::make_unique(from, to); + else { + StringSink saved; + TeeSource tee { from, saved }; + ParseSink ether; + parseDump(ether, tee); + source = std::make_unique(std::move(*saved.s)); + } - // FIXME: race if addToStore doesn't read source? - store->addToStore(info, *source, (RepairFlag) repair, - dontCheckSigs ? NoCheckSigs : CheckSigs); + logger->startWork(); + + // FIXME: race if addToStore doesn't read source? + store->addToStore(info, *source, (RepairFlag) repair, + dontCheckSigs ? NoCheckSigs : CheckSigs); + + logger->stopWork(); + } - logger->stopWork(); break; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 8d01c6667..4ed81d9d8 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -503,9 +503,84 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, conn->to << info.registrationTime << info.narSize << info.ultimate << info.sigs << renderContentAddress(info.ca) << repair << !checkSigs; - bool tunnel = GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21; - if (!tunnel) copyNAR(source, conn->to); - conn.processStderr(0, tunnel ? &source : nullptr); + + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) { + + std::exception_ptr ex; + + struct FramedSink : BufferedSink + { + ConnectionHandle & conn; + std::exception_ptr & ex; + + FramedSink(ConnectionHandle & conn, std::exception_ptr & ex) : conn(conn), ex(ex) + { } + + ~FramedSink() + { + try { + conn->to << 0; + conn->to.flush(); + } catch (...) { + ignoreException(); + } + } + + void write(const unsigned char * data, size_t len) override + { + /* Don't send more data if the remote has + encountered an error. */ + if (ex) { + auto ex2 = ex; + ex = nullptr; + std::rethrow_exception(ex2); + } + conn->to << len; + conn->to(data, len); + }; + }; + + /* Handle log messages / exceptions from the remote on a + separate thread. */ + std::thread stderrThread([&]() + { + try { + conn.processStderr(0, nullptr); + } catch (...) { + ex = std::current_exception(); + } + }); + + Finally joinStderrThread([&]() + { + if (stderrThread.joinable()) { + stderrThread.join(); + if (ex) { + try { + std::rethrow_exception(ex); + } catch (...) { + ignoreException(); + } + } + } + }); + + { + FramedSink sink(conn, ex); + copyNAR(source, sink); + sink.flush(); + } + + stderrThread.join(); + if (ex) + std::rethrow_exception(ex); + + } else if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21) { + conn.processStderr(0, &source); + } else { + copyNAR(source, conn->to); + conn.processStderr(0, nullptr); + } } } diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index 8b538f6da..dcba73116 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -6,7 +6,7 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f -#define PROTOCOL_VERSION 0x116 +#define PROTOCOL_VERSION 0x117 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) From fa8515d7ec160938b0287f405a43aeeca971777e Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 29 Jul 2020 00:57:20 -0500 Subject: [PATCH 28/29] Set regex_constants::match_continuous for quicker search match_continuous limits the search to the current start position, instead of searching the entire file. On libc++, this improves performance dramatically: $ time /nix/store/70ai68dfm6xbzwn26j5n4li9di52ylia-nix-3.0pre20200728_c159f48/bin/nix print-dev-env >/dev/null /nix/store/70ai68dfm6xbzwn26j5n4li9di52ylia-nix-3.0pre20200728_c159f48/bin/ni 2.39s user 0.19s system 64% cpu 4.032 total $ time /nix/store/cwjfxxlp83zln4mfyy1d2dbsx7f6s962-nix-3.0pre20200728_dirty/bin/nix print-dev-env >/dev/null /nix/store/cwjfxxlp83zln4mfyy1d2dbsx7f6s962-nix-3.0pre20200728_dirty/bin/nix 0.09s user 0.05s system 65% cpu 0.204 total Fixes #3874 --- src/nix/develop.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 2f590f53f..a0c119e43 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -68,22 +68,22 @@ BuildEnvironment readEnvironment(const Path & path) std::smatch match; - if (std::regex_search(pos, file.cend(), match, declareRegex)) { + if (std::regex_search(pos, file.cend(), match, declareRegex, std::regex_constants::match_continuous)) { pos = match[0].second; exported.insert(match[1]); } - else if (std::regex_search(pos, file.cend(), match, varRegex)) { + else if (std::regex_search(pos, file.cend(), match, varRegex, std::regex_constants::match_continuous)) { pos = match[0].second; res.env.insert({match[1], Var { .exported = exported.count(match[1]) > 0, .value = match[2] }}); } - else if (std::regex_search(pos, file.cend(), match, assocArrayRegex)) { + else if (std::regex_search(pos, file.cend(), match, assocArrayRegex, std::regex_constants::match_continuous)) { pos = match[0].second; res.env.insert({match[1], Var { .associative = true, .value = match[2] }}); } - else if (std::regex_search(pos, file.cend(), match, functionRegex)) { + else if (std::regex_search(pos, file.cend(), match, functionRegex, std::regex_constants::match_continuous)) { res.bashFunctions = std::string(pos, file.cend()); break; } From f63839bfa402f73e001a5ee412c3f6a6b95d1a38 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 29 Jul 2020 18:04:35 +0200 Subject: [PATCH 29/29] Cleanup --- src/libstore/remote-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 4ed81d9d8..616081aa1 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -545,7 +545,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, std::thread stderrThread([&]() { try { - conn.processStderr(0, nullptr); + conn.processStderr(); } catch (...) { ex = std::current_exception(); }