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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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 679b3b32c952eed3c3be6c450c8189948e5645cd Mon Sep 17 00:00:00 2001 From: Erik Arvstedt Date: Sat, 26 Mar 2022 11:32:37 +0100 Subject: [PATCH 08/28] 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 09/28] 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 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 10/28] 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 11/28] 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 12/28] 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 13/28] 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 14/28] 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 15/28] 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 16/28] 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 17/28] 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 18/28] 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 19/28] 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 20/28] 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 21/28] 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 22/28] 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 23/28] 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 24/28] 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 25/28] 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 26/28] 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 27/28] 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 28/28] 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