From 1467a98d4c2014e512825c87e9056de79408421a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Baylac-Jacqu=C3=A9?= Date: Mon, 1 Aug 2022 11:37:21 +0200 Subject: [PATCH 01/11] derivation-goal.cc: remove bmCheck custom return branch on buildDone Once a derivation goal has been completed, we check whether or not this goal was meant to be repeated to check its output. An early return branch was preventing the worker to reach that repeat code branch, hence breaking the --check command (#2619). It seems like this early return branch is an artifact of a passed refactoring. As far as I can tell, buildDone's main branch also cleanup the tmp directory before returning. --- src/libstore/build/derivation-goal.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3fff2385f..3c0b14029 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -914,12 +914,6 @@ void DerivationGoal::buildDone() outputPaths ); - if (buildMode == bmCheck) { - cleanupPostOutputsRegisteredModeCheck(); - done(BuildResult::Built, std::move(builtOutputs)); - return; - } - cleanupPostOutputsRegisteredModeNonCheck(); /* Repeat the build if necessary. */ From b47b6a418d656ee5408b7773dd9b76770ca0d2a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Baylac-Jacqu=C3=A9?= Date: Wed, 3 Aug 2022 12:05:41 +0200 Subject: [PATCH 02/11] tests/check.sh: add nix3-build check test --- tests/check.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/check.sh b/tests/check.sh index ab48ff865..495202781 100644 --- a/tests/check.sh +++ b/tests/check.sh @@ -40,6 +40,14 @@ nix-build check.nix -A deterministic --argstr checkBuildId $checkBuildId \ if grep -q 'may not be deterministic' $TEST_ROOT/log; then false; fi checkBuildTempDirRemoved $TEST_ROOT/log +nix build -f check.nix deterministic --rebuild --repeat 1 \ + --argstr checkBuildId $checkBuildId --keep-failed --no-link \ + 2> $TEST_ROOT/log +if grep -q 'checking is not possible' $TEST_ROOT/log; then false; fi +# Repeat is set to 1, ie. nix should build deterministic twice. +if [ "$(grep "checking outputs" $TEST_ROOT/log | wc -l)" -ne 2 ]; then false; fi +checkBuildTempDirRemoved $TEST_ROOT/log + nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \ --no-out-link 2> $TEST_ROOT/log checkBuildTempDirRemoved $TEST_ROOT/log @@ -50,6 +58,12 @@ grep 'may not be deterministic' $TEST_ROOT/log [ "$status" = "104" ] checkBuildTempDirRemoved $TEST_ROOT/log +nix build -f check.nix nondeterministic --rebuild --repeat 1 \ + --argstr checkBuildId $checkBuildId --keep-failed --no-link \ + 2> $TEST_ROOT/log || status=$? +grep 'may not be deterministic' $TEST_ROOT/log +checkBuildTempDirRemoved $TEST_ROOT/log + nix-build check.nix -A nondeterministic --argstr checkBuildId $checkBuildId \ --no-out-link --check --keep-failed 2> $TEST_ROOT/log || status=$? grep 'may not be deterministic' $TEST_ROOT/log From 64c3adbe1ad14e043b772c3e981d511922cb06e5 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 8 Aug 2022 15:45:23 -0400 Subject: [PATCH 03/11] install-multi-user: abstract is_root, is_os_linux, is_os_darwin --- scripts/install-multi-user.sh | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 9a18280ef..472d25842 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -59,6 +59,30 @@ headless() { fi } +is_root() { + if [ "$EUID" -eq 0 ]; then + return 0 + else + return 1 + fi +} + +is_os_linux() { + if [ "$(uname -s)" = "Linux" ]; then + return 0 + else + return 1 + fi +} + +is_os_darwin() { + if [ "$(uname -s)" = "Darwin" ]; then + return 0 + else + return 1 + fi +} + contact_us() { echo "You can open an issue at https://github.com/nixos/nix/issues" echo "" @@ -423,7 +447,7 @@ EOF fi done - if [ "$(uname -s)" = "Linux" ] && [ ! -e /run/systemd/system ]; then + if is_os_linux && [ ! -e /run/systemd/system ]; then warning < Date: Mon, 8 Aug 2022 16:01:59 -0400 Subject: [PATCH 04/11] Strip whitespace in installing-binary.md --- doc/manual/src/installation/installing-binary.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index 9fb9c80c3..18fc8fff5 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -13,7 +13,7 @@ for your platform: - multi-user on macOS > **Notes on read-only filesystem root in macOS 10.15 Catalina +** - > + > > - It took some time to support this cleanly. You may see posts, > examples, and tutorials using obsolete workarounds. > - Supporting it cleanly made macOS installs too complex to qualify @@ -75,7 +75,7 @@ should run this under your usual user account, *not* as root. The script will invoke `sudo` as needed. > **Note** -> +> > If you need Nix to use a different group ID or user ID set, you will > have to download the tarball manually and [edit the install > script](#installing-from-a-binary-tarball). @@ -167,7 +167,7 @@ and `/etc/zshrc` which you may remove. removed next. 7. Remove the Nix Store volume: - + ```console sudo diskutil apfs deleteVolume /nix ``` @@ -176,7 +176,7 @@ and `/etc/zshrc` which you may remove. store. > **Note** -> +> > After you complete the steps here, you will still have an empty `/nix` > directory. This is an expected sign of a successful uninstall. The empty > `/nix` directory will disappear the next time you reboot. From 7bb1e913b33499d3ce74929749977774bcc35aed Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 8 Aug 2022 15:46:17 -0400 Subject: [PATCH 05/11] Don't prompt about using sudo if we're already root --- .../src/installation/installing-binary.md | 6 ++--- scripts/install-multi-user.sh | 25 ++++++++----------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index 18fc8fff5..a46735196 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -31,8 +31,8 @@ $ sh <(curl -L https://nixos.org/nix/install) --no-daemon ``` This will perform a single-user installation of Nix, meaning that `/nix` -is owned by the invoking user. You should run this under your usual user -account, *not* as root. The script will invoke `sudo` to create `/nix` +is owned by the invoking user. You can run this under your usual user +account or root. The script will invoke `sudo` to create `/nix` if it doesn’t already exist. If you don’t have `sudo`, you should manually create `/nix` first as root, e.g.: @@ -71,7 +71,7 @@ $ sh <(curl -L https://nixos.org/nix/install) --daemon The multi-user installation of Nix will create build users between the user IDs 30001 and 30032, and a group with the group ID 30000. You -should run this under your usual user account, *not* as root. The script +can run this under your usual user account or root. The script will invoke `sudo` as needed. > **Note** diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 472d25842..e7bdc1227 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -337,10 +337,15 @@ __sudo() { _sudo() { local expl="$1" shift - if ! headless; then + if ! headless || is_root; then __sudo "$expl" "$*" >&2 fi - sudo "$@" + + if is_root; then + env "$@" + else + sudo "$@" + fi } @@ -891,17 +896,6 @@ EOF main() { - # TODO: I've moved this out of validate_starting_assumptions so we - # can fail faster in this case. Sourcing install-darwin... now runs - # `touch /` to detect Read-only root, but it could update times on - # pre-Catalina macOS if run as root user. - if is_root; then - failure < Date: Wed, 10 Aug 2022 18:49:29 -0500 Subject: [PATCH 06/11] docfix: bundlers --- src/nix/bundle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/bundle.md b/src/nix/bundle.md index 2bb70711f..a18161a3c 100644 --- a/src/nix/bundle.md +++ b/src/nix/bundle.md @@ -44,7 +44,7 @@ flake output attributes: * `bundlers..default` -If an attribute *name* is given, `nix run` tries the following flake +If an attribute *name* is given, `nix bundle` tries the following flake output attributes: * `bundlers..` From 3d4489b623deaceef720da7d884a41452f928db6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Aug 2022 15:57:25 +0200 Subject: [PATCH 07/11] Show when we're evaluating a flake --- src/libcmd/installables.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 59162c4df..e097f23b3 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -616,6 +616,8 @@ InstallableFlake::InstallableFlake( std::tuple InstallableFlake::toDerivation() { + Activity act(*logger, lvlTalkative, actUnknown, fmt("evaluating derivation '%s'", what())); + auto attr = getCursor(*state); auto attrPath = attr->getAttrPathStr(); From e62160579f40a0425061c2223e0a303d42736ea2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 16 Aug 2022 14:58:08 +0200 Subject: [PATCH 08/11] nix flake metadata: Don't show "Inputs" if there are no inputs --- src/nix/flake.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nix/flake.cc b/src/nix/flake.cc index e01bc6d10..3967f1102 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -212,7 +212,8 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON ANSI_BOLD "Last modified:" ANSI_NORMAL " %s", std::put_time(std::localtime(&*lastModified), "%F %T")); - logger->cout(ANSI_BOLD "Inputs:" ANSI_NORMAL); + if (!lockedFlake.lockFile.root->inputs.empty()) + logger->cout(ANSI_BOLD "Inputs:" ANSI_NORMAL); std::unordered_set> visited; From c3769c68465bae971ab6bb48cfcdea85b61ea83a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Aug 2022 15:56:08 +0200 Subject: [PATCH 09/11] ProgressBar: Delay before showing a new activity Some activities are numerous but usually very short (e.g. copying a source file to the store) which would cause a lot of flickering. So only show activities that have been running for at least 10 ms. --- src/libmain/progress-bar.cc | 43 ++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index f4306ab91..5183f212f 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -8,6 +8,7 @@ #include #include #include +#include namespace nix { @@ -48,6 +49,7 @@ private: bool visible = true; ActivityId parent; std::optional name; + std::chrono::time_point startTime; }; struct ActivitiesByType @@ -91,10 +93,11 @@ public: state_.lock()->active = isTTY; updateThread = std::thread([&]() { auto state(state_.lock()); + auto nextWakeup = std::chrono::milliseconds::max(); while (state->active) { if (!state->haveUpdate) - state.wait(updateCV); - draw(*state); + state.wait_for(updateCV, nextWakeup); + nextWakeup = draw(*state); state.wait_for(quitCV, std::chrono::milliseconds(50)); } }); @@ -118,7 +121,8 @@ public: updateThread.join(); } - bool isVerbose() override { + bool isVerbose() override + { return printBuildLogs; } @@ -159,11 +163,13 @@ public: if (lvl <= verbosity && !s.empty() && type != actBuildWaiting) log(*state, lvl, s + "..."); - state->activities.emplace_back(ActInfo()); + state->activities.emplace_back(ActInfo { + .s = s, + .type = type, + .parent = parent, + .startTime = std::chrono::steady_clock::now() + }); auto i = std::prev(state->activities.end()); - i->s = s; - i->type = type; - i->parent = parent; state->its.emplace(act, i); state->activitiesByType[type].its.emplace(act, i); @@ -327,10 +333,12 @@ public: updateCV.notify_one(); } - void draw(State & state) + std::chrono::milliseconds draw(State & state) { + auto nextWakeup = std::chrono::milliseconds::max(); + state.haveUpdate = false; - if (!state.active) return; + if (!state.active) return nextWakeup; std::string line; @@ -341,12 +349,25 @@ public: line += "]"; } + auto now = std::chrono::steady_clock::now(); + if (!state.activities.empty()) { if (!status.empty()) line += " "; auto i = state.activities.rbegin(); - while (i != state.activities.rend() && (!i->visible || (i->s.empty() && i->lastLine.empty()))) + while (i != state.activities.rend()) { + if (i->visible && (!i->s.empty() || !i->lastLine.empty())) { + /* Don't show activities until some time has + passed, to avoid displaying very short + activities. */ + auto delay = std::chrono::milliseconds(10); + if (i->startTime + delay < now) + break; + else + nextWakeup = std::min(nextWakeup, std::chrono::duration_cast(delay - (now - i->startTime))); + } ++i; + } if (i != state.activities.rend()) { line += i->s; @@ -366,6 +387,8 @@ public: if (width <= 0) width = std::numeric_limits::max(); writeToStderr("\r" + filterANSIEscapes(line, false, width) + ANSI_NORMAL + "\e[K"); + + return nextWakeup; } std::string getStatus(State & state) From 53e7b7e8ac44704199a868c0f6850ac53a0b8ad1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Aug 2022 12:28:02 +0200 Subject: [PATCH 10/11] Remove warnLargeDump() This message was unhelpful (#1184) and probably misleading since memory is O(1) in most cases now. --- src/libstore/remote-store.cc | 2 -- src/libutil/serialise.cc | 20 -------------------- src/libutil/serialise.hh | 4 +--- 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index bc36aef5d..eaaf9669f 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -580,7 +580,6 @@ ref RemoteStore::addCAToStore( try { conn->to.written = 0; - conn->to.warn = true; connections->incCapacity(); { Finally cleanup([&]() { connections->decCapacity(); }); @@ -591,7 +590,6 @@ ref RemoteStore::addCAToStore( dumpString(contents, conn->to); } } - conn->to.warn = false; conn.processStderr(); } catch (SysError & e) { /* Daemon closed while we were sending the path. Probably OOM diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 8ff904583..2c3597775 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -48,24 +48,9 @@ FdSink::~FdSink() } -size_t threshold = 256 * 1024 * 1024; - -static void warnLargeDump() -{ - warn("dumping very large path (> 256 MiB); this may run out of memory"); -} - - void FdSink::write(std::string_view data) { written += data.size(); - static bool warned = false; - if (warn && !warned) { - if (written > threshold) { - warnLargeDump(); - warned = true; - } - } try { writeFull(fd, data); } catch (SysError & e) { @@ -448,11 +433,6 @@ Error readError(Source & source) void StringSink::operator () (std::string_view data) { - static bool warned = false; - if (!warned && s.size() > threshold) { - warnLargeDump(); - warned = true; - } s.append(data); } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 13da26c6a..84847835a 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -97,19 +97,17 @@ protected: struct FdSink : BufferedSink { int fd; - bool warn = false; size_t written = 0; FdSink() : fd(-1) { } FdSink(int fd) : fd(fd) { } FdSink(FdSink&&) = default; - FdSink& operator=(FdSink && s) + FdSink & operator=(FdSink && s) { flush(); fd = s.fd; s.fd = -1; - warn = s.warn; written = s.written; return *this; } From 81e42e0d3f0345c28f3d19841c89c4b1975c37a7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 27 Jul 2022 16:41:26 +0200 Subject: [PATCH 11/11] Fix onError --- tests/common.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/common.sh.in b/tests/common.sh.in index 79da10199..73c2d2309 100644 --- a/tests/common.sh.in +++ b/tests/common.sh.in @@ -193,7 +193,7 @@ fi onError() { set +x echo "$0: test failed at:" >&2 - for ((i = 1; i < 16; i++)); do + for ((i = 1; i < ${#BASH_SOURCE[@]}; i++)); do if [[ -z ${BASH_SOURCE[i]} ]]; then break; fi echo " ${FUNCNAME[i]} in ${BASH_SOURCE[i]}:${BASH_LINENO[i-1]}" >&2 done