From a168224464c8d4f4b5dc1d06f6e6572e5aac9f4c Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Sat, 4 Jul 2020 18:30:49 -0600 Subject: [PATCH 01/32] spacing --- src/libutil/error.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index a4ee7afc2..4bc19fa5f 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -110,7 +110,6 @@ std::optional getCodeLines(const ErrPos &errPos) int pl = errPos.line - 1; LinesOfCode loc; - do { std::getline(iss, line); From 75bfcf8d1571e94530e21636d5a2bc7613adf964 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Mon, 6 Jul 2020 10:51:48 -0600 Subject: [PATCH 02/32] revamp trace code and test --- src/libutil/error.cc | 27 +++++++++++---------------- src/libutil/tests/logging.cc | 15 +++++++++------ 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 4bc19fa5f..124603c34 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -354,26 +354,21 @@ std::ostream& showErrorInfo(std::ostream &out, const ErrorInfo &einfo, bool show for (auto iter = einfo.traces.rbegin(); iter != einfo.traces.rend(); ++iter) { - try { + out << std::endl << prefix; + out << ANSI_BLUE << "trace: " << ANSI_NORMAL << iter->hint.str(); + + if (iter->pos.has_value() && (*iter->pos)) { + auto pos = iter->pos.value(); out << std::endl << prefix; - out << ANSI_BLUE << "trace: " << ANSI_NORMAL << iter->hint.str(); + printAtPos(prefix, pos, out); - nl = true; - if (*iter->pos) { - auto pos = iter->pos.value(); + auto loc = getCodeLines(pos); + if (loc.has_value()) + { + out << std::endl << prefix; + printCodeLines(out, prefix, pos, *loc); out << std::endl << prefix; - - printAtPos(prefix, pos, out); - auto loc = getCodeLines(pos); - if (loc.has_value()) - { - out << std::endl << prefix; - printCodeLines(out, prefix, pos, *loc); - out << std::endl << prefix; - } } - } catch(const std::bad_optional_access& e) { - out << iter->hint.str() << std::endl; } } } diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc index ef22e9966..ad588055f 100644 --- a/src/libutil/tests/logging.cc +++ b/src/libutil/tests/logging.cc @@ -251,18 +251,19 @@ namespace nix { TEST(addTrace, showTracesWithShowTrace) { SymbolTable testTable; auto problem_file = testTable.create(test_file); - auto oneliner_file = testTable.create(one_liner); + auto invalidfilename = testTable.create("invalid filename"); auto e = AssertionError(ErrorInfo { .name = "wat", - .description = "a well-known problem occurred", + .description = "show-traces", .hint = hintfmt("it has been %1% days since our last error", "zero"), .errPos = Pos(foString, problem_file, 2, 13), }); e.addTrace(Pos(foStdin, oneliner_file, 1, 19), "while trying to compute %1%", 42); e.addTrace(std::nullopt, "while doing something without a %1%", "pos"); + e.addTrace(Pos(foFile, invalidfilename, 100, 1), "missing %s", "nix file"); testing::internal::CaptureStderr(); @@ -271,24 +272,25 @@ namespace nix { logError(e.info()); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m---- show-trace ----\x1B[0m\n\x1B[34;1mtrace: \x1B[0mwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n\n 1| this is the other problem line of code\n | \x1B[31;1m^\x1B[0m\n\n\x1B[34;1mtrace: \x1B[0mwhile doing something without a \x1B[33;1mpos\x1B[0m\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nopening file '\x1B[33;1minvalid filename\x1B[0m': \x1B[33;1mNo such file or directory\x1B[0m\n\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\nshow-traces\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m---- show-trace ----\x1B[0m\n\x1B[34;1mtrace: \x1B[0mwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n\n 1| this is the other problem line of code\n | \x1B[31;1m^\x1B[0m\n\n\x1B[34;1mtrace: \x1B[0mwhile doing something without a \x1B[33;1mpos\x1B[0m\n\x1B[34;1mtrace: \x1B[0mmissing \x1B[33;1mnix file\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(100:1)\x1B[34;1m in file: \x1B[0minvalid filename\n"); } TEST(addTrace, hideTracesWithoutShowTrace) { SymbolTable testTable; auto problem_file = testTable.create(test_file); - auto oneliner_file = testTable.create(one_liner); + auto invalidfilename = testTable.create("invalid filename"); auto e = AssertionError(ErrorInfo { .name = "wat", - .description = "a well-known problem occurred", + .description = "hide traces", .hint = hintfmt("it has been %1% days since our last error", "zero"), .errPos = Pos(foString, problem_file, 2, 13), }); e.addTrace(Pos(foStdin, oneliner_file, 1, 19), "while trying to compute %1%", 42); e.addTrace(std::nullopt, "while doing something without a %1%", "pos"); + e.addTrace(Pos(foFile, invalidfilename, 100, 1), "missing %s", "nix file"); testing::internal::CaptureStderr(); @@ -297,9 +299,10 @@ namespace nix { logError(e.info()); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\na well-known problem occurred\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\nhide traces\n\n 1| previous line of code\n 2| this is the problem line of code\n | \x1B[31;1m^\x1B[0m\n 3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n"); } + /* ---------------------------------------------------------------------------- * hintfmt * --------------------------------------------------------------------------*/ From efd6a8b2304f6db6fc4e0869ddce98548e7b8c95 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Mon, 6 Jul 2020 11:54:53 -0600 Subject: [PATCH 03/32] bump --- src/libutil/error.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 124603c34..fd6f69b7f 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -110,6 +110,7 @@ std::optional getCodeLines(const ErrPos &errPos) int pl = errPos.line - 1; LinesOfCode loc; + do { std::getline(iss, line); From 7c9ece5dcaf6407449616c9d2d302853829c4f7d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 Jul 2020 14:25:43 +0200 Subject: [PATCH 04/32] exportReferencesGraph: Fix support for non-top-level store paths Fixes #3471. --- src/libstore/build.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 347fe1b99..0ef2f288f 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2041,7 +2041,10 @@ void DerivationGoal::startBuilder() if (!std::regex_match(fileName, regex)) throw Error("invalid file name '%s' in 'exportReferencesGraph'", fileName); - auto storePath = worker.store.parseStorePath(*i++); + 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)); /* Write closure info to . */ writeFile(tmpDir + "/" + fileName, From 4055cfee3643ecf921e7c3e1b6ad7388271460d8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 Jul 2020 14:37:47 +0200 Subject: [PATCH 05/32] Fix coverage build --- src/libutil/error.hh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 1b0fb43b8..f4b3f11bb 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -1,8 +1,8 @@ #pragma once - #include "ref.hh" #include "types.hh" +#include "fmt.hh" #include #include @@ -10,7 +10,9 @@ #include #include -#include "fmt.hh" +#include +#include +#include /* Before 4.7, gcc's std::exception uses empty throw() specifiers for * its (virtual) destructor and what() in c++11 mode, in violation of spec From 16ec7785ca51ee6e7e415657a6ecd30c4ea12861 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 8 Jul 2020 15:53:14 +0200 Subject: [PATCH 06/32] Fix 'got unknown message type 1 from Nix daemon' Example: $ nix-build -E 'with import {}; runCommand "foo" { x = runCommand "bar" {} "exit 1"; } "echo foo; exit 1"' warning: unknown setting 'auto-allocate-uids' these 2 derivations will be built: /nix/store/v4fbdbhcdi949929a67g8farwf72zgam-bar.drv /nix/store/k4fsvrjl7cp2xpz7927iv7g0dqj1zyhs-foo.drv warning: unknown setting 'auto-allocate-uids' building '/nix/store/v4fbdbhcdi949929a67g8farwf72zgam-bar.drv'... error: --- Error ----------------------------------------------------------------------------------------------------------------------------------------------------------------- nix-daemon builder for '/nix/store/v4fbdbhcdi949929a67g8farwf72zgam-bar.drv' failed with exit code 1 error: --- Error ------------------------------------------------------------------------------------------------------------------------------------------------------------------ nix-build got unknown message type 1 from Nix daemon --- src/libstore/daemon.cc | 2 +- src/libutil/logging.cc | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index a8fb62e0a..ebc4d0285 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -81,7 +81,7 @@ struct TunnelLogger : public Logger showErrorInfo(oss, ei, false); StringSink buf; - buf << STDERR_NEXT << oss.str() << "\n"; + buf << STDERR_NEXT << oss.str(); enqueueMsg(*buf.s); } diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 90c6afe81..832aee783 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -81,7 +81,6 @@ public: log(ei.level, oss.str()); } - void startActivity(ActivityId act, Verbosity lvl, ActivityType type, const std::string & s, const Fields & fields, ActivityId parent) From 7d8d78f06a637ba6b75285d299b07b81279c422f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 8 Jul 2020 17:01:20 +0200 Subject: [PATCH 07/32] upload-release.pl: Update latest-release branch --- maintainers/upload-release.pl | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/maintainers/upload-release.pl b/maintainers/upload-release.pl index baefe0f12..91ae896d6 100755 --- a/maintainers/upload-release.pl +++ b/maintainers/upload-release.pl @@ -170,15 +170,5 @@ $channelsBucket->add_key( chdir("/home/eelco/Dev/nix-pristine") or die; system("git remote update origin") == 0 or die; system("git tag --force --sign $version $nixRev -m 'Tagging release $version'") == 0 or die; - -# Update the website. -my $siteDir = "/home/eelco/Dev/nixos-homepage-pristine"; - -system("cd $siteDir && git pull") == 0 or die; - -write_file("$siteDir/nix-release.tt", - "[%-\n" . - "latestNixVersion = \"$version\"\n" . - "-%]\n"); - -system("cd $siteDir && git commit -a -m 'Nix $version released'") == 0 or die; +system("git push --tags") == 0 or die; +system("git push --force-with-lease origin $nixRev:refs/heads/latest-release") == 0 or die; From 34f25124ba2ab32b8a95d6b37cd68d7bb85ff2d4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 9 Apr 2018 23:46:20 +0200 Subject: [PATCH 08/32] Make LocalStore::addToStore(srcPath) run in constant memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reduces memory consumption of nix-instantiate \ -E 'with import {}; runCommand "foo" { src = ./blender; } "echo foo"' \ --option nar-buffer-size 10000 (where ./blender is a 1.1 GiB tree) from 1716 to 36 MiB, while still ensuring that we don't do any write I/O for small source paths (up to 'nar-buffer-size' bytes). The downside is that large paths are now always written to a temporary location in the store, even if they produce an already valid store path. Thus, adding large paths might be slower and run out of disk space. ¯\_(ツ)_/¯ Of course, you can always restore the old behaviour by setting 'nar-buffer-size' to a very high value. --- src/libstore/globals.hh | 3 + src/libstore/local-store.cc | 121 +++++++++++++++++++++++++++++++++--- 2 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 58cf08763..4d5eec7bf 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -365,6 +365,9 @@ public: Setting warnDirty{this, true, "warn-dirty", "Whether to warn about dirty Git/Mercurial trees."}; + + Setting narBufferSize{this, 32 * 1024 * 1024, "nar-buffer-size", + "Maximum size of NARs before spilling them to disk."}; }; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index eed225349..b9176ec38 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1098,16 +1098,119 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, { Path srcPath(absPath(_srcPath)); - /* Read the whole path into memory. This is not a very scalable - method for very large paths, but `copyPath' is mainly used for - small files. */ - StringSink sink; - if (method == FileIngestionMethod::Recursive) - dumpPath(srcPath, sink, filter); - else - sink.s = make_ref(readFile(srcPath)); + if (method != FileIngestionMethod::Recursive) + return addToStoreFromDump(readFile(srcPath), name, method, hashAlgo, repair); - return addToStoreFromDump(*sink.s, 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 = + hashAlgo == htSHA256 + ? nullptr + : std::make_unique(hashAlgo); + + /* Read the source path into memory, but only if it's up to + narBufferSize bytes. If it's larger, write it to a temporary + location in the Nix store. If the subsequently computed + destination store path is already valid, we just delete the + temporary path. Otherwise, we move it to the destination store + path. */ + bool inMemory = true; + std::string nar; + + auto source = sinkToSource([&](Sink & sink) { + + LambdaSink sink2([&](const unsigned char * buf, size_t len) { + (*sha256Sink)(buf, len); + if (hashSink) (*hashSink)(buf, len); + + if (inMemory) { + if (nar.size() + len > settings.narBufferSize) { + inMemory = false; + sink << 1; + sink((const unsigned char *) nar.data(), nar.size()); + nar.clear(); + } else { + nar.append((const char *) buf, len); + } + } + + if (!inMemory) sink(buf, len); + }); + + dumpPath(srcPath, sink2, filter); + }); + + std::unique_ptr delTempDir; + Path tempPath; + + try { + /* Wait for the source coroutine to give us some dummy + data. This is so that we don't create the temporary + directory if the NAR fits in memory. */ + readInt(*source); + + auto tempDir = createTempDir(realStoreDir, "add"); + delTempDir = std::make_unique(tempDir); + tempPath = tempDir + "/x"; + + restorePath(tempPath, *source); + + } catch (EndOfFile &) { + if (!inMemory) throw; + /* The NAR fits in memory, so we didn't do restorePath(). */ + } + + auto sha256 = sha256Sink->finish(); + + Hash hash = hashSink ? hashSink->finish().first : sha256.first; + + Path dstPath = makeFixedOutputPath(method, hash, name); + + addTempRoot(dstPath); + + if (repair || !isValidPath(dstPath)) { + + /* The first check above is an optimisation to prevent + unnecessary lock acquisition. */ + + Path realPath = realStoreDir + "/" + baseNameOf(dstPath); + + PathLocks outputLock({realPath}); + + if (repair || !isValidPath(dstPath)) { + + deletePath(realPath); + + autoGC(); + + if (inMemory) { + /* Restore from the NAR in memory. */ + StringSource source(nar); + restorePath(realPath, source); + } else { + /* Move the temporary path we restored above. */ + if (rename(tempPath.c_str(), realPath.c_str())) + throw Error("renaming '%s' to '%s'", tempPath, realPath); + } + + canonicalisePathMetaData(realPath, -1); // FIXME: merge into restorePath + + optimisePath(realPath); + + ValidPathInfo info(dstPath); + info.narHash = sha256.first; + info.narSize = sha256.second; + info.ca = FixedOutputHash { .method = method, .hash = hash }; + registerValidPath(info); + } + + outputLock.setDeletion(true); + } + + return dstPath; } From b981e5aacf3848424264f4a84826f8f9ca33da14 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 8 Jul 2020 22:07:10 +0200 Subject: [PATCH 09/32] Cleanup --- src/libstore/local-store.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index b9176ec38..fa66c1c90 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -976,7 +976,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, PathLocks outputLock; - Path realPath = realStoreDir + "/" + std::string(info.path.to_string()); + auto realPath = Store::toRealPath(info.path); /* Lock the output path. But don't lock if we're being called from a build hook (whose parent process already acquired a @@ -1047,8 +1047,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam /* The first check above is an optimisation to prevent unnecessary lock acquisition. */ - Path realPath = realStoreDir + "/"; - realPath += dstPath.to_string(); + auto realPath = Store::toRealPath(dstPath); PathLocks outputLock({realPath}); @@ -1167,7 +1166,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, Hash hash = hashSink ? hashSink->finish().first : sha256.first; - Path dstPath = makeFixedOutputPath(method, hash, name); + auto dstPath = makeFixedOutputPath(method, hash, name); addTempRoot(dstPath); @@ -1176,7 +1175,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, /* The first check above is an optimisation to prevent unnecessary lock acquisition. */ - Path realPath = realStoreDir + "/" + baseNameOf(dstPath); + auto realPath = Store::toRealPath(dstPath); PathLocks outputLock({realPath}); @@ -1224,8 +1223,7 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s, if (repair || !isValidPath(dstPath)) { - Path realPath = realStoreDir + "/"; - realPath += dstPath.to_string(); + auto realPath = Store::toRealPath(dstPath); PathLocks outputLock({realPath}); From cfe6ea746c3b88887eafda3eaccedc654f61651f Mon Sep 17 00:00:00 2001 From: "Travis A. Everett" Date: Wed, 8 Jul 2020 20:10:22 -0500 Subject: [PATCH 10/32] add temp CI job to test syspolicy impact Starting in Catalina, macOS runs a syspolicyd "assessment" that hits the network for each binary/script executable. It does cache these results, but Nix tends to introduce many "new" executables per build. (You can read more about this at https://github.com/NixOS/nix/issues/3789). This PR adds a temporary, redundant macOS job with these assessments disabled. I'm hoping you can adopt it for a few weeks to help me collect more data on how this affects real projects. --- .github/workflows/test.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7755466a0..47fa041e9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,3 +12,13 @@ jobs: - uses: actions/checkout@v2 - uses: cachix/install-nix-action@v10 - run: nix-build release.nix --arg nix '{ outPath = ./.; revCount = 123; shortRev = "abcdefgh"; }' --arg systems '[ builtins.currentSystem ]' -A installerScript -A perlBindings + macos_perf_test: + runs-on: macos-latest + steps: + - name: Disable syspolicy assessments + run: | + spctl --status + sudo spctl --master-disable + - uses: actions/checkout@v2 + - uses: cachix/install-nix-action@v10 + - run: nix-build release.nix --arg nix '{ outPath = ./.; revCount = 123; shortRev = "abcdefgh"; }' --arg systems '[ builtins.currentSystem ]' -A installerScript -A perlBindings From a2c27022e9afc394e08d34d349587c8903fc1a97 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 9 Jul 2020 15:54:32 +0200 Subject: [PATCH 11/32] LocalStore::addToStore(srcPath): Handle the flat case This helps nix-prefetch-url when using a local store. --- src/libstore/local-store.cc | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index fa66c1c90..69f149841 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1097,16 +1097,13 @@ 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 = - hashAlgo == htSHA256 + method == FileIngestionMethod::Recursive && hashAlgo == htSHA256 ? nullptr : std::make_unique(hashAlgo); @@ -1139,7 +1136,10 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, if (!inMemory) sink(buf, len); }); - dumpPath(srcPath, sink2, filter); + if (method == FileIngestionMethod::Recursive) + dumpPath(srcPath, sink2, filter); + else + readFile(srcPath, sink2); }); std::unique_ptr delTempDir; @@ -1155,7 +1155,10 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, delTempDir = std::make_unique(tempDir); tempPath = tempDir + "/x"; - restorePath(tempPath, *source); + if (method == FileIngestionMethod::Recursive) + restorePath(tempPath, *source); + else + writeFile(tempPath, *source); } catch (EndOfFile &) { if (!inMemory) throw; @@ -1188,7 +1191,10 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, if (inMemory) { /* Restore from the NAR in memory. */ StringSource source(nar); - restorePath(realPath, source); + if (method == FileIngestionMethod::Recursive) + restorePath(realPath, source); + else + writeFile(realPath, source); } else { /* Move the temporary path we restored above. */ if (rename(tempPath.c_str(), realPath.c_str())) From 062a584f12a9216464e1664f019b782f86ce78df Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 10 Jul 2020 11:21:06 +0200 Subject: [PATCH 12/32] .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 13/32] 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 14/32] 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 15/32] 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 16/32] 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; From 64f03635d763bd627f4fb8516c04eb64e881c902 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 12 Jul 2020 16:50:22 +0200 Subject: [PATCH 17/32] 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 18/32] 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 19/32] 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 20/32] 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 21/32] 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 22/32] 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 23/32] 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 24/32] 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 25/32] 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 26/32] 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 27/32] 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 28/32] 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 29/32] 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 30/32] 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 31/32] 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 32/32] 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()))