From 7f5cf87d563a6f8b756c4ef985a6cc5b3b9dbcf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Fri, 18 Feb 2022 13:19:57 +0100 Subject: [PATCH 01/83] Accept and discard fragments in getFlakeRefForCompletion Otherwise trying to complete `nix build foo#bar --update-input ` fails with "unexpected fragment" --- src/libcmd/command.hh | 2 +- src/libcmd/installables.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index 0f6125f11..f1ef9e69c 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -134,7 +134,7 @@ struct InstallableCommand : virtual Args, SourceExprCommand std::optional getFlakeRefForCompletion() override { - return parseFlakeRef(_installable, absPath(".")); + return parseFlakeRefWithFragment(_installable, absPath(".")).first; } private: diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 3209456bf..1af0675c6 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -905,10 +905,10 @@ std::optional InstallablesCommand::getFlakeRefForCompletion() { if (_installables.empty()) { if (useDefaultInstallables()) - return parseFlakeRef(".", absPath(".")); + return parseFlakeRefWithFragment(".", absPath(".")).first; return {}; } - return parseFlakeRef(_installables.front(), absPath(".")); + return parseFlakeRefWithFragment(_installables.front(), absPath(".")).first; } InstallableCommand::InstallableCommand() From 7ddcb3920629bfaa0caf17b54eb63f3f951ba485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Sat, 19 Feb 2022 18:36:02 +0100 Subject: [PATCH 02/83] Add shell completion for --override-input --- src/libcmd/installables.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 1af0675c6..13e85b1bb 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -98,6 +98,14 @@ MixFlakeOptions::MixFlakeOptions() lockFlags.inputOverrides.insert_or_assign( flake::parseInputPath(inputPath), parseFlakeRef(flakeRef, absPath("."), true)); + }}, + .completer = {[&](size_t n, std::string_view prefix) { + if (n == 0) { + if (auto flakeRef = getFlakeRefForCompletion()) + completeFlakeInputPath(getEvalState(), *flakeRef, prefix); + } else if (n == 1) { + completeFlakeRef(getEvalState()->store, prefix); + } }} }); From 5f06a91bf77e45c580202324ba2a5f0338a78b7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Sat, 19 Feb 2022 18:51:18 +0100 Subject: [PATCH 03/83] Fix completion of nested attributes in completeInstallable Without this, completing `nix eval -f file.nix foo.` suggests `bar` instead of `foo.bar`, which messes up the command --- src/libcmd/installables.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 13e85b1bb..c4fefb971 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -187,6 +187,8 @@ Strings SourceExprCommand::getDefaultFlakeAttrPathPrefixes() void SourceExprCommand::completeInstallable(std::string_view prefix) { if (file) { + completionType = ctAttrs; + evalSettings.pureEval = false; auto state = getEvalState(); Expr *e = state->parseExprFromFile( @@ -215,13 +217,14 @@ void SourceExprCommand::completeInstallable(std::string_view prefix) Value v2; state->autoCallFunction(*autoArgs, v1, v2); - completionType = ctAttrs; - if (v2.type() == nAttrs) { for (auto & i : *v2.attrs) { std::string name = i.name; if (name.find(searchWord) == 0) { - completions->add(i.name); + if (prefix_ == "") + completions->add(name); + else + completions->add(prefix_ + "." + name); } } } @@ -249,6 +252,8 @@ void completeFlakeRefWithFragment( if (hash == std::string::npos) { completeFlakeRef(evalState->store, prefix); } else { + completionType = ctAttrs; + auto fragment = prefix.substr(hash + 1); auto flakeRefS = std::string(prefix.substr(0, hash)); // FIXME: do tilde expansion. @@ -264,8 +269,6 @@ void completeFlakeRefWithFragment( flake. */ attrPathPrefixes.push_back(""); - completionType = ctAttrs; - for (auto & attrPathPrefixS : attrPathPrefixes) { auto attrPathPrefix = parseAttrPath(*evalState, attrPathPrefixS); auto attrPathS = attrPathPrefixS + std::string(fragment); From a6d7cd418385e20feab8d7260a7251f218a0d5bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Fri, 18 Feb 2022 13:24:39 +0100 Subject: [PATCH 04/83] Ensure the completion marker is not processed beyond completion I was surprised to see an error mentioning ___COMPLETE___ when trying to complete a flag argument that had no completer implemented --- src/libutil/args.cc | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/libutil/args.cc b/src/libutil/args.cc index f970c0e9e..38c748be0 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -127,11 +127,11 @@ bool Args::processFlag(Strings::iterator & pos, Strings::iterator end) if (flag.handler.arity == ArityAny) break; throw UsageError("flag '%s' requires %d argument(s)", name, flag.handler.arity); } - if (flag.completer) - if (auto prefix = needsCompletion(*pos)) { - anyCompleted = true; + if (auto prefix = needsCompletion(*pos)) { + anyCompleted = true; + if (flag.completer) flag.completer(n, *prefix); - } + } args.push_back(*pos++); } if (!anyCompleted) @@ -146,6 +146,7 @@ bool Args::processFlag(Strings::iterator & pos, Strings::iterator end) && hasPrefix(name, std::string(*prefix, 2))) completions->add("--" + name, flag->description); } + return false; } auto i = longFlags.find(std::string(*pos, 2)); if (i == longFlags.end()) return false; @@ -187,10 +188,12 @@ bool Args::processArgs(const Strings & args, bool finish) { std::vector ss; for (const auto &[n, s] : enumerate(args)) { - ss.push_back(s); - if (exp.completer) - if (auto prefix = needsCompletion(s)) + if (auto prefix = needsCompletion(s)) { + ss.push_back(*prefix); + if (exp.completer) exp.completer(n, *prefix); + } else + ss.push_back(s); } exp.handler.fun(ss); expectedArgs.pop_front(); @@ -322,16 +325,16 @@ MultiCommand::MultiCommand(const Commands & commands_) .optional = true, .handler = {[=](std::string s) { assert(!command); - if (auto prefix = needsCompletion(s)) { - for (auto & [name, command] : commands) - if (hasPrefix(name, *prefix)) - completions->add(name); - } auto i = commands.find(s); if (i == commands.end()) throw UsageError("'%s' is not a recognised command", s); command = {s, i->second()}; command->second->parent = this; + }}, + .completer = {[&](size_t, std::string_view prefix) { + for (auto & [name, command] : commands) + if (hasPrefix(name, prefix)) + completions->add(name); }} }); From 5461ff532d6169be86af703b15cfb49569732cbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Fri, 18 Feb 2022 13:26:40 +0100 Subject: [PATCH 05/83] Make completeDir follow symlinks Allows completing `nix why-depends /run/cur` to /run/current-system --- src/libutil/args.cc | 2 +- src/libutil/util.cc | 9 +++++++++ src/libutil/util.hh | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libutil/args.cc b/src/libutil/args.cc index 38c748be0..b60c609a6 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -290,7 +290,7 @@ static void _completePath(std::string_view prefix, bool onlyDirs) if (glob((std::string(prefix) + "*").c_str(), flags, nullptr, &globbuf) == 0) { for (size_t i = 0; i < globbuf.gl_pathc; ++i) { if (onlyDirs) { - auto st = lstat(globbuf.gl_pathv[i]); + auto st = stat(globbuf.gl_pathv[i]); if (!S_ISDIR(st.st_mode)) continue; } completions->add(globbuf.gl_pathv[i]); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index b833038a9..deaa17a7f 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -215,6 +215,15 @@ bool isDirOrInDir(std::string_view path, std::string_view dir) } +struct stat stat(const Path & path) +{ + struct stat st; + if (stat(path.c_str(), &st)) + throw SysError("getting status of '%1%'", path); + return st; +} + + struct stat lstat(const Path & path) { struct stat st; diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 20591952d..94ae8ab7d 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -77,6 +77,7 @@ bool isInDir(std::string_view path, std::string_view dir); bool isDirOrInDir(std::string_view path, std::string_view dir); /* Get status of `path'. */ +struct stat stat(const Path & path); struct stat lstat(const Path & path); /* Return true iff the given path exists. */ From 55c6906701ee7fc7e915f7889fea86957b020f94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Sat, 19 Feb 2022 14:26:34 +0100 Subject: [PATCH 06/83] Perform tilde expansion when completing flake fragments Allows completing `nix build ~/flake#`. We can implement expansion for `~user` later if needed. Not using wordexp(3) since that expands way too much. --- misc/bash/completion.sh | 2 +- src/libcmd/installables.cc | 4 ++-- src/libutil/args.cc | 7 ++++--- src/libutil/util.cc | 11 +++++++++++ src/libutil/util.hh | 3 +++ 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/misc/bash/completion.sh b/misc/bash/completion.sh index 045053dee..9af695f5a 100644 --- a/misc/bash/completion.sh +++ b/misc/bash/completion.sh @@ -15,7 +15,7 @@ function _complete_nix { else COMPREPLY+=("$completion") fi - done < <(NIX_GET_COMPLETIONS=$cword "${words[@]/#\~/$HOME}" 2>/dev/null) + done < <(NIX_GET_COMPLETIONS=$cword "${words[@]}" 2>/dev/null) __ltrim_colon_completions "$cur" } diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index c4fefb971..a0b8d0af5 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -1,4 +1,5 @@ #include "installables.hh" +#include "util.hh" #include "command.hh" #include "attr-path.hh" #include "common-eval-args.hh" @@ -256,8 +257,7 @@ void completeFlakeRefWithFragment( auto fragment = prefix.substr(hash + 1); auto flakeRefS = std::string(prefix.substr(0, hash)); - // FIXME: do tilde expansion. - auto flakeRef = parseFlakeRef(flakeRefS, absPath(".")); + auto flakeRef = parseFlakeRef(expandTilde(flakeRefS), absPath(".")); auto evalCache = openEvalCache(*evalState, std::make_shared(lockFlake(*evalState, flakeRef, lockFlags))); diff --git a/src/libutil/args.cc b/src/libutil/args.cc index b60c609a6..10f01af5b 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -282,12 +282,13 @@ static void _completePath(std::string_view prefix, bool onlyDirs) { completionType = ctFilenames; glob_t globbuf; - int flags = GLOB_NOESCAPE | GLOB_TILDE; + int flags = GLOB_NOESCAPE; #ifdef GLOB_ONLYDIR if (onlyDirs) flags |= GLOB_ONLYDIR; #endif - if (glob((std::string(prefix) + "*").c_str(), flags, nullptr, &globbuf) == 0) { + // using expandTilde here instead of GLOB_TILDE(_CHECK) so that ~ expands to /home/user/ + if (glob((expandTilde(prefix) + "*").c_str(), flags, nullptr, &globbuf) == 0) { for (size_t i = 0; i < globbuf.gl_pathc; ++i) { if (onlyDirs) { auto st = stat(globbuf.gl_pathv[i]); @@ -295,8 +296,8 @@ static void _completePath(std::string_view prefix, bool onlyDirs) } completions->add(globbuf.gl_pathv[i]); } - globfree(&globbuf); } + globfree(&globbuf); } void completePath(size_t, std::string_view prefix) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index deaa17a7f..d9db24397 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -200,6 +200,17 @@ std::string_view baseNameOf(std::string_view path) } +std::string expandTilde(std::string_view path) +{ + // TODO: expand ~user ? + auto tilde = path.substr(0, 2); + if (tilde == "~/" || tilde == "~") + return getHome() + std::string(path.substr(1)); + else + return std::string(path); +} + + bool isInDir(std::string_view path, std::string_view dir) { return path.substr(0, 1) == "/" diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 94ae8ab7d..a1d0e0e6b 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -68,6 +68,9 @@ Path dirOf(const PathView path); following the final `/' (trailing slashes are removed). */ std::string_view baseNameOf(std::string_view path); +/* Perform tilde expansion on a path. */ +std::string expandTilde(std::string_view path); + /* Check whether 'path' is a descendant of 'dir'. Both paths must be canonicalized. */ bool isInDir(std::string_view path, std::string_view dir); From da7d8daa77c2cce5805947a012060b02be9e4a43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Sat, 19 Feb 2022 16:59:52 +0100 Subject: [PATCH 07/83] Add shell completion for --override-flake Requires moving the MixEvalArgs class from libexpr to libcmd because that's where completeFlakeRef is. --- src/{libexpr => libcmd}/common-eval-args.cc | 4 ++++ src/{libexpr => libcmd}/common-eval-args.hh | 0 2 files changed, 4 insertions(+) rename src/{libexpr => libcmd}/common-eval-args.cc (95%) rename src/{libexpr => libcmd}/common-eval-args.hh (100%) diff --git a/src/libexpr/common-eval-args.cc b/src/libcmd/common-eval-args.cc similarity index 95% rename from src/libexpr/common-eval-args.cc rename to src/libcmd/common-eval-args.cc index e50ff244c..5b6e82388 100644 --- a/src/libexpr/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -7,6 +7,7 @@ #include "registry.hh" #include "flake/flakeref.hh" #include "store-api.hh" +#include "command.hh" namespace nix { @@ -59,6 +60,9 @@ MixEvalArgs::MixEvalArgs() fetchers::Attrs extraAttrs; if (to.subdir != "") extraAttrs["dir"] = to.subdir; fetchers::overrideRegistry(from.input, to.input, extraAttrs); + }}, + .completer = {[&](size_t, std::string_view prefix) { + completeFlakeRef(openStore(), prefix); }} }); diff --git a/src/libexpr/common-eval-args.hh b/src/libcmd/common-eval-args.hh similarity index 100% rename from src/libexpr/common-eval-args.hh rename to src/libcmd/common-eval-args.hh From 2191dab65726012b057402e13132dd7a062d8440 Mon Sep 17 00:00:00 2001 From: Kevin Amado Date: Fri, 11 Mar 2022 08:57:28 -0500 Subject: [PATCH 08/83] nix-fmt: add command --- src/nix/flake.cc | 12 ++++++++++ src/nix/fmt.cc | 53 +++++++++++++++++++++++++++++++++++++++++++++ src/nix/fmt.md | 53 +++++++++++++++++++++++++++++++++++++++++++++ tests/fmt.sh | 28 ++++++++++++++++++++++++ tests/fmt.simple.sh | 1 + tests/local.mk | 1 + 6 files changed, 148 insertions(+) create mode 100644 src/nix/fmt.cc create mode 100644 src/nix/fmt.md create mode 100644 tests/fmt.sh create mode 100755 tests/fmt.simple.sh diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 47a380238..9830ce841 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -527,6 +527,16 @@ struct CmdFlakeCheck : FlakeCommand } } + else if (name == "formatter") { + state->forceAttrs(vOutput, pos); + for (auto & attr : *vOutput.attrs) { + checkSystemName(attr.name, *attr.pos); + checkApp( + fmt("%s.%s", name, attr.name), + *attr.value, *attr.pos); + } + } + else if (name == "packages" || name == "devShells") { state->forceAttrs(vOutput, pos); for (auto & attr : *vOutput.attrs) { @@ -1010,6 +1020,7 @@ struct CmdFlakeShow : FlakeCommand, MixJSON || (attrPath.size() == 1 && ( attrPath[0] == "defaultPackage" || attrPath[0] == "devShell" + || attrPath[0] == "formatter" || attrPath[0] == "nixosConfigurations" || attrPath[0] == "nixosModules" || attrPath[0] == "defaultApp" @@ -1059,6 +1070,7 @@ struct CmdFlakeShow : FlakeCommand, MixJSON else if ( (attrPath.size() == 2 && attrPath[0] == "defaultApp") || + (attrPath.size() == 2 && attrPath[0] == "formatter") || (attrPath.size() == 3 && attrPath[0] == "apps")) { auto aType = visitor.maybeGetAttr("type"); diff --git a/src/nix/fmt.cc b/src/nix/fmt.cc new file mode 100644 index 000000000..e5d44bd38 --- /dev/null +++ b/src/nix/fmt.cc @@ -0,0 +1,53 @@ +#include "command.hh" +#include "run.hh" + +using namespace nix; + +struct CmdFmt : SourceExprCommand { + std::vector args; + + CmdFmt() { expectArgs({.label = "args", .handler = {&args}}); } + + std::string description() override { + return "reformat your code in the standard style"; + } + + std::string doc() override { + return + #include "fmt.md" + ; + } + + Category category() override { return catSecondary; } + + Strings getDefaultFlakeAttrPaths() override { + return Strings{"formatter." + settings.thisSystem.get()}; + } + + Strings getDefaultFlakeAttrPathPrefixes() override { return Strings{}; } + + void run(ref store) { + auto evalState = getEvalState(); + auto evalStore = getEvalStore(); + + auto installable = parseInstallable(store, "."); + auto app = installable->toApp(*evalState).resolve(evalStore, store); + + Strings programArgs{app.program}; + + // Propagate arguments from the CLI + if (args.empty()) { + // Format the current flake out of the box + programArgs.push_back("."); + } else { + // User wants more power, let them decide which paths to include/exclude + for (auto &i : args) { + programArgs.push_back(i); + } + } + + runProgramInStore(store, app.program, programArgs); + }; +}; + +static auto r2 = registerCommand("fmt"); diff --git a/src/nix/fmt.md b/src/nix/fmt.md new file mode 100644 index 000000000..1c78bb36f --- /dev/null +++ b/src/nix/fmt.md @@ -0,0 +1,53 @@ +R""( + +# Examples + +With [nixpkgs-fmt](https://github.com/nix-community/nixpkgs-fmt): + +```nix +# flake.nix +{ + outputs = { nixpkgs, self }: { + formatter.x86_64-linux = nixpkgs.legacyPackages.x86_64-linux.nixpkgs-fmt; + }; +} +``` + +- Format the current flake: `$ nix fmt` + +- Format a specific folder or file: `$ nix fmt ./folder ./file.nix` + +With [nixfmt](https://github.com/serokell/nixfmt): + +```nix +# flake.nix +{ + outputs = { nixpkgs, self }: { + formatter.x86_64-linux = nixpkgs.legacyPackages.x86_64-linux.nixfmt; + }; +} +``` + +- Format specific files: `$ nix fmt ./file1.nix ./file2.nix` + +With [Alejandra](https://github.com/kamadorueda/alejandra): + +```nix +# flake.nix +{ + outputs = { nixpkgs, self }: { + formatter.x86_64-linux = nixpkgs.legacyPackages.x86_64-linux.alejandra; + }; +} +``` + +- Format the current flake: `$ nix fmt` + +- Format a specific folder or file: `$ nix fmt ./folder ./file.nix` + +# Description + +`nix fmt` will rewrite all Nix files (\*.nix) to a canonical format +using the formatter specified in your flake. + +)"" diff --git a/tests/fmt.sh b/tests/fmt.sh new file mode 100644 index 000000000..7df1c82d3 --- /dev/null +++ b/tests/fmt.sh @@ -0,0 +1,28 @@ +source common.sh + +set -o pipefail + +clearStore +rm -rf $TEST_HOME/.cache $TEST_HOME/.config $TEST_HOME/.local + +cp ./simple.nix ./simple.builder.sh ./fmt.simple.sh ./config.nix $TEST_HOME + +cd $TEST_HOME + +nix fmt --help | grep "Format" + +cat << EOF > flake.nix +{ + outputs = _: { + formatter.$system = { + type = "app"; + program = ./fmt.simple.sh; + }; + }; +} +EOF +nix fmt ./file ./folder | grep 'Formatting: ./file ./folder' +nix flake check +nix flake show | grep -P 'x86_64-linux|x86_64-darwin' + +clearStore diff --git a/tests/fmt.simple.sh b/tests/fmt.simple.sh new file mode 100755 index 000000000..4c8c67ebb --- /dev/null +++ b/tests/fmt.simple.sh @@ -0,0 +1 @@ +echo Formatting: "${@}" diff --git a/tests/local.mk b/tests/local.mk index 8032fc38a..b42a5bccb 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -77,6 +77,7 @@ nix_tests = \ post-hook.sh \ function-trace.sh \ flake-local-settings.sh \ + fmt.sh \ eval-store.sh \ why-depends.sh \ import-derivation.sh \ From 247d2cb661218762a8a1fc0ee475a8ca856fcf17 Mon Sep 17 00:00:00 2001 From: Artturin Date: Sat, 26 Mar 2022 00:58:19 +0200 Subject: [PATCH 09/83] scripts/install-systemd-multi-user.sh: fix typo sytemd-tmpfiles -> systemd-tmpfiles --- scripts/install-systemd-multi-user.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-systemd-multi-user.sh b/scripts/install-systemd-multi-user.sh index 24884a023..1d92c5388 100755 --- a/scripts/install-systemd-multi-user.sh +++ b/scripts/install-systemd-multi-user.sh @@ -90,7 +90,7 @@ poly_configure_nix_daemon_service() { ln -sfn /nix/var/nix/profiles/default/$TMPFILES_SRC $TMPFILES_DEST _sudo "to run systemd-tmpfiles once to pick that path up" \ - sytemd-tmpfiles create --prefix=/nix/var/nix + systemd-tmpfiles create --prefix=/nix/var/nix _sudo "to set up the nix-daemon service" \ systemctl link "/nix/var/nix/profiles/default$SERVICE_SRC" From 679b3b32c952eed3c3be6c450c8189948e5645cd Mon Sep 17 00:00:00 2001 From: Erik Arvstedt Date: Sat, 26 Mar 2022 11:32:37 +0100 Subject: [PATCH 10/83] Minor comment fix --- src/libcmd/installables.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 784117569..2dc6c3ff8 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -935,7 +935,7 @@ InstallablesCommand::InstallablesCommand() void InstallablesCommand::prepare() { if (_installables.empty() && useDefaultInstallables()) - // FIXME: commands like "nix install" should not have a + // FIXME: commands like "nix profile install" should not have a // default, probably. _installables.push_back("."); installables = parseInstallables(getStore(), _installables); From 16860a0328094c04c3e877089119a56f9ddfbcd6 Mon Sep 17 00:00:00 2001 From: Erik Arvstedt Date: Sat, 26 Mar 2022 11:32:38 +0100 Subject: [PATCH 11/83] nix eval: Add option `read-only` --- src/libcmd/command.hh | 5 +++-- src/libcmd/installables.cc | 21 +++++++++++++++++++-- src/nix/eval.cc | 2 +- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index 0f6125f11..15bc13c6e 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -85,11 +85,12 @@ struct SourceExprCommand : virtual Args, MixFlakeOptions { std::optional file; std::optional expr; + bool readOnlyMode = false; // FIXME: move this; not all commands (e.g. 'nix run') use it. OperateOn operateOn = OperateOn::Output; - SourceExprCommand(); + SourceExprCommand(bool supportReadOnlyMode = false); std::vector> parseInstallables( ref store, std::vector ss); @@ -128,7 +129,7 @@ struct InstallableCommand : virtual Args, SourceExprCommand { std::shared_ptr installable; - InstallableCommand(); + InstallableCommand(bool supportReadOnlyMode = false); void prepare() override; diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 2dc6c3ff8..e5d69ae14 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -1,3 +1,4 @@ +#include "globals.hh" #include "installables.hh" #include "command.hh" #include "attr-path.hh" @@ -129,7 +130,7 @@ MixFlakeOptions::MixFlakeOptions() }); } -SourceExprCommand::SourceExprCommand() +SourceExprCommand::SourceExprCommand(bool supportReadOnlyMode) { addFlag({ .longName = "file", @@ -157,6 +158,17 @@ SourceExprCommand::SourceExprCommand() .category = installablesCategory, .handler = {&operateOn, OperateOn::Derivation}, }); + + if (supportReadOnlyMode) { + addFlag({ + .longName = "read-only", + .description = + "Do not instantiate each evaluated derivation. " + "This improves performance, but can cause errors when accessing " + "store paths of derivations during evaluation.", + .handler = {&readOnlyMode, true}, + }); + } } Strings SourceExprCommand::getDefaultFlakeAttrPaths() @@ -687,6 +699,10 @@ std::vector> SourceExprCommand::parseInstallables( { std::vector> result; + if (readOnlyMode) { + settings.readOnlyMode = true; + } + if (file || expr) { if (file && expr) throw UsageError("'--file' and '--expr' are exclusive"); @@ -951,7 +967,8 @@ std::optional InstallablesCommand::getFlakeRefForCompletion() return parseFlakeRef(_installables.front(), absPath(".")); } -InstallableCommand::InstallableCommand() +InstallableCommand::InstallableCommand(bool supportReadOnlyMode) + : SourceExprCommand(supportReadOnlyMode) { expectArgs({ .label = "installable", diff --git a/src/nix/eval.cc b/src/nix/eval.cc index 8cd04d5fe..733b93661 100644 --- a/src/nix/eval.cc +++ b/src/nix/eval.cc @@ -16,7 +16,7 @@ struct CmdEval : MixJSON, InstallableCommand std::optional apply; std::optional writeTo; - CmdEval() + CmdEval() : InstallableCommand(true /* supportReadOnlyMode */) { addFlag({ .longName = "raw", From 057f9ee1900312f42efe6c5cebb02b07b4ff2131 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 28 Mar 2022 14:21:35 +0200 Subject: [PATCH 12/83] nix profile install: Don't use queryDerivationOutputMap() Instead get the outputs from Installable::build(). This will also allow 'nix profile install' to support impure derivations. Fixes #6286. --- src/libcmd/installables.cc | 139 ++++++++++++++++++++--------------- src/libcmd/installables.hh | 12 +-- src/libstore/derived-path.hh | 6 ++ src/nix/profile.cc | 35 ++++++--- 4 files changed, 116 insertions(+), 76 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 784117569..955bbe6fb 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -756,55 +756,20 @@ std::shared_ptr SourceExprCommand::parseInstallable( return installables.front(); } -BuiltPaths getBuiltPaths(ref evalStore, ref store, const DerivedPaths & hopefullyBuiltPaths) +BuiltPaths Installable::build( + ref evalStore, + ref store, + Realise mode, + const std::vector> & installables, + BuildMode bMode) { BuiltPaths res; - for (const auto & b : hopefullyBuiltPaths) - std::visit( - overloaded{ - [&](const DerivedPath::Opaque & bo) { - res.push_back(BuiltPath::Opaque{bo.path}); - }, - [&](const DerivedPath::Built & bfd) { - OutputPathMap outputs; - auto drv = evalStore->readDerivation(bfd.drvPath); - auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive - auto drvOutputs = drv.outputsAndOptPaths(*store); - for (auto & output : bfd.outputs) { - if (!outputHashes.count(output)) - throw Error( - "the derivation '%s' doesn't have an output named '%s'", - store->printStorePath(bfd.drvPath), output); - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - auto outputId = - DrvOutput{outputHashes.at(output), output}; - auto realisation = - store->queryRealisation(outputId); - if (!realisation) - throw Error( - "cannot operate on an output of unbuilt " - "content-addressed derivation '%s'", - outputId.to_string()); - outputs.insert_or_assign( - output, realisation->outPath); - } else { - // If ca-derivations isn't enabled, assume that - // the output path is statically known. - assert(drvOutputs.count(output)); - assert(drvOutputs.at(output).second); - outputs.insert_or_assign( - output, *drvOutputs.at(output).second); - } - } - res.push_back(BuiltPath::Built{bfd.drvPath, outputs}); - }, - }, - b.raw()); - + for (auto & [_, builtPath] : build2(evalStore, store, mode, installables, bMode)) + res.push_back(builtPath); return res; } -BuiltPaths Installable::build( +std::vector, BuiltPath>> Installable::build2( ref evalStore, ref store, Realise mode, @@ -815,39 +780,93 @@ BuiltPaths Installable::build( settings.readOnlyMode = true; std::vector pathsToBuild; + std::map>> backmap; for (auto & i : installables) { - auto b = i->toDerivedPaths(); - pathsToBuild.insert(pathsToBuild.end(), b.begin(), b.end()); + for (auto b : i->toDerivedPaths()) { + pathsToBuild.push_back(b); + backmap[b].push_back(i); + } } + std::vector, BuiltPath>> res; + switch (mode) { + case Realise::Nothing: case Realise::Derivation: printMissing(store, pathsToBuild, lvlError); - return getBuiltPaths(evalStore, store, pathsToBuild); + + for (auto & path : pathsToBuild) { + for (auto & installable : backmap[path]) { + std::visit(overloaded { + [&](const DerivedPath::Built & bfd) { + OutputPathMap outputs; + auto drv = evalStore->readDerivation(bfd.drvPath); + auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive + auto drvOutputs = drv.outputsAndOptPaths(*store); + for (auto & output : bfd.outputs) { + if (!outputHashes.count(output)) + throw Error( + "the derivation '%s' doesn't have an output named '%s'", + store->printStorePath(bfd.drvPath), output); + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { + DrvOutput outputId { outputHashes.at(output), output }; + auto realisation = store->queryRealisation(outputId); + if (!realisation) + throw Error( + "cannot operate on an output of unbuilt " + "content-addressed derivation '%s'", + outputId.to_string()); + outputs.insert_or_assign(output, realisation->outPath); + } else { + // If ca-derivations isn't enabled, assume that + // the output path is statically known. + assert(drvOutputs.count(output)); + assert(drvOutputs.at(output).second); + outputs.insert_or_assign( + output, *drvOutputs.at(output).second); + } + } + res.push_back({installable, BuiltPath::Built { bfd.drvPath, outputs }}); + }, + [&](const DerivedPath::Opaque & bo) { + res.push_back({installable, BuiltPath::Opaque { bo.path }}); + }, + }, path.raw()); + } + } + + break; + case Realise::Outputs: { - BuiltPaths res; for (auto & buildResult : store->buildPathsWithResults(pathsToBuild, bMode, evalStore)) { if (!buildResult.success()) buildResult.rethrow(); - std::visit(overloaded { - [&](const DerivedPath::Built & bfd) { - std::map outputs; - for (auto & path : buildResult.builtOutputs) - outputs.emplace(path.first.outputName, path.second.outPath); - res.push_back(BuiltPath::Built { bfd.drvPath, outputs }); - }, - [&](const DerivedPath::Opaque & bo) { - res.push_back(BuiltPath::Opaque { bo.path }); - }, - }, buildResult.path.raw()); + + for (auto & installable : backmap[buildResult.path]) { + std::visit(overloaded { + [&](const DerivedPath::Built & bfd) { + std::map outputs; + for (auto & path : buildResult.builtOutputs) + outputs.emplace(path.first.outputName, path.second.outPath); + res.push_back({installable, BuiltPath::Built { bfd.drvPath, outputs }}); + }, + [&](const DerivedPath::Opaque & bo) { + res.push_back({installable, BuiltPath::Opaque { bo.path }}); + }, + }, buildResult.path.raw()); + } } - return res; + + break; } + default: assert(false); } + + return res; } BuiltPaths Installable::toBuiltPaths( diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index e172b71b0..f4bf0d406 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -98,6 +98,13 @@ struct Installable const std::vector> & installables, BuildMode bMode = bmNormal); + static std::vector, BuiltPath>> build2( + ref evalStore, + ref store, + Realise mode, + const std::vector> & installables, + BuildMode bMode = bmNormal); + static std::set toStorePaths( ref evalStore, ref store, @@ -185,9 +192,4 @@ ref openEvalCache( EvalState & state, std::shared_ptr lockedFlake); -BuiltPaths getBuiltPaths( - ref evalStore, - ref store, - const DerivedPaths & hopefullyBuiltPaths); - } diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh index 8ca0882a4..24a0ae773 100644 --- a/src/libstore/derived-path.hh +++ b/src/libstore/derived-path.hh @@ -25,6 +25,9 @@ struct DerivedPathOpaque { nlohmann::json toJSON(ref store) const; std::string to_string(const Store & store) const; static DerivedPathOpaque parse(const Store & store, std::string_view); + + bool operator < (const DerivedPathOpaque & b) const + { return path < b.path; } }; /** @@ -46,6 +49,9 @@ struct DerivedPathBuilt { std::string to_string(const Store & store) const; static DerivedPathBuilt parse(const Store & store, std::string_view); nlohmann::json toJSON(ref store) const; + + bool operator < (const DerivedPathBuilt & b) const + { return std::make_pair(drvPath, outputs) < std::make_pair(b.drvPath, b.outputs); } }; using _DerivedPathRaw = std::variant< diff --git a/src/nix/profile.cc b/src/nix/profile.cc index da990ddc8..f35947ddb 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -62,22 +62,21 @@ struct ProfileElement return std::tuple(describe(), storePaths) < std::tuple(other.describe(), other.storePaths); } - void updateStorePaths(ref evalStore, ref store, Installable & installable) + void updateStorePaths( + ref evalStore, + ref store, + const BuiltPaths & builtPaths) { // FIXME: respect meta.outputsToInstall storePaths.clear(); - for (auto & buildable : getBuiltPaths(evalStore, store, installable.toDerivedPaths())) { + for (auto & buildable : builtPaths) { std::visit(overloaded { [&](const BuiltPath::Opaque & bo) { storePaths.insert(bo.path); }, [&](const BuiltPath::Built & bfd) { - // TODO: Why are we querying if we know the output - // names already? Is it just to figure out what the - // default one is? - for (auto & output : store->queryDerivationOutputMap(bfd.drvPath)) { + for (auto & output : bfd.outputs) storePaths.insert(output.second); - } }, }, buildable.raw()); } @@ -236,6 +235,16 @@ struct ProfileManifest } }; +static std::map +builtPathsPerInstallable( + const std::vector, BuiltPath>> & builtPaths) +{ + std::map res; + for (auto & [installable, builtPath] : builtPaths) + res[installable.get()].push_back(builtPath); + return res; +} + struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile { std::string description() override @@ -254,7 +263,9 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile { ProfileManifest manifest(*getEvalState(), *profile); - auto builtPaths = Installable::build(getEvalStore(), store, Realise::Outputs, installables, bmNormal); + auto builtPaths = builtPathsPerInstallable( + Installable::build2( + getEvalStore(), store, Realise::Outputs, installables, bmNormal)); for (auto & installable : installables) { ProfileElement element; @@ -269,7 +280,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile }; } - element.updateStorePaths(getEvalStore(), store, *installable); + element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()]); manifest.elements.push_back(std::move(element)); } @@ -457,12 +468,14 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf warn ("Use 'nix profile list' to see the current profile."); } - auto builtPaths = Installable::build(getEvalStore(), store, Realise::Outputs, installables, bmNormal); + auto builtPaths = builtPathsPerInstallable( + Installable::build2( + getEvalStore(), store, Realise::Outputs, installables, bmNormal)); for (size_t i = 0; i < installables.size(); ++i) { auto & installable = installables.at(i); auto & element = manifest.elements[indices.at(i)]; - element.updateStorePaths(getEvalStore(), store, *installable); + element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()]); } updateProfile(manifest.build(store)); From b266fd53dda9c303c55ceb55752c2117011fce69 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 28 Mar 2022 14:58:38 +0200 Subject: [PATCH 13/83] nix {run,shell}: Print a better error message if the store is not local Closes #6317 --- src/nix/run.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/nix/run.cc b/src/nix/run.cc index 033263c36..23e893fbf 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -38,9 +38,12 @@ void runProgramInStore(ref store, unshare(CLONE_NEWUSER) doesn't work in a multithreaded program (which "nix" is), so we exec() a single-threaded helper program (chrootHelper() below) to do the work. */ - auto store2 = store.dynamic_pointer_cast(); + auto store2 = store.dynamic_pointer_cast(); - if (store2 && store->storeDir != store2->getRealStoreDir()) { + if (!store2) + throw Error("store '%s' is not a local store so it does not support command execution", store->getUri()); + + if (store->storeDir != store2->getRealStoreDir()) { Strings helperArgs = { chrootHelperName, store->storeDir, store2->getRealStoreDir(), program }; for (auto & arg : args) helperArgs.push_back(arg); From 390269ed8784b1a73a3310e63eb96a4b62861654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 16 Mar 2022 14:21:09 +0100 Subject: [PATCH 14/83] Simplify the handling of the hash modulo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than having four different but very similar types of hashes, make only one, with a tag indicating whether it corresponds to a regular of deferred derivation. This implies a slight logical change: The original Nix+multiple-outputs model assumed only one hash-modulo per derivation. Adding multiple-outputs CA derivations changed this as these have one hash-modulo per output. This change is now treating each derivation as having one hash modulo per output. This obviously means that we internally loose the guaranty that all the outputs of input-addressed derivations have the same hash modulo. But it turns out that it doesn’t matter because there’s nothing in the code taking advantage of that fact (and it probably shouldn’t anyways). The upside is that it is now much easier to work with these hashes, and we can get rid of a lot of useless `std::visit{ overloaded`. Co-authored-by: John Ericson --- src/libexpr/primops.cc | 48 ++++++++++------------- src/libstore/derivations.cc | 77 +++++++++++-------------------------- src/libstore/derivations.hh | 36 ++++------------- src/libstore/local-store.cc | 9 ++--- src/nix/develop.cc | 4 +- 5 files changed, 56 insertions(+), 118 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f3eb5e925..9f549e52f 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1222,34 +1222,26 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * DerivationOutput::Deferred { }); } - // Regular, non-CA derivation should always return a single hash and not - // hash per output. - auto hashModulo = hashDerivationModulo(*state.store, drv, true); - std::visit(overloaded { - [&](const DrvHash & drvHash) { - auto & h = drvHash.hash; - switch (drvHash.kind) { - case DrvHash::Kind::Deferred: - /* Outputs already deferred, nothing to do */ - break; - case DrvHash::Kind::Regular: - for (auto & [outputName, output] : drv.outputs) { - auto outPath = state.store->makeOutputPath(outputName, h, drvName); - drv.env[outputName] = state.store->printStorePath(outPath); - output = DerivationOutput::InputAddressed { - .path = std::move(outPath), - }; - } - break; - } - }, - [&](const CaOutputHashes &) { - // Shouldn't happen as the toplevel derivation is not CA. - assert(false); - }, - }, - hashModulo.raw()); - + auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true); + switch (hashModulo.kind) { + case DrvHash::Kind::Regular: + for (auto & i : outputs) { + auto h = hashModulo.hashes.at(i); + auto outPath = state.store->makeOutputPath(i, h, drvName); + drv.env[i] = state.store->printStorePath(outPath); + drv.outputs.insert_or_assign( + i, + DerivationOutputInputAddressed { + .path = std::move(outPath), + }); + } + break; + ; + case DrvHash::Kind::Deferred: + for (auto & i : outputs) { + drv.outputs.insert_or_assign(i, DerivationOutputDeferred {}); + } + } } /* Write the resulting term into the Nix store directory. */ diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 7fed80387..85d75523f 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -474,7 +474,7 @@ Sync drvHashes; /* Look up the derivation by value and memoize the `hashDerivationModulo` call. */ -static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath & drvPath) +static const DrvHash pathDerivationModulo(Store & store, const StorePath & drvPath) { { auto hashes = drvHashes.lock(); @@ -509,7 +509,7 @@ static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath & don't leak the provenance of fixed outputs, reducing pointless cache misses as the build itself won't know this. */ -DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) +DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) { auto type = drv.type(); @@ -524,7 +524,10 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m + store.printStorePath(dof.path(store, drv.name, i.first))); outputHashes.insert_or_assign(i.first, std::move(hash)); } - return outputHashes; + return DrvHash{ + .hashes = outputHashes, + .kind = DrvHash::Kind::Regular, + }; } auto kind = std::visit(overloaded { @@ -540,65 +543,36 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m }, }, drv.type().raw()); - /* For other derivations, replace the inputs paths with recursive - calls to this function. */ std::map inputs2; for (auto & [drvPath, inputOutputs0] : drv.inputDrvs) { // Avoid lambda capture restriction with standard / Clang auto & inputOutputs = inputOutputs0; const auto & res = pathDerivationModulo(store, drvPath); - std::visit(overloaded { - // Regular non-CA derivation, replace derivation - [&](const DrvHash & drvHash) { - kind |= drvHash.kind; - inputs2.insert_or_assign(drvHash.hash.to_string(Base16, false), inputOutputs); - }, - // CA derivation's output hashes - [&](const CaOutputHashes & outputHashes) { - std::set justOut = { "out" }; - for (auto & output : inputOutputs) { - /* Put each one in with a single "out" output.. */ - const auto h = outputHashes.at(output); - inputs2.insert_or_assign( - h.to_string(Base16, false), - justOut); - } - }, - }, res.raw()); + if (res.kind == DrvHash::Kind::Deferred) + kind = DrvHash::Kind::Deferred; + for (auto & outputName : inputOutputs) { + const auto h = res.hashes.at(outputName); + inputs2[h.to_string(Base16, false)].insert(outputName); + } } auto hash = hashString(htSHA256, drv.unparse(store, maskOutputs, &inputs2)); - return DrvHash { .hash = hash, .kind = kind }; -} - - -void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept -{ - switch (other) { - case DrvHash::Kind::Regular: - break; - case DrvHash::Kind::Deferred: - self = other; - break; + std::map outputHashes; + for (const auto & [outputName, _] : drv.outputs) { + outputHashes.insert_or_assign(outputName, hash); } + + return DrvHash { + .hashes = outputHashes, + .kind = kind, + }; } std::map staticOutputHashes(Store & store, const Derivation & drv) { - std::map res; - std::visit(overloaded { - [&](const DrvHash & drvHash) { - for (auto & outputName : drv.outputNames()) { - res.insert({outputName, drvHash.hash}); - } - }, - [&](const CaOutputHashes & outputHashes) { - res = outputHashes; - }, - }, hashDerivationModulo(store, drv, true).raw()); - return res; + return hashDerivationModulo(store, drv, true).hashes; } @@ -747,7 +721,7 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); for (auto & [outputName, output] : drv.outputs) { if (std::holds_alternative(output.raw())) { - auto & h = hashModulo.requireNoFixedNonDeferred(); + auto & h = hashModulo.hashes.at(outputName); auto outPath = store.makeOutputPath(outputName, h, drv.name); drv.env[outputName] = store.printStorePath(outPath); output = DerivationOutput::InputAddressed { @@ -758,13 +732,6 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String } -const Hash & DrvHashModulo::requireNoFixedNonDeferred() const { - auto * drvHashOpt = std::get_if(&raw()); - assert(drvHashOpt); - assert(drvHashOpt->kind == DrvHash::Kind::Regular); - return drvHashOpt->hash; -} - static bool tryResolveInput( Store & store, StorePathSet & inputSrcs, StringMap & inputRewrites, const StorePath & inputDrv, const StringSet & inputOutputs) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 8dea90abf..63ea5ef76 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -202,12 +202,14 @@ bool isDerivation(const std::string & fileName); the output name is "out". */ std::string outputPathName(std::string_view drvName, std::string_view outputName); -// known CA drv's output hashes, current just for fixed-output derivations -// whose output hashes are always known since they are fixed up-front. -typedef std::map CaOutputHashes; +// The hashes modulo of a derivation. +// +// Each output is given a hash, although in practice only the content-addressed +// derivations (fixed-output or not) will have a different hash for each +// output. struct DrvHash { - Hash hash; + std::map hashes; enum struct Kind: bool { // Statically determined derivations. @@ -222,28 +224,6 @@ struct DrvHash { void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept; -typedef std::variant< - // Regular normalized derivation hash, and whether it was deferred (because - // an ancestor derivation is a floating content addressed derivation). - DrvHash, - // Fixed-output derivation hashes - CaOutputHashes -> _DrvHashModuloRaw; - -struct DrvHashModulo : _DrvHashModuloRaw { - using Raw = _DrvHashModuloRaw; - using Raw::Raw; - - /* Get hash, throwing if it is per-output CA hashes or a - deferred Drv hash. - */ - const Hash & requireNoFixedNonDeferred() const; - - inline const Raw & raw() const { - return static_cast(*this); - } -}; - /* Returns hashes with the details of fixed-output subderivations expunged. @@ -267,7 +247,7 @@ struct DrvHashModulo : _DrvHashModuloRaw { ATerm, after subderivations have been likewise expunged from that derivation. */ -DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); +DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); /* Return a map associating each output to a hash that uniquely identifies its @@ -276,7 +256,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m std::map staticOutputHashes(Store& store, const Derivation& drv); /* Memoisation of hashDerivationModulo(). */ -typedef std::map DrvHashes; +typedef std::map DrvHashes; // FIXME: global, though at least thread-safe. extern Sync drvHashes; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 46a547db1..60fe53af1 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -695,16 +695,15 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat // combinations that are currently prohibited. drv.type(); - std::optional h; + std::optional hashesModulo; for (auto & i : drv.outputs) { std::visit(overloaded { [&](const DerivationOutput::InputAddressed & doia) { - if (!h) { + if (!hashesModulo) { // somewhat expensive so we do lazily - auto h0 = hashDerivationModulo(*this, drv, true); - h = h0.requireNoFixedNonDeferred(); + hashesModulo = hashDerivationModulo(*this, drv, true); } - StorePath recomputed = makeOutputPath(i.first, *h, drvName); + StorePath recomputed = makeOutputPath(i.first, hashesModulo->hashes.at(i.first), drvName); if (doia.path != recomputed) throw Error("derivation '%s' has incorrect output '%s', should be '%s'", printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed)); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index d2f9b5a6a..7fc74d34e 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -204,10 +204,10 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore output.second = DerivationOutput::Deferred { }; drv.env[output.first] = ""; } - auto h0 = hashDerivationModulo(*evalStore, drv, true); - const Hash & h = h0.requireNoFixedNonDeferred(); + auto hashesModulo = hashDerivationModulo(*evalStore, drv, true); for (auto & output : drv.outputs) { + Hash h = hashesModulo.hashes.at(output.first); auto outPath = store->makeOutputPath(output.first, h, drv.name); output.second = DerivationOutput::InputAddressed { .path = outPath, From 3b26dd51ff0d7b51ec90141bfe2d05b52a4ecfd4 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Tue, 29 Mar 2022 21:05:57 -0400 Subject: [PATCH 15/83] nix-daemon.service: require mounts for /nix/var/nix/db Users may want to mount a filesystem just for the Nix database, with the filesystem's parameters specially tuned for sqlite. For example, on ZFS you might set the recordsize to 64k after changing the database's page size to 65536. --- misc/systemd/nix-daemon.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/misc/systemd/nix-daemon.service.in b/misc/systemd/nix-daemon.service.in index b4badf2ba..24d894898 100644 --- a/misc/systemd/nix-daemon.service.in +++ b/misc/systemd/nix-daemon.service.in @@ -3,6 +3,7 @@ Description=Nix Daemon Documentation=man:nix-daemon https://nixos.org/manual RequiresMountsFor=@storedir@ RequiresMountsFor=@localstatedir@ +RequiresMountsFor=@localstatedir@/nix/db ConditionPathIsReadWrite=@localstatedir@/nix/daemon-socket [Service] From 8dee15cd31f634183c32649e9e62e576e12e5cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 30 Mar 2022 11:42:47 +0200 Subject: [PATCH 16/83] =?UTF-8?q?Don=E2=80=99t=20create=20a=20file=20in=20?= =?UTF-8?q?the=20worktree=20in=20the=20fetchPath=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/fetchPath.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fetchPath.sh b/tests/fetchPath.sh index 8f17638e9..29be38ce2 100644 --- a/tests/fetchPath.sh +++ b/tests/fetchPath.sh @@ -1,6 +1,6 @@ source common.sh -touch foo -t 202211111111 +touch $TEST_ROOT/foo -t 202211111111 # We only check whether 2022-11-1* **:**:** is the last modified date since # `lastModified` is transformed into UTC in `builtins.fetchTarball`. -[[ "$(nix eval --impure --raw --expr "(builtins.fetchTree \"path://$PWD/foo\").lastModifiedDate")" =~ 2022111.* ]] +[[ "$(nix eval --impure --raw --expr "(builtins.fetchTree \"path://$TEST_ROOT/foo\").lastModifiedDate")" =~ 2022111.* ]] From 87f867ef62e7c391955fb1ca3f78bfd532e6666b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 30 Mar 2022 11:43:08 +0200 Subject: [PATCH 17/83] Gitignore the generated systemd nix-daemon conf file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 4b290425a..58e7377fb 100644 --- a/.gitignore +++ b/.gitignore @@ -90,6 +90,7 @@ perl/Makefile.config /misc/systemd/nix-daemon.service /misc/systemd/nix-daemon.socket +/misc/systemd/nix-daemon.conf /misc/upstart/nix-daemon.conf /src/resolve-system-dependencies/resolve-system-dependencies From fa83b865a2cfd21d7e63eedb206c1e07c8178965 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Wed, 30 Mar 2022 15:41:25 +0200 Subject: [PATCH 18/83] libexpr: Throw the correct error in toJSON BaseError::addTrace(...) returns a BaseError, but we want to throw a TypeError instead. Fixes #6336. --- src/libexpr/value-to-json.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index 517da4c01..7b35abca2 100644 --- a/src/libexpr/value-to-json.cc +++ b/src/libexpr/value-to-json.cc @@ -84,7 +84,8 @@ void printValueAsJSON(EvalState & state, bool strict, .msg = hintfmt("cannot convert %1% to JSON", showType(v)), .errPos = v.determinePos(pos) }); - throw e.addTrace(pos, hintfmt("message for the trace")); + e.addTrace(pos, hintfmt("message for the trace")); + throw e; } } From 629edd43ba7550be835660fe5df3b65cc4a515c7 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Wed, 30 Mar 2022 17:30:47 +0200 Subject: [PATCH 19/83] libutil: Change return value of addTrace to void The return value of BaseError::addTrace(...) is never used and error-prone as subclasses calling it will return a BaseError instead of the subclass. This commit changes its return value to be void. --- src/libutil/error.cc | 3 +-- src/libutil/error.hh | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index b2dfb35b2..02bc5caa5 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -9,10 +9,9 @@ namespace nix { const std::string nativeSystem = SYSTEM; -BaseError & BaseError::addTrace(std::optional e, hintformat hint) +void BaseError::addTrace(std::optional e, hintformat hint) { err.traces.push_front(Trace { .pos = e, .hint = hint }); - return *this; } // c++ std::exception descendants must have a 'const char* what()' function. diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 93b789f0b..6a757f9ad 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -175,12 +175,12 @@ public: const ErrorInfo & info() const { calcWhat(); return err; } template - BaseError & addTrace(std::optional e, const std::string & fs, const Args & ... args) + void addTrace(std::optional e, const std::string & fs, const Args & ... args) { - return addTrace(e, hintfmt(fs, args...)); + addTrace(e, hintfmt(fs, args...)); } - BaseError & addTrace(std::optional e, hintformat hint); + void addTrace(std::optional e, hintformat hint); bool hasTrace() const { return !err.traces.empty(); } }; From d77823b502ebc358d33a7719375677fed92291c8 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Wed, 30 Mar 2022 16:10:42 -0400 Subject: [PATCH 20/83] bundler: update default bundler to support new bundler API --- src/nix/bundle.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 7ed558dee..81fb8464a 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -9,7 +9,7 @@ using namespace nix; struct CmdBundle : InstallableCommand { - std::string bundler = "github:matthewbauer/nix-bundle"; + std::string bundler = "github:NixOS/bundlers"; std::optional outLink; CmdBundle() From 50f9f335c92a88e8c9bd3421e6befbcd06cac4ec Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Wed, 30 Mar 2022 16:35:26 -0400 Subject: [PATCH 21/83] profile!: consistent use of url/uri. create new version --- src/nix/profile.cc | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index f35947ddb..b151e48d6 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -97,19 +97,30 @@ struct ProfileManifest auto json = nlohmann::json::parse(readFile(manifestPath)); auto version = json.value("version", 0); - if (version != 1) - throw Error("profile manifest '%s' has unsupported version %d", manifestPath, version); + std::string sUrl; + std::string sOriginalUrl; + switch(version){ + case 1: + sUrl = "uri"; + sOriginalUrl = "originalUri"; + break; + case 2: + sUrl = "url"; + sOriginalUrl = "originalUrl"; + break; + default: + throw Error("profile manifest '%s' has unsupported version %d", manifestPath, version); + } for (auto & e : json["elements"]) { ProfileElement element; for (auto & p : e["storePaths"]) element.storePaths.insert(state.store->parseStorePath((std::string) p)); element.active = e["active"]; - if (e.value("uri", "") != "") { - auto originalUrl = e.value("originalUrl", e["originalUri"]); + if (e.value(sUrl,"") != "") { element.source = ProfileElementSource{ - parseFlakeRef(originalUrl), - parseFlakeRef(e["uri"]), + parseFlakeRef(e[sOriginalUrl]), + parseFlakeRef(e[sUrl]), e["attrPath"] }; } @@ -144,13 +155,13 @@ struct ProfileManifest obj["active"] = element.active; if (element.source) { obj["originalUrl"] = element.source->originalRef.to_string(); - obj["uri"] = element.source->resolvedRef.to_string(); + obj["url"] = element.source->resolvedRef.to_string(); obj["attrPath"] = element.source->attrPath; } array.push_back(obj); } nlohmann::json json; - json["version"] = 1; + json["version"] = 2; json["elements"] = array; return json.dump(); } From 28309352d991f50c9d8b54a5a0ee99995a1a5297 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 10:39:53 +0200 Subject: [PATCH 22/83] replaceEnv(): Pass newEnv by reference --- src/libutil/util.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 70eaf4f9c..59e3aad6d 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -71,13 +71,11 @@ void clearEnv() unsetenv(name.first.c_str()); } -void replaceEnv(std::map newEnv) +void replaceEnv(const std::map & newEnv) { clearEnv(); - for (auto newEnvVar : newEnv) - { + for (auto & newEnvVar : newEnv) setenv(newEnvVar.first.c_str(), newEnvVar.second.c_str(), 1); - } } From 5cd72598feaff3c4bbcc7304a4844768f64a1ee0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 30 Mar 2022 16:31:01 +0200 Subject: [PATCH 23/83] Add support for impure derivations Impure derivations are derivations that can produce a different result every time they're built. Example: stdenv.mkDerivation { name = "impure"; __impure = true; # marks this derivation as impure outputHashAlgo = "sha256"; outputHashMode = "recursive"; buildCommand = "date > $out"; }; Some important characteristics: * This requires the 'impure-derivations' experimental feature. * Impure derivations are not "cached". Thus, running "nix-build" on the example above multiple times will cause a rebuild every time. * They are implemented similar to CA derivations, i.e. the output is moved to a content-addressed path in the store. The difference is that we don't register a realisation in the Nix database. * Pure derivations are not allowed to depend on impure derivations. In the future fixed-output derivations will be allowed to depend on impure derivations, thus forming an "impurity barrier" in the dependency graph. * When sandboxing is enabled, impure derivations can access the network in the same way as fixed-output derivations. In relaxed sandboxing mode, they can access the local filesystem. --- src/libexpr/eval.cc | 1 + src/libexpr/eval.hh | 2 +- src/libexpr/primops.cc | 32 ++- src/libstore/build/derivation-goal.cc | 182 +++++++++------ src/libstore/build/derivation-goal.hh | 7 + src/libstore/build/local-derivation-goal.cc | 21 +- src/libstore/derivations.cc | 235 +++++++++++++++----- src/libstore/derivations.hh | 52 ++++- src/libstore/local-store.cc | 3 + src/libstore/path.cc | 9 + src/libstore/path.hh | 2 + src/libutil/experimental-features.cc | 1 + src/libutil/experimental-features.hh | 1 + src/nix/show-derivation.cc | 4 + tests/impure-derivations.nix | 46 ++++ tests/impure-derivations.sh | 39 ++++ tests/local.mk | 3 +- 17 files changed, 486 insertions(+), 154 deletions(-) create mode 100644 tests/impure-derivations.nix create mode 100644 tests/impure-derivations.sh diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 437c7fc53..b87e06ef5 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -436,6 +436,7 @@ EvalState::EvalState( , sBuilder(symbols.create("builder")) , sArgs(symbols.create("args")) , sContentAddressed(symbols.create("__contentAddressed")) + , sImpure(symbols.create("__impure")) , sOutputHash(symbols.create("outputHash")) , sOutputHashAlgo(symbols.create("outputHashAlgo")) , sOutputHashMode(symbols.create("outputHashMode")) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index e7915dd99..7ed376e8d 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -78,7 +78,7 @@ public: sSystem, sOverrides, sOutputs, sOutputName, sIgnoreNulls, sFile, sLine, sColumn, sFunctor, sToString, sRight, sWrong, sStructuredAttrs, sBuilder, sArgs, - sContentAddressed, + sContentAddressed, sImpure, sOutputHash, sOutputHashAlgo, sOutputHashMode, sRecurseForDerivations, sDescription, sSelf, sEpsilon, sStartSet, sOperator, sKey, sPath, diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 9f549e52f..eaf04320e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -989,6 +989,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * PathSet context; bool contentAddressed = false; + bool isImpure = false; std::optional outputHash; std::string outputHashAlgo; auto ingestionMethod = FileIngestionMethod::Flat; @@ -1051,6 +1052,12 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * settings.requireExperimentalFeature(Xp::CaDerivations); } + else if (i->name == state.sImpure) { + isImpure = state.forceBool(*i->value, pos); + if (isImpure) + settings.requireExperimentalFeature(Xp::ImpureDerivations); + } + /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ else if (i->name == state.sArgs) { @@ -1197,15 +1204,28 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * }); } - else if (contentAddressed) { + else if (contentAddressed || isImpure) { + if (contentAddressed && isImpure) + throw EvalError({ + .msg = hintfmt("derivation cannot be both content-addressed and impure"), + .errPos = posDrvName + }); + HashType ht = parseHashType(outputHashAlgo); for (auto & i : outputs) { drv.env[i] = hashPlaceholder(i); - drv.outputs.insert_or_assign(i, - DerivationOutput::CAFloating { - .method = ingestionMethod, - .hashType = ht, - }); + if (isImpure) + drv.outputs.insert_or_assign(i, + DerivationOutput::Impure { + .method = ingestionMethod, + .hashType = ht, + }); + else + drv.outputs.insert_or_assign(i, + DerivationOutput::CAFloating { + .method = ingestionMethod, + .hashType = ht, + }); } } diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3d1c4fbc1..2f3490829 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -204,9 +204,34 @@ void DerivationGoal::haveDerivation() { trace("have derivation"); + parsedDrv = std::make_unique(drvPath, *drv); + if (!drv->type().hasKnownOutputPaths()) settings.requireExperimentalFeature(Xp::CaDerivations); + if (!drv->type().isPure()) { + settings.requireExperimentalFeature(Xp::ImpureDerivations); + + for (auto & [outputName, output] : drv->outputs) { + auto randomPath = StorePath::random(outputPathName(drv->name, outputName)); + assert(!worker.store.isValidPath(randomPath)); + initialOutputs.insert({ + outputName, + InitialOutput { + .wanted = true, + .outputHash = impureOutputHash, + .known = InitialOutputStatus { + .path = randomPath, + .status = PathStatus::Absent + } + } + }); + } + + gaveUpOnSubstitution(); + return; + } + for (auto & i : drv->outputsAndOptPaths(worker.store)) if (i.second.second) worker.store.addTempRoot(*i.second.second); @@ -230,9 +255,6 @@ void DerivationGoal::haveDerivation() return; } - parsedDrv = std::make_unique(drvPath, *drv); - - /* We are first going to try to create the invalid output paths through substitutes. If that doesn't work, we'll build them. */ @@ -266,6 +288,8 @@ void DerivationGoal::outputsSubstitutionTried() { trace("all outputs substituted (maybe)"); + assert(drv->type().isPure()); + if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) { done(BuildResult::TransientFailure, {}, Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", @@ -315,9 +339,21 @@ void DerivationGoal::outputsSubstitutionTried() void DerivationGoal::gaveUpOnSubstitution() { /* The inputs must be built before we can build this goal. */ + inputDrvOutputs.clear(); if (useDerivation) - for (auto & i : dynamic_cast(drv.get())->inputDrvs) + for (auto & i : dynamic_cast(drv.get())->inputDrvs) { + /* Ensure that pure derivations don't depend on impure + derivations. */ + if (drv->type().isPure()) { + auto inputDrv = worker.evalStore.readDerivation(i.first); + if (!inputDrv.type().isPure()) + throw Error("pure derivation '%s' depends on impure derivation '%s'", + worker.store.printStorePath(drvPath), + worker.store.printStorePath(i.first)); + } + addWaitee(worker.makeDerivationGoal(i.first, i.second, buildMode == bmRepair ? bmRepair : bmNormal)); + } /* Copy the input sources from the eval store to the build store. */ @@ -345,6 +381,8 @@ void DerivationGoal::gaveUpOnSubstitution() void DerivationGoal::repairClosure() { + assert(drv->type().isPure()); + /* If we're repairing, we now know that our own outputs are valid. Now check whether the other paths in the outputs closure are good. If not, then start derivation goals for the derivations @@ -452,22 +490,24 @@ void DerivationGoal::inputsRealised() drvs. */ : true); }, + [&](const DerivationType::Impure &) { + return true; + } }, drvType.raw()); - if (resolveDrv) - { + if (resolveDrv && !fullDrv.inputDrvs.empty()) { settings.requireExperimentalFeature(Xp::CaDerivations); /* We are be able to resolve this derivation based on the - now-known results of dependencies. If so, we become a stub goal - aliasing that resolved derivation goal */ - std::optional attempt = fullDrv.tryResolve(worker.store); + now-known results of dependencies. If so, we become a + stub goal aliasing that resolved derivation goal. */ + std::optional attempt = fullDrv.tryResolve(worker.store, inputDrvOutputs); assert(attempt); Derivation drvResolved { *std::move(attempt) }; auto pathResolved = writeDerivation(worker.store, drvResolved); - auto msg = fmt("Resolved derivation: '%s' -> '%s'", + auto msg = fmt("resolved derivation: '%s' -> '%s'", worker.store.printStorePath(drvPath), worker.store.printStorePath(pathResolved)); act = std::make_unique(*logger, lvlInfo, actBuildWaiting, msg, @@ -488,21 +528,13 @@ void DerivationGoal::inputsRealised() /* Add the relevant output closures of the input derivation `i' as input paths. Only add the closures of output paths that are specified as inputs. */ - assert(worker.evalStore.isValidPath(drvPath)); - auto outputs = worker.evalStore.queryPartialDerivationOutputMap(depDrvPath); - for (auto & j : wantedDepOutputs) { - if (outputs.count(j) > 0) { - auto optRealizedInput = outputs.at(j); - if (!optRealizedInput) - throw Error( - "derivation '%s' requires output '%s' from input derivation '%s', which is supposedly realized already, yet we still don't know what path corresponds to that output", - worker.store.printStorePath(drvPath), j, worker.store.printStorePath(depDrvPath)); - worker.store.computeFSClosure(*optRealizedInput, inputPaths); - } else + for (auto & j : wantedDepOutputs) + if (auto outPath = get(inputDrvOutputs, { depDrvPath, j })) + worker.store.computeFSClosure(*outPath, inputPaths); + else throw Error( "derivation '%s' requires non-existent output '%s' from input derivation '%s'", worker.store.printStorePath(drvPath), j, worker.store.printStorePath(depDrvPath)); - } } } @@ -923,7 +955,7 @@ void DerivationGoal::buildDone() st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - derivationType.isImpure() || diskFull ? BuildResult::TransientFailure : + derivationType.needsNetworkAccess() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; } @@ -934,60 +966,52 @@ void DerivationGoal::buildDone() void DerivationGoal::resolvedFinished() { + trace("resolved derivation finished"); + assert(resolvedDrvGoal); auto resolvedDrv = *resolvedDrvGoal->drv; - - auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); - - StorePathSet outputPaths; - - // `wantedOutputs` might be empty, which means “all the outputs” - auto realWantedOutputs = wantedOutputs; - if (realWantedOutputs.empty()) - realWantedOutputs = resolvedDrv.outputNames(); + auto & resolvedResult = resolvedDrvGoal->buildResult; DrvOutputs builtOutputs; - for (auto & wantedOutput : realWantedOutputs) { - assert(initialOutputs.count(wantedOutput) != 0); - assert(resolvedHashes.count(wantedOutput) != 0); - auto realisation = worker.store.queryRealisation( - DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} - ); - // We've just built it, but maybe the build failed, in which case the - // realisation won't be there - if (realisation) { - auto newRealisation = *realisation; - newRealisation.id = DrvOutput{initialOutputs.at(wantedOutput).outputHash, wantedOutput}; - newRealisation.signatures.clear(); - newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation->outPath); - signRealisation(newRealisation); - worker.store.registerDrvOutput(newRealisation); - outputPaths.insert(realisation->outPath); - builtOutputs.emplace(realisation->id, *realisation); - } else { - // If we don't have a realisation, then it must mean that something - // failed when building the resolved drv - assert(!buildResult.success()); + if (resolvedResult.success()) { + auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); + + StorePathSet outputPaths; + + // `wantedOutputs` might be empty, which means “all the outputs” + auto realWantedOutputs = wantedOutputs; + if (realWantedOutputs.empty()) + realWantedOutputs = resolvedDrv.outputNames(); + + for (auto & wantedOutput : realWantedOutputs) { + assert(initialOutputs.count(wantedOutput) != 0); + assert(resolvedHashes.count(wantedOutput) != 0); + auto realisation = resolvedResult.builtOutputs.at( + DrvOutput { resolvedHashes.at(wantedOutput), wantedOutput }); + if (drv->type().isPure()) { + auto newRealisation = realisation; + newRealisation.id = DrvOutput { initialOutputs.at(wantedOutput).outputHash, wantedOutput }; + newRealisation.signatures.clear(); + newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); + signRealisation(newRealisation); + worker.store.registerDrvOutput(newRealisation); + } + outputPaths.insert(realisation.outPath); + builtOutputs.emplace(realisation.id, realisation); } + + runPostBuildHook( + worker.store, + *logger, + drvPath, + outputPaths + ); } - runPostBuildHook( - worker.store, - *logger, - drvPath, - outputPaths - ); - - auto status = [&]() { - auto & resolvedResult = resolvedDrvGoal->buildResult; - switch (resolvedResult.status) { - case BuildResult::AlreadyValid: - return BuildResult::ResolvesToAlreadyValid; - default: - return resolvedResult.status; - } - }(); + auto status = resolvedResult.status; + if (status == BuildResult::AlreadyValid) + status = BuildResult::ResolvesToAlreadyValid; done(status, std::move(builtOutputs)); } @@ -1236,6 +1260,7 @@ void DerivationGoal::flushLine() std::map> DerivationGoal::queryPartialDerivationOutputMap() { + assert(drv->type().isPure()); if (!useDerivation || drv->type().hasKnownOutputPaths()) { std::map> res; for (auto & [name, output] : drv->outputs) @@ -1248,6 +1273,7 @@ std::map> DerivationGoal::queryPartialDeri OutputPathMap DerivationGoal::queryDerivationOutputMap() { + assert(drv->type().isPure()); if (!useDerivation || drv->type().hasKnownOutputPaths()) { OutputPathMap res; for (auto & [name, output] : drv->outputsAndOptPaths(worker.store)) @@ -1261,6 +1287,8 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap() std::pair DerivationGoal::checkPathValidity() { + if (!drv->type().isPure()) return { false, {} }; + bool checkHash = buildMode == bmRepair; auto wantedOutputsLeft = wantedOutputs; DrvOutputs validOutputs; @@ -1304,6 +1332,7 @@ std::pair DerivationGoal::checkPathValidity() if (info.wanted && info.known && info.known->isValid()) validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path }); } + // If we requested all the outputs via the empty set, we are always fine. // If we requested specific elements, the loop above removes all the valid // ones, so any that are left must be invalid. @@ -1343,7 +1372,6 @@ void DerivationGoal::done( if (ex) // FIXME: strip: "error: " buildResult.errorMsg = ex->what(); - amDone(buildResult.success() ? ecSuccess : ecFailed, ex); if (buildResult.status == BuildResult::TimedOut) worker.timedOut = true; if (buildResult.status == BuildResult::PermanentFailure) @@ -1370,7 +1398,21 @@ void DerivationGoal::done( fs.open(traceBuiltOutputsFile, std::fstream::out); fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl; } + + amDone(buildResult.success() ? ecSuccess : ecFailed, ex); } +void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) +{ + Goal::waiteeDone(waitee, result); + + if (waitee->buildResult.success()) + if (auto bfd = std::get_if(&waitee->buildResult.path)) + for (auto & [output, realisation] : waitee->buildResult.builtOutputs) + inputDrvOutputs.insert_or_assign( + { bfd->drvPath, output.outputName }, + realisation.outPath); +} + } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index f556b6f25..2d8bfd592 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -57,6 +57,11 @@ struct DerivationGoal : public Goal them. */ StringSet wantedOutputs; + /* Mapping from input derivations + output names to actual store + paths. This is filled in by waiteeDone() as each dependency + finishes, before inputsRealised() is reached, */ + std::map, StorePath> inputDrvOutputs; + /* Whether additional wanted outputs have been added. */ bool needRestart = false; @@ -224,6 +229,8 @@ struct DerivationGoal : public Goal DrvOutputs builtOutputs = {}, std::optional ex = {}); + void waiteeDone(GoalPtr waitee, ExitCode result) override; + StorePathSet exportReferences(const StorePathSet & storePaths); }; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index b176f318b..a6c07314f 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -395,7 +395,7 @@ void LocalDerivationGoal::startBuilder() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = !(derivationType.isImpure()) && !noChroot; + useChroot = !derivationType.needsNetworkAccess() && !noChroot; } auto & localStore = getLocalStore(); @@ -608,7 +608,7 @@ void LocalDerivationGoal::startBuilder() "nogroup:x:65534:\n", sandboxGid())); /* Create /etc/hosts with localhost entry. */ - if (!(derivationType.isImpure())) + if (!derivationType.needsNetworkAccess()) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -796,7 +796,7 @@ void LocalDerivationGoal::startBuilder() us. */ - if (!(derivationType.isImpure())) + if (!derivationType.needsNetworkAccess()) privateNetwork = true; userNamespaceSync.create(); @@ -1060,7 +1060,7 @@ void LocalDerivationGoal::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (derivationType.isImpure()) { + if (derivationType.needsNetworkAccess()) { for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) env[i] = getEnv(i).value_or(""); } @@ -1674,7 +1674,7 @@ void LocalDerivationGoal::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (derivationType.isImpure()) { + if (derivationType.needsNetworkAccess()) { // Only use nss functions to resolve hosts and // services. Don’t use it for anything else that may // be configured for this system. This limits the @@ -2399,6 +2399,13 @@ DrvOutputs LocalDerivationGoal::registerOutputs() assert(false); }, + [&](const DerivationOutput::Impure & doi) { + return newInfoFromCA(DerivationOutput::CAFloating { + .method = doi.method, + .hashType = doi.hashType, + }); + }, + }, output.raw()); /* FIXME: set proper permissions in restorePath() so @@ -2609,7 +2616,9 @@ DrvOutputs LocalDerivationGoal::registerOutputs() }, .outPath = newInfo.path }; - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) + && drv->type().isPure()) + { signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 85d75523f..75e0178bb 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -25,26 +25,42 @@ std::optional DerivationOutput::path(const Store & store, std::string [](const DerivationOutput::Deferred &) -> std::optional { return std::nullopt; }, + [](const DerivationOutput::Impure &) -> std::optional { + return std::nullopt; + }, }, raw()); } -StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const { +StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const +{ return store.makeFixedOutputPath( hash.method, hash.hash, outputPathName(drvName, outputName)); } -bool DerivationType::isCA() const { +bool DerivationType::isCA() const +{ /* Normally we do the full `std::visit` to make sure we have exhaustively handled all variants, but so long as there is a variant called `ContentAddressed`, it must be the only one for which `isCA` is true for this to make sense!. */ - return std::holds_alternative(raw()); + return std::visit(overloaded { + [](const InputAddressed & ia) { + return false; + }, + [](const ContentAddressed & ca) { + return true; + }, + [](const Impure &) { + return true; + }, + }, raw()); } -bool DerivationType::isFixed() const { +bool DerivationType::isFixed() const +{ return std::visit(overloaded { [](const InputAddressed & ia) { return false; @@ -52,10 +68,14 @@ bool DerivationType::isFixed() const { [](const ContentAddressed & ca) { return ca.fixed; }, + [](const Impure &) { + return false; + }, }, raw()); } -bool DerivationType::hasKnownOutputPaths() const { +bool DerivationType::hasKnownOutputPaths() const +{ return std::visit(overloaded { [](const InputAddressed & ia) { return !ia.deferred; @@ -63,11 +83,15 @@ bool DerivationType::hasKnownOutputPaths() const { [](const ContentAddressed & ca) { return ca.fixed; }, + [](const Impure &) { + return false; + }, }, raw()); } -bool DerivationType::isImpure() const { +bool DerivationType::needsNetworkAccess() const +{ return std::visit(overloaded { [](const InputAddressed & ia) { return false; @@ -75,6 +99,25 @@ bool DerivationType::isImpure() const { [](const ContentAddressed & ca) { return !ca.pure; }, + [](const Impure &) { + return true; + }, + }, raw()); +} + + +bool DerivationType::isPure() const +{ + return std::visit(overloaded { + [](const InputAddressed & ia) { + return true; + }, + [](const ContentAddressed & ca) { + return true; + }, + [](const Impure &) { + return false; + }, }, raw()); } @@ -176,7 +219,16 @@ static DerivationOutput parseDerivationOutput(const Store & store, hashAlgo = hashAlgo.substr(2); } const auto hashType = parseHashType(hashAlgo); - if (hash != "") { + if (hash == "impure") { + settings.requireExperimentalFeature(Xp::ImpureDerivations); + assert(pathS == ""); + return DerivationOutput { + .output = DerivationOutputImpure { + .method = std::move(method), + .hashType = std::move(hashType), + }, + }; + } else if (hash != "") { validatePath(pathS); return DerivationOutput::CAFixed { .hash = FixedOutputHash { @@ -345,6 +397,12 @@ std::string Derivation::unparse(const Store & store, bool maskOutputs, s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); + }, + [&](const DerivationOutputImpure & doi) { + // FIXME + s += ','; printUnquotedString(s, ""); + s += ','; printUnquotedString(s, makeFileIngestionPrefix(doi.method) + printHashType(doi.hashType)); + s += ','; printUnquotedString(s, "impure"); } }, i.second.raw()); s += ')'; @@ -410,8 +468,14 @@ std::string outputPathName(std::string_view drvName, std::string_view outputName DerivationType BasicDerivation::type() const { - std::set inputAddressedOutputs, fixedCAOutputs, floatingCAOutputs, deferredIAOutputs; + std::set + inputAddressedOutputs, + fixedCAOutputs, + floatingCAOutputs, + deferredIAOutputs, + impureOutputs; std::optional floatingHashType; + for (auto & i : outputs) { std::visit(overloaded { [&](const DerivationOutput::InputAddressed &) { @@ -426,43 +490,78 @@ DerivationType BasicDerivation::type() const floatingHashType = dof.hashType; } else { if (*floatingHashType != dof.hashType) - throw Error("All floating outputs must use the same hash type"); + throw Error("all floating outputs must use the same hash type"); } }, [&](const DerivationOutput::Deferred &) { - deferredIAOutputs.insert(i.first); + deferredIAOutputs.insert(i.first); + }, + [&](const DerivationOutput::Impure &) { + impureOutputs.insert(i.first); }, }, i.second.raw()); } - if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { - throw Error("Must have at least one output"); - } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { + if (inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && impureOutputs.empty()) + throw Error("must have at least one output"); + + if (!inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && impureOutputs.empty()) return DerivationType::InputAddressed { .deferred = false, }; - } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { + + if (inputAddressedOutputs.empty() + && !fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && impureOutputs.empty()) + { if (fixedCAOutputs.size() > 1) // FIXME: Experimental feature? - throw Error("Only one fixed output is allowed for now"); + throw Error("only one fixed output is allowed for now"); if (*fixedCAOutputs.begin() != "out") - throw Error("Single fixed output must be named \"out\""); + throw Error("single fixed output must be named \"out\""); return DerivationType::ContentAddressed { .pure = false, .fixed = true, }; - } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && ! floatingCAOutputs.empty() && deferredIAOutputs.empty()) { + } + + if (inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && !floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && impureOutputs.empty()) return DerivationType::ContentAddressed { .pure = true, .fixed = false, }; - } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && !deferredIAOutputs.empty()) { + + if (inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && !deferredIAOutputs.empty() + && impureOutputs.empty()) return DerivationType::InputAddressed { .deferred = true, }; - } else { - throw Error("Can't mix derivation output types"); - } + + if (inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && !impureOutputs.empty()) + return DerivationType::Impure { }; + + throw Error("can't mix derivation output types"); } @@ -524,12 +623,22 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut + store.printStorePath(dof.path(store, drv.name, i.first))); outputHashes.insert_or_assign(i.first, std::move(hash)); } - return DrvHash{ + return DrvHash { .hashes = outputHashes, .kind = DrvHash::Kind::Regular, }; } + if (!type.isPure()) { + std::map outputHashes; + for (const auto & [outputName, _] : drv.outputs) + outputHashes.insert_or_assign(outputName, impureOutputHash); + return DrvHash { + .hashes = outputHashes, + .kind = DrvHash::Kind::Deferred, + }; + } + auto kind = std::visit(overloaded { [](const DerivationType::InputAddressed & ia) { /* This might be a "pesimistically" deferred output, so we don't @@ -541,6 +650,9 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut ? DrvHash::Kind::Regular : DrvHash::Kind::Deferred; }, + [](const DerivationType::Impure &) -> DrvHash::Kind { + assert(false); + } }, drv.type().raw()); std::map inputs2; @@ -599,7 +711,8 @@ StringSet BasicDerivation::outputNames() const return names; } -DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const { +DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const +{ DerivationOutputsAndOptPaths outsAndOptPaths; for (auto output : outputs) outsAndOptPaths.insert(std::make_pair( @@ -610,7 +723,8 @@ DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & s return outsAndOptPaths; } -std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath) { +std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath) +{ auto nameWithSuffix = drvPath.name(); constexpr std::string_view extension = ".drv"; assert(hasSuffix(nameWithSuffix, extension)); @@ -672,6 +786,11 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr << "" << ""; }, + [&](const DerivationOutput::Impure & doi) { + out << "" + << (makeFileIngestionPrefix(doi.method) + printHashType(doi.hashType)) + << "impure"; + }, }, i.second.raw()); } worker_proto::write(store, out, drv.inputSrcs); @@ -697,21 +816,19 @@ std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath } -static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) { - - debug("Rewriting the derivation"); - - for (auto &rewrite: rewrites) { +static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) +{ + for (auto & rewrite : rewrites) { debug("rewriting %s as %s", rewrite.first, rewrite.second); } drv.builder = rewriteStrings(drv.builder, rewrites); - for (auto & arg: drv.args) { + for (auto & arg : drv.args) { arg = rewriteStrings(arg, rewrites); } StringPairs newEnv; - for (auto & envVar: drv.env) { + for (auto & envVar : drv.env) { auto envName = rewriteStrings(envVar.first, rewrites); auto envValue = rewriteStrings(envVar.second, rewrites); newEnv.emplace(envName, envValue); @@ -732,48 +849,48 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String } -static bool tryResolveInput( - Store & store, StorePathSet & inputSrcs, StringMap & inputRewrites, - const StorePath & inputDrv, const StringSet & inputOutputs) +std::optional Derivation::tryResolve(Store & store) const { - auto inputDrvOutputs = store.queryPartialDerivationOutputMap(inputDrv); + std::map, StorePath> inputDrvOutputs; - auto getOutput = [&](const std::string & outputName) { - auto & actualPathOpt = inputDrvOutputs.at(outputName); - if (!actualPathOpt) - warn("output %s of input %s missing, aborting the resolving", - outputName, - store.printStorePath(inputDrv) - ); - return actualPathOpt; - }; + for (auto & input : inputDrvs) + for (auto & [outputName, outputPath] : store.queryPartialDerivationOutputMap(input.first)) + if (outputPath) + inputDrvOutputs.insert_or_assign({input.first, outputName}, *outputPath); - for (auto & outputName : inputOutputs) { - auto actualPathOpt = getOutput(outputName); - if (!actualPathOpt) return false; - auto actualPath = *actualPathOpt; - inputRewrites.emplace( - downstreamPlaceholder(store, inputDrv, outputName), - store.printStorePath(actualPath)); - inputSrcs.insert(std::move(actualPath)); - } - - return true; + return tryResolve(store, inputDrvOutputs); } -std::optional Derivation::tryResolve(Store & store) { +std::optional Derivation::tryResolve( + Store & store, + const std::map, StorePath> & inputDrvOutputs) const +{ BasicDerivation resolved { *this }; // Input paths that we'll want to rewrite in the derivation StringMap inputRewrites; - for (auto & [inputDrv, inputOutputs] : inputDrvs) - if (!tryResolveInput(store, resolved.inputSrcs, inputRewrites, inputDrv, inputOutputs)) - return std::nullopt; + for (auto & [inputDrv, inputOutputs] : inputDrvs) { + for (auto & outputName : inputOutputs) { + if (auto actualPath = get(inputDrvOutputs, { inputDrv, outputName })) { + inputRewrites.emplace( + downstreamPlaceholder(store, inputDrv, outputName), + store.printStorePath(*actualPath)); + resolved.inputSrcs.insert(*actualPath); + } else { + warn("output '%s' of input '%s' missing, aborting the resolving", + outputName, + store.printStorePath(inputDrv)); + return {}; + } + } + } rewriteDerivation(store, resolved, inputRewrites); return resolved; } +const Hash impureOutputHash = hashString(htSHA256, "impure"); + } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 63ea5ef76..b62e40786 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -41,15 +41,26 @@ struct DerivationOutputCAFloating }; /* Input-addressed output which depends on a (CA) derivation whose hash isn't - * known atm + * known yet. */ struct DerivationOutputDeferred {}; +/* Impure output which is moved to a content-addressed location (like + CAFloating) but isn't registered as a realization. + */ +struct DerivationOutputImpure +{ + /* information used for expected hash computation */ + FileIngestionMethod method; + HashType hashType; +}; + typedef std::variant< DerivationOutputInputAddressed, DerivationOutputCAFixed, DerivationOutputCAFloating, - DerivationOutputDeferred + DerivationOutputDeferred, + DerivationOutputImpure > _DerivationOutputRaw; struct DerivationOutput : _DerivationOutputRaw @@ -61,6 +72,7 @@ struct DerivationOutput : _DerivationOutputRaw using CAFixed = DerivationOutputCAFixed; using CAFloating = DerivationOutputCAFloating; using Deferred = DerivationOutputDeferred; + using Impure = DerivationOutputImpure; /* Note, when you use this function you should make sure that you're passing the right derivation name. When in doubt, you should use the safer @@ -94,9 +106,13 @@ struct DerivationType_ContentAddressed { bool fixed; }; +struct DerivationType_Impure { +}; + typedef std::variant< DerivationType_InputAddressed, - DerivationType_ContentAddressed + DerivationType_ContentAddressed, + DerivationType_Impure > _DerivationTypeRaw; struct DerivationType : _DerivationTypeRaw { @@ -104,7 +120,7 @@ struct DerivationType : _DerivationTypeRaw { using Raw::Raw; using InputAddressed = DerivationType_InputAddressed; using ContentAddressed = DerivationType_ContentAddressed; - + using Impure = DerivationType_Impure; /* Do the outputs of the derivation have paths calculated from their content, or from the derivation itself? */ @@ -114,10 +130,13 @@ struct DerivationType : _DerivationTypeRaw { non-CA derivations. */ bool isFixed() const; - /* Is the derivation impure and needs to access non-deterministic resources, or - pure and can be sandboxed? Note that whether or not we actually sandbox the - derivation is controlled separately. Never true for non-CA derivations. */ - bool isImpure() const; + /* Whether the derivation needs to access the network. Note that + whether or not we actually sandbox the derivation is controlled + separately. Never true for non-CA derivations. */ + bool needsNetworkAccess() const; + + /* FIXME */ + bool isPure() const; /* Does the derivation knows its own output paths? Only true when there's no floating-ca derivation involved in the @@ -173,7 +192,14 @@ struct Derivation : BasicDerivation added directly to input sources. 2. Input placeholders are replaced with realized input store paths. */ - std::optional tryResolve(Store & store); + std::optional tryResolve(Store & store) const; + + /* Like the above, but instead of querying the Nix database for + realisations, uses a given mapping from input derivation paths + + output names to actual output store paths. */ + std::optional tryResolve( + Store & store, + const std::map, StorePath> & inputDrvOutputs) const; Derivation() = default; Derivation(const BasicDerivation & bd) : BasicDerivation(bd) { } @@ -211,7 +237,7 @@ std::string outputPathName(std::string_view drvName, std::string_view outputName struct DrvHash { std::map hashes; - enum struct Kind: bool { + enum struct Kind : bool { // Statically determined derivations. // This hash will be directly used to compute the output paths Regular, @@ -252,8 +278,10 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut /* Return a map associating each output to a hash that uniquely identifies its derivation (modulo the self-references). + + FIXME: what is the Hash in this map? */ -std::map staticOutputHashes(Store& store, const Derivation& drv); +std::map staticOutputHashes(Store & store, const Derivation & drv); /* Memoisation of hashDerivationModulo(). */ typedef std::map DrvHashes; @@ -286,4 +314,6 @@ std::string hashPlaceholder(const std::string_view outputName); dependency which is a CA derivation. */ std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath, std::string_view outputName); +extern const Hash impureOutputHash; + } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 60fe53af1..d77fff963 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -719,6 +719,9 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat [&](const DerivationOutput::Deferred &) { /* Nothing to check */ }, + [&](const DerivationOutput::Impure &) { + /* Nothing to check */ + }, }, i.second.raw()); } } diff --git a/src/libstore/path.cc b/src/libstore/path.cc index e642abcd5..392db225e 100644 --- a/src/libstore/path.cc +++ b/src/libstore/path.cc @@ -1,5 +1,7 @@ #include "store-api.hh" +#include + namespace nix { static void checkName(std::string_view path, std::string_view name) @@ -41,6 +43,13 @@ bool StorePath::isDerivation() const StorePath StorePath::dummy("ffffffffffffffffffffffffffffffff-x"); +StorePath StorePath::random(std::string_view name) +{ + Hash hash(htSHA1); + randombytes_buf(hash.hash, hash.hashSize); + return StorePath(hash, name); +} + StorePath Store::parseStorePath(std::string_view path) const { auto p = canonPath(std::string(path)); diff --git a/src/libstore/path.hh b/src/libstore/path.hh index e65fee622..77fd0f8dc 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -58,6 +58,8 @@ public: } static StorePath dummy; + + static StorePath random(std::string_view name); }; typedef std::set StorePathSet; diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 01f318fa3..e033a4116 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -7,6 +7,7 @@ namespace nix { std::map stringifiedXpFeatures = { { Xp::CaDerivations, "ca-derivations" }, + { Xp::ImpureDerivations, "impure-derivations" }, { Xp::Flakes, "flakes" }, { Xp::NixCommand, "nix-command" }, { Xp::RecursiveNix, "recursive-nix" }, diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index b5140dcfe..3a254b423 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -16,6 +16,7 @@ namespace nix { enum struct ExperimentalFeature { CaDerivations, + ImpureDerivations, Flakes, NixCommand, RecursiveNix, diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 0d9655732..fb46b4dbf 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -77,6 +77,10 @@ struct CmdShowDerivation : InstallablesCommand outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); }, [&](const DerivationOutput::Deferred &) {}, + [&](const DerivationOutput::Impure & doi) { + outputObj.attr("hashAlgo", makeFileIngestionPrefix(doi.method) + printHashType(doi.hashType)); + outputObj.attr("impure", true); + }, }, output.raw()); } } diff --git a/tests/impure-derivations.nix b/tests/impure-derivations.nix new file mode 100644 index 000000000..ba7d53146 --- /dev/null +++ b/tests/impure-derivations.nix @@ -0,0 +1,46 @@ +with import ./config.nix; + +rec { + + impure = mkDerivation { + name = "impure"; + outputs = [ "out" "stuff" ]; + buildCommand = + '' + x=$(< $TEST_ROOT/counter) + mkdir $out $stuff + echo $x > $out/n + ln -s $out/n $stuff/bla + printf $((x + 1)) > $TEST_ROOT/counter + ''; + __impure = true; + outputHashAlgo = "sha256"; + outputHashMode = "recursive"; + impureEnvVars = [ "TEST_ROOT" ]; + }; + + impureOnImpure = mkDerivation { + name = "impure-on-impure"; + buildCommand = + '' + x=$(< ${impure}/n) + mkdir $out + printf X$x > $out/n + ln -s ${impure.stuff} $out/symlink + ln -s $out $out/self + ''; + __impure = true; + outputHashAlgo = "sha256"; + outputHashMode = "recursive"; + }; + + # This is not allowed. + inputAddressed = mkDerivation { + name = "input-addressed"; + buildCommand = + '' + cat ${impure} > $out + ''; + }; + +} diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh new file mode 100644 index 000000000..cb1eaf84a --- /dev/null +++ b/tests/impure-derivations.sh @@ -0,0 +1,39 @@ +source common.sh + +requireDaemonNewerThan "2.8pre20220311" + +enableFeatures "ca-derivations ca-references impure-derivations" + +clearStore + +# Basic test of impure derivations: building one a second time should not use the previous result. +printf 0 > $TEST_ROOT/counter + +json=$(nix build -L --no-link --json --file ./impure-derivations.nix impure) +path1=$(echo $json | jq -r .[].outputs.out) +path1_stuff=$(echo $json | jq -r .[].outputs.stuff) +[[ $(< $path1/n) = 0 ]] +[[ $(< $path1_stuff/bla) = 0 ]] + +[[ $(nix path-info --json $path1 | jq .[].ca) =~ fixed:r:sha256: ]] + +path2=$(nix build -L --no-link --json --file ./impure-derivations.nix impure | jq -r .[].outputs.out) +[[ $(< $path2/n) = 1 ]] + +# Test impure derivations that depend on impure derivations. +path3=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure -vvvvv | jq -r .[].outputs.out) +[[ $(< $path3/n) = X2 ]] + +path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure -vvvvv | jq -r .[].outputs.out) +[[ $(< $path4/n) = X3 ]] + +# Test that (self-)references work. +[[ $(< $path4/symlink/bla) = 3 ]] +[[ $(< $path4/self/n) = X3 ]] + +# Input-addressed derivations cannot depend on impure derivations directly. +nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>&1 | grep 'depends on impure derivation' + +drvPath=$(nix eval --json --file ./impure-derivations.nix impure.drvPath | jq -r .) +[[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.out.impure") = true ]] +[[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.stuff.impure") = true ]] diff --git a/tests/local.mk b/tests/local.mk index 97971dd76..668b34500 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -97,7 +97,8 @@ nix_tests = \ nix-profile.sh \ suggestions.sh \ store-ping.sh \ - fetchClosure.sh + fetchClosure.sh \ + impure-derivations.sh ifeq ($(HAVE_LIBCPUID), 1) nix_tests += compute-levels.sh From 18935e8b9f152f18705e738d4b8cc6fce97c8b02 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 11 Mar 2022 13:23:23 +0100 Subject: [PATCH 24/83] Support fixed-output derivations depending on impure derivations --- src/libstore/build/derivation-goal.cc | 9 +++++---- src/libstore/misc.cc | 7 ++++--- tests/impure-derivations.nix | 22 ++++++++++++++++++++++ tests/impure-derivations.sh | 19 +++++++++++++++++-- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 2f3490829..542a6f6ea 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -342,9 +342,9 @@ void DerivationGoal::gaveUpOnSubstitution() inputDrvOutputs.clear(); if (useDerivation) for (auto & i : dynamic_cast(drv.get())->inputDrvs) { - /* Ensure that pure derivations don't depend on impure - derivations. */ - if (drv->type().isPure()) { + /* Ensure that pure, non-fixed-output derivations don't + depend on impure derivations. */ + if (drv->type().isPure() && !drv->type().isFixed()) { auto inputDrv = worker.evalStore.readDerivation(i.first); if (!inputDrv.type().isPure()) throw Error("pure derivation '%s' depends on impure derivation '%s'", @@ -993,7 +993,8 @@ void DerivationGoal::resolvedFinished() auto newRealisation = realisation; newRealisation.id = DrvOutput { initialOutputs.at(wantedOutput).outputHash, wantedOutput }; newRealisation.signatures.clear(); - newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); + if (!drv->type().isFixed()) + newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 1f0bae7fe..2bbd7aa70 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -277,15 +277,15 @@ std::map drvOutputReferences( { std::set inputRealisations; - for (const auto& [inputDrv, outputNames] : drv.inputDrvs) { + for (const auto & [inputDrv, outputNames] : drv.inputDrvs) { auto outputHashes = staticOutputHashes(store, store.readDerivation(inputDrv)); - for (const auto& outputName : outputNames) { + for (const auto & outputName : outputNames) { auto thisRealisation = store.queryRealisation( DrvOutput{outputHashes.at(outputName), outputName}); if (!thisRealisation) throw Error( - "output '%s' of derivation '%s' isn’t built", outputName, + "output '%s' of derivation '%s' isn't built", outputName, store.printStorePath(inputDrv)); inputRealisations.insert(*thisRealisation); } @@ -295,4 +295,5 @@ std::map drvOutputReferences( return drvOutputReferences(Realisation::closure(store, inputRealisations), info->references); } + } diff --git a/tests/impure-derivations.nix b/tests/impure-derivations.nix index ba7d53146..2fed56fe7 100644 --- a/tests/impure-derivations.nix +++ b/tests/impure-derivations.nix @@ -7,6 +7,7 @@ rec { outputs = [ "out" "stuff" ]; buildCommand = '' + echo impure x=$(< $TEST_ROOT/counter) mkdir $out $stuff echo $x > $out/n @@ -23,6 +24,7 @@ rec { name = "impure-on-impure"; buildCommand = '' + echo impure-on-impure x=$(< ${impure}/n) mkdir $out printf X$x > $out/n @@ -43,4 +45,24 @@ rec { ''; }; + contentAddressed = mkDerivation { + name = "content-addressed"; + buildCommand = + '' + echo content-addressed + x=$(< ${impureOnImpure}/n) + printf ''${x:0:1} > $out + ''; + outputHashAlgo = "sha256"; + outputHashMode = "recursive"; + outputHash = "sha256-eBYxcgkuWuiqs4cKNgKwkb3vY/HR0vVsJnqe8itJGcQ="; + }; + + inputAddressedAfterCA = mkDerivation { + name = "input-addressed-after-ca"; + buildCommand = + '' + cat ${contentAddressed} > $out + ''; + }; } diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh index cb1eaf84a..85e3e09cc 100644 --- a/tests/impure-derivations.sh +++ b/tests/impure-derivations.sh @@ -21,10 +21,10 @@ path2=$(nix build -L --no-link --json --file ./impure-derivations.nix impure | j [[ $(< $path2/n) = 1 ]] # Test impure derivations that depend on impure derivations. -path3=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure -vvvvv | jq -r .[].outputs.out) +path3=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure | jq -r .[].outputs.out) [[ $(< $path3/n) = X2 ]] -path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure -vvvvv | jq -r .[].outputs.out) +path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure | jq -r .[].outputs.out) [[ $(< $path4/n) = X3 ]] # Test that (self-)references work. @@ -37,3 +37,18 @@ nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>& drvPath=$(nix eval --json --file ./impure-derivations.nix impure.drvPath | jq -r .) [[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.out.impure") = true ]] [[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.stuff.impure") = true ]] + +# Fixed-output derivations *can* depend on impure derivations. +path5=$(nix build -L --no-link --json --file ./impure-derivations.nix contentAddressed | jq -r .[].outputs.out) +[[ $(< $path5) = X ]] +[[ $(< $TEST_ROOT/counter) = 5 ]] + +# And they should not be rebuilt. +path5=$(nix build -L --no-link --json --file ./impure-derivations.nix contentAddressed | jq -r .[].outputs.out) +[[ $(< $path5) = X ]] +[[ $(< $TEST_ROOT/counter) = 5 ]] + +# Input-addressed derivations can depend on fixed-output derivations that depend on impure derivations. +path6=$(nix build -L --no-link --json --file ./impure-derivations.nix inputAddressedAfterCA | jq -r .[].outputs.out) +[[ $(< $path6) = X ]] +[[ $(< $TEST_ROOT/counter) = 5 ]] From b2ae922747adfe187d02c1bfca8231c2d8bceb75 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 11 Mar 2022 15:56:22 +0100 Subject: [PATCH 25/83] tests/impure-derivations.sh: Restart daemon --- tests/impure-derivations.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh index 85e3e09cc..1b33a785c 100644 --- a/tests/impure-derivations.sh +++ b/tests/impure-derivations.sh @@ -3,6 +3,7 @@ source common.sh requireDaemonNewerThan "2.8pre20220311" enableFeatures "ca-derivations ca-references impure-derivations" +restartDaemon clearStore From 162beb25955adcedeed76e97510feb577d4f86db Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 30 Mar 2022 17:01:32 +0200 Subject: [PATCH 26/83] Fix test --- tests/impure-derivations.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh index 1b33a785c..aab5cb61a 100644 --- a/tests/impure-derivations.sh +++ b/tests/impure-derivations.sh @@ -10,7 +10,7 @@ clearStore # Basic test of impure derivations: building one a second time should not use the previous result. printf 0 > $TEST_ROOT/counter -json=$(nix build -L --no-link --json --file ./impure-derivations.nix impure) +json=$(nix build -L --no-link --json --file ./impure-derivations.nix impure.all) path1=$(echo $json | jq -r .[].outputs.out) path1_stuff=$(echo $json | jq -r .[].outputs.stuff) [[ $(< $path1/n) = 0 ]] From d7fc33c8426768040b322a060b5a50433b3a78e6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 15:59:14 +0200 Subject: [PATCH 27/83] Fix macOS build --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index a6c07314f..108c661c0 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1918,7 +1918,7 @@ void LocalDerivationGoal::runChild() sandboxProfile += "(import \"sandbox-defaults.sb\")\n"; - if (derivationType.isImpure()) + if (derivationType.needsNetworkAccess()) sandboxProfile += "(import \"sandbox-network.sb\")\n"; /* Add the output paths we'll use at build-time to the chroot */ From 4e043c2f32af3d3d49c53a2d06746e2fd6967836 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:01:50 +0200 Subject: [PATCH 28/83] Document isPure() --- src/libstore/derivations.hh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index b62e40786..98e59b64e 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -135,7 +135,10 @@ struct DerivationType : _DerivationTypeRaw { separately. Never true for non-CA derivations. */ bool needsNetworkAccess() const; - /* FIXME */ + /* Whether the derivation is expected to produce the same result + every time, and therefore it only needs to be built once. This + is only false for derivations that have the attribute '__impure + = true'. */ bool isPure() const; /* Does the derivation knows its own output paths? From e279fbb16a99896101006ec00277a5d9d50f5040 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:06:40 +0200 Subject: [PATCH 29/83] needsNetworkAccess() -> isSandboxed() --- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 12 ++++++------ src/libstore/derivations.cc | 8 ++++---- src/libstore/derivations.hh | 10 ++++++---- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 542a6f6ea..6582497bd 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -955,7 +955,7 @@ void DerivationGoal::buildDone() st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - derivationType.needsNetworkAccess() || diskFull ? BuildResult::TransientFailure : + !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; } diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 108c661c0..40ef706a6 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -395,7 +395,7 @@ void LocalDerivationGoal::startBuilder() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = !derivationType.needsNetworkAccess() && !noChroot; + useChroot = derivationType.isSandboxed() && !noChroot; } auto & localStore = getLocalStore(); @@ -608,7 +608,7 @@ void LocalDerivationGoal::startBuilder() "nogroup:x:65534:\n", sandboxGid())); /* Create /etc/hosts with localhost entry. */ - if (!derivationType.needsNetworkAccess()) + if (derivationType.isSandboxed()) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -796,7 +796,7 @@ void LocalDerivationGoal::startBuilder() us. */ - if (!derivationType.needsNetworkAccess()) + if (derivationType.isSandboxed()) privateNetwork = true; userNamespaceSync.create(); @@ -1060,7 +1060,7 @@ void LocalDerivationGoal::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (derivationType.needsNetworkAccess()) { + if (!derivationType.isSandboxed()) { for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) env[i] = getEnv(i).value_or(""); } @@ -1674,7 +1674,7 @@ void LocalDerivationGoal::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (derivationType.needsNetworkAccess()) { + if (!derivationType.isSandboxed()) { // Only use nss functions to resolve hosts and // services. Don’t use it for anything else that may // be configured for this system. This limits the @@ -1918,7 +1918,7 @@ void LocalDerivationGoal::runChild() sandboxProfile += "(import \"sandbox-defaults.sb\")\n"; - if (derivationType.needsNetworkAccess()) + if (!derivationType.isSandboxed()) sandboxProfile += "(import \"sandbox-network.sb\")\n"; /* Add the output paths we'll use at build-time to the chroot */ diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 75e0178bb..b4fb77f9f 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -90,17 +90,17 @@ bool DerivationType::hasKnownOutputPaths() const } -bool DerivationType::needsNetworkAccess() const +bool DerivationType::isSandboxed() const { return std::visit(overloaded { [](const InputAddressed & ia) { - return false; + return true; }, [](const ContentAddressed & ca) { - return !ca.pure; + return ca.pure; }, [](const Impure &) { - return true; + return false; }, }, raw()); } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 98e59b64e..489948c30 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -130,10 +130,12 @@ struct DerivationType : _DerivationTypeRaw { non-CA derivations. */ bool isFixed() const; - /* Whether the derivation needs to access the network. Note that - whether or not we actually sandbox the derivation is controlled - separately. Never true for non-CA derivations. */ - bool needsNetworkAccess() const; + /* Whether the derivation is fully sandboxed. If false, the + sandbox is opened up, e.g. the derivation has access to the + network. Note that whether or not we actually sandbox the + derivation is controlled separately. Always true for non-CA + derivations. */ + bool isSandboxed() const; /* Whether the derivation is expected to produce the same result every time, and therefore it only needs to be built once. This From 6051cc954b990fa57a6d3b75bd4b0aaaceb0ca82 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:12:25 +0200 Subject: [PATCH 30/83] Rename 'pure' -> 'sandboxed' for consistency --- src/libstore/derivations.cc | 6 +++--- src/libstore/derivations.hh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index b4fb77f9f..cc04119c3 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -97,7 +97,7 @@ bool DerivationType::isSandboxed() const return true; }, [](const ContentAddressed & ca) { - return ca.pure; + return ca.sandboxed; }, [](const Impure &) { return false; @@ -530,7 +530,7 @@ DerivationType BasicDerivation::type() const if (*fixedCAOutputs.begin() != "out") throw Error("single fixed output must be named \"out\""); return DerivationType::ContentAddressed { - .pure = false, + .sandboxed = false, .fixed = true, }; } @@ -541,7 +541,7 @@ DerivationType BasicDerivation::type() const && deferredIAOutputs.empty() && impureOutputs.empty()) return DerivationType::ContentAddressed { - .pure = true, + .sandboxed = true, .fixed = false, }; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 489948c30..af198a767 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -102,7 +102,7 @@ struct DerivationType_InputAddressed { }; struct DerivationType_ContentAddressed { - bool pure; + bool sandboxed; bool fixed; }; From a99af85a770df462985b621c4c3dd710b8487f44 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:39:18 +0200 Subject: [PATCH 31/83] Fix macOS build --- src/libstore/derivations.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index cc04119c3..1c695de82 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -222,11 +222,9 @@ static DerivationOutput parseDerivationOutput(const Store & store, if (hash == "impure") { settings.requireExperimentalFeature(Xp::ImpureDerivations); assert(pathS == ""); - return DerivationOutput { - .output = DerivationOutputImpure { - .method = std::move(method), - .hashType = std::move(hashType), - }, + return DerivationOutput::Impure { + .method = std::move(method), + .hashType = std::move(hashType), }; } else if (hash != "") { validatePath(pathS); From 75370972847a1b992055085f39b38f1f659e5275 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:56:44 +0200 Subject: [PATCH 32/83] Provide default values for outputHashAlgo and outputHashMode --- src/libexpr/primops.cc | 19 +++++++++++-------- tests/impure-derivations.nix | 5 ----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index eaf04320e..969391725 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -991,8 +991,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * bool contentAddressed = false; bool isImpure = false; std::optional outputHash; - std::string outputHashAlgo; - auto ingestionMethod = FileIngestionMethod::Flat; + std::optional outputHashAlgo; + std::optional ingestionMethod; StringSet outputs; outputs.insert("out"); @@ -1190,15 +1190,16 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * .errPos = posDrvName }); - std::optional ht = parseHashTypeOpt(outputHashAlgo); + std::optional ht = parseHashTypeOpt(outputHashAlgo.value_or("sha256")); Hash h = newHashAllowEmpty(*outputHash, ht); - auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName); + auto method = ingestionMethod.value_or(FileIngestionMethod::Flat); + auto outPath = state.store->makeFixedOutputPath(method, h, drvName); drv.env["out"] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign("out", DerivationOutput::CAFixed { .hash = FixedOutputHash { - .method = ingestionMethod, + .method = method, .hash = std::move(h), }, }); @@ -1211,19 +1212,21 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * .errPos = posDrvName }); - HashType ht = parseHashType(outputHashAlgo); + auto ht = parseHashType(outputHashAlgo.value_or("sha256")); + auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive); + for (auto & i : outputs) { drv.env[i] = hashPlaceholder(i); if (isImpure) drv.outputs.insert_or_assign(i, DerivationOutput::Impure { - .method = ingestionMethod, + .method = method, .hashType = ht, }); else drv.outputs.insert_or_assign(i, DerivationOutput::CAFloating { - .method = ingestionMethod, + .method = method, .hashType = ht, }); } diff --git a/tests/impure-derivations.nix b/tests/impure-derivations.nix index 2fed56fe7..98547e6c1 100644 --- a/tests/impure-derivations.nix +++ b/tests/impure-derivations.nix @@ -15,8 +15,6 @@ rec { printf $((x + 1)) > $TEST_ROOT/counter ''; __impure = true; - outputHashAlgo = "sha256"; - outputHashMode = "recursive"; impureEnvVars = [ "TEST_ROOT" ]; }; @@ -32,8 +30,6 @@ rec { ln -s $out $out/self ''; __impure = true; - outputHashAlgo = "sha256"; - outputHashMode = "recursive"; }; # This is not allowed. @@ -53,7 +49,6 @@ rec { x=$(< ${impureOnImpure}/n) printf ''${x:0:1} > $out ''; - outputHashAlgo = "sha256"; outputHashMode = "recursive"; outputHash = "sha256-eBYxcgkuWuiqs4cKNgKwkb3vY/HR0vVsJnqe8itJGcQ="; }; From d63a5f5dd3b47e629295eb68264b4a6aadc65aa7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 17:33:06 +0200 Subject: [PATCH 33/83] Update release notes --- doc/manual/src/release-notes/rl-next.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 2ec864ee4..4f3c9ce41 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -14,3 +14,21 @@ This function is only available if you enable the experimental feature `fetch-closure`. + +* New experimental feature: *impure derivations*. These are + derivations that can produce a different result every time they're + built. Here is an example: + + ```nix + stdenv.mkDerivation { + name = "impure"; + __impure = true; # marks this derivation as impure + buildCommand = "date > $out"; + } + ``` + + Running `nix build` twice on this expression will build the + derivation twice, producing two different content-addressed store + paths. Like fixed-output derivations, impure derivations have access + to the network. Only fixed-output derivations and impure derivations + can depend on an impure derivation. From 6377442c983f4399d0a997238b898298e349fc1b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 17:38:15 +0200 Subject: [PATCH 34/83] tests/impure-derivations.sh: Ensure that inputAddressed build fails --- tests/impure-derivations.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh index aab5cb61a..35ae3f5d3 100644 --- a/tests/impure-derivations.sh +++ b/tests/impure-derivations.sh @@ -5,6 +5,8 @@ requireDaemonNewerThan "2.8pre20220311" enableFeatures "ca-derivations ca-references impure-derivations" restartDaemon +set -o pipefail + clearStore # Basic test of impure derivations: building one a second time should not use the previous result. @@ -33,7 +35,7 @@ path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnIm [[ $(< $path4/self/n) = X3 ]] # Input-addressed derivations cannot depend on impure derivations directly. -nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>&1 | grep 'depends on impure derivation' +(! nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>&1) | grep 'depends on impure derivation' drvPath=$(nix eval --json --file ./impure-derivations.nix impure.drvPath | jq -r .) [[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.out.impure") = true ]] From 7492030ed7d1d570b71e832b610150054a9f095b Mon Sep 17 00:00:00 2001 From: Artturin Date: Thu, 31 Mar 2022 22:14:53 +0300 Subject: [PATCH 35/83] scripts/install-systemd-multi-user.sh: fix another typo --- scripts/install-systemd-multi-user.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-systemd-multi-user.sh b/scripts/install-systemd-multi-user.sh index 1d92c5388..62397127a 100755 --- a/scripts/install-systemd-multi-user.sh +++ b/scripts/install-systemd-multi-user.sh @@ -90,7 +90,7 @@ poly_configure_nix_daemon_service() { ln -sfn /nix/var/nix/profiles/default/$TMPFILES_SRC $TMPFILES_DEST _sudo "to run systemd-tmpfiles once to pick that path up" \ - systemd-tmpfiles create --prefix=/nix/var/nix + systemd-tmpfiles --create --prefix=/nix/var/nix _sudo "to set up the nix-daemon service" \ systemctl link "/nix/var/nix/profiles/default$SERVICE_SRC" From fdfe737867920ed0ec29ba8ffb7d19854d8658bb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 1 Apr 2022 12:40:49 +0200 Subject: [PATCH 36/83] Fix handling of outputHash when outputHashAlgo is not specified https://hydra.nixos.org/build/171351131 --- src/libexpr/primops.cc | 7 +++---- tests/ca/content-addressed.nix | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 969391725..73817dbdd 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -991,7 +991,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * bool contentAddressed = false; bool isImpure = false; std::optional outputHash; - std::optional outputHashAlgo; + std::string outputHashAlgo; std::optional ingestionMethod; StringSet outputs; @@ -1190,8 +1190,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * .errPos = posDrvName }); - std::optional ht = parseHashTypeOpt(outputHashAlgo.value_or("sha256")); - Hash h = newHashAllowEmpty(*outputHash, ht); + auto h = newHashAllowEmpty(*outputHash, parseHashTypeOpt(outputHashAlgo)); auto method = ingestionMethod.value_or(FileIngestionMethod::Flat); auto outPath = state.store->makeFixedOutputPath(method, h, drvName); @@ -1212,7 +1211,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * .errPos = posDrvName }); - auto ht = parseHashType(outputHashAlgo.value_or("sha256")); + auto ht = parseHashTypeOpt(outputHashAlgo).value_or(htSHA256); auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive); for (auto & i : outputs) { diff --git a/tests/ca/content-addressed.nix b/tests/ca/content-addressed.nix index d328fc92c..1be3eeb6e 100644 --- a/tests/ca/content-addressed.nix +++ b/tests/ca/content-addressed.nix @@ -64,8 +64,7 @@ rec { dependentFixedOutput = mkDerivation { name = "dependent-fixed-output"; outputHashMode = "recursive"; - outputHashAlgo = "sha256"; - outputHash = "sha256-QvtAMbUl/uvi+LCObmqOhvNOapHdA2raiI4xG5zI5pA="; + outputHash = "sha512-7aJcmSuEuYP5tGKcmGY8bRr/lrCjJlOxP2mIUjO/vMQeg6gx/65IbzRWES8EKiPDOs9z+wF30lEfcwxM/cT4pw=="; buildCommand = '' cat ${dependentCA}/dep echo foo > $out From 435848cef12d065b209c82c96ce3a8bfa5e6a051 Mon Sep 17 00:00:00 2001 From: aszlig Date: Fri, 1 Apr 2022 09:23:43 -0700 Subject: [PATCH 37/83] libutil: Fix restoring mount namespace I regularly pass around simple scripts by using nix-shell as the script interpreter, eg. like this: #!/usr/bin/env nix-shell #!nix-shell -p dd_rescue coreutils bash -i bash While this works most of the time, I recently had one occasion where it would not and the above would result in the following: $ sudo ./myscript.sh bash: ./myscript.sh: No such file or directory Note the "sudo" here, because this error only occurs if we're root. The reason for the latter is because running Nix as root means that we can directly access the store, which makes sure we use a filesystem namespace to make the store writable. XXX - REWORD! So when stracing the process, I stumbled on the following sequence: openat(AT_FDCWD, "/proc/self/ns/mnt", O_RDONLY) = 3 unshare(CLONE_NEWNS) = 0 ... later ... getcwd("/the/real/cwd", 4096) = 14 setns(3, CLONE_NEWNS) = 0 getcwd("/", 4096) = 2 In the whole strace output there are no calls to chdir() whatsoever, so I decided to look into the kernel source to see what else could change directories and found this[1]: /* Update the pwd and root */ set_fs_pwd(fs, &root); set_fs_root(fs, &root); The set_fs_pwd() call is roughly equivalent to a chdir() syscall and this is called when the setns() syscall is invoked[2]. [1]: https://github.com/torvalds/linux/blob/b14ffae378aa1db993e62b01392e70d1e585fb23/fs/namespace.c#L4659 [2]: https://github.com/torvalds/linux/blob/b14ffae378aa1db993e62b01392e70d1e585fb23/kernel/nsproxy.c#L346 --- src/libutil/util.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 59e3aad6d..e62672717 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1688,13 +1689,17 @@ void setStackSize(size_t stackSize) #endif } +#if __linux__ static AutoCloseFD fdSavedMountNamespace; +std::optional savedCwd; +#endif void saveMountNamespace() { #if __linux__ static std::once_flag done; std::call_once(done, []() { + savedCwd.emplace(std::filesystem::current_path()); AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) throw SysError("saving parent mount namespace"); @@ -1712,6 +1717,12 @@ void restoreMountNamespace() } catch (Error & e) { debug(e.msg()); } + try { + if (savedCwd) + std::filesystem::current_path(*savedCwd); + } catch (std::filesystem::filesystem_error const &e) { + debug(e.what()); + } #endif } From 7f5caaa7c0f151520d05d4662415ac09d4cf34b0 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Fri, 1 Apr 2022 10:24:31 -0700 Subject: [PATCH 38/83] libutil: Don't use std::filesystem Just in case making libutil depend on std::filesystem is unacceptable, here is the non-filesystem approach. --- src/libutil/util.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index e62672717..0b18f1027 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -1691,7 +1690,7 @@ void setStackSize(size_t stackSize) #if __linux__ static AutoCloseFD fdSavedMountNamespace; -std::optional savedCwd; +std::optional savedCwd; #endif void saveMountNamespace() @@ -1699,7 +1698,11 @@ void saveMountNamespace() #if __linux__ static std::once_flag done; std::call_once(done, []() { - savedCwd.emplace(std::filesystem::current_path()); + char* cwd = getcwd(NULL, 0); + if (cwd == NULL) throw SysError("getting cwd"); + savedCwd.emplace(cwd); + free(cwd); + AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) throw SysError("saving parent mount namespace"); @@ -1717,11 +1720,8 @@ void restoreMountNamespace() } catch (Error & e) { debug(e.msg()); } - try { - if (savedCwd) - std::filesystem::current_path(*savedCwd); - } catch (std::filesystem::filesystem_error const &e) { - debug(e.what()); + if (savedCwd && chdir(savedCwd->c_str()) == -1) { + throw SysError("restoring cwd"); } #endif } From 2a45cf54e4201a894254604676dcb51f4c1a471c Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Fri, 1 Apr 2022 12:20:34 -0700 Subject: [PATCH 39/83] libutil: Properly guard self-allocating getcwd on GNU It's a GNU extension, as pointed out by pennae. --- src/libutil/util.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 0b18f1027..28ab77adc 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1698,10 +1698,19 @@ void saveMountNamespace() #if __linux__ static std::once_flag done; std::call_once(done, []() { - char* cwd = getcwd(NULL, 0); - if (cwd == NULL) throw SysError("getting cwd"); +#ifdef __GNU__ + // getcwd allocating its return value is a GNU extension. + char *cwd = getcwd(NULL, 0); + if (cwd == NULL) +#else + char cwd[PATH_MAX]; + if (!getcwd(cwd, sizeof(cwd))) +#endif + throw SysError("getting cwd"); savedCwd.emplace(cwd); +#ifdef __GNU__ free(cwd); +#endif AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) From c1e2ce4515c12ff0b4b1c3ab6d1e057507104979 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Thu, 31 Mar 2022 20:30:53 -0400 Subject: [PATCH 40/83] fix(run): set applyNixConfig lockFlag --- src/nix/run.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix/run.cc b/src/nix/run.cc index 23e893fbf..25a8fa8d3 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -182,6 +182,7 @@ struct CmdRun : InstallableCommand { auto state = getEvalState(); + lockFlags.applyNixConfig = true; auto app = installable->toApp(*state).resolve(getEvalStore(), store); Strings allArgs{app.program}; From a4a1de69dcc3c6e0c40a093d67b5f20568a5f31e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 4 Apr 2022 16:49:39 +0200 Subject: [PATCH 41/83] Add missing #include --- src/libstore/build-result.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index cb6d19b8e..7f9bcce6d 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -1,6 +1,7 @@ #pragma once #include "realisation.hh" +#include "derived-path.hh" #include #include From e5b70d47aa656833e4609106f9f1ef48b67664cc Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 08:19:49 -0700 Subject: [PATCH 42/83] libutil: cleanup savedCwd logic Co-authored-by: Eelco Dolstra --- src/libutil/util.cc | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 28ab77adc..9b34d5d72 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1698,20 +1698,7 @@ void saveMountNamespace() #if __linux__ static std::once_flag done; std::call_once(done, []() { -#ifdef __GNU__ - // getcwd allocating its return value is a GNU extension. - char *cwd = getcwd(NULL, 0); - if (cwd == NULL) -#else - char cwd[PATH_MAX]; - if (!getcwd(cwd, sizeof(cwd))) -#endif - throw SysError("getting cwd"); - savedCwd.emplace(cwd); -#ifdef __GNU__ - free(cwd); -#endif - + savedCwd = absPath("."); AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) throw SysError("saving parent mount namespace"); From e135d223f66f77f2b03a11b979d714e582364013 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 08:32:45 -0700 Subject: [PATCH 43/83] libutil: save fd to cwd instead of cwd itself --- src/libutil/util.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 9b34d5d72..a00a03978 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1690,7 +1690,7 @@ void setStackSize(size_t stackSize) #if __linux__ static AutoCloseFD fdSavedMountNamespace; -std::optional savedCwd; +static AutoCloseFD fdSavedCwd; #endif void saveMountNamespace() @@ -1698,11 +1698,15 @@ void saveMountNamespace() #if __linux__ static std::once_flag done; std::call_once(done, []() { - savedCwd = absPath("."); AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) throw SysError("saving parent mount namespace"); fdSavedMountNamespace = std::move(fd); + + fd = open("/proc/self/cwd", O_RDONLY); + if (!fd) + throw SysError("saving cwd"); + fdSavedCwd = std::move(fd); }); #endif } @@ -1716,7 +1720,7 @@ void restoreMountNamespace() } catch (Error & e) { debug(e.msg()); } - if (savedCwd && chdir(savedCwd->c_str()) == -1) { + if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { throw SysError("restoring cwd"); } #endif From f89b0f7846f2177b65eebdbff7f24a255d66819e Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 08:33:59 -0700 Subject: [PATCH 44/83] libutil: `try` restoring the cwd from fdSavedCwd --- src/libutil/util.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index a00a03978..1f800f3f4 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1717,12 +1717,12 @@ void restoreMountNamespace() try { if (fdSavedMountNamespace && setns(fdSavedMountNamespace.get(), CLONE_NEWNS) == -1) throw SysError("restoring parent mount namespace"); + if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { + throw SysError("restoring cwd"); + } } catch (Error & e) { debug(e.msg()); } - if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { - throw SysError("restoring cwd"); - } #endif } From 10b9c1b2b269b96dcb8b3d298491fa143d1663e8 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 10:16:30 -0700 Subject: [PATCH 45/83] libutil: save cwd fd in restoreMountNamespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This doesn't work very well (maybe I'm misunderstanding the desired implementation): : ~/w/vc/nix ; doas outputs/out/bin/nix --experimental-features 'nix-command flakes' develop -c pwd pwd: couldn't find directory entry in ‘../../../..’ with matching i-node --- src/libutil/util.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 1f800f3f4..701545589 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1690,7 +1690,6 @@ void setStackSize(size_t stackSize) #if __linux__ static AutoCloseFD fdSavedMountNamespace; -static AutoCloseFD fdSavedCwd; #endif void saveMountNamespace() @@ -1702,11 +1701,6 @@ void saveMountNamespace() if (!fd) throw SysError("saving parent mount namespace"); fdSavedMountNamespace = std::move(fd); - - fd = open("/proc/self/cwd", O_RDONLY); - if (!fd) - throw SysError("saving cwd"); - fdSavedCwd = std::move(fd); }); #endif } @@ -1715,6 +1709,10 @@ void restoreMountNamespace() { #if __linux__ try { + AutoCloseFD fdSavedCwd = open("/proc/self/cwd", O_RDONLY); + if (!fdSavedCwd) { + throw SysError("saving cwd"); + } if (fdSavedMountNamespace && setns(fdSavedMountNamespace.get(), CLONE_NEWNS) == -1) throw SysError("restoring parent mount namespace"); if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { From 56009b2639dc878be11f94d096fae56ac14dcd1d Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 10:21:56 -0700 Subject: [PATCH 46/83] libutil: don't save cwd fd, use path instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saving the cwd fd didn't actually work well -- prior to this commit, the following would happen: : ~/w/vc/nix ; doas outputs/out/bin/nix --experimental-features 'nix-command flakes' run nixpkgs#coreutils -- --coreutils-prog=pwd pwd: couldn't find directory entry in ‘../../../..’ with matching i-node : ~/w/vc/nix ; doas outputs/out/bin/nix --experimental-features 'nix-command flakes' develop -c pwd pwd: couldn't find directory entry in ‘../../../..’ with matching i-node --- src/libutil/util.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 701545589..3132e7161 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1709,13 +1709,11 @@ void restoreMountNamespace() { #if __linux__ try { - AutoCloseFD fdSavedCwd = open("/proc/self/cwd", O_RDONLY); - if (!fdSavedCwd) { - throw SysError("saving cwd"); - } + auto savedCwd = absPath("."); + if (fdSavedMountNamespace && setns(fdSavedMountNamespace.get(), CLONE_NEWNS) == -1) throw SysError("restoring parent mount namespace"); - if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { + if (chdir(savedCwd.c_str()) == -1) { throw SysError("restoring cwd"); } } catch (Error & e) { From 5abe3f4aa61f33ac2124a624763e067257541871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 5 Apr 2022 11:03:43 +0200 Subject: [PATCH 47/83] Allow `welcomeText` when checking a flake template Fix https://github.com/NixOS/nix/issues/6321 --- src/nix/flake.cc | 2 +- tests/flakes.sh | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 47a380238..ce7dc101a 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -463,7 +463,7 @@ struct CmdFlakeCheck : FlakeCommand for (auto & attr : *v.attrs) { std::string name(attr.name); - if (name != "path" && name != "description") + if (name != "path" && name != "description" && name != "welcomeText") throw Error("template '%s' has unsupported attribute '%s'", attrPath, name); } } catch (Error & e) { diff --git a/tests/flakes.sh b/tests/flakes.sh index ea629ae70..46e6a7982 100644 --- a/tests/flakes.sh +++ b/tests/flakes.sh @@ -376,6 +376,9 @@ cat > $templatesDir/flake.nix < Date: Tue, 5 Apr 2022 13:34:25 +0200 Subject: [PATCH 48/83] manual: Add some anchor targets for the nix.conf options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For each `nix.conf` option, add an empty html node with a unique `id` that can be used as an anchor target. Also make the name of the option be a link to that target so that it’s easily discoverable. We can’t rewrite the whole list as an html definition list like it’s done for the builtins because these options also appear in a man page, and the manpage renderer (lowdown) can’t render arbitrary html. But the hack here allows to keep the manpage and have the links in the html version. Fix https://github.com/NixOS/nix/issues/5745 --- doc/manual/generate-options.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/manual/generate-options.nix b/doc/manual/generate-options.nix index 84d90beb6..2d586fa1b 100644 --- a/doc/manual/generate-options.nix +++ b/doc/manual/generate-options.nix @@ -6,7 +6,8 @@ options: concatStrings (map (name: let option = options.${name}; in - " - `${name}` \n\n" + " - [`${name}`](#conf-${name})" + + "

\n\n" + concatStrings (map (s: " ${s}\n") (splitLines option.description)) + "\n\n" + (if option.documentDefault then " **Default:** " + ( From 9a640afc1e63bed28926cb44fe85137fb7d2d2d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 5 Apr 2022 11:33:46 +0200 Subject: [PATCH 49/83] doctor: Always show the output Fix https://github.com/NixOS/nix/issues/6342 --- src/nix/doctor.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/doctor.cc b/src/nix/doctor.cc index 4f3003448..ea87e3d87 100644 --- a/src/nix/doctor.cc +++ b/src/nix/doctor.cc @@ -24,12 +24,12 @@ std::string formatProtocol(unsigned int proto) } bool checkPass(const std::string & msg) { - logger->log(ANSI_GREEN "[PASS] " ANSI_NORMAL + msg); + notice(ANSI_GREEN "[PASS] " ANSI_NORMAL + msg); return true; } bool checkFail(const std::string & msg) { - logger->log(ANSI_RED "[FAIL] " ANSI_NORMAL + msg); + notice(ANSI_RED "[FAIL] " ANSI_NORMAL + msg); return false; } From f98d76ff1aeaaf18ea68a86f9eb91dd73b2d6ce4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Apr 2022 14:13:26 +0200 Subject: [PATCH 50/83] rl-2.7.md: Fix title --- doc/manual/src/release-notes/rl-2.7.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/src/release-notes/rl-2.7.md b/doc/manual/src/release-notes/rl-2.7.md index dc92a9cde..2f3879422 100644 --- a/doc/manual/src/release-notes/rl-2.7.md +++ b/doc/manual/src/release-notes/rl-2.7.md @@ -1,4 +1,4 @@ -# Release X.Y (2022-03-07) +# Release 2.7 (2022-03-07) * Nix will now make some helpful suggestions when you mistype something on the command line. For instance, if you type `nix build From 8d6c937d6a233106dcf9c3e0f0e1c60148b7a71e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Apr 2022 16:41:40 +0200 Subject: [PATCH 51/83] flake.lock: Update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/82891b5e2c2359d7e58d08849e4c89511ab94234' (2021-09-28) → 'github:NixOS/nixpkgs/530a53dcbc9437363471167a5e4762c5fcfa34a1' (2022-02-19) --- flake.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake.lock b/flake.lock index 61eccb73c..cd79fa85e 100644 --- a/flake.lock +++ b/flake.lock @@ -18,11 +18,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1632864508, - "narHash": "sha256-d127FIvGR41XbVRDPVvozUPQ/uRHbHwvfyKHwEt5xFM=", + "lastModified": 1645296114, + "narHash": "sha256-y53N7TyIkXsjMpOG7RhvqJFGDacLs9HlyHeSTBioqYU=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "82891b5e2c2359d7e58d08849e4c89511ab94234", + "rev": "530a53dcbc9437363471167a5e4762c5fcfa34a1", "type": "github" }, "original": { From 1fa03934791189f02208e1807d822128b216f087 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Tue, 5 Apr 2022 21:16:11 +0200 Subject: [PATCH 52/83] libutil: Reserve memory when en/decoding base64 The size of the output when encoding to and decoding from base64 is (roughly) known so we can allocate it in advance to prevent reallocation. --- src/libutil/util.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 59e3aad6d..cca66d5bf 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1471,6 +1471,7 @@ constexpr char base64Chars[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuv std::string base64Encode(std::string_view s) { std::string res; + res.reserve((s.size() + 2) / 3 * 4); int data = 0, nbits = 0; for (char c : s) { @@ -1502,6 +1503,9 @@ std::string base64Decode(std::string_view s) }(); std::string res; + // Some sequences are missing the padding consisting of up to two '='. + // vvv + res.reserve((s.size() + 2) / 4 * 3); unsigned int d = 0, bits = 0; for (char c : s) { From 513652d5947c1923a004318ea453cab786cabda1 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Tue, 5 Apr 2022 22:13:45 +0200 Subject: [PATCH 53/83] tokenizeString: Fix semantic mistake `string_view::find_first_not_of(...)` and `string_view::find_first_of(...)` return `string_view::npos` on error not `string::npos`. --- 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 59e3aad6d..8ab385837 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1259,9 +1259,9 @@ template C tokenizeString(std::string_view s, std::string_view separato { C result; auto pos = s.find_first_not_of(separators, 0); - while (pos != std::string::npos) { + while (pos != std::string_view::npos) { auto end = s.find_first_of(separators, pos + 1); - if (end == std::string::npos) end = s.size(); + if (end == std::string_view::npos) end = s.size(); result.insert(result.end(), std::string(s, pos, end - pos)); pos = s.find_first_not_of(separators, end); } From 589f6f267b009bc2856597995db360f910e69a6f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 11:52:51 +0200 Subject: [PATCH 54/83] fetchClosure: Don't allow URL query parameters Allowing this is a potential security hole, since it allows the user to specify parameters like 'local-nar-cache'. --- src/libexpr/primops/fetchClosure.cc | 9 ++++++++- tests/fetchClosure.sh | 12 ++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index efeb93daf..821eba698 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -61,6 +61,12 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args .errPos = pos }); + if (!parsedURL.query.empty()) + throw Error({ + .msg = hintfmt("'fetchClosure' does not support URL query parameters (in '%s')", *fromStoreUrl), + .errPos = pos + }); + auto fromStore = openStore(parsedURL.to_string()); if (toCA) { @@ -87,7 +93,8 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args }); } } else { - copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); + if (!state.store->isValidPath(*fromPath)) + copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); toPath = fromPath; } diff --git a/tests/fetchClosure.sh b/tests/fetchClosure.sh index 0c905ac43..96e4bb741 100644 --- a/tests/fetchClosure.sh +++ b/tests/fetchClosure.sh @@ -56,3 +56,15 @@ nix copy --to file://$cacheDir $caPath fromPath = $caPath; } ") = $caPath ]] + +# Check that URL query parameters aren't allowed. +clearStore +narCache=$TEST_ROOT/nar-cache +rm -rf $narCache +(! nix eval -v --raw --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir?local-nar-cache=$narCache\"; + fromPath = $caPath; + } +") +(! [ -e $narCache ]) From 318936366d5198123ae9ba0292538529f706dce8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 12:41:18 +0200 Subject: [PATCH 55/83] Fix empty 'nix copy' error message This was caused by SubstitutionGoal not setting the errorMsg field in its BuildResult. We now get a more descriptive message than in 2.7.0, e.g. error: path '/nix/store/13mh...' is required, but there is no substituter that can build it instead of the misleading (since there was no build) error: build of '/nix/store/13mh...' failed Fixes #6295. --- .../build/drv-output-substitution-goal.cc | 2 +- src/libstore/build/substitution-goal.cc | 19 ++++++++++++++----- src/libstore/build/substitution-goal.hh | 5 ++++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index e50292c1e..b7f7b5ab1 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -41,7 +41,7 @@ void DrvOutputSubstitutionGoal::tryNext() if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal with it. */ - debug("drv output '%s' is required, but there is no substituter that can provide it", id.to_string()); + debug("derivation output '%s' is required, but there is no substituter that can provide it", id.to_string()); /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 31e6dbc9f..ca5218627 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -24,9 +24,16 @@ PathSubstitutionGoal::~PathSubstitutionGoal() } -void PathSubstitutionGoal::done(ExitCode result, BuildResult::Status status) +void PathSubstitutionGoal::done( + ExitCode result, + BuildResult::Status status, + std::optional errorMsg) { buildResult.status = status; + if (errorMsg) { + debug(*errorMsg); + buildResult.errorMsg = *errorMsg; + } amDone(result); } @@ -67,12 +74,14 @@ void PathSubstitutionGoal::tryNext() if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal with it. */ - debug("path '%s' is required, but there is no substituter that can build it", worker.store.printStorePath(storePath)); /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a build. */ - done(substituterFailed ? ecFailed : ecNoSubstituters, BuildResult::NoSubstituters); + done( + substituterFailed ? ecFailed : ecNoSubstituters, + BuildResult::NoSubstituters, + fmt("path '%s' is required, but there is no substituter that can build it", worker.store.printStorePath(storePath))); if (substituterFailed) { worker.failedSubstitutions++; @@ -169,10 +178,10 @@ void PathSubstitutionGoal::referencesValid() trace("all references realised"); if (nrFailed > 0) { - debug("some references of path '%s' could not be realised", worker.store.printStorePath(storePath)); done( nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed, - BuildResult::DependencyFailed); + BuildResult::DependencyFailed, + fmt("some references of path '%s' could not be realised", worker.store.printStorePath(storePath))); return; } diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index 946f13841..a73f8e666 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -53,7 +53,10 @@ struct PathSubstitutionGoal : public Goal /* Content address for recomputing store path */ std::optional ca; - void done(ExitCode result, BuildResult::Status status); + void done( + ExitCode result, + BuildResult::Status status, + std::optional errorMsg = {}); public: PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); From a7b12c6bd90cd44432bdaf45cb32b0af916d499c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 13:34:25 +0200 Subject: [PATCH 56/83] curl: Use --fail to catch errors --- scripts/check-hydra-status.sh | 2 +- scripts/install.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/check-hydra-status.sh b/scripts/check-hydra-status.sh index 5e2f03429..e62705e94 100644 --- a/scripts/check-hydra-status.sh +++ b/scripts/check-hydra-status.sh @@ -14,7 +14,7 @@ curl -sS -H 'Accept: application/json' https://hydra.nixos.org/jobset/nix/master someBuildFailed=0 for buildId in $BUILDS_FOR_LATEST_EVAL; do - buildInfo=$(curl -sS -H 'Accept: application/json' "https://hydra.nixos.org/build/$buildId") + buildInfo=$(curl --fail -sS -H 'Accept: application/json' "https://hydra.nixos.org/build/$buildId") finished=$(echo "$buildInfo" | jq -r '.finished') diff --git a/scripts/install.in b/scripts/install.in index 38d1fb36f..af5f71080 100755 --- a/scripts/install.in +++ b/scripts/install.in @@ -82,7 +82,7 @@ if [ "$(uname -s)" != "Darwin" ]; then fi if command -v curl > /dev/null 2>&1; then - fetch() { curl -L "$1" -o "$2"; } + fetch() { curl --fail -L "$1" -o "$2"; } elif command -v wget > /dev/null 2>&1; then fetch() { wget "$1" -O "$2"; } else From 1e1cd6e7a926097683da366983cc362c2430867d Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Wed, 6 Apr 2022 17:33:23 +0200 Subject: [PATCH 57/83] libfetchers: Fix assertion The filter expects all paths to have a prefix of the raw `actualUrl`, but `Store::addToStore(...)` provides absolute canonicalized paths. To fix this create an absolute and canonicalized path from the `actualUrl` and use it instead. Fixes #6195. --- src/libfetchers/git.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index d75c5d3ae..f8433bc28 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -285,9 +285,11 @@ struct GitInputScheme : InputScheme auto files = tokenizeString>( runProgram("git", true, gitOpts), "\0"s); + Path actualPath(absPath(actualUrl)); + PathFilter filter = [&](const Path & p) -> bool { - assert(hasPrefix(p, actualUrl)); - std::string file(p, actualUrl.size() + 1); + assert(hasPrefix(p, actualPath)); + std::string file(p, actualPath.size() + 1); auto st = lstat(p); @@ -300,13 +302,13 @@ struct GitInputScheme : InputScheme return files.count(file); }; - auto storePath = store->addToStore(input.getName(), actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); + auto storePath = store->addToStore(input.getName(), actualPath, FileIngestionMethod::Recursive, htSHA256, filter); // FIXME: maybe we should use the timestamp of the last // modified dirty file? input.attrs.insert_or_assign( "lastModified", - hasHead ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); + hasHead ? std::stoull(runProgram("git", true, { "-C", actualPath, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); return {std::move(storePath), input}; } From b9c969a86667a7b16a7efa1df0e3d090ef2ead72 Mon Sep 17 00:00:00 2001 From: Rehno Lindeque Date: Wed, 6 Apr 2022 12:20:39 -0400 Subject: [PATCH 58/83] nix flake check: Warn about deprecated nixosModule output --- src/nix/flake.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix/flake.cc b/src/nix/flake.cc index ce7dc101a..a876bb3af 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -508,6 +508,7 @@ struct CmdFlakeCheck : FlakeCommand name == "defaultBundler" ? "bundlers..default" : name == "overlay" ? "overlays.default" : name == "devShell" ? "devShells..default" : + name == "nixosModule" ? "nixosModules.default" : ""; if (replacement != "") warn("flake output attribute '%s' is deprecated; use '%s' instead", name, replacement); From 5ff4c42608bdf54d6f889f196f59a6c20c1631d6 Mon Sep 17 00:00:00 2001 From: Rehno Lindeque Date: Wed, 6 Apr 2022 12:24:35 -0400 Subject: [PATCH 59/83] Update release notes --- doc/manual/src/release-notes/rl-next.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 4f3c9ce41..8c8c0fd41 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -32,3 +32,11 @@ paths. Like fixed-output derivations, impure derivations have access to the network. Only fixed-output derivations and impure derivations can depend on an impure derivation. + +* The `nixosModule` flake output attribute has been renamed consistent + with the `.default` renames in nix 2.7. + + * `nixosModule` → `nixosModules.default` + + As before, the old output will continue to work, but `nix flake check` will + issue a warning about it. From 305d3a0ec3c7d53a5ceffee239c6cd4949f99423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 7 Apr 2022 17:31:12 +0200 Subject: [PATCH 60/83] Test fetchgit with path containing a `.` segment --- tests/fetchGit.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index ac23be5c0..9179e2071 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -7,7 +7,9 @@ fi clearStore -repo=$TEST_ROOT/git +# Intentionally not in a canonical form +# See https://github.com/NixOS/nix/issues/6195 +repo=$TEST_ROOT/./git export _NIX_FORCE_HTTP=1 From 2c2fd4946f96e6839ecbfb4cf61318d8910e7e8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20Ga=C5=82kowski?= Date: Wed, 6 Apr 2022 19:28:12 +0200 Subject: [PATCH 61/83] don't assume that rev is a SHA1 hash This was a problem when writing a fetcher that uses e.g. sha256 hashes for revisions. This doesn't actually do anything new, but allows for creating such fetchers in the future (perhaps when support for Git's SHA256 object format gains more popularity). --- src/libfetchers/fetchers.cc | 15 ++++++++++++--- src/libutil/hash.cc | 2 +- src/libutil/hash.hh | 2 -- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 976f40d3b..6957d2da4 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -238,9 +238,18 @@ std::optional Input::getRef() const std::optional Input::getRev() const { - if (auto s = maybeGetStrAttr(attrs, "rev")) - return Hash::parseAny(*s, htSHA1); - return {}; + std::optional hash = {}; + + if (auto s = maybeGetStrAttr(attrs, "rev")) { + try { + hash = Hash::parseAnyPrefixed(*s); + } catch (BadHash &e) { + // Default to sha1 for backwards compatibility with existing flakes + hash = Hash::parseAny(*s, htSHA1); + } + } + + return hash; } std::optional Input::getRevCount() const diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index a4d632161..d2fd0c15a 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -155,7 +155,7 @@ static std::pair, bool> getParsedTypeAndSRI(std::string_ { bool isSRI = false; - // Parse the has type before the separater, if there was one. + // Parse the hash type before the separator, if there was one. std::optional optParsedType; { auto hashRaw = splitPrefixTo(rest, ':'); diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 56b5938b3..00f70a572 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -93,13 +93,11 @@ public: std::string gitRev() const { - assert(type == htSHA1); return to_string(Base16, false); } std::string gitShortRev() const { - assert(type == htSHA1); return std::string(to_string(Base16, false), 0, 7); } From 4f29cf1a1d906f35054f8b318df7c0d3a9117bb3 Mon Sep 17 00:00:00 2001 From: Martin Schwaighofer Date: Thu, 7 Apr 2022 11:28:20 +0200 Subject: [PATCH 62/83] installer: ask for confirmation on multi-user install without systemd On Linux a user can go through all the way through the multi-user install and find out at the end that they now have to manually configure their init system to launch the nix daemon. I suspect that for a significant number of users this is not what they wanted. They might prefer a single-user install. Now they have to manually uninstall nix before they can go through the single-user install. This introduces a confirmation dialog before the install in that specific situation to make sure that they want to proceed. See also: https://github.com/NixOS/nix/issues/4999#issuecomment-1064188080 This closes #4999 but rejecting it and closing that issue anyways would also be valid. --- scripts/install-multi-user.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 69b6676ea..b79a9c23a 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -423,6 +423,18 @@ EOF fi done + if [ "$(uname -s)" = "Linux" ] && [ ! -e /run/systemd/system ]; then + warning < Date: Wed, 6 Apr 2022 13:02:26 +0200 Subject: [PATCH 63/83] Remove unused Error.name field --- src/libstore/sqlite.cc | 1 - src/libutil/error.cc | 3 --- src/libutil/error.hh | 1 - src/libutil/serialise.cc | 5 ++--- 4 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index e6ecadd7f..1d82b4ab1 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -215,7 +215,6 @@ void handleSQLiteBusy(const SQLiteBusy & e) if (now > lastWarned + 10) { lastWarned = now; logWarning({ - .name = "Sqlite busy", .msg = hintfmt(e.what()) }); } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 02bc5caa5..9172f67a6 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -21,12 +21,9 @@ const std::string & BaseError::calcWhat() const if (what_.has_value()) return *what_; else { - err.name = sname(); - std::ostringstream oss; showErrorInfo(oss, err, loggerSettings.showTrace); what_ = oss.str(); - return *what_; } } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 6a757f9ad..005c2c225 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -109,7 +109,6 @@ struct Trace { struct ErrorInfo { Verbosity level; - std::string name; // FIXME: rename hintformat msg; std::optional errPos; std::list traces; diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 6445b3f1b..8ff904583 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -357,7 +357,7 @@ Sink & operator << (Sink & sink, const Error & ex) sink << "Error" << info.level - << info.name + << "Error" // removed << info.msg.str() << 0 // FIXME: info.errPos << info.traces.size(); @@ -426,11 +426,10 @@ Error readError(Source & source) auto type = readString(source); assert(type == "Error"); auto level = (Verbosity) readInt(source); - auto name = readString(source); + auto name = readString(source); // removed auto msg = readString(source); ErrorInfo info { .level = level, - .name = name, .msg = hintformat(std::move(format("%s") % msg)), }; auto havePos = readNum(source); From 8bd9ebf52c9f7c5381d071250660a48d133b5e87 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 15:05:08 +0200 Subject: [PATCH 64/83] Error: Remove unused sname() method --- src/libstore/filetransfer.hh | 2 -- src/libutil/error.hh | 5 ----- src/libutil/experimental-features.hh | 4 ---- 3 files changed, 11 deletions(-) diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index ca61e3937..1ad96bc10 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -123,8 +123,6 @@ public: template FileTransferError(FileTransfer::Error error, std::optional response, const Args & ... args); - - virtual const char* sname() const override { return "FileTransferError"; } }; bool isUri(std::string_view s); diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 005c2c225..348018f57 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -161,8 +161,6 @@ public: : err(e) { } - virtual const char* sname() const { return "BaseError"; } - #ifdef EXCEPTION_NEEDS_THROW_SPEC ~BaseError() throw () { }; const char * what() const throw () { return calcWhat().c_str(); } @@ -189,7 +187,6 @@ public: { \ public: \ using superClass::superClass; \ - virtual const char* sname() const override { return #newClass; } \ } MakeError(Error, BaseError); @@ -209,8 +206,6 @@ public: auto hf = hintfmt(args...); err.msg = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo)); } - - virtual const char* sname() const override { return "SysError"; } }; } diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index 3a254b423..266e41a22 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -49,10 +49,6 @@ public: ExperimentalFeature missingFeature; MissingExperimentalFeature(ExperimentalFeature); - virtual const char * sname() const override - { - return "MissingExperimentalFeature"; - } }; } From c68963eaea7005b5bfb954e50fefb36c36084414 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 Apr 2022 11:48:30 +0200 Subject: [PATCH 65/83] Remove duplicate "error:" --- src/libstore/build-result.hh | 2 ++ src/libstore/build/derivation-goal.cc | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index 7f9bcce6d..24fb1f763 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -31,6 +31,8 @@ struct BuildResult ResolvesToAlreadyValid, NoSubstituters, } status = MiscFailure; + + // FIXME: include entire ErrorInfo object. std::string errorMsg; std::string toString() const { diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 6582497bd..53f212c1d 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1371,8 +1371,7 @@ void DerivationGoal::done( { buildResult.status = status; if (ex) - // FIXME: strip: "error: " - buildResult.errorMsg = ex->what(); + buildResult.errorMsg = fmt("%s", normaltxt(ex->info().msg)); if (buildResult.status == BuildResult::TimedOut) worker.timedOut = true; if (buildResult.status == BuildResult::PermanentFailure) From f3d3587ab39130e8d87d290b8cedabb3e6e9d715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 7 Apr 2022 21:09:39 +0200 Subject: [PATCH 66/83] Allow empty path segments in urls Valid per https://datatracker.ietf.org/doc/html/rfc3986#section-3.3 (and also somewhat frequently happening for local paths) --- src/libutil/tests/url.cc | 4 ++-- src/libutil/url-parts.hh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libutil/tests/url.cc b/src/libutil/tests/url.cc index f20e2dc41..c3b233797 100644 --- a/src/libutil/tests/url.cc +++ b/src/libutil/tests/url.cc @@ -178,7 +178,7 @@ namespace nix { } TEST(parseURL, parseFileURLWithQueryAndFragment) { - auto s = "file:///none/of/your/business"; + auto s = "file:///none/of//your/business"; auto parsed = parseURL(s); ParsedURL expected { @@ -186,7 +186,7 @@ namespace nix { .base = "", .scheme = "file", .authority = "", - .path = "/none/of/your/business", + .path = "/none/of//your/business", .query = (StringMap) { }, .fragment = "", }; diff --git a/src/libutil/url-parts.hh b/src/libutil/url-parts.hh index da10a6bbc..d5e6a2736 100644 --- a/src/libutil/url-parts.hh +++ b/src/libutil/url-parts.hh @@ -18,7 +18,7 @@ const static std::string userRegex = "(?:(?:" + unreservedRegex + "|" + pctEncod const static std::string authorityRegex = "(?:" + userRegex + "@)?" + hostRegex + "(?::[0-9]+)?"; const static std::string pcharRegex = "(?:" + unreservedRegex + "|" + pctEncoded + "|" + subdelimsRegex + "|[:@])"; const static std::string queryRegex = "(?:" + pcharRegex + "|[/? \"])*"; -const static std::string segmentRegex = "(?:" + pcharRegex + "+)"; +const static std::string segmentRegex = "(?:" + pcharRegex + "*)"; const static std::string absPathRegex = "(?:(?:/" + segmentRegex + ")*/?)"; const static std::string pathRegex = "(?:" + segmentRegex + "(?:/" + segmentRegex + ")*/?)"; From 770f7371f33a56f7214001981a698113477ccec4 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Sat, 9 Apr 2022 17:00:14 +0200 Subject: [PATCH 67/83] libfetchers: Replace regex to clarify intent --- src/libfetchers/git.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index f8433bc28..91dd332a2 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -28,9 +28,7 @@ static std::string readHead(const Path & path) static bool isNotDotGitDirectory(const Path & path) { - static const std::regex gitDirRegex("^(?:.*/)?\\.git$"); - - return not std::regex_match(path, gitDirRegex); + return baseNameOf(path) != ".git"; } struct GitInputScheme : InputScheme From d6b75295797af332f2cba635531b2019571319e2 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Sat, 9 Apr 2022 19:10:23 +0200 Subject: [PATCH 68/83] libfetchers: Fix assertion (Mercurial) See commit 1e1cd6e7a for more information. --- src/libfetchers/mercurial.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 8b82e9daa..51cf35bf4 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -178,9 +178,11 @@ struct MercurialInputScheme : InputScheme auto files = tokenizeString>( runHg({ "status", "-R", actualUrl, "--clean", "--modified", "--added", "--no-status", "--print0" }), "\0"s); + Path actualPath(absPath(actualUrl)); + PathFilter filter = [&](const Path & p) -> bool { - assert(hasPrefix(p, actualUrl)); - std::string file(p, actualUrl.size() + 1); + assert(hasPrefix(p, actualPath)); + std::string file(p, actualPath.size() + 1); auto st = lstat(p); @@ -193,7 +195,7 @@ struct MercurialInputScheme : InputScheme return files.count(file); }; - auto storePath = store->addToStore(input.getName(), actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); + auto storePath = store->addToStore(input.getName(), actualPath, FileIngestionMethod::Recursive, htSHA256, filter); return {std::move(storePath), input}; } From 38125a47ab512446dd78d3d0f1ed2d52e1d9cbd2 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Sat, 9 Apr 2022 23:39:00 +0200 Subject: [PATCH 69/83] Test fetchMercurial with path containing a `.` segment --- tests/fetchMercurial.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/fetchMercurial.sh b/tests/fetchMercurial.sh index 726840664..5c64ffd26 100644 --- a/tests/fetchMercurial.sh +++ b/tests/fetchMercurial.sh @@ -7,7 +7,9 @@ fi clearStore -repo=$TEST_ROOT/hg +# Intentionally not in a canonical form +# See https://github.com/NixOS/nix/issues/6195 +repo=$TEST_ROOT/./hg rm -rf $repo ${repo}-tmp $TEST_HOME/.cache/nix @@ -28,6 +30,12 @@ echo world > $repo/hello hg commit --cwd $repo -m 'Bla2' rev2=$(hg log --cwd $repo -r tip --template '{node}') +# Fetch an unclean branch. +echo unclean > $repo/hello +path=$(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).outPath") +[[ $(cat $path/hello) = unclean ]] +hg revert --cwd $repo --all + # Fetch the default branch. path=$(nix eval --impure --raw --expr "(builtins.fetchMercurial file://$repo).outPath") [[ $(cat $path/hello) = world ]] From 63d9a818194f940fcd2da8fb68bef303c984f300 Mon Sep 17 00:00:00 2001 From: Sebastian Blunt Date: Sun, 10 Apr 2022 21:09:04 -0700 Subject: [PATCH 70/83] Log builder args and environment variables Previously it only logged the builder's path, this changes it to log the arguments at the same log level, and the environment variables at the vomit level. This helped me debug https://github.com/svanderburg/node2nix/issues/75 --- src/libstore/build/local-derivation-goal.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 40ef706a6..4c91fa4fb 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -704,6 +704,9 @@ void LocalDerivationGoal::startBuilder() /* Run the builder. */ printMsg(lvlChatty, "executing builder '%1%'", drv->builder); + printMsg(lvlChatty, "using builder args '%1%'", concatStringsSep(" ", drv->args)); + for (auto & i : drv->env) + printMsg(lvlVomit, "setting builder env variable '%1%'='%2%'", i.first, i.second); /* Create the log file. */ Path logFile = openLogFile(); From 5fc73c276b369aee13185ae6c9c8faed35490460 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 11 Apr 2022 22:01:20 +0000 Subject: [PATCH 71/83] build(deps): bump cachix/install-nix-action from 16 to 17 Bumps [cachix/install-nix-action](https://github.com/cachix/install-nix-action) from 16 to 17. - [Release notes](https://github.com/cachix/install-nix-action/releases) - [Commits](https://github.com/cachix/install-nix-action/compare/v16...v17) --- updated-dependencies: - dependency-name: cachix/install-nix-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 09436b7e3..4ecb050dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v2.4.0 with: fetch-depth: 0 - - uses: cachix/install-nix-action@v16 + - uses: cachix/install-nix-action@v17 - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV - uses: cachix/cachix-action@v10 if: needs.check_cachix.outputs.secret == 'true' @@ -50,7 +50,7 @@ jobs: with: fetch-depth: 0 - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV - - uses: cachix/install-nix-action@v16 + - uses: cachix/install-nix-action@v17 - uses: cachix/cachix-action@v10 with: name: '${{ env.CACHIX_NAME }}' @@ -69,7 +69,7 @@ jobs: steps: - uses: actions/checkout@v2.4.0 - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV - - uses: cachix/install-nix-action@v16 + - uses: cachix/install-nix-action@v17 with: install_url: '${{needs.installer.outputs.installerURL}}' install_options: "--tarball-url-prefix https://${{ env.CACHIX_NAME }}.cachix.org/serve" @@ -86,7 +86,7 @@ jobs: - uses: actions/checkout@v2.4.0 with: fetch-depth: 0 - - uses: cachix/install-nix-action@v16 + - uses: cachix/install-nix-action@v17 - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV - run: echo NIX_VERSION="$(nix-instantiate --eval -E '(import ./default.nix).defaultPackage.${builtins.currentSystem}.version' | tr -d \")" >> $GITHUB_ENV - uses: cachix/cachix-action@v10 From 2769e43f61d827e1b8fe46257450986d76319243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20Ga=C5=82kowski?= Date: Fri, 8 Apr 2022 19:38:43 +0200 Subject: [PATCH 72/83] assert hash types for Git and Mercurial --- src/libfetchers/git.cc | 8 ++++++++ src/libfetchers/mercurial.cc | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index d75c5d3ae..220442479 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -189,8 +189,16 @@ struct GitInputScheme : InputScheme if (submodules) cacheType += "-submodules"; if (allRefs) cacheType += "-all-refs"; + auto checkHashType = [&](const std::optional & hash) + { + if (hash.has_value() && !(hash->type == htSHA1 || hash->type == htSHA256)) + throw Error("Hash '%s' is not supported by Git. Supported types are sha1 and sha256.", hash->to_string(Base16, true)); + }; + auto getLockedAttrs = [&]() { + checkHashType(input.getRev()); + return Attrs({ {"type", cacheType}, {"name", name}, diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 8b82e9daa..19ff98030 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -201,8 +201,17 @@ struct MercurialInputScheme : InputScheme if (!input.getRef()) input.attrs.insert_or_assign("ref", "default"); + auto checkHashType = [&](const std::optional & hash) + { + if (hash.has_value() && hash->type != htSHA1) + throw Error("Hash '%s' is not supported by Mercurial. Only sha1 is supported.", hash->to_string(Base16, true)); + }; + + auto getLockedAttrs = [&]() { + checkHashType(input.getRev()); + return Attrs({ {"type", "hg"}, {"name", name}, From dc9510c8d73e5180172359a577fb96d01bbada1d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 13 Apr 2022 12:10:29 +0000 Subject: [PATCH 73/83] Bump actions/checkout from 2 to 3 Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 3. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v2...v3) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/backport.yml | 2 +- .github/workflows/ci.yml | 8 ++++---- .github/workflows/hydra_status.yml | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/backport.yml b/.github/workflows/backport.yml index ec7ab4516..dd481160f 100644 --- a/.github/workflows/backport.yml +++ b/.github/workflows/backport.yml @@ -8,7 +8,7 @@ jobs: if: github.repository_owner == 'NixOS' && github.event.pull_request.merged == true && (github.event_name != 'labeled' || startsWith('backport', github.event.label.name)) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 with: ref: ${{ github.event.pull_request.head.sha }} # required to find all branches diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4ecb050dd..d01ef4768 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: runs-on: ${{ matrix.os }} timeout-minutes: 60 steps: - - uses: actions/checkout@v2.4.0 + - uses: actions/checkout@v3 with: fetch-depth: 0 - uses: cachix/install-nix-action@v17 @@ -46,7 +46,7 @@ jobs: outputs: installerURL: ${{ steps.prepare-installer.outputs.installerURL }} steps: - - uses: actions/checkout@v2.4.0 + - uses: actions/checkout@v3 with: fetch-depth: 0 - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV @@ -67,7 +67,7 @@ jobs: os: [ubuntu-latest, macos-latest] runs-on: ${{ matrix.os }} steps: - - uses: actions/checkout@v2.4.0 + - uses: actions/checkout@v3 - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV - uses: cachix/install-nix-action@v17 with: @@ -83,7 +83,7 @@ jobs: needs.check_cachix.outputs.secret == 'true' runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2.4.0 + - uses: actions/checkout@v3 with: fetch-depth: 0 - uses: cachix/install-nix-action@v17 diff --git a/.github/workflows/hydra_status.yml b/.github/workflows/hydra_status.yml index b97076bd7..53e69cb2d 100644 --- a/.github/workflows/hydra_status.yml +++ b/.github/workflows/hydra_status.yml @@ -9,7 +9,7 @@ jobs: if: github.repository_owner == 'NixOS' runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2.4.0 + - uses: actions/checkout@v3 with: fetch-depth: 0 - run: bash scripts/check-hydra-status.sh From d89840b103e57e81e5245c3fe9edfbf7c3477ad5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 14 Apr 2022 14:04:19 +0200 Subject: [PATCH 74/83] Make InstallableFlake::toValue() and toDerivation() behave consistently In particular, this means that 'nix eval` (which uses toValue()) no longer auto-calls functions or functors (because AttrCursor::findAlongAttrPath() doesn't). Fixes #6152. Also use ref<> in a few places, and don't return attrpaths from getCursor() because cursors already have a getAttrPath() method. --- src/libcmd/installables.cc | 117 +++++++++++++++++-------------------- src/libcmd/installables.hh | 12 +++- src/libexpr/eval-cache.cc | 4 +- src/libexpr/eval-cache.hh | 4 +- src/nix/app.cc | 4 +- src/nix/flake.cc | 2 +- src/nix/search.cc | 4 +- 7 files changed, 72 insertions(+), 75 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 955bbe6fb..1bb5d6300 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -334,16 +334,16 @@ DerivedPath Installable::toDerivedPath() return std::move(buildables[0]); } -std::vector, std::string>> +std::vector> Installable::getCursors(EvalState & state) { auto evalCache = std::make_shared(std::nullopt, state, [&]() { return toValue(state).first; }); - return {{evalCache->getRoot(), ""}}; + return {evalCache->getRoot()}; } -std::pair, std::string> +ref Installable::getCursor(EvalState & state) { auto cursors = getCursors(state); @@ -566,43 +566,21 @@ InstallableFlake::InstallableFlake( std::tuple InstallableFlake::toDerivation() { - auto lockedFlake = getLockedFlake(); + auto attr = getCursor(*state); - auto cache = openEvalCache(*state, lockedFlake); - auto root = cache->getRoot(); + auto attrPath = attr->getAttrPathStr(); - Suggestions suggestions; + if (!attr->isDerivation()) + throw Error("flake output attribute '%s' is not a derivation", attrPath); - for (auto & attrPath : getActualAttrPaths()) { - debug("trying flake output attribute '%s'", attrPath); + auto drvPath = attr->forceDerivation(); - auto attrOrSuggestions = root->findAlongAttrPath( - parseAttrPath(*state, attrPath), - true - ); + auto drvInfo = DerivationInfo { + std::move(drvPath), + attr->getAttr(state->sOutputName)->getString() + }; - if (!attrOrSuggestions) { - suggestions += attrOrSuggestions.getSuggestions(); - continue; - } - - auto attr = *attrOrSuggestions; - - if (!attr->isDerivation()) - throw Error("flake output attribute '%s' is not a derivation", attrPath); - - auto drvPath = attr->forceDerivation(); - - auto drvInfo = DerivationInfo { - std::move(drvPath), - attr->getAttr(state->sOutputName)->getString() - }; - - return {attrPath, lockedFlake->flake.lockedRef, std::move(drvInfo)}; - } - - throw Error(suggestions, "flake '%s' does not provide attribute %s", - flakeRef, showAttrPaths(getActualAttrPaths())); + return {attrPath, getLockedFlake()->flake.lockedRef, std::move(drvInfo)}; } std::vector InstallableFlake::toDerivations() @@ -614,33 +592,10 @@ std::vector InstallableFlake::toDerivations() std::pair InstallableFlake::toValue(EvalState & state) { - auto lockedFlake = getLockedFlake(); - - auto vOutputs = getFlakeOutputs(state, *lockedFlake); - - auto emptyArgs = state.allocBindings(0); - - Suggestions suggestions; - - for (auto & attrPath : getActualAttrPaths()) { - try { - auto [v, pos] = findAlongAttrPath(state, attrPath, *emptyArgs, *vOutputs); - state.forceValue(*v, pos); - return {v, pos}; - } catch (AttrPathNotFound & e) { - suggestions += e.info().suggestions; - } - } - - throw Error( - suggestions, - "flake '%s' does not provide attribute %s", - flakeRef, - showAttrPaths(getActualAttrPaths()) - ); + return {&getCursor(state)->forceValue(), noPos}; } -std::vector, std::string>> +std::vector> InstallableFlake::getCursors(EvalState & state) { auto evalCache = openEvalCache(state, @@ -648,21 +603,55 @@ InstallableFlake::getCursors(EvalState & state) auto root = evalCache->getRoot(); - std::vector, std::string>> res; + std::vector> res; for (auto & attrPath : getActualAttrPaths()) { auto attr = root->findAlongAttrPath(parseAttrPath(state, attrPath)); - if (attr) res.push_back({*attr, attrPath}); + if (attr) res.push_back(ref(*attr)); } return res; } +ref InstallableFlake::getCursor(EvalState & state) +{ + auto lockedFlake = getLockedFlake(); + + auto cache = openEvalCache(state, lockedFlake); + auto root = cache->getRoot(); + + Suggestions suggestions; + + auto attrPaths = getActualAttrPaths(); + + for (auto & attrPath : attrPaths) { + debug("trying flake output attribute '%s'", attrPath); + + auto attrOrSuggestions = root->findAlongAttrPath( + parseAttrPath(state, attrPath), + true + ); + + if (!attrOrSuggestions) { + suggestions += attrOrSuggestions.getSuggestions(); + continue; + } + + return *attrOrSuggestions; + } + + throw Error( + suggestions, + "flake '%s' does not provide attribute %s", + flakeRef, + showAttrPaths(attrPaths)); +} + std::shared_ptr InstallableFlake::getLockedFlake() const { - flake::LockFlags lockFlagsApplyConfig = lockFlags; - lockFlagsApplyConfig.applyNixConfig = true; if (!_lockedFlake) { + flake::LockFlags lockFlagsApplyConfig = lockFlags; + lockFlagsApplyConfig.applyNixConfig = true; _lockedFlake = std::make_shared(lockFlake(*state, flakeRef, lockFlagsApplyConfig)); } return _lockedFlake; diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index f4bf0d406..b847f8939 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -80,10 +80,10 @@ struct Installable return {}; } - virtual std::vector, std::string>> + virtual std::vector> getCursors(EvalState & state); - std::pair, std::string> + virtual ref getCursor(EvalState & state); virtual FlakeRef nixpkgsFlakeRef() const @@ -180,9 +180,15 @@ struct InstallableFlake : InstallableValue std::pair toValue(EvalState & state) override; - std::vector, std::string>> + /* Get a cursor to every attrpath in getActualAttrPaths() that + exists. */ + std::vector> getCursors(EvalState & state) override; + /* Get a cursor to the first attrpath in getActualAttrPaths() that + exists, or throw an exception with suggestions if none exists. */ + ref getCursor(EvalState & state) override; + std::shared_ptr getLockedFlake() const; FlakeRef nixpkgsFlakeRef() const override; diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 54fa9b741..7d3fd01a4 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -306,9 +306,9 @@ Value * EvalCache::getRootValue() return *value; } -std::shared_ptr EvalCache::getRoot() +ref EvalCache::getRoot() { - return std::make_shared(ref(shared_from_this()), std::nullopt); + return make_ref(ref(shared_from_this()), std::nullopt); } AttrCursor::AttrCursor( diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index c9a9bf471..b0709ebc2 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -33,7 +33,7 @@ public: EvalState & state, RootLoader rootLoader); - std::shared_ptr getRoot(); + ref getRoot(); }; enum AttrType { @@ -104,6 +104,8 @@ public: ref getAttr(std::string_view name); + /* Get an attribute along a chain of attrsets. Note that this does + not auto-call functors or functions. */ OrSuggestions> findAlongAttrPath(const std::vector & attrPath, bool force = false); std::string getString(); diff --git a/src/nix/app.cc b/src/nix/app.cc index 803d028f0..55efccdee 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -61,7 +61,7 @@ std::string resolveString(Store & store, const std::string & toResolve, const Bu UnresolvedApp Installable::toApp(EvalState & state) { - auto [cursor, attrPath] = getCursor(state); + auto cursor = getCursor(state); auto type = cursor->getAttr("type")->getString(); @@ -101,7 +101,7 @@ UnresolvedApp Installable::toApp(EvalState & state) } else - throw Error("attribute '%s' has unsupported type '%s'", attrPath, type); + throw Error("attribute '%s' has unsupported type '%s'", cursor->getAttrPathStr(), type); } // FIXME: move to libcmd diff --git a/src/nix/flake.cc b/src/nix/flake.cc index a876bb3af..66c315e5a 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -705,7 +705,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand defaultTemplateAttrPathsPrefixes, lockFlags); - auto [cursor, attrPath] = installable.getCursor(*evalState); + auto cursor = installable.getCursor(*evalState); auto templateDirAttr = cursor->getAttr("path"); auto templateDir = templateDirAttr->getString(); diff --git a/src/nix/search.cc b/src/nix/search.cc index e9307342c..e96a85ea2 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -165,8 +165,8 @@ struct CmdSearch : InstallableCommand, MixJSON } }; - for (auto & [cursor, prefix] : installable->getCursors(*state)) - visit(*cursor, parseAttrPath(*state, prefix), true); + for (auto & cursor : installable->getCursors(*state)) + visit(*cursor, cursor->getAttrPath(), true); if (!json && !results) throw Error("no results for the given search term(s)!"); From 9b41239d8fdcc3fe50febe718c15833ebc224354 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Fri, 25 Mar 2022 13:36:41 -0400 Subject: [PATCH 75/83] fix: ensure apps are apps and packages are packages --- doc/manual/src/release-notes/rl-next.md | 4 ++++ src/nix/app.cc | 9 ++++++++ tests/flakes-run.sh | 29 +++++++++++++++++++++++++ tests/local.mk | 1 + 4 files changed, 43 insertions(+) create mode 100644 tests/flakes-run.sh diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 8c8c0fd41..6ebbe5eb4 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -40,3 +40,7 @@ As before, the old output will continue to work, but `nix flake check` will issue a warning about it. + +* `nix run` is now stricter wrt what it accepts: + * Members of `apps` are now required to be apps (as defined in [the manual](https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-run.html#apps)) + * Member of `packages` or `legacyPackages` cannot be of type "app" when used by `nix run`. diff --git a/src/nix/app.cc b/src/nix/app.cc index 803d028f0..a43566496 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -65,6 +65,15 @@ UnresolvedApp Installable::toApp(EvalState & state) auto type = cursor->getAttr("type")->getString(); + std::string expected; + if (hasPrefix(attrPath,"apps.")) { + expected = "app"; + } else { + expected = "derivation"; + } + if (type != expected) { + throw Error("Attribute '%s' should have type '%s'.", attrPath, expected); + } if (type == "app") { auto [program, context] = cursor->getAttr("program")->getStringWithContext(); diff --git a/tests/flakes-run.sh b/tests/flakes-run.sh new file mode 100644 index 000000000..c8035431c --- /dev/null +++ b/tests/flakes-run.sh @@ -0,0 +1,29 @@ +source common.sh + +clearStore +rm -rf $TEST_HOME/.cache $TEST_HOME/.config $TEST_HOME/.local +cp ./shell-hello.nix ./config.nix $TEST_HOME +cd $TEST_HOME + +cat < flake.nix +{ + outputs = {self}: { + packages.$system.PkgAsPkg = (import ./shell-hello.nix).hello; + packages.$system.AppAsApp = self.packages.$system.AppAsApp; + + apps.$system.PkgAsApp = self.packages.$system.PkgAsPkg; + apps.$system.AppAsApp = { + type = "app"; + program = "\${(import ./shell-hello.nix).hello}/bin/hello"; + }; + }; +} +EOF +nix run --no-write-lock-file .#AppAsApp +nix run --no-write-lock-file .#PkgAsPkg + +! nix run --no-write-lock-file .#PkgAsApp || fail "'nix run' shouldn’t accept an 'app' defined under 'packages'" +! nix run --no-write-lock-file .#AppAsPkg || fail "elements of 'apps' should be of type 'app'" + +clearStore + diff --git a/tests/local.mk b/tests/local.mk index 668b34500..51536188c 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -1,5 +1,6 @@ nix_tests = \ flakes.sh \ + flakes-run.sh \ ca/gc.sh \ gc.sh \ remote-store.sh \ From 25c85f5a0e838bf34a20804e06ce7e6574bdfd74 Mon Sep 17 00:00:00 2001 From: Alex Ameen Date: Sun, 17 Apr 2022 17:14:38 -0500 Subject: [PATCH 76/83] doc: document nix.conf connect-timeout default --- src/libstore/filetransfer.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index 1ad96bc10..fcdf1c9d1 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -31,7 +31,7 @@ struct FileTransferSettings : Config R"( The timeout (in seconds) for establishing connections in the binary cache substituter. It corresponds to `curl`’s - `--connect-timeout` option. + `--connect-timeout` option. The default 0 means no limit. )"}; Setting stalledDownloadTimeout{ From e5c934cd48c25eff28c956e414af9b5ff1c6523a Mon Sep 17 00:00:00 2001 From: Alex Ameen Date: Sun, 17 Apr 2022 18:17:37 -0500 Subject: [PATCH 77/83] doc: rephrase connect-timeout help message Co-authored-by: Cole Helbling --- src/libstore/filetransfer.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index fcdf1c9d1..40e7cf52c 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -31,7 +31,7 @@ struct FileTransferSettings : Config R"( The timeout (in seconds) for establishing connections in the binary cache substituter. It corresponds to `curl`’s - `--connect-timeout` option. The default 0 means no limit. + `--connect-timeout` option. A value of 0 means no limit. )"}; Setting stalledDownloadTimeout{ From 8b659eacce046326fa510f83b505b0ad326e60ef Mon Sep 17 00:00:00 2001 From: Robert Helgesson Date: Mon, 18 Apr 2022 17:14:15 +0200 Subject: [PATCH 78/83] Add .tgz as tarball extension in documentation Support for the `tgz` shorthand was added in 52f5fa948a4784b6a9b707770f4beee6a8674dee. --- src/nix/flake.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/flake.md b/src/nix/flake.md index d59915eeb..7d179a6c4 100644 --- a/src/nix/flake.md +++ b/src/nix/flake.md @@ -177,8 +177,8 @@ Currently the `type` attribute can be one of the following: attribute `url`. In URL form, the schema must be `http://`, `https://` or `file://` - URLs and the extension must be `.zip`, `.tar`, `.tar.gz`, `.tar.xz`, - `.tar.bz2` or `.tar.zst`. + URLs and the extension must be `.zip`, `.tar`, `.tgz`, `.tar.gz`, + `.tar.xz`, `.tar.bz2` or `.tar.zst`. * `github`: A more efficient way to fetch repositories from GitHub. The following attributes are required: From 2016b7142ad1282981ad1505085fff0ac9c7d66c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 19 Apr 2022 12:09:12 +0200 Subject: [PATCH 79/83] Fix compilation, style fixes --- src/nix/app.cc | 15 +++++---------- tests/flakes-run.sh | 16 ++++++++-------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/nix/app.cc b/src/nix/app.cc index bd6988066..6b6b31a12 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -62,22 +62,17 @@ std::string resolveString(Store & store, const std::string & toResolve, const Bu UnresolvedApp Installable::toApp(EvalState & state) { auto cursor = getCursor(state); + auto attrPath = cursor->getAttrPath(); auto type = cursor->getAttr("type")->getString(); - std::string expected; - if (hasPrefix(attrPath,"apps.")) { - expected = "app"; - } else { - expected = "derivation"; - } - if (type != expected) { - throw Error("Attribute '%s' should have type '%s'.", attrPath, expected); - } + std::string expected = !attrPath.empty() && attrPath[0] == "apps" ? "app" : "derivation"; + if (type != expected) + throw Error("attribute '%s' should have type '%s'", cursor->getAttrPathStr(), expected); + if (type == "app") { auto [program, context] = cursor->getAttr("program")->getStringWithContext(); - std::vector context2; for (auto & [path, name] : context) context2.push_back({path, {name}}); diff --git a/tests/flakes-run.sh b/tests/flakes-run.sh index c8035431c..88fc3e628 100644 --- a/tests/flakes-run.sh +++ b/tests/flakes-run.sh @@ -8,22 +8,22 @@ cd $TEST_HOME cat < flake.nix { outputs = {self}: { - packages.$system.PkgAsPkg = (import ./shell-hello.nix).hello; - packages.$system.AppAsApp = self.packages.$system.AppAsApp; + packages.$system.pkgAsPkg = (import ./shell-hello.nix).hello; + packages.$system.appAsApp = self.packages.$system.appAsApp; - apps.$system.PkgAsApp = self.packages.$system.PkgAsPkg; - apps.$system.AppAsApp = { + apps.$system.pkgAsApp = self.packages.$system.pkgAsPkg; + apps.$system.appAsApp = { type = "app"; program = "\${(import ./shell-hello.nix).hello}/bin/hello"; }; }; } EOF -nix run --no-write-lock-file .#AppAsApp -nix run --no-write-lock-file .#PkgAsPkg +nix run --no-write-lock-file .#appAsApp +nix run --no-write-lock-file .#pkgAsPkg -! nix run --no-write-lock-file .#PkgAsApp || fail "'nix run' shouldn’t accept an 'app' defined under 'packages'" -! nix run --no-write-lock-file .#AppAsPkg || fail "elements of 'apps' should be of type 'app'" +! nix run --no-write-lock-file .#pkgAsApp || fail "'nix run' shouldn’t accept an 'app' defined under 'packages'" +! nix run --no-write-lock-file .#appAsPkg || fail "elements of 'apps' should be of type 'app'" clearStore From c9e58aa5ff75351a5bb5af1258e019beef13721a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 19 Apr 2022 20:48:13 +0200 Subject: [PATCH 80/83] Require formatters to be packages Because of 9b41239d8fdcc3fe50febe718c15833ebc224354, a formatter can no longer be a package *or* an app. So let's require it to be a package for now. --- src/nix/app.cc | 4 ++-- src/nix/flake.cc | 3 +-- tests/fmt.sh | 10 ++++++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/nix/app.cc b/src/nix/app.cc index 6b6b31a12..df7303e15 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -35,7 +35,7 @@ struct InstallableDerivedPath : Installable /** * Return the rewrites that are needed to resolve a string whose context is - * included in `dependencies` + * included in `dependencies`. */ StringPairs resolveRewrites(Store & store, const BuiltPaths dependencies) { @@ -51,7 +51,7 @@ StringPairs resolveRewrites(Store & store, const BuiltPaths dependencies) } /** - * Resolve the given string assuming the given context + * Resolve the given string assuming the given context. */ std::string resolveString(Store & store, const std::string & toResolve, const BuiltPaths dependencies) { diff --git a/src/nix/flake.cc b/src/nix/flake.cc index dbd157248..04b23ed0f 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -1038,7 +1038,7 @@ struct CmdFlakeShow : FlakeCommand, MixJSON } else if ( - (attrPath.size() == 2 && (attrPath[0] == "defaultPackage" || attrPath[0] == "devShell")) + (attrPath.size() == 2 && (attrPath[0] == "defaultPackage" || attrPath[0] == "devShell" || attrPath[0] == "formatter")) || (attrPath.size() == 3 && (attrPath[0] == "checks" || attrPath[0] == "packages" || attrPath[0] == "devShells")) ) { @@ -1071,7 +1071,6 @@ struct CmdFlakeShow : FlakeCommand, MixJSON else if ( (attrPath.size() == 2 && attrPath[0] == "defaultApp") || - (attrPath.size() == 2 && attrPath[0] == "formatter") || (attrPath.size() == 3 && attrPath[0] == "apps")) { auto aType = visitor.maybeGetAttr("type"); diff --git a/tests/fmt.sh b/tests/fmt.sh index 7df1c82d3..2b482f14a 100644 --- a/tests/fmt.sh +++ b/tests/fmt.sh @@ -14,10 +14,12 @@ nix fmt --help | grep "Format" cat << EOF > flake.nix { outputs = _: { - formatter.$system = { - type = "app"; - program = ./fmt.simple.sh; - }; + formatter.$system = + with import ./config.nix; + mkDerivation { + name = "formatter"; + buildCommand = "mkdir -p \$out/bin; cp \${./fmt.simple.sh} \$out/bin/formatter"; + }; }; } EOF From 1cdad1074c42bb0b086f09b083db2f3c0f930b0e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 19 Apr 2022 21:12:33 +0200 Subject: [PATCH 81/83] Move rl-next.md to rl-2.8.md --- doc/manual/src/SUMMARY.md.in | 1 + doc/manual/src/release-notes/rl-2.8.md | 53 +++++++++++++++++++++++++ doc/manual/src/release-notes/rl-next.md | 45 --------------------- 3 files changed, 54 insertions(+), 45 deletions(-) create mode 100644 doc/manual/src/release-notes/rl-2.8.md diff --git a/doc/manual/src/SUMMARY.md.in b/doc/manual/src/SUMMARY.md.in index f0f9457d2..860222337 100644 --- a/doc/manual/src/SUMMARY.md.in +++ b/doc/manual/src/SUMMARY.md.in @@ -72,6 +72,7 @@ - [CLI guideline](contributing/cli-guideline.md) - [Release Notes](release-notes/release-notes.md) - [Release X.Y (202?-??-??)](release-notes/rl-next.md) + - [Release 2.8 (2022-04-19)](release-notes/rl-2.8.md) - [Release 2.7 (2022-03-07)](release-notes/rl-2.7.md) - [Release 2.6 (2022-01-24)](release-notes/rl-2.6.md) - [Release 2.5 (2021-12-13)](release-notes/rl-2.5.md) diff --git a/doc/manual/src/release-notes/rl-2.8.md b/doc/manual/src/release-notes/rl-2.8.md new file mode 100644 index 000000000..9778e8c3a --- /dev/null +++ b/doc/manual/src/release-notes/rl-2.8.md @@ -0,0 +1,53 @@ +# Release 2.8 (2022-04-19) + +* New experimental command: `nix fmt`, which applies a formatter + defined by the `formatter.` flake output to the Nix + expressions in a flake. + +* Various Nix commands can now read expressions from standard input + using `--file -`. + +* New experimental builtin function `builtins.fetchClosure` that + copies a closure from a binary cache at evaluation time and rewrites + it to content-addressed form (if it isn't already). Like + `builtins.storePath`, this allows importing pre-built store paths; + the difference is that it doesn't require the user to configure + binary caches and trusted public keys. + + This function is only available if you enable the experimental + feature `fetch-closure`. + +* New experimental feature: *impure derivations*. These are + derivations that can produce a different result every time they're + built. Here is an example: + + ```nix + stdenv.mkDerivation { + name = "impure"; + __impure = true; # marks this derivation as impure + buildCommand = "date > $out"; + } + ``` + + Running `nix build` twice on this expression will build the + derivation twice, producing two different content-addressed store + paths. Like fixed-output derivations, impure derivations have access + to the network. Only fixed-output derivations and impure derivations + can depend on an impure derivation. + +* `nix store make-content-addressable` has been renamed to `nix store + make-content-addressed`. + +* The `nixosModule` flake output attribute has been renamed consistent + with the `.default` renames in Nix 2.7. + + * `nixosModule` → `nixosModules.default` + + As before, the old output will continue to work, but `nix flake check` will + issue a warning about it. + +* `nix run` is now stricter in what it accepts: members of the `apps` + flake output are now required to be apps (as defined in [the + manual](https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-run.html#apps)), + and members of `packages` or `legacyPackages` must be derivations + (not apps). diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 6ebbe5eb4..c869b5e2f 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -1,46 +1 @@ # Release X.Y (202?-??-??) - -* Various nix commands can now read expressions from stdin with `--file -`. - -* `nix store make-content-addressable` has been renamed to `nix store - make-content-addressed`. - -* New experimental builtin function `builtins.fetchClosure` that - copies a closure from a binary cache at evaluation time and rewrites - it to content-addressed form (if it isn't already). Like - `builtins.storePath`, this allows importing pre-built store paths; - the difference is that it doesn't require the user to configure - binary caches and trusted public keys. - - This function is only available if you enable the experimental - feature `fetch-closure`. - -* New experimental feature: *impure derivations*. These are - derivations that can produce a different result every time they're - built. Here is an example: - - ```nix - stdenv.mkDerivation { - name = "impure"; - __impure = true; # marks this derivation as impure - buildCommand = "date > $out"; - } - ``` - - Running `nix build` twice on this expression will build the - derivation twice, producing two different content-addressed store - paths. Like fixed-output derivations, impure derivations have access - to the network. Only fixed-output derivations and impure derivations - can depend on an impure derivation. - -* The `nixosModule` flake output attribute has been renamed consistent - with the `.default` renames in nix 2.7. - - * `nixosModule` → `nixosModules.default` - - As before, the old output will continue to work, but `nix flake check` will - issue a warning about it. - -* `nix run` is now stricter wrt what it accepts: - * Members of `apps` are now required to be apps (as defined in [the manual](https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-run.html#apps)) - * Member of `packages` or `legacyPackages` cannot be of type "app" when used by `nix run`. From a3c843e6357be9d1cf84c1e030b8e5169815d827 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 19 Apr 2022 21:47:13 +0200 Subject: [PATCH 82/83] Fix 'nix fmt' test --- tests/fmt.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fmt.sh b/tests/fmt.sh index 2b482f14a..bc05118ff 100644 --- a/tests/fmt.sh +++ b/tests/fmt.sh @@ -25,6 +25,6 @@ cat << EOF > flake.nix EOF nix fmt ./file ./folder | grep 'Formatting: ./file ./folder' nix flake check -nix flake show | grep -P 'x86_64-linux|x86_64-darwin' +nix flake show | grep -P "package 'formatter'" clearStore From ee57f91413c9d01f1027eccbe01f7706c94919ac Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 19 Apr 2022 21:48:17 +0200 Subject: [PATCH 83/83] Bump version --- .version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.version b/.version index 6533b6687..f3ac133c5 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -2.8.0 \ No newline at end of file +2.9.0 \ No newline at end of file