From ff6867ab94cbe9ddcb4ba18d68a4a2dcb79b865d Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sun, 12 May 2019 15:53:40 +0200 Subject: [PATCH 01/13] build: move needsHashRewrite initialization to startBuilder The value of useChroot is not set yet in the constructor, resulting in hash rewriting being enabled in certain cases where it should not be. Fixes #2801 --- src/libstore/build.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 91eb97dfb..30825add4 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -997,13 +997,6 @@ DerivationGoal::DerivationGoal(const Path & drvPath, const StringSet & wantedOut , wantedOutputs(wantedOutputs) , buildMode(buildMode) { -#if __linux__ - needsHashRewrite = !useChroot; -#else - /* Darwin requires hash rewriting even when sandboxing is enabled. */ - needsHashRewrite = true; -#endif - state = &DerivationGoal::getDerivation; name = (format("building of '%1%'") % drvPath).str(); trace("created"); @@ -1852,6 +1845,13 @@ void DerivationGoal::startBuilder() #endif } +#if __linux__ + needsHashRewrite = !useChroot; +#else + /* Darwin requires hash rewriting even when sandboxing is enabled. */ + needsHashRewrite = true; +#endif + /* If `build-users-group' is not empty, then we have to build as one of the members of that group. */ if (settings.buildUsersGroup != "" && getuid() == 0) { From d75bdb5793e5ebf9e480f5a0012d141347725801 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sun, 12 May 2019 16:46:21 +0200 Subject: [PATCH 02/13] build: add test for sandboxed --check --- tests/linux-sandbox.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/linux-sandbox.sh b/tests/linux-sandbox.sh index acfd46c54..52967d07d 100644 --- a/tests/linux-sandbox.sh +++ b/tests/linux-sandbox.sh @@ -25,3 +25,6 @@ nix path-info -r $outPath | grep input-2 nix ls-store -R -l $outPath | grep foobar nix cat-store $outPath/foobar | grep FOOBAR + +# Test --check without hash rewriting. +nix-build dependencies.nix --no-out-link --check --sandbox-paths /nix/store From c78686e411e0a14cff51836fe6c35d7584171df3 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 10 May 2019 16:39:31 -0400 Subject: [PATCH 03/13] build: run diff-hook under --check and document diff-hook --- .../advanced-topics/advanced-topics.xml | 1 + doc/manual/advanced-topics/diff-hook.xml | 207 ++++++++++++++++++ doc/manual/command-ref/conf-file.xml | 81 ++++++- src/libstore/build.cc | 30 ++- 4 files changed, 303 insertions(+), 16 deletions(-) create mode 100644 doc/manual/advanced-topics/diff-hook.xml diff --git a/doc/manual/advanced-topics/advanced-topics.xml b/doc/manual/advanced-topics/advanced-topics.xml index b710f9f2b..c304367aa 100644 --- a/doc/manual/advanced-topics/advanced-topics.xml +++ b/doc/manual/advanced-topics/advanced-topics.xml @@ -7,5 +7,6 @@ Advanced Topics + diff --git a/doc/manual/advanced-topics/diff-hook.xml b/doc/manual/advanced-topics/diff-hook.xml new file mode 100644 index 000000000..d2613f6df --- /dev/null +++ b/doc/manual/advanced-topics/diff-hook.xml @@ -0,0 +1,207 @@ + + +Verifying Build Reproducibility with <option linkend="conf-diff-hook">diff-hook</option> + +Check build reproducibility by running builds multiple times +and comparing their results. + +Specify a program with Nix's to +compare build results when two builds produce different results. Note: +this hook is only executed if the results are not the same, this hook +is not used for determining if the results are the same. + +For purposes of demonstration, we'll use the following Nix file, +deterministic.nix for testing: + + +let + inherit (import <nixpkgs> {}) runCommand; +in { + stable = runCommand "stable" {} '' + touch $out + ''; + + unstable = runCommand "unstable" {} '' + echo $RANDOM > $out + ''; +} + + +Additionally, nix.conf contains: + + +diff-hook = /etc/nix/my-diff-hook +run-diff-hook = true + + +where /etc/nix/my-diff-hook is an executable +file containing: + + +#!/bin/sh +exec >&2 +echo "For derivation $3:" +/run/current-system/sw/bin/runuser -u nobody -- /run/current-system/sw/bin/diff -r "$1" "$2" + + + + The diff hook can be run as root. Take care to run as little + as possible as root, for this example we use runuser + to drop privileges. + + + + +
+ + Spot-Checking Build Determinism + + + + Verify a path which already exists in the Nix store by passing + to the build command. + + + If the build passes and is deterministic, Nix will exit with a + status code of 0: + + +$ nix-build ./deterministic.nix -A stable +these derivations will be built: + /nix/store/z98fasz2jqy9gs0xbvdj939p27jwda38-stable.drv +building '/nix/store/z98fasz2jqy9gs0xbvdj939p27jwda38-stable.drv'... +/nix/store/yyxlzw3vqaas7wfp04g0b1xg51f2czgq-stable + +$ nix-build ./deterministic.nix -A stable --check +checking outputs of '/nix/store/z98fasz2jqy9gs0xbvdj939p27jwda38-stable.drv'... +/nix/store/yyxlzw3vqaas7wfp04g0b1xg51f2czgq-stable + + + If the build is not deterministic, Nix will exit with a status + code of 1: + + +$ nix-build ./deterministic.nix -A unstable +these derivations will be built: + /nix/store/cgl13lbj1w368r5z8gywipl1ifli7dhk-unstable.drv +building '/nix/store/cgl13lbj1w368r5z8gywipl1ifli7dhk-unstable.drv'... +/nix/store/krpqk0l9ib0ibi1d2w52z293zw455cap-unstable + +$ nix-build ./deterministic.nix -A unstable --check +checking outputs of '/nix/store/cgl13lbj1w368r5z8gywipl1ifli7dhk-unstable.drv'... +error: derivation '/nix/store/cgl13lbj1w368r5z8gywipl1ifli7dhk-unstable.drv' may not be deterministic: output '/nix/store/krpqk0l9ib0ibi1d2w52z293zw455cap-unstable' differs + + +In the Nix daemon's log, we will now see: + +For derivation /nix/store/cgl13lbj1w368r5z8gywipl1ifli7dhk-unstable.drv: +1c1 +< 8108 +--- +> 30204 + + + + Using with + will cause Nix to keep the second build's output in a special, + .check path: + + +$ nix-build ./deterministic.nix -A unstable --check --keep-failed +checking outputs of '/nix/store/cgl13lbj1w368r5z8gywipl1ifli7dhk-unstable.drv'... +note: keeping build directory '/tmp/nix-build-unstable.drv-0' +error: derivation '/nix/store/cgl13lbj1w368r5z8gywipl1ifli7dhk-unstable.drv' may not be deterministic: output '/nix/store/krpqk0l9ib0ibi1d2w52z293zw455cap-unstable' differs from '/nix/store/krpqk0l9ib0ibi1d2w52z293zw455cap-unstable.check' + + + In particular, notice the + /nix/store/krpqk0l9ib0ibi1d2w52z293zw455cap-unstable.check + output. Nix has copied the build results to that directory where you + can examine it. + + + <literal>.check</literal> paths are not registered store paths + + Check paths are not protected against garbage collection, + and this path will be deleted on the next garbage collection. + + The path is guaranteed to be alive for the duration of + 's execution, but may be deleted + any time after. + + If the comparison is performed as part of automated tooling, + please use the diff-hook or author your tooling to handle the case + where the build was not deterministic and also a check path does + not exist. + + + + is only usable if the derivation has + been built on the system already. If the derivation has not been + built Nix will fail with the error: + +error: some outputs of '/nix/store/hzi1h60z2qf0nb85iwnpvrai3j2w7rr6-unstable.drv' are not valid, so checking is not possible + + + Run the build without , and then try with + again. + +
+ +
+ + Automatic and Optionally Enforced Determinism Verification + + + + Automatically verify every build at build time by executing the + build multiple times. + + + + Setting and + in your + nix.conf permits the automated verification + of every build Nix performs. + + + + The following configuration will run each build three times, and + will require the build to be deterministic: + + +enforce-determinism = true +repeat = 2 + + + + + Setting to false as in + the following configuration will run the build multiple times, + execute the build hook, but will allow the build to succeed even + if it does not build reproducibly: + + +enforce-determinism = false +repeat = 1 + + + + + An example output of this configuration: + +$ nix-build ./test.nix -A unstable +these derivations will be built: + /nix/store/ch6llwpr2h8c3jmnf3f2ghkhx59aa97f-unstable.drv +building '/nix/store/ch6llwpr2h8c3jmnf3f2ghkhx59aa97f-unstable.drv' (round 1/2)... +building '/nix/store/ch6llwpr2h8c3jmnf3f2ghkhx59aa97f-unstable.drv' (round 2/2)... +output '/nix/store/6xg356v9gl03hpbbg8gws77n19qanh02-unstable' of '/nix/store/ch6llwpr2h8c3jmnf3f2ghkhx59aa97f-unstable.drv' differs from '/nix/store/6xg356v9gl03hpbbg8gws77n19qanh02-unstable.check' from previous round +/nix/store/6xg356v9gl03hpbbg8gws77n19qanh02-unstable + + +
+ diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml index f0da1f612..a1a5d6e12 100644 --- a/doc/manual/command-ref/conf-file.xml +++ b/doc/manual/command-ref/conf-file.xml @@ -1,7 +1,9 @@ + + xml:id="sec-conf-file" + version="5"> nix.conf @@ -240,6 +242,64 @@ false. + diff-hook + + + Absolute path to an executable capable of diffing build results. + The hook executes if is + true, and the output of a build is known to not be the same. + This program is not executed to determine if two results are the + same. + + + + + The root user executes the diff hook in a daemonised + installation. See for + information on using the diff hook safely. + + + + The diff hook program receives three parameters: + + + + + A path to the previous build's results + + + + + + A path to the current build's results + + + + + + The path to the build's derivation + + + + + The diff hook should not print data to stderr or stdout, as + output is not displayed to the user. However, if information is + printed, it will be printed in the nix-daemon + log. + + When using the Nix daemon, diff-hook must + be set in the nix.conf configuration file, and + cannot be passed at the command line. + + + + + + enforce-determinism + + See . + + extra-sandbox-paths @@ -595,9 +655,9 @@ password my-password they are deterministic. The default value is 0. If the value is non-zero, every build is repeated the specified number of times. If the contents of any of the runs differs from the - previous ones, the build is rejected and the resulting store paths - are not registered as “valid” in Nix’s database. - + previous ones and is + true, the build is rejected and the resulting store paths are not + registered as “valid” in Nix’s database. require-sigs @@ -628,6 +688,19 @@ password my-password + run-diff-hook + + + If true, enable the execution of . + + + + When using the Nix daemon, run-diff-hook must + be set in the nix.conf configuration file, + and cannot be passed at the command line. + + + sandbox diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 91eb97dfb..026828c53 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -461,6 +461,19 @@ static void commonChildInit(Pipe & logPipe) close(fdDevNull); } +void handleDiffHook(Path tryA, Path tryB, Path drvPath) +{ + auto diffHook = settings.diffHook; + if (diffHook != "" && settings.runDiffHook) { + try { + auto diff = runProgram(diffHook, true, {tryA, tryB, drvPath}); + if (diff != "") + printError(chomp(diff)); + } catch (Error & error) { + printError("diff hook execution failed: %s", error.what()); + } + } +} ////////////////////////////////////////////////////////////////////// @@ -3039,8 +3052,7 @@ void DerivationGoal::registerOutputs() InodesSeen inodesSeen; Path checkSuffix = ".check"; - bool runDiffHook = settings.runDiffHook; - bool keepPreviousRound = settings.keepFailed || runDiffHook; + bool keepPreviousRound = settings.keepFailed || settings.runDiffHook; std::exception_ptr delayedException; @@ -3185,11 +3197,14 @@ void DerivationGoal::registerOutputs() if (!worker.store.isValidPath(path)) continue; auto info = *worker.store.queryPathInfo(path); if (hash.first != info.narHash) { + handleDiffHook(path, actualPath, drvPath); + if (settings.keepFailed) { Path dst = worker.store.toRealPath(path + checkSuffix); deletePath(dst); if (rename(actualPath.c_str(), dst.c_str())) throw SysError(format("renaming '%1%' to '%2%'") % actualPath % dst); + throw Error(format("derivation '%1%' may not be deterministic: output '%2%' differs from '%3%'") % drvPath % path % dst); } else @@ -3254,16 +3269,7 @@ void DerivationGoal::registerOutputs() ? fmt("output '%1%' of '%2%' differs from '%3%' from previous round", i->second.path, drvPath, prev) : fmt("output '%1%' of '%2%' differs from previous round", i->second.path, drvPath); - auto diffHook = settings.diffHook; - if (prevExists && diffHook != "" && runDiffHook) { - try { - auto diff = runProgram(diffHook, true, {prev, i->second.path}); - if (diff != "") - printError(chomp(diff)); - } catch (Error & error) { - printError("diff hook execution failed: %s", error.what()); - } - } + handleDiffHook(prev, i->second.path, drvPath); if (settings.enforceDeterminism) throw NotDeterministic(msg); From 6df61db0600ca73ccd51e3e5bec5312a04e99da1 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Fri, 10 May 2019 20:59:39 -0400 Subject: [PATCH 04/13] diff hook: execute as the build user, and pass the temp dir --- doc/manual/advanced-topics/diff-hook.xml | 12 +++---- doc/manual/command-ref/conf-file.xml | 20 ++++++++---- src/libstore/build.cc | 41 +++++++++++++++++------- src/libutil/util.cc | 4 +-- src/libutil/util.hh | 2 ++ 5 files changed, 51 insertions(+), 28 deletions(-) diff --git a/doc/manual/advanced-topics/diff-hook.xml b/doc/manual/advanced-topics/diff-hook.xml index d2613f6df..fb4bf819f 100644 --- a/doc/manual/advanced-topics/diff-hook.xml +++ b/doc/manual/advanced-topics/diff-hook.xml @@ -46,17 +46,15 @@ file containing: #!/bin/sh exec >&2 echo "For derivation $3:" -/run/current-system/sw/bin/runuser -u nobody -- /run/current-system/sw/bin/diff -r "$1" "$2" +/run/current-system/sw/bin/diff -r "$1" "$2" - - The diff hook can be run as root. Take care to run as little - as possible as root, for this example we use runuser - to drop privileges. - - +The diff hook is executed by the same user and group who ran the +build. However, the diff hook does not have write access to the store +path just built. +
Spot-Checking Build Determinism diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml index a1a5d6e12..c5f90481b 100644 --- a/doc/manual/command-ref/conf-file.xml +++ b/doc/manual/command-ref/conf-file.xml @@ -252,13 +252,11 @@ false</literal>.</para> same. </para> - <warning> - <para> - The root user executes the diff hook in a daemonised - installation. See <xref linkend="chap-diff-hook" /> for - information on using the diff hook safely. - </para> - </warning> + <para> + The diff hook is executed by the same user and group who ran the + build. However, the diff hook does not have write access to the + store path just built. + </para> <para>The diff hook program receives three parameters:</para> @@ -280,6 +278,14 @@ false</literal>.</para> The path to the build's derivation </para> </listitem> + + <listitem> + <para> + The path to the build's scratch directory. This directory + will exist only if the build was run with + <option>--keep-failed</option>. + </para> + </listitem> </orderedlist> <para>The diff hook should not print data to stderr or stdout, as diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 026828c53..f38d2eaa0 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -461,17 +461,26 @@ static void commonChildInit(Pipe & logPipe) close(fdDevNull); } -void handleDiffHook(Path tryA, Path tryB, Path drvPath) +void handleDiffHook(bool allowVfork, uid_t uid, uid_t gid, Path tryA, Path tryB, Path drvPath, Path tmpDir) { auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { - try { - auto diff = runProgram(diffHook, true, {tryA, tryB, drvPath}); - if (diff != "") - printError(chomp(diff)); - } catch (Error & error) { - printError("diff hook execution failed: %s", error.what()); - } + auto wrapper = [&]() { + if (setgid(gid) == -1) + throw SysError("setgid failed"); + if (setuid(uid) == -1) + throw SysError("setuid failed"); + + try { + auto diff = runProgram(diffHook, true, {tryA, tryB, drvPath, tmpDir}); + if (diff != "") + printError(chomp(diff)); + } catch (Error & error) { + printError("diff hook execution failed: %s", error.what()); + } + }; + + doFork(allowVfork, wrapper); } } @@ -3197,14 +3206,18 @@ void DerivationGoal::registerOutputs() if (!worker.store.isValidPath(path)) continue; auto info = *worker.store.queryPathInfo(path); if (hash.first != info.narHash) { - handleDiffHook(path, actualPath, drvPath); - - if (settings.keepFailed) { + if (settings.runDiffHook || settings.keepFailed) { Path dst = worker.store.toRealPath(path + checkSuffix); deletePath(dst); if (rename(actualPath.c_str(), dst.c_str())) throw SysError(format("renaming '%1%' to '%2%'") % actualPath % dst); + handleDiffHook( + !buildUser && !drv->isBuiltin(), + buildUser ? buildUser->getUID() : getuid(), + buildUser ? buildUser->getGID() : getgid(), + path, dst, drvPath, tmpDir); + throw Error(format("derivation '%1%' may not be deterministic: output '%2%' differs from '%3%'") % drvPath % path % dst); } else @@ -3269,7 +3282,11 @@ void DerivationGoal::registerOutputs() ? fmt("output '%1%' of '%2%' differs from '%3%' from previous round", i->second.path, drvPath, prev) : fmt("output '%1%' of '%2%' differs from previous round", i->second.path, drvPath); - handleDiffHook(prev, i->second.path, drvPath); + handleDiffHook( + !buildUser && !drv->isBuiltin(), + buildUser ? buildUser->getUID() : getuid(), + buildUser ? buildUser->getGID() : getgid(), + prev, i->second.path, drvPath, tmpDir); if (settings.enforceDeterminism) throw NotDeterministic(msg); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index a71705665..0f4d3d92b 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -914,8 +914,8 @@ void killUser(uid_t uid) /* Wrapper around vfork to prevent the child process from clobbering the caller's stack frame in the parent. */ -static pid_t doFork(bool allowVfork, std::function<void()> fun) __attribute__((noinline)); -static pid_t doFork(bool allowVfork, std::function<void()> fun) +pid_t doFork(bool allowVfork, std::function<void()> fun) __attribute__((noinline)); +pid_t doFork(bool allowVfork, std::function<void()> fun) { #ifdef __linux__ pid_t pid = allowVfork ? vfork() : fork(); diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 54936a5cb..824a35b98 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -265,6 +265,8 @@ string runProgram(Path program, bool searchPath = false, const Strings & args = Strings(), const std::optional<std::string> & input = {}); +pid_t doFork(bool allowVfork, std::function<void()> fun); + struct RunOptions { Path program; From dde8eeb39ae9fb73011462c74e5fa6405e432147 Mon Sep 17 00:00:00 2001 From: Graham Christensen <graham@grahamc.com> Date: Sat, 11 May 2019 15:57:38 -0400 Subject: [PATCH 05/13] chdir, setgroups --- src/libstore/build.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index f38d2eaa0..8397cd0d1 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -466,8 +466,12 @@ void handleDiffHook(bool allowVfork, uid_t uid, uid_t gid, Path tryA, Path tryB, auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { auto wrapper = [&]() { + if (chdir("/") == -1) + throw SysError("chdir / failed"); if (setgid(gid) == -1) throw SysError("setgid failed"); + if (setgroups(0, 0) == -1) + throw SysError("setgroups failed"); if (setuid(uid) == -1) throw SysError("setuid failed"); From b4a05edbfe49f87555fd284dfb0d6c56ed43217d Mon Sep 17 00:00:00 2001 From: Graham Christensen <graham@grahamc.com> Date: Sat, 11 May 2019 16:35:53 -0400 Subject: [PATCH 06/13] runProgram: support gid, uid, chdir --- src/libstore/build.cc | 32 ++++++++++++++------------------ src/libutil/util.cc | 15 +++++++++++++-- src/libutil/util.hh | 5 +++-- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 8397cd0d1..8902e22bd 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -465,26 +465,22 @@ void handleDiffHook(bool allowVfork, uid_t uid, uid_t gid, Path tryA, Path tryB, { auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { - auto wrapper = [&]() { - if (chdir("/") == -1) - throw SysError("chdir / failed"); - if (setgid(gid) == -1) - throw SysError("setgid failed"); - if (setgroups(0, 0) == -1) - throw SysError("setgroups failed"); - if (setuid(uid) == -1) - throw SysError("setuid failed"); + try { + RunOptions diffHookOptions(diffHook,{tryA, tryB, drvPath, tmpDir}); + diffHookOptions.searchPath = true; + diffHookOptions.uid = uid; + diffHookOptions.gid = gid; + diffHookOptions.chdir = "/"; - try { - auto diff = runProgram(diffHook, true, {tryA, tryB, drvPath, tmpDir}); - if (diff != "") - printError(chomp(diff)); - } catch (Error & error) { - printError("diff hook execution failed: %s", error.what()); - } - }; + auto diffRes = runProgram(diffHookOptions); + if (!statusOk(diffRes.first)) + throw ExecError(diffRes.first, fmt("diff-hook program '%1%' %2%", diffHook, statusToString(diffRes.first))); - doFork(allowVfork, wrapper); + if (diffRes.second != "") + printError(chomp(diffRes.second)); + } catch (Error & error) { + printError("diff hook execution failed: %s", error.what()); + } } } diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 0f4d3d92b..55b9144f3 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -16,6 +16,7 @@ #include <future> #include <fcntl.h> +#include <grp.h> #include <limits.h> #include <pwd.h> #include <sys/ioctl.h> @@ -914,8 +915,8 @@ void killUser(uid_t uid) /* Wrapper around vfork to prevent the child process from clobbering the caller's stack frame in the parent. */ -pid_t doFork(bool allowVfork, std::function<void()> fun) __attribute__((noinline)); -pid_t doFork(bool allowVfork, std::function<void()> fun) +static pid_t doFork(bool allowVfork, std::function<void()> fun) __attribute__((noinline)); +static pid_t doFork(bool allowVfork, std::function<void()> fun) { #ifdef __linux__ pid_t pid = allowVfork ? vfork() : fork(); @@ -1025,6 +1026,16 @@ void runProgram2(const RunOptions & options) if (source && dup2(in.readSide.get(), STDIN_FILENO) == -1) throw SysError("dupping stdin"); + //if (options.chdir && chdir((*options.chdir).c_str()) == -1) + // throw SysError("chdir failed"); + if (options.gid && setgid(*options.gid) == -1) + throw SysError("setgid failed"); + /* Drop all other groups if we're setgid. */ + if (options.gid && setgroups(0, 0) == -1) + throw SysError("setgroups failed"); + if (options.uid && setuid(*options.uid) == -1) + throw SysError("setuid failed"); + Strings args_(options.args); args_.push_front(options.program); diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 824a35b98..7c57d0afa 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -265,10 +265,11 @@ string runProgram(Path program, bool searchPath = false, const Strings & args = Strings(), const std::optional<std::string> & input = {}); -pid_t doFork(bool allowVfork, std::function<void()> fun); - struct RunOptions { + std::optional<uid_t> uid; + std::optional<uid_t> gid; + std::optional<Path> chdir; Path program; bool searchPath = true; Strings args; From a5efe617862484ab7dd234a495d315e7b08aa519 Mon Sep 17 00:00:00 2001 From: Graham Christensen <graham@grahamc.com> Date: Sun, 12 May 2019 13:23:30 -0400 Subject: [PATCH 07/13] Clarify where output from the diff hook goes. --- doc/manual/command-ref/conf-file.xml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml index c5f90481b..24fbf28cf 100644 --- a/doc/manual/command-ref/conf-file.xml +++ b/doc/manual/command-ref/conf-file.xml @@ -288,10 +288,11 @@ false</literal>.</para> </listitem> </orderedlist> - <para>The diff hook should not print data to stderr or stdout, as - output is not displayed to the user. However, if information is - printed, it will be printed in the <command>nix-daemon</command> - log.</para> + <para> + The stderr and stdout output from the diff hook will not be + displayed to the user. Instead, it will print to the nix-daemon's + log. + </para> <para>When using the Nix daemon, <literal>diff-hook</literal> must be set in the <filename>nix.conf</filename> configuration file, and From 73b797c207e1c7a0fd9059d2cf1e3479502f8f1b Mon Sep 17 00:00:00 2001 From: Graham Christensen <graham@grahamc.com> Date: Sun, 12 May 2019 13:44:22 -0400 Subject: [PATCH 08/13] handleDiffHook: stop passing allowVfork --- src/libstore/build.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 8902e22bd..b07461013 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -461,7 +461,7 @@ static void commonChildInit(Pipe & logPipe) close(fdDevNull); } -void handleDiffHook(bool allowVfork, uid_t uid, uid_t gid, Path tryA, Path tryB, Path drvPath, Path tmpDir) +void handleDiffHook(uid_t uid, uid_t gid, Path tryA, Path tryB, Path drvPath, Path tmpDir) { auto diffHook = settings.diffHook; if (diffHook != "" && settings.runDiffHook) { @@ -3213,7 +3213,6 @@ void DerivationGoal::registerOutputs() throw SysError(format("renaming '%1%' to '%2%'") % actualPath % dst); handleDiffHook( - !buildUser && !drv->isBuiltin(), buildUser ? buildUser->getUID() : getuid(), buildUser ? buildUser->getGID() : getgid(), path, dst, drvPath, tmpDir); @@ -3283,7 +3282,6 @@ void DerivationGoal::registerOutputs() : fmt("output '%1%' of '%2%' differs from previous round", i->second.path, drvPath); handleDiffHook( - !buildUser && !drv->isBuiltin(), buildUser ? buildUser->getUID() : getuid(), buildUser ? buildUser->getGID() : getgid(), prev, i->second.path, drvPath, tmpDir); From ce02fc74b2db35e45906865c8a3ce2e98871eeb8 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan <daiderd@gmail.com> Date: Sun, 12 May 2019 22:47:41 +0200 Subject: [PATCH 09/13] build: make needsHashRewrite a method --- src/libstore/build.cc | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 30825add4..79dcdddbe 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -803,9 +803,6 @@ private: /* Whether we're currently doing a chroot build. */ bool useChroot = false; - /* Whether we need to perform hash rewriting if there are valid output paths. */ - bool needsHashRewrite; - Path chrootRootDir; /* RAII object to delete the chroot directory. */ @@ -885,6 +882,9 @@ public: Worker & worker, BuildMode buildMode = bmNormal); ~DerivationGoal(); + /* Whether we need to perform hash rewriting if there are valid output paths. */ + bool needsHashRewrite(); + void timedOut() override; string key() override @@ -1037,6 +1037,17 @@ DerivationGoal::~DerivationGoal() } +inline bool DerivationGoal::needsHashRewrite() +{ +#if __linux__ + return !useChroot; +#else + /* Darwin requires hash rewriting even when sandboxing is enabled. */ + return true; +#endif +} + + void DerivationGoal::killChild() { if (pid != -1) { @@ -1845,13 +1856,6 @@ void DerivationGoal::startBuilder() #endif } -#if __linux__ - needsHashRewrite = !useChroot; -#else - /* Darwin requires hash rewriting even when sandboxing is enabled. */ - needsHashRewrite = true; -#endif - /* If `build-users-group' is not empty, then we have to build as one of the members of that group. */ if (settings.buildUsersGroup != "" && getuid() == 0) { @@ -2083,7 +2087,7 @@ void DerivationGoal::startBuilder() #endif } - if (needsHashRewrite) { + if (needsHashRewrite()) { if (pathExists(homeDir)) throw Error(format("directory '%1%' exists; please remove it") % homeDir); @@ -3067,7 +3071,7 @@ void DerivationGoal::registerOutputs() if (buildMode != bmCheck) actualPath = worker.store.toRealPath(path); } - if (needsHashRewrite) { + if (needsHashRewrite()) { Path redirected = redirectedOutputs[path]; if (buildMode == bmRepair && redirectedBadOutputs.find(path) != redirectedBadOutputs.end() From f1b8e9efe77014655f059b44afa05c38990dc4aa Mon Sep 17 00:00:00 2001 From: Graham Christensen <graham@grahamc.com> Date: Sun, 12 May 2019 17:03:01 -0400 Subject: [PATCH 10/13] runProgram: Uncomment chdir support --- src/libutil/util.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 55b9144f3..17aee2d5c 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1026,8 +1026,8 @@ void runProgram2(const RunOptions & options) if (source && dup2(in.readSide.get(), STDIN_FILENO) == -1) throw SysError("dupping stdin"); - //if (options.chdir && chdir((*options.chdir).c_str()) == -1) - // throw SysError("chdir failed"); + if (options.chdir && chdir((*options.chdir).c_str()) == -1) + throw SysError("chdir failed"); if (options.gid && setgid(*options.gid) == -1) throw SysError("setgid failed"); /* Drop all other groups if we're setgid. */ From 3fd5425f948afa5122ff6b0aad60a6b961b57161 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra <edolstra@gmail.com> Date: Wed, 15 May 2019 13:13:14 +0200 Subject: [PATCH 11/13] Fix shellcheck error https://hydra.nixos.org/build/93359951 --- 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 4dd249923..fc999d336 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -26,7 +26,7 @@ fi if [ "$(uname -s)" = "Darwin" ]; then macos_major=$(sw_vers -productVersion | cut -d '.' -f 2) macos_minor=$(sw_vers -productVersion | cut -d '.' -f 3) - if [ "$macos_major" -lt 12 ] || ([ "$macos_major" -eq 12 ] && [ "$macos_minor" -lt 6 ]); then + if [ "$macos_major" -lt 12 ] || { [ "$macos_major" -eq 12 ] && [ "$macos_minor" -lt 6 ]; }; then echo "$0: macOS $(sw_vers -productVersion) is not supported, upgrade to 10.12.6 or higher" exit 1 fi From b6eb8a2d7e2ea8b083fdac15f537679ffe633183 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra <edolstra@gmail.com> Date: Wed, 15 May 2019 14:30:09 +0200 Subject: [PATCH 12/13] nix-profile: Add all channels to $NIX_PATH Fixes #2709. --- scripts/nix-profile.sh.in | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/scripts/nix-profile.sh.in b/scripts/nix-profile.sh.in index db03e16ba..f3cfa157c 100644 --- a/scripts/nix-profile.sh.in +++ b/scripts/nix-profile.sh.in @@ -51,10 +51,9 @@ if [ -n "$HOME" ] && [ -n "$USER" ]; then unset __nix_defexpr fi - # Append ~/.nix-defexpr/channels/nixpkgs to $NIX_PATH so that - # <nixpkgs> paths work when the user has fetched the Nixpkgs - # channel. - export NIX_PATH="${NIX_PATH:+$NIX_PATH:}nixpkgs=$HOME/.nix-defexpr/channels/nixpkgs" + # Append ~/.nix-defexpr/channels to $NIX_PATH so that <nixpkgs> + # paths work when the user has fetched the Nixpkgs channel. + export NIX_PATH=${NIX_PATH:+$NIX_PATH:}$HOME/.nix-defexpr/channels # Set up environment. # This part should be kept in sync with nixpkgs:nixos/modules/programs/environment.nix From 66b8a62101cb1dfe2e368346cf99efd32e9328ae Mon Sep 17 00:00:00 2001 From: Eelco Dolstra <edolstra@gmail.com> Date: Wed, 15 May 2019 17:33:56 +0200 Subject: [PATCH 13/13] nix: Add --print-build-logs flag This causes 'nix' to print build log output to stderr rather than showing the last log line in the progress bar. Log lines are prefixed by the name of the derivation (minus the version string), e.g. binutils> make[1]: Leaving directory '/build/binutils-2.31.1' binutils-wrapper> unpacking sources binutils-wrapper> patching sources ... binutils-wrapper> Using dynamic linker: '/nix/store/kr51dlsj9v5cr4n8700jliyz8v5b2q7q-bootstrap-stage0-glibc/lib/ld-linux-x86-64.so.2' bootstrap-stage2-gcc-wrapper> unpacking sources ... linux-headers> unpacking sources linux-headers> unpacking source archive /nix/store/8javli69jhj3bkql2c35gsj5vl91p382-linux-4.19.16.tar.xz --- src/libutil/util.hh | 1 + src/nix/main.cc | 10 ++++++++-- src/nix/progress-bar.cc | 39 +++++++++++++++++++++++++++++---------- src/nix/progress-bar.hh | 2 +- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 9f239bff3..23360de2e 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -401,6 +401,7 @@ void ignoreException(); /* Some ANSI escape sequences. */ #define ANSI_NORMAL "\e[0m" #define ANSI_BOLD "\e[1m" +#define ANSI_FAINT "\e[2m" #define ANSI_RED "\e[31;1m" #define ANSI_GREEN "\e[32;1m" #define ANSI_BLUE "\e[34;1m" diff --git a/src/nix/main.cc b/src/nix/main.cc index 64c1dc357..4f87ad72b 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -20,6 +20,8 @@ std::string programPath; struct NixArgs : virtual MultiCommand, virtual MixCommonArgs { + bool printBuildLogs = false; + NixArgs() : MultiCommand(*RegisterCommand::commands), MixCommonArgs("nix") { mkFlag() @@ -41,6 +43,11 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs throw Exit(); }); + mkFlag() + .longName("print-build-logs") + .description("print full build logs on stderr") + .set(&printBuildLogs, true); + mkFlag() .longName("version") .description("show version information") @@ -98,8 +105,7 @@ void mainWrapped(int argc, char * * argv) Finally f([]() { stopProgressBar(); }); - if (isatty(STDERR_FILENO)) - startProgressBar(); + startProgressBar(args.printBuildLogs); args.command->prepare(); args.command->run(); diff --git a/src/nix/progress-bar.cc b/src/nix/progress-bar.cc index 40b905ba3..304f918cc 100644 --- a/src/nix/progress-bar.cc +++ b/src/nix/progress-bar.cc @@ -2,6 +2,7 @@ #include "util.hh" #include "sync.hh" #include "store-api.hh" +#include "names.hh" #include <atomic> #include <map> @@ -38,6 +39,7 @@ private: std::map<ActivityType, uint64_t> expectedByType; bool visible = true; ActivityId parent; + std::optional<std::string> name; }; struct ActivitiesByType @@ -68,10 +70,16 @@ private: std::condition_variable quitCV, updateCV; + bool printBuildLogs; + bool isTTY; + public: - ProgressBar() + ProgressBar(bool printBuildLogs, bool isTTY) + : printBuildLogs(printBuildLogs) + , isTTY(isTTY) { + state_.lock()->active = isTTY; updateThread = std::thread([&]() { auto state(state_.lock()); while (state->active) { @@ -109,8 +117,14 @@ public: void log(State & state, Verbosity lvl, const std::string & s) { - writeToStderr("\r\e[K" + s + ANSI_NORMAL "\n"); - draw(state); + if (state.active) { + writeToStderr("\r\e[K" + s + ANSI_NORMAL "\n"); + draw(state); + } else { + auto s2 = s + ANSI_NORMAL "\n"; + if (!isTTY) s2 = filterANSIEscapes(s2, true); + writeToStderr(s2); + } } void startActivity(ActivityId act, Verbosity lvl, ActivityType type, @@ -141,6 +155,7 @@ public: auto nrRounds = getI(fields, 3); if (nrRounds != 1) i->s += fmt(" (round %d/%d)", curRound, nrRounds); + i->name = DrvName(name).name; } if (type == actSubstitute) { @@ -217,11 +232,15 @@ public: auto i = state->its.find(act); assert(i != state->its.end()); ActInfo info = *i->second; - state->activities.erase(i->second); - info.lastLine = lastLine; - state->activities.emplace_back(info); - i->second = std::prev(state->activities.end()); - update(); + if (printBuildLogs) { + log(*state, lvlInfo, ANSI_FAINT + info.name.value_or("unnamed") + "> " + ANSI_NORMAL + lastLine); + } else { + state->activities.erase(i->second); + info.lastLine = lastLine; + state->activities.emplace_back(info); + i->second = std::prev(state->activities.end()); + update(); + } } } @@ -395,9 +414,9 @@ public: } }; -void startProgressBar() +void startProgressBar(bool printBuildLogs) { - logger = new ProgressBar(); + logger = new ProgressBar(printBuildLogs, isatty(STDERR_FILENO)); } void stopProgressBar() diff --git a/src/nix/progress-bar.hh b/src/nix/progress-bar.hh index af8eda5a8..4d61175c2 100644 --- a/src/nix/progress-bar.hh +++ b/src/nix/progress-bar.hh @@ -4,7 +4,7 @@ namespace nix { -void startProgressBar(); +void startProgressBar(bool printBuildLogs = false); void stopProgressBar();