From dbe4c043d75f90f30932d326d2c80fd030b1f06d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Forsman?= Date: Wed, 27 Mar 2019 16:18:53 +0100 Subject: [PATCH 01/12] install-multi-user: reduce max-jobs from 32 to 1 Having max-jobs = 32 ($NIX_USER_COUNT is hardcoded to that value) may severely overload the machine. The nix.conf(5) manual page says max-jobs defaults to 1, so let's use that value. NOTE: Both max-jobs and cores are now being set to their default value, so they can be removed alltogether. --- scripts/install-multi-user.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 4b65783a2..6a76761cc 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -742,7 +742,7 @@ place_nix_configuration() { cat < "$SCRATCH/nix.conf" build-users-group = $NIX_BUILD_GROUP_NAME -max-jobs = $NIX_USER_COUNT +max-jobs = 1 cores = 1 EOF _sudo "to place the default nix daemon configuration (part 2)" \ From 07d9981f34c8423e74a8dc1b4f978fe58b421aee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Forsman?= Date: Wed, 27 Mar 2019 16:24:58 +0100 Subject: [PATCH 02/12] install-multi-user: remove unneeded settings from nix.conf Hardcoding the "max-jobs" and "cores" settings in nix.conf at install time, to the same value as Nix' built-in default, makes little sense to me. --- scripts/install-multi-user.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 6a76761cc..1e1db21c3 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -741,9 +741,6 @@ setup_default_profile() { place_nix_configuration() { cat < "$SCRATCH/nix.conf" build-users-group = $NIX_BUILD_GROUP_NAME - -max-jobs = 1 -cores = 1 EOF _sudo "to place the default nix daemon configuration (part 2)" \ install -m 0664 "$SCRATCH/nix.conf" /etc/nix/nix.conf From 4b0d6133836113572418f383ddd14c92bfebc768 Mon Sep 17 00:00:00 2001 From: JorisE Date: Tue, 4 Jun 2019 14:12:03 +0200 Subject: [PATCH 03/12] Minor typo --- doc/manual/expressions/language-constructs.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/expressions/language-constructs.xml b/doc/manual/expressions/language-constructs.xml index 923b5d8c4..0d0cbbe15 100644 --- a/doc/manual/expressions/language-constructs.xml +++ b/doc/manual/expressions/language-constructs.xml @@ -43,7 +43,7 @@ encountered).. Let-expressions -A let-expression allows you define local variables for an +A let-expression allows you to define local variables for an expression. For instance, From 9e0f5f803f6cbfe9925cef69a0e58cbf5375bfaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Baylac-Jacqu=C3=A9?= Date: Sat, 8 Jun 2019 00:41:19 +0200 Subject: [PATCH 04/12] Daemon: warn when an untrusted user cannot override a setting In a daemon-based Nix setup, some options cannot be overridden by a client unless the client's user is considered trusted. Currently, if an untrusted user tries to override one of those options, we are silently ignoring it. This can be pretty confusing in certain situations. e.g. a user thinks he disabled the sandbox when in reality he did not. We are now sending a warning message letting know the user some options have been ignored. Related to #1761. --- src/nix-daemon/nix-daemon.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index 8d63b8f36..973f64a34 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -574,7 +574,7 @@ static void performOp(TunnelLogger * logger, ref store, else if (setSubstituters(settings.extraSubstituters)) ; else - debug("ignoring untrusted setting '%s'", name); + warn("ignoring the user-specified setting '%s', because it is a restricted setting and you are not a trusted user.", name); } catch (UsageError & e) { warn(e.what()); } From 34fa8ce9179b17cc4cd13ae49b69fccf393271a0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sat, 15 Jun 2019 16:34:06 +0200 Subject: [PATCH 05/12] nix: Support -j flag --- src/libmain/common-args.cc | 9 +++++++++ src/libmain/shared.cc | 4 ---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libmain/common-args.cc b/src/libmain/common-args.cc index 4c35a4199..9e1d7cee6 100644 --- a/src/libmain/common-args.cc +++ b/src/libmain/common-args.cc @@ -35,6 +35,15 @@ MixCommonArgs::MixCommonArgs(const string & programName) } }); + mkFlag() + .longName("max-jobs") + .shortName('j') + .label("jobs") + .description("maximum number of parallel builds") + .handler([=](std::string s) { + settings.set("max-jobs", s); + }); + std::string cat = "config"; globalConfig.convertToArgs(*this, cat); diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 4ed34e54d..cd752f467 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -175,10 +175,6 @@ LegacyArgs::LegacyArgs(const std::string & programName, .description("build from source if substitution fails") .set(&(bool&) settings.tryFallback, true); - mkFlag1('j', "max-jobs", "jobs", "maximum number of parallel builds", [=](std::string s) { - settings.set("max-jobs", s); - }); - auto intSettingAlias = [&](char shortName, const std::string & longName, const std::string & description, const std::string & dest) { mkFlag(shortName, longName, description, [=](unsigned int n) { From 5064971ded3e2cc7c6b80d3166270a87a9650c09 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 May 2019 21:15:45 +0200 Subject: [PATCH 06/12] Fix test failures when $TMPDIR changes (cherry picked from commit c38c726eb5d447c7e9d894d57cd05ac46c173ddd) --- tests/build-dry.sh | 6 +++--- tests/nix-copy-ssh.sh | 2 +- tests/nix-shell.sh | 8 ++++---- tests/placeholders.sh | 2 -- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/build-dry.sh b/tests/build-dry.sh index 610e6070c..e72533e70 100644 --- a/tests/build-dry.sh +++ b/tests/build-dry.sh @@ -8,13 +8,13 @@ clearStore clearCache # Ensure this builds successfully first -nix build -f dependencies.nix +nix build --no-link -f dependencies.nix clearStore clearCache # Try --dry-run using old command first -nix-build dependencies.nix --dry-run 2>&1 | grep "will be built" +nix-build --no-out-link dependencies.nix --dry-run 2>&1 | grep "will be built" # Now new command: nix build -f dependencies.nix --dry-run 2>&1 | grep "will be built" @@ -27,7 +27,7 @@ clearCache # Try --dry-run using new command first nix build -f dependencies.nix --dry-run 2>&1 | grep "will be built" # Now old command: -nix-build dependencies.nix --dry-run 2>&1 | grep "will be built" +nix-build --no-out-link dependencies.nix --dry-run 2>&1 | grep "will be built" fi ################################################### diff --git a/tests/nix-copy-ssh.sh b/tests/nix-copy-ssh.sh index 6aba667a4..eb801548d 100644 --- a/tests/nix-copy-ssh.sh +++ b/tests/nix-copy-ssh.sh @@ -7,7 +7,7 @@ remoteRoot=$TEST_ROOT/store2 chmod -R u+w "$remoteRoot" || true rm -rf "$remoteRoot" -outPath=$(nix-build dependencies.nix) +outPath=$(nix-build --no-out-link dependencies.nix) nix copy --to "ssh://localhost?store=$NIX_STORE_DIR&remote-store=$remoteRoot%3fstore=$NIX_STORE_DIR%26real=$remoteRoot$NIX_STORE_DIR" $outPath diff --git a/tests/nix-shell.sh b/tests/nix-shell.sh index 6024ea399..ee502dddb 100644 --- a/tests/nix-shell.sh +++ b/tests/nix-shell.sh @@ -27,13 +27,13 @@ output=$(nix-shell --pure --keep SELECTED_IMPURE_VAR shell.nix -A shellDrv --run # Test nix-shell on a .drv symlink # Legacy: absolute path and .drv extension required -nix-instantiate shell.nix -A shellDrv --indirect --add-root shell.drv -[[ $(nix-shell --pure $PWD/shell.drv --run \ +nix-instantiate shell.nix -A shellDrv --indirect --add-root $TEST_ROOT/shell.drv +[[ $(nix-shell --pure $TEST_ROOT/shell.drv --run \ 'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX"') = " - foo - bar" ]] # New behaviour: just needs to resolve to a derivation in the store -nix-instantiate shell.nix -A shellDrv --indirect --add-root shell -[[ $(nix-shell --pure shell --run \ +nix-instantiate shell.nix -A shellDrv --indirect --add-root $TEST_ROOT/shell +[[ $(nix-shell --pure $TEST_ROOT/shell --run \ 'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX"') = " - foo - bar" ]] # Test nix-shell -p diff --git a/tests/placeholders.sh b/tests/placeholders.sh index 071cfe2dc..cd1bb7bc2 100644 --- a/tests/placeholders.sh +++ b/tests/placeholders.sh @@ -18,5 +18,3 @@ nix-build --no-out-link -E ' "; } ' - -echo XYZZY From 26bc876ae6e7ceff8b37ba5c2c1043ce216cf384 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sat, 15 Jun 2019 16:45:00 +0200 Subject: [PATCH 07/12] nix: Add -L alias for --print-build-logs --- src/nix/main.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix/main.cc b/src/nix/main.cc index 4f87ad72b..25e321b86 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -45,6 +45,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs mkFlag() .longName("print-build-logs") + .shortName('L') .description("print full build logs on stderr") .set(&printBuildLogs, true); From b693029ca076abf8a4084a053a122ffcdf4d61b2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 16 Jun 2019 09:43:20 +0200 Subject: [PATCH 08/12] Style fix --- src/nix-daemon/nix-daemon.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index 973f64a34..e88aaf636 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -574,7 +574,7 @@ static void performOp(TunnelLogger * logger, ref store, else if (setSubstituters(settings.extraSubstituters)) ; else - warn("ignoring the user-specified setting '%s', because it is a restricted setting and you are not a trusted user.", name); + warn("ignoring the user-specified setting '%s', because it is a restricted setting and you are not a trusted user", name); } catch (UsageError & e) { warn(e.what()); } From e84c26564507388f2e8b7c7f2b00eb2b57856da1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 17 May 2019 22:29:15 +0200 Subject: [PATCH 09/12] Run builds in a pseudo-terminal This allows many programs (e.g. gcc, clang, cmake) to print colorized log output (assuming $TERM is set to a value like "xterm"). There are other ways to get colors, in particular setting CLICOLOR_FORCE, but they're less widely supported and can break programs that parse tool output. --- src/libstore/build.cc | 54 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0bd738809..9de7613f8 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -1558,8 +1559,8 @@ void DerivationGoal::buildDone() if (hook) { hook->builderOut.readSide = -1; hook->fromHook.readSide = -1; - } - else builderOut.readSide = -1; + } else + builderOut.readSide = -1; /* Close the log file. */ closeLogFile(); @@ -2181,7 +2182,43 @@ void DerivationGoal::startBuilder() Path logFile = openLogFile(); /* Create a pipe to get the output of the builder. */ - builderOut.create(); + //builderOut.create(); + + builderOut.readSide = posix_openpt(O_RDWR | O_NOCTTY); + if (!builderOut.readSide) + throw SysError("opening pseudoterminal master"); + + std::string slaveName(ptsname(builderOut.readSide.get())); + + if (chmod(slaveName.c_str(), 0600)) + throw SysError("changing mode of pseudoterminal slave"); + + if (buildUser && chown(slaveName.c_str(), buildUser->getUID(), 0)) + throw SysError("changing owner of pseudoterminal slave"); + + #if 0 + // Mount the pt in the sandbox so that the "tty" command works. + // FIXME: this doesn't work with the new devpts in the sandbox. + if (useChroot) + dirsInChroot[slaveName] = {slaveName, false}; + #endif + + if (unlockpt(builderOut.readSide.get())) + throw SysError("unlocking pseudoterminal"); + + builderOut.writeSide = open(slaveName.c_str(), O_RDWR | O_NOCTTY); + if (!builderOut.writeSide) + throw SysError("opening pseudoterminal slave"); + + // Put the pt into raw mode to prevent \n -> \r\n translation. + struct termios term; + if (tcgetattr(builderOut.writeSide.get(), &term)) + throw SysError("getting pseudoterminal attributes"); + + cfmakeraw(&term); + + if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term)) + throw SysError("putting pseudoterminal into raw mode"); result.startTime = time(0); @@ -4361,14 +4398,15 @@ void Worker::waitForInput() for (auto & k : fds2) { if (FD_ISSET(k, &fds)) { ssize_t rd = read(k, buffer.data(), buffer.size()); - if (rd == -1) { - if (errno != EINTR) - throw SysError(format("reading from %1%") - % goal->getName()); - } else if (rd == 0) { + // FIXME: is there a cleaner way to handle pt close + // than EIO? Is this even standard? + if (rd == 0 || (rd == -1 && errno == EIO)) { debug(format("%1%: got EOF") % goal->getName()); goal->handleEOF(k); j->fds.erase(k); + } else if (rd == -1) { + if (errno != EINTR) + throw SysError("%s: read failed", goal->getName()); } else { printMsg(lvlVomit, format("%1%: read %2% bytes") % goal->getName() % rd); From 82ca6ef390b752faf1bf27b22daa34b29a4b00d4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 19 May 2019 16:56:08 +0200 Subject: [PATCH 10/12] Set $TERM --- src/libstore/build.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 9de7613f8..5a75403cf 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2443,6 +2443,9 @@ void DerivationGoal::initEnv() may change that in the future. So tell the builder which file descriptor to use for that. */ env["NIX_LOG_FD"] = "2"; + + /* Trigger colored output in various tools. */ + env["TERM"] = "xterm-256color"; } From 2743bf0bb1727056ce6d6d6be4e38c3251aac1f8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 16 Jun 2019 20:02:40 +0200 Subject: [PATCH 11/12] Hopefully fix macOS tests --- src/libstore/build.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 5a75403cf..cccf0c7c6 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2190,11 +2190,13 @@ void DerivationGoal::startBuilder() std::string slaveName(ptsname(builderOut.readSide.get())); - if (chmod(slaveName.c_str(), 0600)) - throw SysError("changing mode of pseudoterminal slave"); + if (buildUser) { + if (chmod(slaveName.c_str(), 0600)) + throw SysError("changing mode of pseudoterminal slave"); - if (buildUser && chown(slaveName.c_str(), buildUser->getUID(), 0)) - throw SysError("changing owner of pseudoterminal slave"); + if (chown(slaveName.c_str(), buildUser->getUID(), 0)) + throw SysError("changing owner of pseudoterminal slave"); + } #if 0 // Mount the pt in the sandbox so that the "tty" command works. From 3cc1125595d97b4ab7369e37e4ad22f4cfecb8b2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 17 Jun 2019 08:08:04 +0200 Subject: [PATCH 12/12] Another attempt at getting pseudoterminals to work on macOS --- src/libstore/build.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index cccf0c7c6..5b38bcf3c 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2196,6 +2196,9 @@ void DerivationGoal::startBuilder() if (chown(slaveName.c_str(), buildUser->getUID(), 0)) throw SysError("changing owner of pseudoterminal slave"); + } else { + if (grantpt(builderOut.readSide.get())) + throw SysError("granting access to pseudoterminal slave"); } #if 0