From 062a584f12a9216464e1664f019b782f86ce78df Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jul 2020 11:21:06 +0200 Subject: [PATCH 1/5] .dir-locals.el: Set c-block-comment-prefix --- .dir-locals.el | 1 + 1 file changed, 1 insertion(+) diff --git a/.dir-locals.el b/.dir-locals.el index 8b21d4e40..a2d1dc48d 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -1,6 +1,7 @@ ((c++-mode . ( (c-file-style . "k&r") (c-basic-offset . 4) + (c-block-comment-prefix . " ") (indent-tabs-mode . nil) (tab-width . 4) (show-trailing-whitespace . t) From 06e3dd9005c1904a17a14a62dcd19813e2c261dc Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jul 2020 11:22:48 +0200 Subject: [PATCH 2/5] nix-prefetch-url: Run in constant memory when using RemoteStore Fixes #3684. --- src/nix-prefetch-url/nix-prefetch-url.cc | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 40b05a2f3..8ca85e894 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -10,6 +10,7 @@ #include "../nix/legacy.hh" #include "progress-bar.hh" #include "tarfile.hh" +#include "archive.hh" #include @@ -200,8 +201,10 @@ static int _main(int argc, char * * argv) tmpFile = unpacked; } - /* FIXME: inefficient; addToStore() will also hash - this. */ + /* FIXME: inefficient: we're reading/hashing 'tmpFile' + three times. */ + auto [narHash, narSize] = hashPath(htSHA256, tmpFile); + hash = unpack ? hashPath(ht, tmpFile).first : hashFile(ht, tmpFile); if (expectedHash != Hash(ht) && expectedHash != hash) @@ -209,13 +212,17 @@ static int _main(int argc, char * * argv) const auto recursive = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; - /* Copy the file to the Nix store. FIXME: if RemoteStore - implemented addToStoreFromDump() and downloadFile() - supported a sink, we could stream the download directly - into the Nix store. */ - storePath = store->addToStore(name, tmpFile, recursive, ht); + storePath = store->makeFixedOutputPath(recursive, hash, name); - assert(*storePath == store->makeFixedOutputPath(recursive, hash, name)); + /* Copy the file to the Nix store. */ + ValidPathInfo info(*storePath); + info.narHash = narHash; + info.narSize = narSize; + info.ca = FixedOutputHash { .method = recursive, .hash = hash }; + auto source = sinkToSource([&](Sink & sink) { + dumpPath(tmpFile, sink); + }); + store->addToStore(info, *source); } stopProgressBar(); From 7f1a86d57c548b4db44f589ffae2f6491d3b2d4c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jul 2020 12:51:56 +0200 Subject: [PATCH 3/5] nix-store --add-fixed: Run in constant memory --- src/nix-store/nix-store.cc | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 7d81bf54f..c0274d4b6 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -174,10 +174,10 @@ static void opAdd(Strings opFlags, Strings opArgs) store. */ static void opAddFixed(Strings opFlags, Strings opArgs) { - auto recursive = FileIngestionMethod::Flat; + auto method = FileIngestionMethod::Flat; for (auto & i : opFlags) - if (i == "--recursive") recursive = FileIngestionMethod::Recursive; + if (i == "--recursive") method = FileIngestionMethod::Recursive; else throw UsageError("unknown flag '%1%'", i); if (opArgs.empty()) @@ -186,8 +186,23 @@ static void opAddFixed(Strings opFlags, Strings opArgs) HashType hashAlgo = parseHashType(opArgs.front()); opArgs.pop_front(); - for (auto & i : opArgs) - cout << fmt("%s\n", store->printStorePath(store->addToStore(std::string(baseNameOf(i)), i, recursive, hashAlgo))); + for (auto & i : opArgs) { + auto hash = method == FileIngestionMethod::Recursive + ? hashPath(hashAlgo, i).first + : hashFile(hashAlgo, i); + auto [narHash, narSize] = hashPath(htSHA256, i); + ValidPathInfo info(store->makeFixedOutputPath(method, hash, baseNameOf(i))); + info.narHash = narHash; + info.narSize = narSize; + info.ca = FixedOutputHash { .method = method, .hash = hash }; + + auto source = sinkToSource([&](Sink & sink) { + dumpPath(i, sink); + }); + store->addToStore(info, *source); + + std::cout << fmt("%s\n", store->printStorePath(info.path)); + } } From 5dff49f661cd221fc457d1a4660cd36f28266dc5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jul 2020 13:21:37 +0200 Subject: [PATCH 4/5] Factor out commonality between nix-prefetch-url and nix-store --add-fixed --- src/libstore/content-address.cc | 12 +++++++++ src/libstore/content-address.hh | 2 ++ src/libstore/store-api.cc | 32 ++++++++++++++++++++++ src/libstore/store-api.hh | 7 +++++ src/nix-prefetch-url/nix-prefetch-url.cc | 34 +++++++----------------- src/nix-store/nix-store.cc | 19 ++----------- 6 files changed, 64 insertions(+), 42 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 3d753836f..6cb69d0a9 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -82,4 +82,16 @@ std::string renderContentAddress(std::optional ca) { return ca ? renderContentAddress(*ca) : ""; } +Hash getContentAddressHash(const ContentAddress & ca) +{ + return std::visit(overloaded { + [](TextHash th) { + return th.hash; + }, + [](FixedOutputHash fsh) { + return fsh.hash; + } + }, ca); +} + } diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index ba4797f5b..22a039242 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -53,4 +53,6 @@ ContentAddress parseContentAddress(std::string_view rawCa); std::optional parseContentAddressOpt(std::string_view rawCaOpt); +Hash getContentAddressHash(const ContentAddress & ca); + } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index c0a8bc9f6..46587a49a 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -7,6 +7,7 @@ #include "json.hh" #include "derivations.hh" #include "url.hh" +#include "archive.hh" #include @@ -221,6 +222,37 @@ StorePath Store::computeStorePathForText(const string & name, const string & s, } +ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath, + FileIngestionMethod method, HashType hashAlgo, + std::optional expectedCAHash) +{ + /* FIXME: inefficient: we're reading/hashing 'tmpFile' three + times. */ + + auto hash = method == FileIngestionMethod::Recursive + ? hashPath(hashAlgo, srcPath).first + : hashFile(hashAlgo, srcPath); + + if (expectedCAHash && expectedCAHash != hash) + throw Error("hash mismatch for '%s'", srcPath); + + auto [narHash, narSize] = hashPath(htSHA256, srcPath); + ValidPathInfo info(makeFixedOutputPath(method, hash, name)); + info.narHash = narHash; + info.narSize = narSize; + info.ca = FixedOutputHash { .method = method, .hash = hash }; + + if (!isValidPath(info.path)) { + auto source = sinkToSource([&](Sink & sink) { + dumpPath(srcPath, sink); + }); + addToStore(info, *source); + } + + return info; +} + + Store::Store(const Params & params) : Config(params) , state({(size_t) pathInfoCacheSize}) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index b122e05d6..b1dd1f478 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -450,6 +450,13 @@ public: FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) = 0; + /* Copy the contents of a path to the store and register the + validity the resulting path, using a constant amount of + memory. */ + ValidPathInfo addToStoreSlow(std::string_view name, const Path & srcPath, + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, + std::optional expectedCAHash = {}); + // FIXME: remove? virtual StorePath addToStoreFromDump(const string & dump, const string & name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 8ca85e894..961e7fb6d 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -10,7 +10,6 @@ #include "../nix/legacy.hh" #include "progress-bar.hh" #include "tarfile.hh" -#include "archive.hh" #include @@ -154,14 +153,15 @@ 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); + std::optional expectedHash; + Hash hash; std::optional storePath; if (args.size() == 2) { expectedHash = Hash(args[1], ht); const auto recursive = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; - storePath = store->makeFixedOutputPath(recursive, expectedHash, name); + storePath = store->makeFixedOutputPath(recursive, *expectedHash, name); if (store->isValidPath(*storePath)) - hash = expectedHash; + hash = *expectedHash; else storePath.reset(); } @@ -201,28 +201,12 @@ static int _main(int argc, char * * argv) tmpFile = unpacked; } - /* FIXME: inefficient: we're reading/hashing 'tmpFile' - three times. */ - auto [narHash, narSize] = hashPath(htSHA256, tmpFile); + const auto method = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; - hash = unpack ? hashPath(ht, tmpFile).first : hashFile(ht, tmpFile); - - if (expectedHash != Hash(ht) && expectedHash != hash) - throw Error("hash mismatch for '%1%'", uri); - - const auto recursive = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; - - storePath = store->makeFixedOutputPath(recursive, hash, name); - - /* Copy the file to the Nix store. */ - ValidPathInfo info(*storePath); - info.narHash = narHash; - info.narSize = narSize; - info.ca = FixedOutputHash { .method = recursive, .hash = hash }; - auto source = sinkToSource([&](Sink & sink) { - dumpPath(tmpFile, sink); - }); - store->addToStore(info, *source); + auto info = store->addToStoreSlow(name, tmpFile, method, ht, expectedHash); + storePath = info.path; + assert(info.ca); + hash = getContentAddressHash(*info.ca); } stopProgressBar(); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index c0274d4b6..4fa179105 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -186,23 +186,8 @@ static void opAddFixed(Strings opFlags, Strings opArgs) HashType hashAlgo = parseHashType(opArgs.front()); opArgs.pop_front(); - for (auto & i : opArgs) { - auto hash = method == FileIngestionMethod::Recursive - ? hashPath(hashAlgo, i).first - : hashFile(hashAlgo, i); - auto [narHash, narSize] = hashPath(htSHA256, i); - ValidPathInfo info(store->makeFixedOutputPath(method, hash, baseNameOf(i))); - info.narHash = narHash; - info.narSize = narSize; - info.ca = FixedOutputHash { .method = method, .hash = hash }; - - auto source = sinkToSource([&](Sink & sink) { - dumpPath(i, sink); - }); - store->addToStore(info, *source); - - std::cout << fmt("%s\n", store->printStorePath(info.path)); - } + for (auto & i : opArgs) + std::cout << fmt("%s\n", store->printStorePath(store->addToStoreSlow(baseNameOf(i), i, method, hashAlgo).path)); } From 8efa23bb996161af74f89401902450e51e9d4b54 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jul 2020 15:56:24 +0200 Subject: [PATCH 5/5] Avoid a redundant hash --- src/libstore/store-api.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 46587a49a..8d46bb436 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -229,14 +229,17 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath, /* FIXME: inefficient: we're reading/hashing 'tmpFile' three times. */ + auto [narHash, narSize] = hashPath(htSHA256, srcPath); + auto hash = method == FileIngestionMethod::Recursive - ? hashPath(hashAlgo, srcPath).first + ? hashAlgo == htSHA256 + ? narHash + : hashPath(hashAlgo, srcPath).first : hashFile(hashAlgo, srcPath); if (expectedCAHash && expectedCAHash != hash) throw Error("hash mismatch for '%s'", srcPath); - auto [narHash, narSize] = hashPath(htSHA256, srcPath); ValidPathInfo info(makeFixedOutputPath(method, hash, name)); info.narHash = narHash; info.narSize = narSize;