From d25923263e0eb70f37536e2fe5b59237f6fce5cc Mon Sep 17 00:00:00 2001 From: Brian McKenna Date: Sat, 21 Dec 2019 07:40:23 +1100 Subject: [PATCH 1/9] Disable use-sqlite-wal under WSL Before: $ nix-channel --update unpacking channels... warning: SQLite database '/nix/var/nix/db/db.sqlite' is busy (SQLITE_PROTOCOL) warning: SQLite database '/nix/var/nix/db/db.sqlite' is busy (SQLITE_PROTOCOL) warning: SQLite database '/nix/var/nix/db/db.sqlite' is busy (SQLITE_PROTOCOL) warning: SQLite database '/nix/var/nix/db/db.sqlite' is busy (SQLITE_PROTOCOL) warning: SQLite database '/nix/var/nix/db/db.sqlite' is busy (SQLITE_PROTOCOL) After: $ inst/bin/nix-channel --update unpacking channels... created 1 symlinks in user environment I've seen complaints that "sandbox" caused problems under WSL but I'm having no problems. I think recent changes could have fixed the issue. --- src/libstore/globals.cc | 9 +++++++++ src/libstore/globals.hh | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index cec85edca..5f1ae5ab5 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -117,6 +117,15 @@ void Settings::requireExperimentalFeature(const std::string & name) throw Error("experimental Nix feature '%s' is disabled", name); } +bool Settings::isWSL1() +{ + struct utsname utsbuf; + uname(&utsbuf); + // WSL1 uses -Microsoft suffix + // WSL2 uses -microsoft-standard suffix + return hasSuffix(utsbuf.release, "-Microsoft"); +} + const string nixVersion = PACKAGE_VERSION; template<> void BaseSetting::set(const std::string & str) diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 41e511c6c..782870547 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -34,6 +34,8 @@ class Settings : public Config { StringSet getDefaultSystemFeatures(); + bool isWSL1(); + public: Settings(); @@ -130,7 +132,7 @@ public: Setting fsyncMetadata{this, true, "fsync-metadata", "Whether SQLite should use fsync()."}; - Setting useSQLiteWAL{this, true, "use-sqlite-wal", + Setting useSQLiteWAL{this, !isWSL1(), "use-sqlite-wal", "Whether SQLite should use WAL mode."}; Setting syncBeforeRegistering{this, false, "sync-before-registering", From 10414d467b392f285007edeb9e5ca897057adddf Mon Sep 17 00:00:00 2001 From: Michael Forney Date: Sat, 21 Dec 2019 21:30:38 -0800 Subject: [PATCH 2/9] Pass -P to cp to preserve symlinks This is commonly the default behavior with -R, but POSIX leaves the default unspecified. --- scripts/install-nix-from-closure.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 3f1581854..3fea7e056 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -102,7 +102,7 @@ for i in $(cd "$self/store" >/dev/null && echo ./*); do rm -rf "$i_tmp" fi if ! [ -e "$dest/store/$i" ]; then - cp -Rp "$self/store/$i" "$i_tmp" + cp -RPp "$self/store/$i" "$i_tmp" chmod -R a-w "$i_tmp" chmod +w "$i_tmp" mv "$i_tmp" "$dest/store/$i" From 43eb7b6756a6441c8901342d657d299a6efdefbc Mon Sep 17 00:00:00 2001 From: Michael Forney Date: Sun, 22 Dec 2019 00:06:51 -0800 Subject: [PATCH 3/9] Pass -J to tar for xz decompression Some tar implementations can't auto-detect compression formats, so they must be specified explicitly. --- scripts/install.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install.in b/scripts/install.in index 902758b13..76cb3b1fd 100644 --- a/scripts/install.in +++ b/scripts/install.in @@ -56,7 +56,7 @@ fi unpack=$tmpDir/unpack mkdir -p "$unpack" -tar -xf "$tarball" -C "$unpack" || oops "failed to unpack '$url'" +tar -xJf "$tarball" -C "$unpack" || oops "failed to unpack '$url'" script=$(echo "$unpack"/*/install) From c502831a1d087a406e80dede8cf14e3e97531006 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Wed, 1 Jan 2020 20:50:40 -0500 Subject: [PATCH 4/9] exportReferencesGraph: support working Before, we would get: [deploy@bastion:~]$ nix-store -r /nix/store/grfnl76cahwls0igd2by2pqv0dimi8h2-nixos-system-eris-19.09.20191213.03f3def.drv these derivations will be built: /nix/store/3ka4ihvwh6wsyhpd2qa9f59506mnxvx1-initrd-linux-4.19.88.drv /nix/store/ssxwmll7v21did1c8j027q0m8w6pg41i-unit-prometheus-alertmanager-irc-notifier.service.drv /nix/store/mvyvkj46ay7pp7b1znqbkck2mq98k0qd-unit-script-network-local-commands-start.drv /nix/store/vsl1y9mz38qfk6pyirjwnfzfggz5akg6-unit-network-local-commands.service.drv /nix/store/wi5ighfwwb83fdmav6z6n2fw6npm9ffl-unit-prometheus-hydra-exporter.service.drv /nix/store/x0qkv535n75pbl3xn6nn1w7qkrg9wwyg-unit-prometheus-packet-sd.service.drv /nix/store/lv491znsjxdf51xnfxh9ld7r1zg14d52-unit-script-packet-sd-env-key-pre-start.drv /nix/store/nw4nzlca49agsajvpibx7zg5b873gk9f-unit-script-packet-sd-env-key-start.drv /nix/store/x674wwabdwjrkhnykair4c8mpxa9532w-unit-packet-sd-env-key.service.drv /nix/store/ywivz64ilb1ywlv652pkixw3vxzfvgv8-unit-wireguard-wg0.service.drv /nix/store/v3b648293g3zl8pnn0m1345nvmyd8dwb-unit-script-acme-selfsigned-status.nixos.org-start.drv /nix/store/zci5d3zvr6fgdicz6k7jjka6lmx0v3g4-unit-acme-selfsigned-status.nixos.org.service.drv /nix/store/f6pwvnm63d0kw5df0v7sipd1rkhqxk5g-system-units.drv /nix/store/iax8071knxk9c7krpm9jqg0lcrawf4lc-etc.drv /nix/store/grfnl76cahwls0igd2by2pqv0dimi8h2-nixos-system-eris-19.09.20191213.03f3def.drv error: invalid file name 'closure-init-0' in 'exportReferencesGraph' This was tough to debug, I didn't figure out which one was broken until I did: nix-store -r /nix/store/grfnl76cahwls0igd2by2pqv0dimi8h2-nixos-system-eris-19.09.20191213.03f3def.drv 2>&1 | grep nix/store | xargs -n1 nix-store -r and then looking at the remaining build graph: $ nix-store -r /nix/store/grfnl76cahwls0igd2by2pqv0dimi8h2-nixos-system-eris-19.09.20191213.03f3def.drv these derivations will be built: /nix/store/3ka4ihvwh6wsyhpd2qa9f59506mnxvx1-initrd-linux-4.19.88.drv /nix/store/grfnl76cahwls0igd2by2pqv0dimi8h2-nixos-system-eris-19.09.20191213.03f3def.drv error: invalid file name 'closure-init-0' in 'exportReferencesGraph' and knowing the initrd build is before the system, then: $ nix show-derivation /nix/store/3ka4ihvwh6wsyhpd2qa9f59506mnxvx1-initrd-linux-4.19.88.drv { "/nix/store/3ka4ihvwh6wsyhpd2qa9f59506mnxvx1-initrd-linux-4.19.88.drv": { [...] "exportReferencesGraph": "closure-init-0 /nix/store/...-stage-1-init.sh closure-mdadm.conf-1 /nix/store/...-mdadm.conf closure-ubuntu.conf-2 ...", [...] } } I then searched the repo for "in 'exportReferencesGraph'", found this recently updated regex, and realized it was missing a "-". --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index de0ce9ae1..5d681d279 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1985,7 +1985,7 @@ void DerivationGoal::startBuilder() throw BuildError(format("odd number of tokens in 'exportReferencesGraph': '%1%'") % s); for (Strings::iterator i = ss.begin(); i != ss.end(); ) { string fileName = *i++; - static std::regex regex("[A-Za-z_][A-Za-z0-9_.]*"); + static std::regex regex("[A-Za-z_][A-Za-z0-9_.-]*"); if (!std::regex_match(fileName, regex)) throw Error("invalid file name '%s' in 'exportReferencesGraph'", fileName); From 515c0a263e137a00e82f7d981284dbe54db23247 Mon Sep 17 00:00:00 2001 From: Puck Meerburg Date: Thu, 2 Jan 2020 17:20:57 +0000 Subject: [PATCH 5/9] passAsFile: hash the attribute name instead of numbering sequentially This makes the paths consistent without relying on ordering. Co-authored-by: edef --- src/libstore/build.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 5d681d279..9ee3b04e3 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2455,12 +2455,12 @@ void DerivationGoal::initTmpDir() { if (!parsedDrv->getStructuredAttrs()) { StringSet passAsFile = tokenizeString(get(drv->env, "passAsFile").value_or("")); - int fileNr = 0; for (auto & i : drv->env) { if (passAsFile.find(i.first) == passAsFile.end()) { env[i.first] = i.second; } else { - string fn = ".attr-" + std::to_string(fileNr++); + auto hash = hashString(htSHA256, i.first); + string fn = ".attr-" + hash.to_string(); Path p = tmpDir + "/" + fn; writeFile(p, i.second); chownToBuilder(p); From c65a6fa86aef7bdf51fb4fba7bd31d265619ba3f Mon Sep 17 00:00:00 2001 From: edef Date: Thu, 2 Jan 2020 23:41:48 +0000 Subject: [PATCH 6/9] passAsFile: leave out the hash prefix Having a colon in the path may cause issues, and having the hash function indicated isn't actually necessary. We now verify the path format in the tests to prevent regressions. --- src/libstore/build.cc | 2 +- tests/pass-as-file.sh | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 9ee3b04e3..78f39fed1 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2460,7 +2460,7 @@ void DerivationGoal::initTmpDir() { env[i.first] = i.second; } else { auto hash = hashString(htSHA256, i.first); - string fn = ".attr-" + hash.to_string(); + string fn = ".attr-" + hash.to_string(Base32, false); Path p = tmpDir + "/" + fn; writeFile(p, i.second); chownToBuilder(p); diff --git a/tests/pass-as-file.sh b/tests/pass-as-file.sh index 3dfe10baa..2c0bc5031 100644 --- a/tests/pass-as-file.sh +++ b/tests/pass-as-file.sh @@ -10,6 +10,7 @@ mkDerivation { passAsFile = [ \"foo\" ]; foo = [ \"xyzzy\" ]; builder = builtins.toFile \"builder.sh\" '' + [ \"\$(basename \$fooPath)\" = .attr-1bp7cri8hplaz6hbz0v4f0nl44rl84q1sg25kgwqzipzd1mv89ic ] [ \"\$(cat \$fooPath)\" = xyzzy ] touch \$out ''; From b33fefcb92e02725fc77b6adfabab1c1f43e9aa1 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sat, 4 Jan 2020 17:47:52 +0100 Subject: [PATCH 7/9] build: recover store path when replacing fails This shouldn't happen in normal circumstances, but just in case attempt to move the temporary path back if possible. --- src/libstore/build.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 78f39fed1..257d70ca9 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1513,8 +1513,10 @@ void replaceValidPath(const Path & storePath, const Path tmpPath) Path oldPath = (format("%1%.old-%2%-%3%") % storePath % getpid() % random()).str(); if (pathExists(storePath)) rename(storePath.c_str(), oldPath.c_str()); - if (rename(tmpPath.c_str(), storePath.c_str()) == -1) + if (rename(tmpPath.c_str(), storePath.c_str()) == -1) { + rename(oldPath.c_str(), storePath.c_str()); // attempt to recover throw SysError("moving '%s' to '%s'", tmpPath, storePath); + } deletePath(oldPath); } From 7d448bc966021e63e9c7b96530cee885f3dcd68d Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sat, 4 Jan 2020 20:23:59 +0100 Subject: [PATCH 8/9] build: fix path repairing when hash rewriting is required Handle store path repairing on darwin when sandboxing is enabled. Unlike on linux sandboxing on darwin still requires hash rewriting. --- src/libstore/build.cc | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 257d70ca9..0674f936b 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3567,19 +3567,6 @@ void DerivationGoal::registerOutputs() if (!missingPaths.count(i.second.path)) continue; Path actualPath = path; - if (useChroot) { - actualPath = chrootRootDir + path; - if (pathExists(actualPath)) { - /* Move output paths from the chroot to the Nix store. */ - if (buildMode == bmRepair) - replaceValidPath(path, actualPath); - else - if (buildMode != bmCheck && rename(actualPath.c_str(), worker.store.toRealPath(path).c_str()) == -1) - throw SysError(format("moving build output '%1%' from the sandbox to the Nix store") % path); - } - if (buildMode != bmCheck) actualPath = worker.store.toRealPath(path); - } - if (needsHashRewrite()) { auto r = redirectedOutputs.find(i.second.path); if (r != redirectedOutputs.end()) { @@ -3591,6 +3578,17 @@ void DerivationGoal::registerOutputs() if (buildMode == bmCheck) actualPath = redirected; } + } else if (useChroot) { + actualPath = chrootRootDir + path; + if (pathExists(actualPath)) { + /* Move output paths from the chroot to the Nix store. */ + if (buildMode == bmRepair) + replaceValidPath(path, actualPath); + else + if (buildMode != bmCheck && rename(actualPath.c_str(), worker.store.toRealPath(path).c_str()) == -1) + throw SysError(format("moving build output '%1%' from the sandbox to the Nix store") % path); + } + if (buildMode != bmCheck) actualPath = worker.store.toRealPath(path); } struct stat st; From cb90e382b5b6e177ea902b3909fd1897643ae3cd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 5 Jan 2020 16:21:34 +0100 Subject: [PATCH 9/9] Hide FunctionCallTrace constructor/destructor This prevents them from being inlined. On gcc 9, this reduces the stack size needed for nix-instantiate '' -A texlive.combined.scheme-full --dry-run from 12.9 MiB to 4.8 MiB. --- src/libexpr/eval.cc | 4 +--- src/libexpr/function-trace.cc | 17 +++++++++++++++++ src/libexpr/function-trace.hh | 14 ++------------ 3 files changed, 20 insertions(+), 15 deletions(-) create mode 100644 src/libexpr/function-trace.cc diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 6539a346a..1c787645d 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1106,9 +1106,7 @@ void EvalState::callPrimOp(Value & fun, Value & arg, Value & v, const Pos & pos) void EvalState::callFunction(Value & fun, Value & arg, Value & v, const Pos & pos) { - std::unique_ptr trace; - if (evalSettings.traceFunctionCalls) - trace = std::make_unique(pos); + auto trace = evalSettings.traceFunctionCalls ? std::make_unique(pos) : nullptr; forceValue(fun, pos); diff --git a/src/libexpr/function-trace.cc b/src/libexpr/function-trace.cc new file mode 100644 index 000000000..af1486f78 --- /dev/null +++ b/src/libexpr/function-trace.cc @@ -0,0 +1,17 @@ +#include "function-trace.hh" + +namespace nix { + +FunctionCallTrace::FunctionCallTrace(const Pos & pos) : pos(pos) { + auto duration = std::chrono::high_resolution_clock::now().time_since_epoch(); + auto ns = std::chrono::duration_cast(duration); + printMsg(lvlInfo, "function-trace entered %1% at %2%", pos, ns.count()); +} + +FunctionCallTrace::~FunctionCallTrace() { + auto duration = std::chrono::high_resolution_clock::now().time_since_epoch(); + auto ns = std::chrono::duration_cast(duration); + printMsg(lvlInfo, "function-trace exited %1% at %2%", pos, ns.count()); +} + +} diff --git a/src/libexpr/function-trace.hh b/src/libexpr/function-trace.hh index 2c39b7430..472f2045e 100644 --- a/src/libexpr/function-trace.hh +++ b/src/libexpr/function-trace.hh @@ -9,17 +9,7 @@ namespace nix { struct FunctionCallTrace { const Pos & pos; - - FunctionCallTrace(const Pos & pos) : pos(pos) { - auto duration = std::chrono::high_resolution_clock::now().time_since_epoch(); - auto ns = std::chrono::duration_cast(duration); - printMsg(lvlInfo, "function-trace entered %1% at %2%", pos, ns.count()); - } - - ~FunctionCallTrace() { - auto duration = std::chrono::high_resolution_clock::now().time_since_epoch(); - auto ns = std::chrono::duration_cast(duration); - printMsg(lvlInfo, "function-trace exited %1% at %2%", pos, ns.count()); - } + FunctionCallTrace(const Pos & pos); + ~FunctionCallTrace(); }; }