From 64f03635d763bd627f4fb8516c04eb64e881c902 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 12 Jul 2020 16:50:22 +0200 Subject: [PATCH 01/17] Fix ANSI color constants The `m` acts as termination-symbol when declaring graphics. Because of this, the `;1m` doesn't have any effect and is directly printed to the console: ``` $ nix repl > builtins.fetchGit { /* ... */ } { outPath = "/nix/store/s0f0iz4a41cxx2h055lmh6p2d5k5bc6r-source"; rev = "e73e45b723a9a6eecb98bd5f3df395d9ab3633b6"; revCount = ;1m428; shortRev = "e73e45b"; submodules = ;1mfalse; } ``` Introduced by 6403508f5a2fcf073b2a0d4e5fcf5f5ebb890384. --- src/libutil/ansicolor.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/ansicolor.hh b/src/libutil/ansicolor.hh index a38c2d798..ae741f867 100644 --- a/src/libutil/ansicolor.hh +++ b/src/libutil/ansicolor.hh @@ -11,7 +11,7 @@ namespace nix { #define ANSI_GREEN "\e[32;1m" #define ANSI_YELLOW "\e[33;1m" #define ANSI_BLUE "\e[34;1m" -#define ANSI_MAGENTA "\e[35m;1m" -#define ANSI_CYAN "\e[36m;1m" +#define ANSI_MAGENTA "\e[35;1m" +#define ANSI_CYAN "\e[36;1m" } From 41bdf429ecc6cb7a1ebdd004649c3cec0511ab22 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 13:32:33 +0200 Subject: [PATCH 02/17] Add a test for NAR listing generation --- tests/binary-cache.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index 17b63d978..28af3ca5e 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -182,3 +182,22 @@ clearCacheCache nix-store -r $outPath --substituters "file://$cacheDir2 file://$cacheDir" --trusted-public-keys "$publicKey" fi # HAVE_LIBSODIUM + + +unset _NIX_FORCE_HTTP + + +# Test NAR listing generation. +clearCache + +outPath=$(nix-build --no-out-link -E ' + with import ./config.nix; + mkDerivation { + name = "nar-listing"; + buildCommand = "mkdir $out; echo foo > $out/bar; ln -s xyzzy $out/link"; + } +') + +nix copy --to file://$cacheDir?write-nar-listing=1 $outPath + +[[ $(cat $cacheDir/$(basename $outPath).ls) = '{"version":1,"root":{"type":"directory","entries":{"bar":{"type":"regular","size":4,"narOffset":232},"link":{"type":"symlink","target":"xyzzy"}}}}' ]] From 2900a441f5e2a05bc10186e37b4084acb7eca83c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 13:37:22 +0200 Subject: [PATCH 03/17] Add a test for DWARF debug info index generation --- tests/binary-cache.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index 28af3ca5e..59cdfef2e 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -201,3 +201,19 @@ outPath=$(nix-build --no-out-link -E ' nix copy --to file://$cacheDir?write-nar-listing=1 $outPath [[ $(cat $cacheDir/$(basename $outPath).ls) = '{"version":1,"root":{"type":"directory","entries":{"bar":{"type":"regular","size":4,"narOffset":232},"link":{"type":"symlink","target":"xyzzy"}}}}' ]] + + +# Test debug info index generation. +clearCache + +outPath=$(nix-build --no-out-link -E ' + with import ./config.nix; + mkDerivation { + name = "debug-info"; + buildCommand = "mkdir -p $out/lib/debug/.build-id/02; echo foo > $out/lib/debug/.build-id/02/623eda209c26a59b1a8638ff7752f6b945c26b.debug"; + } +') + +nix copy --to "file://$cacheDir?index-debug-info=1&compression=none" $outPath + +[[ $(cat $cacheDir/debuginfo/02623eda209c26a59b1a8638ff7752f6b945c26b.debug) = '{"archive":"../nar/100vxs724qr46phz8m24iswmg9p3785hsyagz0kchf6q6gf06sw6.nar","member":"lib/debug/.build-id/02/623eda209c26a59b1a8638ff7752f6b945c26b.debug"}' ]] From 1d01ae816b80eaefb0996a9605d00a3031ecd4d9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 14:35:01 +0200 Subject: [PATCH 04/17] Fix 'nix verify --all' on a binary cache and add a test --- src/libstore/local-binary-cache-store.cc | 4 +++- src/libstore/s3-binary-cache-store.cc | 2 +- src/libstore/store-api.cc | 12 +++++++++--- src/libstore/store-api.hh | 9 ++++++--- tests/binary-cache.sh | 4 ++++ 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 48aca478c..215c016f5 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -52,7 +52,9 @@ protected: if (entry.name.size() != 40 || !hasSuffix(entry.name, ".narinfo")) continue; - paths.insert(parseStorePath(storeDir + "/" + entry.name.substr(0, entry.name.size() - 8))); + paths.insert(parseStorePath( + storeDir + "/" + entry.name.substr(0, entry.name.size() - 8) + + "-" + MissingName)); } return paths; diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 427dd48ce..f85563766 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -410,7 +410,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore for (auto object : contents) { auto & key = object.GetKey(); if (key.size() != 40 || !hasSuffix(key, ".narinfo")) continue; - paths.insert(parseStorePath(storeDir + "/" + key.substr(0, key.size() - 8) + "-unknown")); + paths.insert(parseStorePath(storeDir + "/" + key.substr(0, key.size() - 8) + "-" + MissingName)); } marker = res.GetNextMarker(); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 8d46bb436..0c6788b69 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -390,7 +390,7 @@ void Store::queryPathInfo(const StorePath & storePath, auto callbackPtr = std::make_shared(std::move(callback)); queryPathInfoUncached(storePath, - {[this, storePath{printStorePath(storePath)}, hashPart, callbackPtr](std::future> fut) { + {[this, storePathS{printStorePath(storePath)}, hashPart, callbackPtr](std::future> fut) { try { auto info = fut.get(); @@ -403,9 +403,15 @@ void Store::queryPathInfo(const StorePath & storePath, state_->pathInfoCache.upsert(hashPart, PathInfoCacheValue { .value = info }); } - if (!info || info->path != parseStorePath(storePath)) { + auto storePath = parseStorePath(storePathS); + + if (!info + || info->path.hashPart() != storePath.hashPart() + || (storePath.name() != MissingName && info->path.name() != storePath.name()) + ) + { stats.narInfoMissing++; - throw InvalidPath("path '%s' is not valid", storePath); + throw InvalidPath("path '%s' is not valid", storePathS); } (*callbackPtr)(ref(info)); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index b1dd1f478..0be0021f5 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -384,13 +384,16 @@ public: SubstituteFlag maybeSubstitute = NoSubstitute); /* Query the set of all valid paths. Note that for some store - backends, the name part of store paths may be omitted - (i.e. you'll get /nix/store/ rather than + backends, the name part of store paths may be replaced by 'x' + (i.e. you'll get /nix/store/-x rather than /nix/store/-). Use queryPathInfo() to obtain the - full store path. */ + full store path. FIXME: should return a set of + std::variant to get rid of this hack. */ virtual StorePathSet queryAllValidPaths() { unsupported("queryAllValidPaths"); } + constexpr static const char * MissingName = "x"; + /* Query information about a valid path. It is permitted to omit the name part of the store path. */ ref queryPathInfo(const StorePath & path); diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index 59cdfef2e..17d7a2df6 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -187,6 +187,10 @@ fi # HAVE_LIBSODIUM unset _NIX_FORCE_HTTP +# Test 'nix verify --all' on a binary cache. +nix verify -vvvvv --all --store file://$cacheDir --no-trust + + # Test NAR listing generation. clearCache From 143a5f32ed2ce37ce6edddf36ed7ed984532541f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 16:01:00 +0200 Subject: [PATCH 05/17] Add a test for local NAR caching --- tests/binary-cache.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index 17d7a2df6..40f1a4f76 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -191,6 +191,20 @@ unset _NIX_FORCE_HTTP nix verify -vvvvv --all --store file://$cacheDir --no-trust +# Test local NAR caching. +narCache=$TEST_ROOT/nar-cache +rm -rf $narCache +mkdir $narCache + +[[ $(nix cat-store --store "file://$cacheDir?local-nar-cache=$narCache" $outPath/foobar) = FOOBAR ]] + +rm -rfv "$cacheDir/nar" + +[[ $(nix cat-store --store "file://$cacheDir?local-nar-cache=$narCache" $outPath/foobar) = FOOBAR ]] + +(! nix cat-store --store file://$cacheDir $outPath/foobar) + + # Test NAR listing generation. clearCache From c0dd05131e08102d560a737ff68c66bc8314f5fa Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 16:19:37 +0200 Subject: [PATCH 06/17] toStorePath(): Return a StorePath and the suffix --- src/libexpr/eval.cc | 2 +- src/libexpr/primops.cc | 6 +++--- src/libstore/build.cc | 4 ++-- src/libstore/gc.cc | 30 ++++++++++++++------------- src/libstore/local-fs-store.cc | 6 +++--- src/libstore/path.cc | 2 -- src/libstore/remote-fs-accessor.cc | 33 +++++++++++++++--------------- src/libstore/remote-fs-accessor.hh | 6 +++--- src/libstore/store-api.cc | 10 ++++----- src/libstore/store-api.hh | 8 ++++---- src/nix/installables.cc | 14 ++++++------- 11 files changed, 60 insertions(+), 61 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index c1a9af9b2..58066fa48 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -366,7 +366,7 @@ EvalState::EvalState(const Strings & _searchPath, ref store) if (store->isInStore(r.second)) { StorePathSet closure; - store->computeFSClosure(store->parseStorePath(store->toStorePath(r.second)), closure); + store->computeFSClosure(store->toStorePath(r.second).first, closure); for (auto & path : closure) allowedPaths->insert(store->printStorePath(path)); } else diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index dec917b38..1c6fd5b33 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -883,10 +883,10 @@ static void prim_storePath(EvalState & state, const Pos & pos, Value * * args, V .hint = hintfmt("path '%1%' is not in the Nix store", path), .errPos = pos }); - Path path2 = state.store->toStorePath(path); + auto path2 = state.store->toStorePath(path).first; if (!settings.readOnlyMode) - state.store->ensurePath(state.store->parseStorePath(path2)); - context.insert(path2); + state.store->ensurePath(path2); + context.insert(state.store->printStorePath(path2)); mkString(v, path, context); } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0ef2f288f..96f7f2603 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2044,7 +2044,7 @@ void DerivationGoal::startBuilder() auto storePathS = *i++; if (!worker.store.isInStore(storePathS)) throw BuildError("'exportReferencesGraph' contains a non-store path '%1%'", storePathS); - auto storePath = worker.store.parseStorePath(worker.store.toStorePath(storePathS)); + auto storePath = worker.store.toStorePath(storePathS).first; /* Write closure info to . */ writeFile(tmpDir + "/" + fileName, @@ -2083,7 +2083,7 @@ void DerivationGoal::startBuilder() for (auto & i : dirsInChroot) try { if (worker.store.isInStore(i.second.source)) - worker.store.computeFSClosure(worker.store.parseStorePath(worker.store.toStorePath(i.second.source)), closure); + worker.store.computeFSClosure(worker.store.toStorePath(i.second.source).first, closure); } catch (InvalidPath & e) { } catch (Error & e) { throw Error("while processing 'sandbox-paths': %s", e.what()); diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 57fb20845..aaed5c218 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -262,11 +262,13 @@ void LocalStore::findTempRoots(FDs & fds, Roots & tempRoots, bool censor) void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) { auto foundRoot = [&](const Path & path, const Path & target) { - auto storePath = maybeParseStorePath(toStorePath(target)); - if (storePath && isValidPath(*storePath)) - roots[std::move(*storePath)].emplace(path); - else - printInfo("skipping invalid root from '%1%' to '%2%'", path, target); + try { + auto storePath = toStorePath(target).first; + if (isValidPath(storePath)) + roots[std::move(storePath)].emplace(path); + else + printInfo("skipping invalid root from '%1%' to '%2%'", path, target); + } catch (BadStorePath &) { } }; try { @@ -472,15 +474,15 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor) for (auto & [target, links] : unchecked) { if (!isInStore(target)) continue; - Path pathS = toStorePath(target); - if (!isStorePath(pathS)) continue; - auto path = parseStorePath(pathS); - if (!isValidPath(path)) continue; - debug("got additional root '%1%'", pathS); - if (censor) - roots[path].insert(censored); - else - roots[path].insert(links.begin(), links.end()); + try { + auto path = toStorePath(target).first; + if (!isValidPath(path)) continue; + debug("got additional root '%1%'", printStorePath(path)); + if (censor) + roots[path].insert(censored); + else + roots[path].insert(links.begin(), links.end()); + } catch (BadStorePath &) { } } } diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index dd96d2578..2f1d9663a 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -20,9 +20,9 @@ struct LocalStoreAccessor : public FSAccessor Path toRealPath(const Path & path) { - Path storePath = store->toStorePath(path); - if (!store->isValidPath(store->parseStorePath(storePath))) - throw InvalidPath("path '%1%' is not a valid store path", storePath); + auto storePath = store->toStorePath(path).first; + if (!store->isValidPath(storePath)) + throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); return store->getRealStoreDir() + std::string(path, store->storeDir.size()); } diff --git a/src/libstore/path.cc b/src/libstore/path.cc index b3d8ce95c..dc9dc3897 100644 --- a/src/libstore/path.cc +++ b/src/libstore/path.cc @@ -2,8 +2,6 @@ namespace nix { -MakeError(BadStorePath, Error); - static void checkName(std::string_view path, std::string_view name) { if (name.empty()) diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc index bd698d781..2d02a181b 100644 --- a/src/libstore/remote-fs-accessor.cc +++ b/src/libstore/remote-fs-accessor.cc @@ -16,26 +16,26 @@ RemoteFSAccessor::RemoteFSAccessor(ref store, const Path & cacheDir) createDirs(cacheDir); } -Path RemoteFSAccessor::makeCacheFile(const Path & storePath, const std::string & ext) +Path RemoteFSAccessor::makeCacheFile(std::string_view hashPart, const std::string & ext) { assert(cacheDir != ""); - return fmt("%s/%s.%s", cacheDir, store->parseStorePath(storePath).hashPart(), ext); + return fmt("%s/%s.%s", cacheDir, hashPart, ext); } -void RemoteFSAccessor::addToCache(const Path & storePath, const std::string & nar, +void RemoteFSAccessor::addToCache(std::string_view hashPart, const std::string & nar, ref narAccessor) { - nars.emplace(storePath, narAccessor); + nars.emplace(hashPart, narAccessor); if (cacheDir != "") { try { std::ostringstream str; JSONPlaceholder jsonRoot(str); listNar(jsonRoot, narAccessor, "", true); - writeFile(makeCacheFile(storePath, "ls"), str.str()); + writeFile(makeCacheFile(hashPart, "ls"), str.str()); /* FIXME: do this asynchronously. */ - writeFile(makeCacheFile(storePath, "nar"), nar); + writeFile(makeCacheFile(hashPart, "nar"), nar); } catch (...) { ignoreException(); @@ -47,23 +47,22 @@ std::pair, Path> RemoteFSAccessor::fetch(const Path & path_) { auto path = canonPath(path_); - auto storePath = store->toStorePath(path); - std::string restPath = std::string(path, storePath.size()); + auto [storePath, restPath] = store->toStorePath(path); - if (!store->isValidPath(store->parseStorePath(storePath))) - throw InvalidPath("path '%1%' is not a valid store path", storePath); + if (!store->isValidPath(storePath)) + throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); - auto i = nars.find(storePath); + auto i = nars.find(std::string(storePath.hashPart())); if (i != nars.end()) return {i->second, restPath}; StringSink sink; std::string listing; Path cacheFile; - if (cacheDir != "" && pathExists(cacheFile = makeCacheFile(storePath, "nar"))) { + if (cacheDir != "" && pathExists(cacheFile = makeCacheFile(storePath.hashPart(), "nar"))) { try { - listing = nix::readFile(makeCacheFile(storePath, "ls")); + listing = nix::readFile(makeCacheFile(storePath.hashPart(), "ls")); auto narAccessor = makeLazyNarAccessor(listing, [cacheFile](uint64_t offset, uint64_t length) { @@ -81,7 +80,7 @@ std::pair, Path> RemoteFSAccessor::fetch(const Path & path_) return buf; }); - nars.emplace(storePath, narAccessor); + nars.emplace(storePath.hashPart(), narAccessor); return {narAccessor, restPath}; } catch (SysError &) { } @@ -90,15 +89,15 @@ std::pair, Path> RemoteFSAccessor::fetch(const Path & path_) *sink.s = nix::readFile(cacheFile); auto narAccessor = makeNarAccessor(sink.s); - nars.emplace(storePath, narAccessor); + nars.emplace(storePath.hashPart(), narAccessor); return {narAccessor, restPath}; } catch (SysError &) { } } - store->narFromPath(store->parseStorePath(storePath), sink); + store->narFromPath(storePath, sink); auto narAccessor = makeNarAccessor(sink.s); - addToCache(storePath, *sink.s, narAccessor); + addToCache(storePath.hashPart(), *sink.s, narAccessor); return {narAccessor, restPath}; } diff --git a/src/libstore/remote-fs-accessor.hh b/src/libstore/remote-fs-accessor.hh index 4afb3be95..347cf5764 100644 --- a/src/libstore/remote-fs-accessor.hh +++ b/src/libstore/remote-fs-accessor.hh @@ -10,7 +10,7 @@ class RemoteFSAccessor : public FSAccessor { ref store; - std::map> nars; + std::map> nars; Path cacheDir; @@ -18,9 +18,9 @@ class RemoteFSAccessor : public FSAccessor friend class BinaryCacheStore; - Path makeCacheFile(const Path & storePath, const std::string & ext); + Path makeCacheFile(std::string_view hashPart, const std::string & ext); - void addToCache(const Path & storePath, const std::string & nar, + void addToCache(std::string_view hashPart, const std::string & nar, ref narAccessor); public: diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 0c6788b69..e67f4e359 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -21,15 +21,15 @@ bool Store::isInStore(const Path & path) const } -Path Store::toStorePath(const Path & path) const +std::pair Store::toStorePath(const Path & path) const { if (!isInStore(path)) throw Error("path '%1%' is not in the Nix store", path); Path::size_type slash = path.find('/', storeDir.size() + 1); if (slash == Path::npos) - return path; + return {parseStorePath(path), ""}; else - return Path(path, 0, slash); + return {parseStorePath(std::string_view(path).substr(0, slash)), path.substr(slash)}; } @@ -42,14 +42,14 @@ Path Store::followLinksToStore(std::string_view _path) const path = absPath(target, dirOf(path)); } if (!isInStore(path)) - throw NotInStore("path '%1%' is not in the Nix store", path); + throw BadStorePath("path '%1%' is not in the Nix store", path); return path; } StorePath Store::followLinksToStorePath(std::string_view path) const { - return parseStorePath(toStorePath(followLinksToStore(path))); + return toStorePath(followLinksToStore(path)).first; } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 0be0021f5..6f5e5f93a 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -31,7 +31,7 @@ MakeError(InvalidPath, Error); MakeError(Unsupported, Error); MakeError(SubstituteGone, Error); MakeError(SubstituterDisabled, Error); -MakeError(NotInStore, Error); +MakeError(BadStorePath, Error); class FSAccessor; @@ -317,9 +317,9 @@ public: the Nix store. */ bool isStorePath(std::string_view path) const; - /* Chop off the parts after the top-level store name, e.g., - /nix/store/abcd-foo/bar => /nix/store/abcd-foo. */ - Path toStorePath(const Path & path) const; + /* Split a path like /nix/store/-/ into + /nix/store/- and /. */ + std::pair toStorePath(const Path & path) const; /* Follow symlinks until we end up with a path in the Nix store. */ Path followLinksToStore(std::string_view path) const; diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 708a0dc88..1b04b10d5 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -94,8 +94,8 @@ struct InstallableStorePath : Installable ref store; StorePath storePath; - InstallableStorePath(ref store, const Path & storePath) - : store(store), storePath(store->parseStorePath(storePath)) { } + InstallableStorePath(ref store, StorePath && storePath) + : store(store), storePath(std::move(storePath)) { } std::string what() override { return store->printStorePath(storePath); } @@ -228,11 +228,11 @@ static std::vector> parseInstallables( result.push_back(std::make_shared(cmd, s)); else if (s.find("/") != std::string::npos) { - - auto path = store->toStorePath(store->followLinksToStore(s)); - - if (store->isStorePath(path)) - result.push_back(std::make_shared(store, path)); + try { + result.push_back(std::make_shared( + store, + store->toStorePath(store->followLinksToStore(s)).first)); + } catch (BadStorePath &) { } } else if (s == "" || std::regex_match(s, attrPathRegex)) From 400f1a9b59d95861b4dd171c45e2dc6cf263ab99 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jul 2020 18:55:24 +0200 Subject: [PATCH 07/17] Store::pathInfoToJSON(): Use consistent format for downloadHash --- src/libstore/store-api.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index e67f4e359..bc3fafa35 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -556,7 +556,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(hashBase, true)); if (narInfo->fileSize) jsonPath.attr("downloadSize", narInfo->fileSize); if (showClosureSize) From fc84c358d9e55e9ba1d939d8974f6deef629848e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jul 2020 20:58:02 +0200 Subject: [PATCH 08/17] Make 'nix copy' to file:// binary caches run in constant memory --- src/libstore/binary-cache-store.cc | 112 +++++++++++++++-------- src/libstore/binary-cache-store.hh | 6 +- src/libstore/daemon.cc | 2 +- src/libstore/export-import.cc | 33 ++----- src/libstore/http-binary-cache-store.cc | 4 +- src/libstore/local-binary-cache-store.cc | 30 +++--- src/libstore/s3-binary-cache-store.cc | 3 +- src/libutil/archive.hh | 4 +- src/libutil/serialise.hh | 13 +++ 9 files changed, 120 insertions(+), 87 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 9f52ddafa..166041b6c 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -57,6 +57,14 @@ void BinaryCacheStore::init() } } +void BinaryCacheStore::upsertFile(const std::string & path, + const std::string & data, + const std::string & mimeType) +{ + StringSource source(data); + upsertFile(path, source, mimeType); +} + void BinaryCacheStore::getFile(const std::string & path, Callback> callback) noexcept { @@ -113,13 +121,70 @@ void BinaryCacheStore::writeNarInfo(ref narInfo) diskCache->upsertNarInfo(getUri(), hashPart, std::shared_ptr(narInfo)); } +AutoCloseFD openFile(const Path & path) +{ + auto fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); + if (!fd) + throw SysError("opening file '%1%'", path); + return fd; +} + +struct FileSource : FdSource +{ + AutoCloseFD fd2; + + FileSource(const Path & path) + : fd2(openFile(path)) + { + fd = fd2.get(); + } +}; + void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource, RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) { - // FIXME: See if we can use the original source to reduce memory usage. - auto nar = make_ref(narSource.drain()); + assert(info.narHash && info.narSize); - if (!repair && isValidPath(info.path)) return; + if (!repair && isValidPath(info.path)) { + // FIXME: copyNAR -> null sink + narSource.drain(); + return; + } + + auto [fdTemp, fnTemp] = createTempFile(); + + auto now1 = std::chrono::steady_clock::now(); + + HashSink fileHashSink(htSHA256); + + { + FdSink fileSink(fdTemp.get()); + TeeSink teeSink(fileSink, fileHashSink); + auto compressionSink = makeCompressionSink(compression, teeSink); + copyNAR(narSource, *compressionSink); + compressionSink->finish(); + } + + auto now2 = std::chrono::steady_clock::now(); + + auto narInfo = make_ref(info); + narInfo->narSize = info.narSize; + narInfo->narHash = info.narHash; + narInfo->compression = compression; + auto [fileHash, fileSize] = fileHashSink.finish(); + narInfo->fileHash = fileHash; + narInfo->fileSize = fileSize; + narInfo->url = "nar/" + narInfo->fileHash.to_string(Base32, false) + ".nar" + + (compression == "xz" ? ".xz" : + compression == "bzip2" ? ".bz2" : + compression == "br" ? ".br" : + ""); + + auto duration = std::chrono::duration_cast(now2 - now1).count(); + printMsg(lvlTalkative, "copying path '%1%' (%2% bytes, compressed %3$.1f%% in %4% ms) to binary cache", + printStorePath(narInfo->path), info.narSize, + ((1.0 - (double) fileSize / info.narSize) * 100.0), + duration); /* Verify that all references are valid. This may do some .narinfo reads, but typically they'll already be cached. */ @@ -132,16 +197,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource printStorePath(info.path), printStorePath(ref)); } - assert(nar->compare(0, narMagic.size(), narMagic) == 0); - - auto narInfo = make_ref(info); - - narInfo->narSize = nar->size(); - narInfo->narHash = hashString(htSHA256, *nar); - - if (info.narHash && info.narHash != narInfo->narHash) - throw Error("refusing to copy corrupted path '%1%' to binary cache", printStorePath(info.path)); - + #if 0 auto accessor_ = std::dynamic_pointer_cast(accessor); auto narAccessor = makeNarAccessor(nar); @@ -166,27 +222,9 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource upsertFile(std::string(info.path.to_string()) + ".ls", jsonOut.str(), "application/json"); } + #endif - /* Compress the NAR. */ - narInfo->compression = compression; - auto now1 = std::chrono::steady_clock::now(); - auto narCompressed = compress(compression, *nar, parallelCompression); - auto now2 = std::chrono::steady_clock::now(); - narInfo->fileHash = hashString(htSHA256, *narCompressed); - narInfo->fileSize = narCompressed->size(); - - auto duration = std::chrono::duration_cast(now2 - now1).count(); - printMsg(lvlTalkative, "copying path '%1%' (%2% bytes, compressed %3$.1f%% in %4% ms) to binary cache", - printStorePath(narInfo->path), narInfo->narSize, - ((1.0 - (double) narCompressed->size() / nar->size()) * 100.0), - duration); - - narInfo->url = "nar/" + narInfo->fileHash.to_string(Base32, false) + ".nar" - + (compression == "xz" ? ".xz" : - compression == "bzip2" ? ".bz2" : - compression == "br" ? ".br" : - ""); - + #if 0 /* Optionally maintain an index of DWARF debug info files consisting of JSON files named 'debuginfo/' that specify the NAR file and member containing the debug info. */ @@ -243,16 +281,18 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource threadPool.process(); } } + #endif /* Atomically write the NAR file. */ if (repair || !fileExists(narInfo->url)) { stats.narWrite++; - upsertFile(narInfo->url, *narCompressed, "application/x-nix-nar"); + FileSource source(fnTemp); + upsertFile(narInfo->url, source, "application/x-nix-nar"); } else stats.narWriteAverted++; - stats.narWriteBytes += nar->size(); - stats.narWriteCompressedBytes += narCompressed->size(); + stats.narWriteBytes += info.narSize; + stats.narWriteCompressedBytes += fileSize; stats.narWriteCompressionTimeMs += duration; /* Atomically write the NAR info file.*/ diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 52ef8aa7a..4f0379533 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -36,9 +36,13 @@ public: virtual bool fileExists(const std::string & path) = 0; virtual void upsertFile(const std::string & path, - const std::string & data, + Source & source, const std::string & mimeType) = 0; + void upsertFile(const std::string & path, + const std::string & data, + const std::string & mimeType); + /* Note: subclasses must implement at least one of the two following getFile() methods. */ diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index ebc4d0285..f05f4739d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -731,7 +731,7 @@ static void performOp(TunnelLogger * logger, ref store, if (GET_PROTOCOL_MINOR(clientVersion) >= 21) source = std::make_unique(from, to); else { - TeeSink tee(from); + TeeParseSink tee(from); parseDump(tee, tee.source); saved = std::move(*tee.source.data); source = std::make_unique(saved); diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index 57b7e9590..cfed8ccd8 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -7,24 +7,6 @@ namespace nix { -struct HashAndWriteSink : Sink -{ - Sink & writeSink; - HashSink hashSink; - HashAndWriteSink(Sink & writeSink) : writeSink(writeSink), hashSink(htSHA256) - { - } - virtual void operator () (const unsigned char * data, size_t len) - { - writeSink(data, len); - hashSink(data, len); - } - Hash currentHash() - { - return hashSink.currentHash().first; - } -}; - void Store::exportPaths(const StorePathSet & paths, Sink & sink) { auto sorted = topoSortPaths(paths); @@ -47,23 +29,24 @@ void Store::exportPath(const StorePath & path, Sink & sink) { auto info = queryPathInfo(path); - HashAndWriteSink hashAndWriteSink(sink); + HashSink hashSink(htSHA256); + TeeSink teeSink(sink, hashSink); - narFromPath(path, hashAndWriteSink); + narFromPath(path, teeSink); /* Refuse to export paths that have changed. This prevents filesystem corruption from spreading to other machines. Don't complain if the stored hash is zero (unknown). */ - Hash hash = hashAndWriteSink.currentHash(); + Hash hash = hashSink.currentHash().first; 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)); - hashAndWriteSink + teeSink << exportMagic << printStorePath(path); - writeStorePaths(*this, hashAndWriteSink, info->references); - hashAndWriteSink + writeStorePaths(*this, teeSink, info->references); + teeSink << (info->deriver ? printStorePath(*info->deriver) : "") << 0; } @@ -77,7 +60,7 @@ StorePaths Store::importPaths(Source & source, std::shared_ptr acces if (n != 1) throw Error("input doesn't look like something created by 'nix-store --export'"); /* Extract the NAR from the source. */ - TeeSink tee(source); + TeeParseSink tee(source); parseDump(tee, tee.source); uint32_t magic = readInt(source); diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 451a64785..d9a292368 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -100,11 +100,11 @@ protected: } void upsertFile(const std::string & path, - const std::string & data, + Source & source, const std::string & mimeType) override { auto req = FileTransferRequest(cacheUri + "/" + path); - req.data = std::make_shared(data); // FIXME: inefficient + req.data = std::make_shared(source.drain()); req.mimeType = mimeType; try { getFileTransfer()->upload(req); diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 215c016f5..3d531d3a7 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -31,8 +31,17 @@ protected: bool fileExists(const std::string & path) override; void upsertFile(const std::string & path, - const std::string & data, - const std::string & mimeType) override; + Source & source, + const std::string & mimeType) + { + auto path2 = binaryCacheDir + "/" + path; + Path tmp = path2 + ".tmp." + std::to_string(getpid()); + AutoDelete del(tmp, false); + writeFile(tmp, source); + if (rename(tmp.c_str(), path2.c_str())) + throw SysError("renaming '%1%' to '%2%'", tmp, path2); + del.cancel(); + } void getFile(const std::string & path, Sink & sink) override { @@ -70,28 +79,11 @@ void LocalBinaryCacheStore::init() BinaryCacheStore::init(); } -static void atomicWrite(const Path & path, const std::string & s) -{ - Path tmp = path + ".tmp." + std::to_string(getpid()); - AutoDelete del(tmp, false); - writeFile(tmp, s); - if (rename(tmp.c_str(), path.c_str())) - throw SysError("renaming '%1%' to '%2%'", tmp, path); - del.cancel(); -} - bool LocalBinaryCacheStore::fileExists(const std::string & path) { return pathExists(binaryCacheDir + "/" + path); } -void LocalBinaryCacheStore::upsertFile(const std::string & path, - const std::string & data, - const std::string & mimeType) -{ - atomicWrite(binaryCacheDir + "/" + path, data); -} - static RegisterStoreImplementation regStore([]( const std::string & uri, const Store::Params & params) -> std::shared_ptr diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index f85563766..57f16101d 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -355,9 +355,10 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore stats.put++; } - void upsertFile(const std::string & path, const std::string & data, + void upsertFile(const std::string & path, Source & source, const std::string & mimeType) override { + auto data = source.drain(); if (narinfoCompression != "" && hasSuffix(path, ".narinfo")) uploadFile(path, *compress(narinfoCompression, data), mimeType, narinfoCompression); else if (lsCompression != "" && hasSuffix(path, ".ls")) diff --git a/src/libutil/archive.hh b/src/libutil/archive.hh index 768fe2536..795e9ce02 100644 --- a/src/libutil/archive.hh +++ b/src/libutil/archive.hh @@ -63,11 +63,11 @@ struct ParseSink virtual void createSymlink(const Path & path, const string & target) { }; }; -struct TeeSink : ParseSink +struct TeeParseSink : ParseSink { TeeSource source; - TeeSink(Source & source) : source(source) { } + TeeParseSink(Source & source) : source(source) { } }; void parseDump(ParseSink & sink, Source & source); diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index a04118512..bd651fb7d 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -166,6 +166,19 @@ struct StringSource : Source }; +/* A sink that writes all incoming data to two other sinks. */ +struct TeeSink : Sink +{ + Sink & sink1, & sink2; + TeeSink(Sink & sink1, Sink & sink2) : sink1(sink1), sink2(sink2) { } + virtual void operator () (const unsigned char * data, size_t len) + { + sink1(data, len); + sink2(data, len); + } +}; + + /* Adapter class of a Source that saves all data read to `s'. */ struct TeeSource : Source { From 0a9da00a10fa27a3e3b98439cb0a7d5e79135b58 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 17:30:42 +0200 Subject: [PATCH 09/17] NarAccessor: Run in constant memory --- src/libstore/binary-cache-store.cc | 16 +++++------ src/libstore/daemon.cc | 9 +++--- src/libstore/export-import.cc | 6 ++-- src/libstore/nar-accessor.cc | 46 +++++++++++++++++++----------- src/libstore/nar-accessor.hh | 4 +++ src/libutil/archive.hh | 3 +- src/libutil/serialise.hh | 10 +++---- 7 files changed, 57 insertions(+), 37 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 166041b6c..5851722d7 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -155,13 +155,17 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource auto now1 = std::chrono::steady_clock::now(); + /* Read the NAR simultaneously into a CompressionSink+FileSink (to + write the compressed NAR to disk), into a HashSink (to get the + NAR hash), and into a NarAccessor (to get the NAR listing). */ HashSink fileHashSink(htSHA256); - + std::shared_ptr narAccessor; { FdSink fileSink(fdTemp.get()); TeeSink teeSink(fileSink, fileHashSink); auto compressionSink = makeCompressionSink(compression, teeSink); - copyNAR(narSource, *compressionSink); + TeeSource teeSource(narSource, *compressionSink); + narAccessor = makeNarAccessor(teeSource); compressionSink->finish(); } @@ -200,10 +204,9 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource #if 0 auto accessor_ = std::dynamic_pointer_cast(accessor); - auto narAccessor = makeNarAccessor(nar); - if (accessor_) accessor_->addToCache(printStorePath(info.path), *nar, narAccessor); + #endif /* Optionally write a JSON file containing a listing of the contents of the NAR. */ @@ -216,15 +219,13 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource { auto res = jsonRoot.placeholder("root"); - listNar(res, narAccessor, "", true); + listNar(res, ref(narAccessor), "", true); } } upsertFile(std::string(info.path.to_string()) + ".ls", jsonOut.str(), "application/json"); } - #endif - #if 0 /* Optionally maintain an index of DWARF debug info files consisting of JSON files named 'debuginfo/' that specify the NAR file and member containing the debug info. */ @@ -281,7 +282,6 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource threadPool.process(); } } - #endif /* Atomically write the NAR file. */ if (repair || !fileExists(narInfo->url)) { diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index f05f4739d..536f4e738 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -391,7 +391,8 @@ static void performOp(TunnelLogger * logger, ref store, } HashType hashAlgo = parseHashType(s); - TeeSource savedNAR(from); + StringSink savedNAR; + TeeSource savedNARSource(from, savedNAR); RetrieveRegularNARSink savedRegular; if (method == FileIngestionMethod::Recursive) { @@ -399,7 +400,7 @@ static void performOp(TunnelLogger * logger, ref store, a string so that we can pass it to addToStoreFromDump(). */ ParseSink sink; /* null sink; just parse the NAR */ - parseDump(sink, savedNAR); + parseDump(sink, savedNARSource); } else parseDump(savedRegular, from); @@ -407,7 +408,7 @@ static void performOp(TunnelLogger * logger, ref store, if (!savedRegular.regular) throw Error("regular file expected"); auto path = store->addToStoreFromDump( - method == FileIngestionMethod::Recursive ? *savedNAR.data : savedRegular.s, + method == FileIngestionMethod::Recursive ? *savedNAR.s : savedRegular.s, baseName, method, hashAlgo); @@ -733,7 +734,7 @@ static void performOp(TunnelLogger * logger, ref store, else { TeeParseSink tee(from); parseDump(tee, tee.source); - saved = std::move(*tee.source.data); + saved = std::move(*tee.saved.s); source = std::make_unique(saved); } diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index cfed8ccd8..ae31cbcb0 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -77,15 +77,15 @@ StorePaths Store::importPaths(Source & source, std::shared_ptr acces if (deriver != "") info.deriver = parseStorePath(deriver); - info.narHash = hashString(htSHA256, *tee.source.data); - info.narSize = tee.source.data->size(); + info.narHash = hashString(htSHA256, *tee.saved.s); + info.narSize = tee.saved.s->size(); // Ignore optional legacy signature. if (readInt(source) == 1) readString(source); // Can't use underlying source, which would have been exhausted - auto source = StringSource { *tee.source.data }; + auto source = StringSource { *tee.saved.s }; addToStore(info, source, NoRepair, checkSigs, accessor); res.push_back(info.path); diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc index ca663d837..d884a131e 100644 --- a/src/libstore/nar-accessor.cc +++ b/src/libstore/nar-accessor.cc @@ -18,7 +18,7 @@ struct NarMember /* If this is a regular file, position of the contents of this file in the NAR. */ - size_t start = 0, size = 0; + uint64_t start = 0, size = 0; std::string target; @@ -34,17 +34,19 @@ struct NarAccessor : public FSAccessor NarMember root; - struct NarIndexer : ParseSink, StringSource + struct NarIndexer : ParseSink, Source { NarAccessor & acc; + Source & source; std::stack parents; - std::string currentStart; bool isExec = false; - NarIndexer(NarAccessor & acc, const std::string & nar) - : StringSource(nar), acc(acc) + uint64_t pos = 0; + + NarIndexer(NarAccessor & acc, Source & source) + : acc(acc), source(source) { } void createMember(const Path & path, NarMember member) { @@ -79,31 +81,38 @@ struct NarAccessor : public FSAccessor void preallocateContents(unsigned long long size) override { - currentStart = string(s, pos, 16); - assert(size <= std::numeric_limits::max()); - parents.top()->size = (size_t)size; + assert(size <= std::numeric_limits::max()); + parents.top()->size = (uint64_t) size; parents.top()->start = pos; } void receiveContents(unsigned char * data, unsigned int len) override - { - // Sanity check - if (!currentStart.empty()) { - assert(len < 16 || currentStart == string((char *) data, 16)); - currentStart.clear(); - } - } + { } void createSymlink(const Path & path, const string & target) override { createMember(path, NarMember{FSAccessor::Type::tSymlink, false, 0, 0, target}); } + + size_t read(unsigned char * data, size_t len) override + { + auto n = source.read(data, len); + pos += n; + return n; + } }; NarAccessor(ref nar) : nar(nar) { - NarIndexer indexer(*this, *nar); + StringSource source(*nar); + NarIndexer indexer(*this, source); + parseDump(indexer, indexer); + } + + NarAccessor(Source & source) + { + NarIndexer indexer(*this, source); parseDump(indexer, indexer); } @@ -219,6 +228,11 @@ ref makeNarAccessor(ref nar) return make_ref(nar); } +ref makeNarAccessor(Source & source) +{ + return make_ref(source); +} + ref makeLazyNarAccessor(const std::string & listing, GetNarBytes getNarBytes) { diff --git a/src/libstore/nar-accessor.hh b/src/libstore/nar-accessor.hh index 2871199de..8af1272f6 100644 --- a/src/libstore/nar-accessor.hh +++ b/src/libstore/nar-accessor.hh @@ -6,10 +6,14 @@ namespace nix { +struct Source; + /* Return an object that provides access to the contents of a NAR file. */ ref makeNarAccessor(ref nar); +ref makeNarAccessor(Source & source); + /* Create a NAR accessor from a NAR listing (in the format produced by listNar()). The callback getNarBytes(offset, length) is used by the readFile() method of the accessor to get the contents of files diff --git a/src/libutil/archive.hh b/src/libutil/archive.hh index 795e9ce02..302b1bb18 100644 --- a/src/libutil/archive.hh +++ b/src/libutil/archive.hh @@ -65,9 +65,10 @@ struct ParseSink struct TeeParseSink : ParseSink { + StringSink saved; TeeSource source; - TeeParseSink(Source & source) : source(source) { } + TeeParseSink(Source & source) : source(source, saved) { } }; void parseDump(ParseSink & sink, Source & source); diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index bd651fb7d..84a4eb001 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -179,17 +179,17 @@ struct TeeSink : Sink }; -/* Adapter class of a Source that saves all data read to `s'. */ +/* Adapter class of a Source that saves all data read to a sink. */ struct TeeSource : Source { Source & orig; - ref data; - TeeSource(Source & orig) - : orig(orig), data(make_ref()) { } + Sink & sink; + TeeSource(Source & orig, Sink & sink) + : orig(orig), sink(sink) { } size_t read(unsigned char * data, size_t len) { size_t n = orig.read(data, len); - this->data->append((const char *) data, n); + sink(data, len); return n; } }; From 545bb2ed03001cd7a80a90f73eb500f396c043a1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 17:37:44 +0200 Subject: [PATCH 10/17] Remove 'accessor' from addToStore() This is only used by hydra-queue-runner and it's better to implement it there. --- perl/lib/Nix/Store.xs | 2 +- src/libstore/binary-cache-store.cc | 13 +++---------- src/libstore/binary-cache-store.hh | 3 +-- src/libstore/build.cc | 5 ++--- src/libstore/daemon.cc | 4 ++-- src/libstore/export-import.cc | 4 ++-- src/libstore/legacy-ssh-store.cc | 3 +-- src/libstore/local-store.cc | 2 +- src/libstore/local-store.hh | 3 +-- src/libstore/remote-store.cc | 2 +- src/libstore/remote-store.hh | 3 +-- src/libstore/store-api.hh | 6 ++---- src/nix-store/nix-store.cc | 4 ++-- 13 files changed, 20 insertions(+), 34 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 945ed49c7..f14c3f73f 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -182,7 +182,7 @@ void importPaths(int fd, int dontCheckSigs) PPCODE: try { FdSource source(fd); - store()->importPaths(source, nullptr, dontCheckSigs ? NoCheckSigs : CheckSigs); + store()->importPaths(source, dontCheckSigs ? NoCheckSigs : CheckSigs); } catch (Error & e) { croak("%s", e.what()); } diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 5851722d7..e71240558 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -141,7 +141,7 @@ struct FileSource : FdSource }; void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource, - RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) + RepairFlag repair, CheckSigsFlag checkSigs) { assert(info.narHash && info.narSize); @@ -201,13 +201,6 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource printStorePath(info.path), printStorePath(ref)); } - #if 0 - auto accessor_ = std::dynamic_pointer_cast(accessor); - - if (accessor_) - accessor_->addToCache(printStorePath(info.path), *nar, narAccessor); - #endif - /* Optionally write a JSON file containing a listing of the contents of the NAR. */ if (writeNARListing) { @@ -391,7 +384,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath ValidPathInfo info(makeFixedOutputPath(method, h, name)); auto source = StringSource { *sink.s }; - addToStore(info, source, repair, CheckSigs, nullptr); + addToStore(info, source, repair, CheckSigs); return std::move(info.path); } @@ -406,7 +399,7 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s StringSink sink; dumpString(s, sink); auto source = StringSource { *sink.s }; - addToStore(info, source, repair, CheckSigs, nullptr); + addToStore(info, source, repair, CheckSigs); } return std::move(info.path); diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 4f0379533..2bf8d56b4 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -79,8 +79,7 @@ public: { unsupported("queryPathFromHashPart"); } void addToStore(const ValidPathInfo & info, Source & narSource, - RepairFlag repair, CheckSigsFlag checkSigs, - std::shared_ptr accessor) override; + RepairFlag repair, CheckSigsFlag checkSigs) override; StorePath addToStore(const string & name, const Path & srcPath, FileIngestionMethod method, HashType hashAlgo, diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 96f7f2603..ac2e67574 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2768,10 +2768,9 @@ struct RestrictedStore : public LocalFSStore { throw Error("addToStore"); } void addToStore(const ValidPathInfo & info, Source & narSource, - RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, - std::shared_ptr accessor = 0) override + RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs) override { - next->addToStore(info, narSource, repair, checkSigs, accessor); + next->addToStore(info, narSource, repair, checkSigs); goal.addDependency(info.path); } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 536f4e738..db7139374 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -443,7 +443,7 @@ static void performOp(TunnelLogger * logger, ref store, case wopImportPaths: { logger->startWork(); TunnelSource source(from, to); - auto paths = store->importPaths(source, nullptr, + auto paths = store->importPaths(source, trusted ? NoCheckSigs : CheckSigs); logger->stopWork(); Strings paths2; @@ -742,7 +742,7 @@ static void performOp(TunnelLogger * logger, ref store, // FIXME: race if addToStore doesn't read source? store->addToStore(info, *source, (RepairFlag) repair, - dontCheckSigs ? NoCheckSigs : CheckSigs, nullptr); + dontCheckSigs ? NoCheckSigs : CheckSigs); logger->stopWork(); break; diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc index ae31cbcb0..082d0f1d1 100644 --- a/src/libstore/export-import.cc +++ b/src/libstore/export-import.cc @@ -51,7 +51,7 @@ void Store::exportPath(const StorePath & path, Sink & sink) << 0; } -StorePaths Store::importPaths(Source & source, std::shared_ptr accessor, CheckSigsFlag checkSigs) +StorePaths Store::importPaths(Source & source, CheckSigsFlag checkSigs) { StorePaths res; while (true) { @@ -86,7 +86,7 @@ StorePaths Store::importPaths(Source & source, std::shared_ptr acces // Can't use underlying source, which would have been exhausted auto source = StringSource { *tee.saved.s }; - addToStore(info, source, NoRepair, checkSigs, accessor); + addToStore(info, source, NoRepair, checkSigs); res.push_back(info.path); } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 5657aa593..a8bd8a972 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -126,8 +126,7 @@ struct LegacySSHStore : public Store } void addToStore(const ValidPathInfo & info, Source & source, - RepairFlag repair, CheckSigsFlag checkSigs, - std::shared_ptr accessor) override + RepairFlag repair, CheckSigsFlag checkSigs) override { debug("adding path '%s' to remote host '%s'", printStorePath(info.path), host); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 69f149841..26b226fe8 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -962,7 +962,7 @@ const PublicKeys & LocalStore::getPublicKeys() void LocalStore::addToStore(const ValidPathInfo & info, Source & source, - RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) + RepairFlag repair, CheckSigsFlag checkSigs) { if (!info.narHash) throw Error("cannot add path '%s' because it lacks a hash", printStorePath(info.path)); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index ff36cb00e..c0e5d0286 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -143,8 +143,7 @@ public: SubstitutablePathInfos & infos) override; void addToStore(const ValidPathInfo & info, Source & source, - RepairFlag repair, CheckSigsFlag checkSigs, - std::shared_ptr accessor) override; + RepairFlag repair, CheckSigsFlag checkSigs) override; StorePath addToStore(const string & name, const Path & srcPath, FileIngestionMethod method, HashType hashAlgo, diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 118aadf7e..9af4364b7 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -466,7 +466,7 @@ std::optional RemoteStore::queryPathFromHashPart(const std::string & void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, - RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr accessor) + RepairFlag repair, CheckSigsFlag checkSigs) { auto conn(getConnection()); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index fb2052752..3c1b78b6a 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -60,8 +60,7 @@ public: SubstitutablePathInfos & infos) override; void addToStore(const ValidPathInfo & info, Source & nar, - RepairFlag repair, CheckSigsFlag checkSigs, - std::shared_ptr accessor) override; + RepairFlag repair, CheckSigsFlag checkSigs) override; StorePath addToStore(const string & name, const Path & srcPath, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 6f5e5f93a..a4be0411e 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -442,8 +442,7 @@ public: /* Import a path into the store. */ virtual void addToStore(const ValidPathInfo & info, Source & narSource, - RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, - std::shared_ptr accessor = 0) = 0; + RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs) = 0; /* Copy the contents of a path to the store and register the validity the resulting path. The resulting path is returned. @@ -626,8 +625,7 @@ public: the Nix store. Optionally, the contents of the NARs are preloaded into the specified FS accessor to speed up subsequent access. */ - StorePaths importPaths(Source & source, std::shared_ptr accessor, - CheckSigsFlag checkSigs = CheckSigs); + StorePaths importPaths(Source & source, CheckSigsFlag checkSigs = CheckSigs); struct Stats { diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 4fa179105..23bb48d88 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -671,7 +671,7 @@ static void opImport(Strings opFlags, Strings opArgs) if (!opArgs.empty()) throw UsageError("no arguments expected"); FdSource source(STDIN_FILENO); - auto paths = store->importPaths(source, nullptr, NoCheckSigs); + auto paths = store->importPaths(source, NoCheckSigs); for (auto & i : paths) cout << fmt("%s\n", store->printStorePath(i)) << std::flush; @@ -878,7 +878,7 @@ static void opServe(Strings opFlags, Strings opArgs) case cmdImportPaths: { if (!writeAllowed) throw Error("importing paths is not allowed"); - store->importPaths(in, nullptr, NoCheckSigs); // FIXME: should we skip sig checking? + store->importPaths(in, NoCheckSigs); // FIXME: should we skip sig checking? out << 1; // indicate success break; } From 493961b6899e7f3471e7efa24ed251c7723adbcd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 18:22:56 +0200 Subject: [PATCH 11/17] Remove istringstream_nocopy --- src/libstore/derivations.cc | 7 +- src/libstore/s3-binary-cache-store.cc | 5 +- src/libutil/hash.cc | 1 - src/libutil/istringstream_nocopy.hh | 92 --------------------------- 4 files changed, 5 insertions(+), 100 deletions(-) delete mode 100644 src/libutil/istringstream_nocopy.hh diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 42551ef6b..f325e511a 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -4,7 +4,6 @@ #include "util.hh" #include "worker-protocol.hh" #include "fs-accessor.hh" -#include "istringstream_nocopy.hh" namespace nix { @@ -101,7 +100,7 @@ static StringSet parseStrings(std::istream & str, bool arePaths) } -static DerivationOutput parseDerivationOutput(const Store & store, istringstream_nocopy & str) +static DerivationOutput parseDerivationOutput(const Store & store, std::istringstream & str) { expect(str, ","); auto path = store.parseStorePath(parsePath(str)); expect(str, ","); auto hashAlgo = parseString(str); @@ -129,10 +128,10 @@ static DerivationOutput parseDerivationOutput(const Store & store, istringstream } -static Derivation parseDerivation(const Store & store, const string & s) +static Derivation parseDerivation(const Store & store, std::string && s) { Derivation drv; - istringstream_nocopy str(s); + std::istringstream str(std::move(s)); expect(str, "Derive(["); /* Parse the list of outputs. */ diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 57f16101d..31ad4a3be 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -7,7 +7,6 @@ #include "globals.hh" #include "compression.hh" #include "filetransfer.hh" -#include "istringstream_nocopy.hh" #include #include @@ -266,7 +265,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore const std::string & mimeType, const std::string & contentEncoding) { - auto stream = std::make_shared(data); + auto stream = std::make_shared(data); auto maxThreads = std::thread::hardware_concurrency(); @@ -333,7 +332,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore if (contentEncoding != "") request.SetContentEncoding(contentEncoding); - auto stream = std::make_shared(data); + auto stream = std::make_shared(data); request.SetBody(stream); diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 1a3e7c5d8..f0d7754d1 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -8,7 +8,6 @@ #include "hash.hh" #include "archive.hh" #include "util.hh" -#include "istringstream_nocopy.hh" #include #include diff --git a/src/libutil/istringstream_nocopy.hh b/src/libutil/istringstream_nocopy.hh deleted file mode 100644 index f7beac578..000000000 --- a/src/libutil/istringstream_nocopy.hh +++ /dev/null @@ -1,92 +0,0 @@ -/* This file provides a variant of std::istringstream that doesn't - copy its string argument. This is useful for large strings. The - caller must ensure that the string object is not destroyed while - it's referenced by this object. */ - -#pragma once - -#include -#include - -template , class Allocator = std::allocator> -class basic_istringbuf_nocopy : public std::basic_streambuf -{ -public: - typedef std::basic_string string_type; - - typedef typename std::basic_streambuf::off_type off_type; - - typedef typename std::basic_streambuf::pos_type pos_type; - - typedef typename std::basic_streambuf::int_type int_type; - - typedef typename std::basic_streambuf::traits_type traits_type; - -private: - const string_type & s; - - off_type off; - -public: - basic_istringbuf_nocopy(const string_type & s) : s{s}, off{0} - { - } - -private: - pos_type seekoff(off_type off, std::ios_base::seekdir dir, std::ios_base::openmode which) - { - if (which & std::ios_base::in) { - this->off = dir == std::ios_base::beg - ? off - : (dir == std::ios_base::end - ? s.size() + off - : this->off + off); - } - return pos_type(this->off); - } - - pos_type seekpos(pos_type pos, std::ios_base::openmode which) - { - return seekoff(pos, std::ios_base::beg, which); - } - - std::streamsize showmanyc() - { - return s.size() - off; - } - - int_type underflow() - { - if (typename string_type::size_type(off) == s.size()) - return traits_type::eof(); - return traits_type::to_int_type(s[off]); - } - - int_type uflow() - { - if (typename string_type::size_type(off) == s.size()) - return traits_type::eof(); - return traits_type::to_int_type(s[off++]); - } - - int_type pbackfail(int_type ch) - { - if (off == 0 || (ch != traits_type::eof() && ch != s[off - 1])) - return traits_type::eof(); - - return traits_type::to_int_type(s[--off]); - } - -}; - -template , class Allocator = std::allocator> -class basic_istringstream_nocopy : public std::basic_iostream -{ - typedef basic_istringbuf_nocopy buf_type; - buf_type buf; -public: - basic_istringstream_nocopy(const typename buf_type::string_type & s) : - std::basic_iostream(&buf), buf(s) {}; -}; - -typedef basic_istringstream_nocopy istringstream_nocopy; From 7c2fef0a819481058d49c469c115bb0668b7016b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 20:07:19 +0200 Subject: [PATCH 12/17] Make 'nix copy' to s3:// binary caches run in constant memory --- src/libstore/binary-cache-store.cc | 11 +++---- src/libstore/binary-cache-store.hh | 4 +-- src/libstore/http-binary-cache-store.cc | 4 +-- src/libstore/local-binary-cache-store.cc | 5 ++-- src/libstore/s3-binary-cache-store.cc | 37 ++++++++++++++---------- src/libutil/serialise.hh | 23 +++++++++++++++ 6 files changed, 57 insertions(+), 27 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index e71240558..b791c125b 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include @@ -58,11 +59,10 @@ void BinaryCacheStore::init() } void BinaryCacheStore::upsertFile(const std::string & path, - const std::string & data, + std::string && data, const std::string & mimeType) { - StringSource source(data); - upsertFile(path, source, mimeType); + upsertFile(path, std::make_shared(std::move(data)), mimeType); } void BinaryCacheStore::getFile(const std::string & path, @@ -279,8 +279,9 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource /* Atomically write the NAR file. */ if (repair || !fileExists(narInfo->url)) { stats.narWrite++; - FileSource source(fnTemp); - upsertFile(narInfo->url, source, "application/x-nix-nar"); + upsertFile(narInfo->url, + std::make_shared(fnTemp, std::ios_base::in), + "application/x-nix-nar"); } else stats.narWriteAverted++; diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 2bf8d56b4..9bcdf5901 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -36,11 +36,11 @@ public: virtual bool fileExists(const std::string & path) = 0; virtual void upsertFile(const std::string & path, - Source & source, + std::shared_ptr> istream, const std::string & mimeType) = 0; void upsertFile(const std::string & path, - const std::string & data, + std::string && data, const std::string & mimeType); /* Note: subclasses must implement at least one of the two diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index d9a292368..c1ceb08cf 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -100,11 +100,11 @@ protected: } void upsertFile(const std::string & path, - Source & source, + std::shared_ptr> istream, const std::string & mimeType) override { auto req = FileTransferRequest(cacheUri + "/" + path); - req.data = std::make_shared(source.drain()); + req.data = std::make_shared(StreamToSourceAdapter(istream).drain()); req.mimeType = mimeType; try { getFileTransfer()->upload(req); diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 3d531d3a7..87d8334d7 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -31,12 +31,13 @@ protected: bool fileExists(const std::string & path) override; void upsertFile(const std::string & path, - Source & source, - const std::string & mimeType) + std::shared_ptr> istream, + const std::string & mimeType) override { auto path2 = binaryCacheDir + "/" + path; Path tmp = path2 + ".tmp." + std::to_string(getpid()); AutoDelete del(tmp, false); + StreamToSourceAdapter source(istream); writeFile(tmp, source); if (rename(tmp.c_str(), path2.c_str())) throw SysError("renaming '%1%' to '%2%'", tmp, path2); diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 31ad4a3be..1b7dff085 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -261,12 +261,11 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore std::shared_ptr transferManager; std::once_flag transferManagerCreated; - void uploadFile(const std::string & path, const std::string & data, + void uploadFile(const std::string & path, + std::shared_ptr> istream, const std::string & mimeType, const std::string & contentEncoding) { - auto stream = std::make_shared(data); - auto maxThreads = std::thread::hardware_concurrency(); static std::shared_ptr @@ -306,7 +305,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore std::shared_ptr transferHandle = transferManager->UploadFile( - stream, bucketName, path, mimeType, + istream, bucketName, path, mimeType, Aws::Map(), nullptr /*, contentEncoding */); @@ -332,9 +331,7 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore if (contentEncoding != "") request.SetContentEncoding(contentEncoding); - auto stream = std::make_shared(data); - - request.SetBody(stream); + request.SetBody(istream); auto result = checkAws(fmt("AWS error uploading '%s'", path), s3Helper.client->PutObject(request)); @@ -346,26 +343,34 @@ struct S3BinaryCacheStoreImpl : public S3BinaryCacheStore std::chrono::duration_cast(now2 - now1) .count(); - printInfo(format("uploaded 's3://%1%/%2%' (%3% bytes) in %4% ms") % - bucketName % path % data.size() % duration); + auto size = istream->tellg(); + + printInfo("uploaded 's3://%s/%s' (%d bytes) in %d ms", + bucketName, path, size, duration); stats.putTimeMs += duration; - stats.putBytes += data.size(); + stats.putBytes += size; stats.put++; } - void upsertFile(const std::string & path, Source & source, + void upsertFile(const std::string & path, + std::shared_ptr> istream, const std::string & mimeType) override { - auto data = source.drain(); + auto compress = [&](std::string compression) + { + auto compressed = nix::compress(compression, StreamToSourceAdapter(istream).drain()); + return std::make_shared(std::move(*compressed)); + }; + if (narinfoCompression != "" && hasSuffix(path, ".narinfo")) - uploadFile(path, *compress(narinfoCompression, data), mimeType, narinfoCompression); + uploadFile(path, compress(narinfoCompression), mimeType, narinfoCompression); else if (lsCompression != "" && hasSuffix(path, ".ls")) - uploadFile(path, *compress(lsCompression, data), mimeType, lsCompression); + uploadFile(path, compress(lsCompression), mimeType, lsCompression); else if (logCompression != "" && hasPrefix(path, "log/")) - uploadFile(path, *compress(logCompression, data), mimeType, logCompression); + uploadFile(path, compress(logCompression), mimeType, logCompression); else - uploadFile(path, data, mimeType, ""); + uploadFile(path, istream, mimeType, ""); } void getFile(const std::string & path, Sink & sink) override diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 84a4eb001..8386a4991 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -349,4 +349,27 @@ Source & operator >> (Source & in, bool & b) } +/* An adapter that converts a std::basic_istream into a source. */ +struct StreamToSourceAdapter : Source +{ + std::shared_ptr> istream; + + StreamToSourceAdapter(std::shared_ptr> istream) + : istream(istream) + { } + + size_t read(unsigned char * data, size_t len) override + { + if (!istream->read((char *) data, len)) { + if (istream->eof()) { + if (istream->gcount() == 0) + throw EndOfFile("end of file"); + } else + throw Error("I/O error in StreamToSourceAdapter"); + } + return istream->gcount(); + } +}; + + } From 9502c0e2ebec45a5615c6b570cc99de17ebb542b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 20:12:44 +0200 Subject: [PATCH 13/17] nix verify: Show correct path when using --all on a binary cache --- src/nix/verify.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/nix/verify.cc b/src/nix/verify.cc index bb5e4529b..ce90b0f6d 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -77,13 +77,16 @@ struct CmdVerify : StorePathsCommand try { checkInterrupt(); - Activity act2(*logger, lvlInfo, actUnknown, fmt("checking '%s'", storePath)); - MaintainCount> mcActive(active); update(); auto info = store->queryPathInfo(store->parseStorePath(storePath)); + // Note: info->path can be different from storePath + // for binary cache stores when using --all (since we + // can't enumerate names efficiently). + Activity act2(*logger, lvlInfo, actUnknown, fmt("checking '%s'", store->printStorePath(info->path))); + if (!noContents) { std::unique_ptr hashSink; From 43b8e96d30fa2b4e5f2f5144dc905c2d944a4e15 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Jul 2020 20:17:00 +0200 Subject: [PATCH 14/17] Fix 'nix verify --all' on a binary cache (cached case) --- src/libstore/store-api.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index bc3fafa35..5554befa4 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -351,6 +351,14 @@ ref Store::queryPathInfo(const StorePath & storePath) } +static bool goodStorePath(const StorePath & expected, const StorePath & actual) +{ + return + expected.hashPart() == actual.hashPart() + && (expected.name() == Store::MissingName || expected.name() == actual.name()); +} + + void Store::queryPathInfo(const StorePath & storePath, Callback> callback) noexcept { @@ -378,7 +386,7 @@ void Store::queryPathInfo(const StorePath & storePath, state_->pathInfoCache.upsert(hashPart, res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{ .value = res.second }); if (res.first == NarInfoDiskCache::oInvalid || - res.second->path != storePath) + !goodStorePath(storePath, res.second->path)) throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); } return callback(ref(res.second)); @@ -405,11 +413,7 @@ void Store::queryPathInfo(const StorePath & storePath, auto storePath = parseStorePath(storePathS); - if (!info - || info->path.hashPart() != storePath.hashPart() - || (storePath.name() != MissingName && info->path.name() != storePath.name()) - ) - { + if (!info || !goodStorePath(storePath, info->path)); { stats.narInfoMissing++; throw InvalidPath("path '%s' is not valid", storePathS); } From 926c3a6664a9dfb288ca35af8aae40c4e4a2badb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 14 Jul 2020 11:55:54 +0200 Subject: [PATCH 15/17] Doh --- src/libstore/store-api.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 5554befa4..5b9f79049 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -413,7 +413,7 @@ void Store::queryPathInfo(const StorePath & storePath, auto storePath = parseStorePath(storePathS); - if (!info || !goodStorePath(storePath, info->path)); { + if (!info || !goodStorePath(storePath, info->path)) { stats.narInfoMissing++; throw InvalidPath("path '%s' is not valid", storePathS); } From cff2157185912025c24a1b9dc99056161634176c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 15 Jul 2020 12:49:03 +0200 Subject: [PATCH 16/17] Revert "LocalStore::addToStore(srcPath): Handle the flat case" This reverts commit a2c27022e9afc394e08d34d349587c8903fc1a97. See addToStoreSlow(), we don't need to handle this case efficiently anymore. In fact, we can almost remove the method/hashAlgo arguments since the non-recursive and/or non-SHA256 are almost not used anymore. --- src/libstore/local-store.cc | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 26b226fe8..b3f4b3f7d 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1097,13 +1097,16 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, { Path srcPath(absPath(_srcPath)); + if (method != FileIngestionMethod::Recursive) + return addToStoreFromDump(readFile(srcPath), name, method, hashAlgo, repair); + /* For computing the NAR hash. */ auto sha256Sink = std::make_unique(htSHA256); /* For computing the store path. In recursive SHA-256 mode, this is the same as the NAR hash, so no need to do it again. */ std::unique_ptr hashSink = - method == FileIngestionMethod::Recursive && hashAlgo == htSHA256 + hashAlgo == htSHA256 ? nullptr : std::make_unique(hashAlgo); @@ -1136,10 +1139,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, if (!inMemory) sink(buf, len); }); - if (method == FileIngestionMethod::Recursive) - dumpPath(srcPath, sink2, filter); - else - readFile(srcPath, sink2); + dumpPath(srcPath, sink2, filter); }); std::unique_ptr delTempDir; @@ -1155,10 +1155,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, delTempDir = std::make_unique(tempDir); tempPath = tempDir + "/x"; - if (method == FileIngestionMethod::Recursive) - restorePath(tempPath, *source); - else - writeFile(tempPath, *source); + restorePath(tempPath, *source); } catch (EndOfFile &) { if (!inMemory) throw; @@ -1191,10 +1188,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, if (inMemory) { /* Restore from the NAR in memory. */ StringSource source(nar); - if (method == FileIngestionMethod::Recursive) - restorePath(realPath, source); - else - writeFile(realPath, source); + restorePath(realPath, source); } else { /* Move the temporary path we restored above. */ if (rename(tempPath.c_str(), realPath.c_str())) From 36a124260361ba8dfa43bf43a067dcc48064c93f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 15 Jul 2020 21:08:46 +0200 Subject: [PATCH 17/17] nix why-depends: Fix shortest path calculation This was completely broken since d8972317fc4314864619cadd5620ae780da657a3. --- src/nix/why-depends.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index 167c974ee..49da01c0a 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -106,7 +106,11 @@ struct CmdWhyDepends : SourceExprCommand std::map graph; for (auto & path : closure) - graph.emplace(path, Node { .path = path, .refs = store->queryPathInfo(path)->references }); + graph.emplace(path, Node { + .path = path, + .refs = store->queryPathInfo(path)->references, + .dist = path == dependencyPath ? 0 : inf + }); // Transpose the graph. for (auto & node : graph) @@ -115,8 +119,6 @@ struct CmdWhyDepends : SourceExprCommand /* Run Dijkstra's shortest path algorithm to get the distance of every path in the closure to 'dependency'. */ - graph.emplace(dependencyPath, Node { .path = dependencyPath, .dist = 0 }); - std::priority_queue queue; queue.push(&graph.at(dependencyPath));