From 7c9ece5dcaf6407449616c9d2d302853829c4f7d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 Jul 2020 14:25:43 +0200 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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()))