From 8c54a01df5ee59e4acf151dba8077a9842e8bdc5 Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Mon, 13 Mar 2023 21:14:19 +0100 Subject: [PATCH 01/28] nix: develop: always force SHELL to chosen shell SHELL was inherited from the system environment. This resulted in a new shell being started, but with SHELL still referring to the system shell and not the one used by nix-develop. Applications like make, use SHELL to run commands, which meant that top-level commands are run inside the nix-develop-shell, but sub-commands are ran inside the system shell. This setenv forces SHELL to always be set to the shell used by nix-develop. --- src/nix/develop.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 38482ed42..4a561e52b 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -293,7 +293,6 @@ struct Common : InstallableCommand, MixProfile "NIX_LOG_FD", "NIX_REMOTE", "PPID", - "SHELL", "SHELLOPTS", "SSL_CERT_FILE", // FIXME: only want to ignore /no-cert-file.crt "TEMP", @@ -643,6 +642,10 @@ struct CmdDevelop : Common, MixEnvironment ignoreException(); } + // Override SHELL with the one chosen for this environment. + // This is to make sure the system shell doesn't leak into the build environment. + setenv("SHELL", shell.data(), 1); + // If running a phase or single command, don't want an interactive shell running after // Ctrl-C, so don't pass --rcfile auto args = phase || !command.empty() ? Strings{std::string(baseNameOf(shell)), rcFilePath} From ceab20d056a119317fb29eb0e06dfd0eb0b9d8ad Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Mon, 13 Nov 2023 22:04:34 +0100 Subject: [PATCH 02/28] nix: develop: add tests for interactive shell --- tests/functional/flakes/develop.sh | 75 ++++++++++++++++++++++++++++++ tests/functional/local.mk | 1 + 2 files changed, 76 insertions(+) create mode 100644 tests/functional/flakes/develop.sh diff --git a/tests/functional/flakes/develop.sh b/tests/functional/flakes/develop.sh new file mode 100644 index 000000000..59f731239 --- /dev/null +++ b/tests/functional/flakes/develop.sh @@ -0,0 +1,75 @@ +source ../common.sh + +clearStore +rm -rf $TEST_HOME/.cache $TEST_HOME/.config $TEST_HOME/.local + +# Create flake under test. +cp ../shell-hello.nix ../config.nix $TEST_HOME/ +cat <$TEST_HOME/flake.nix +{ + inputs.nixpkgs.url = "$TEST_HOME/nixpkgs"; + outputs = {self, nixpkgs}: { + packages.$system.hello = (import ./config.nix).mkDerivation { + name = "hello"; + outputs = [ "out" "dev" ]; + meta.outputsToInstall = [ "out" ]; + buildCommand = ""; + }; + }; +} +EOF + +# Create fake nixpkgs flake. +mkdir -p $TEST_HOME/nixpkgs +cp ../config.nix ../shell.nix $TEST_HOME/nixpkgs +cat <$TEST_HOME/nixpkgs/flake.nix +{ + outputs = {self}: { + legacyPackages.$system.bashInteractive = (import ./shell.nix {}).bashInteractive; + }; +} +EOF + +cd $TEST_HOME + +# Test whether `nix develop` passes through environment variables. +[[ "$( + ENVVAR=a nix develop --no-write-lock-file .#hello < Date: Thu, 16 Nov 2023 15:12:31 +0100 Subject: [PATCH 03/28] fixup! nix: develop: add tests for interactive shell --- tests/functional/common/vars-and-functions.sh.in | 1 + tests/functional/flakes/develop.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/functional/common/vars-and-functions.sh.in b/tests/functional/common/vars-and-functions.sh.in index 848988af9..02773bf60 100644 --- a/tests/functional/common/vars-and-functions.sh.in +++ b/tests/functional/common/vars-and-functions.sh.in @@ -45,6 +45,7 @@ if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then DAEMON_PATH="${NIX_DAEMON_PACKAGE}/bin:$DAEMON_PATH" fi coreutils=@coreutils@ +lsof=@lsof@ export dot=@dot@ export SHELL="@bash@" diff --git a/tests/functional/flakes/develop.sh b/tests/functional/flakes/develop.sh index 59f731239..db23ca0c0 100644 --- a/tests/functional/flakes/develop.sh +++ b/tests/functional/flakes/develop.sh @@ -54,7 +54,7 @@ BASH_INTERACTIVE_EXECUTABLE="$PWD/bash-interactive/bin/bash" [[ "$( nix develop --no-write-lock-file .#hello <&1 | grep -o '/.*/bash' EOF )" -ef "$BASH_INTERACTIVE_EXECUTABLE" ]] From 06a745120bc8fe7625954e970c61028f8a42c31e Mon Sep 17 00:00:00 2001 From: Bob van der Linden Date: Sun, 26 Nov 2023 21:27:46 +0100 Subject: [PATCH 04/28] nix: develop: remove test for interactive shell executable --- tests/functional/flakes/develop.sh | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/functional/flakes/develop.sh b/tests/functional/flakes/develop.sh index db23ca0c0..e1e53d364 100644 --- a/tests/functional/flakes/develop.sh +++ b/tests/functional/flakes/develop.sh @@ -50,14 +50,6 @@ EOF nix build --no-write-lock-file './nixpkgs#bashInteractive' --out-link ./bash-interactive BASH_INTERACTIVE_EXECUTABLE="$PWD/bash-interactive/bin/bash" -# Test whether `nix develop` uses nixpkgs#bashInteractive shell. -[[ "$( - nix develop --no-write-lock-file .#hello <&1 | grep -o '/.*/bash' -EOF -)" -ef "$BASH_INTERACTIVE_EXECUTABLE" ]] - # Test whether `nix develop` sets `SHELL` to nixpkgs#bashInteractive shell. [[ "$( SHELL=custom nix develop --no-write-lock-file .#hello < Date: Thu, 21 Dec 2023 16:48:29 +0100 Subject: [PATCH 05/28] nix profile: Remove indices --- src/nix/profile-list.md | 2 -- src/nix/profile-remove.md | 7 ------ src/nix/profile-upgrade.md | 7 ------ src/nix/profile.cc | 39 ++++++++++----------------------- tests/functional/nix-profile.sh | 11 +++++----- 5 files changed, 16 insertions(+), 50 deletions(-) diff --git a/src/nix/profile-list.md b/src/nix/profile-list.md index facfdf0d6..9811b9ec9 100644 --- a/src/nix/profile-list.md +++ b/src/nix/profile-list.md @@ -7,14 +7,12 @@ R""( ```console # nix profile list Name: gdb - Index: 0 Flake attribute: legacyPackages.x86_64-linux.gdb Original flake URL: flake:nixpkgs Locked flake URL: github:NixOS/nixpkgs/7b38b03d76ab71bdc8dc325e3f6338d984cc35ca Store paths: /nix/store/indzcw5wvlhx6vwk7k4iq29q15chvr3d-gdb-11.1 Name: blender-bin - Index: 1 Flake attribute: packages.x86_64-linux.default Original flake URL: flake:blender-bin Locked flake URL: github:edolstra/nix-warez/91f2ffee657bf834e4475865ae336e2379282d34?dir=blender diff --git a/src/nix/profile-remove.md b/src/nix/profile-remove.md index c994b79bd..1f6532250 100644 --- a/src/nix/profile-remove.md +++ b/src/nix/profile-remove.md @@ -8,13 +8,6 @@ R""( # nix profile remove hello ``` -* Remove a package by index - *(deprecated, will be removed in a future version)*: - - ```console - # nix profile remove 3 - ``` - * Remove all packages: ```console diff --git a/src/nix/profile-upgrade.md b/src/nix/profile-upgrade.md index 47103edfc..432b8fa94 100644 --- a/src/nix/profile-upgrade.md +++ b/src/nix/profile-upgrade.md @@ -15,13 +15,6 @@ R""( # nix profile upgrade hello ``` -* Upgrade a specific package by index - *(deprecated, will be removed in a future version)*: - - ```console - # nix profile upgrade 0 - ``` - # Description This command upgrades a previously installed package in a Nix profile, diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 1d89815e2..517693cd4 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -470,40 +470,28 @@ public: std::string pattern; std::regex reg; }; - typedef std::variant Matcher; + typedef std::variant Matcher; std::vector getMatchers(ref store) { std::vector res; - auto anyIndexMatchers = false; - for (auto & s : _matchers) { - if (auto n = string2Int(s)) { - res.push_back(*n); - anyIndexMatchers = true; - } + if (auto n = string2Int(s)) + throw Error("'nix profile' no longer supports indices ('%d')", *n); else if (store->isStorePath(s)) res.push_back(s); else res.push_back(RegexPattern{s,std::regex(s, std::regex::extended | std::regex::icase)}); } - if (anyIndexMatchers) { - warn("Indices are deprecated and will be removed in a future version!\n" - " Refer to packages by their `Name` as printed by `nix profile list`.\n" - " See https://github.com/NixOS/nix/issues/9171 for more information."); - } - return res; } - bool matches(const Store & store, const ProfileElement & element, size_t pos, const std::vector & matchers) + bool matches(const Store & store, const ProfileElement & element, const std::vector & matchers) { for (auto & matcher : matchers) { - if (auto n = std::get_if(&matcher)) { - if (*n == pos) return true; - } else if (auto path = std::get_if(&matcher)) { + if (auto path = std::get_if(&matcher)) { if (element.storePaths.count(store.parseStorePath(*path))) return true; } else if (auto regex = std::get_if(&matcher)) { if (std::regex_match(element.name, regex->reg)) @@ -539,7 +527,7 @@ struct CmdProfileRemove : virtual EvalCommand, MixDefaultProfile, MixProfileElem for (size_t i = 0; i < oldManifest.elements.size(); ++i) { auto & element(oldManifest.elements[i]); - if (!matches(*store, element, i, matchers)) { + if (!matches(*store, element, matchers)) { newManifest.elements.push_back(std::move(element)); } else { notice("removing '%s'", element.identifier()); @@ -553,11 +541,9 @@ struct CmdProfileRemove : virtual EvalCommand, MixDefaultProfile, MixProfileElem if (removedCount == 0) { for (auto matcher: matchers) { - if (const size_t * index = std::get_if(&matcher)){ - warn("'%d' is not a valid index", *index); - } else if (const Path * path = std::get_if(&matcher)){ + if (const Path * path = std::get_if(&matcher)) { warn("'%s' does not match any paths", *path); - } else if (const RegexPattern * regex = std::get_if(&matcher)){ + } else if (const RegexPattern * regex = std::get_if(&matcher)) { warn("'%s' does not match any packages", regex->pattern); } } @@ -595,7 +581,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf for (size_t i = 0; i < manifest.elements.size(); ++i) { auto & element(manifest.elements[i]); - if (!matches(*store, element, i, matchers)) { + if (!matches(*store, element, matchers)) { continue; } @@ -657,11 +643,9 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf if (upgradedCount == 0) { if (matchedCount == 0) { for (auto & matcher : matchers) { - if (const size_t * index = std::get_if(&matcher)){ - warn("'%d' is not a valid index", *index); - } else if (const Path * path = std::get_if(&matcher)){ + if (const Path * path = std::get_if(&matcher)) { warn("'%s' does not match any paths", *path); - } else if (const RegexPattern * regex = std::get_if(&matcher)){ + } else if (const RegexPattern * regex = std::get_if(&matcher)) { warn("'%s' does not match any packages", regex->pattern); } } @@ -715,7 +699,6 @@ struct CmdProfileList : virtual EvalCommand, virtual StoreCommand, MixDefaultPro logger->cout("Name: " ANSI_BOLD "%s" ANSI_NORMAL "%s", element.name, element.active ? "" : " " ANSI_RED "(inactive)" ANSI_NORMAL); - logger->cout("Index: %s", i); if (element.source) { logger->cout("Flake attribute: %s%s", element.source->attrPath, element.source->outputs.to_string()); logger->cout("Original flake URL: %s", element.source->originalRef.to_string()); diff --git a/tests/functional/nix-profile.sh b/tests/functional/nix-profile.sh index eced4d3f1..618b6241d 100644 --- a/tests/functional/nix-profile.sh +++ b/tests/functional/nix-profile.sh @@ -49,7 +49,7 @@ cp ./config.nix $flake1Dir/ nix-env -f ./user-envs.nix -i foo-1.0 nix profile list | grep -A2 'Name:.*foo' | grep 'Store paths:.*foo-1.0' nix profile install $flake1Dir -L -nix profile list | grep -A4 'Index:.*1' | grep 'Locked flake URL:.*narHash' +nix profile list | grep -A4 'Name:.*flake1' | grep 'Locked flake URL:.*narHash' [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]] [ -e $TEST_HOME/.nix-profile/share/man ] (! [ -e $TEST_HOME/.nix-profile/include ]) @@ -58,9 +58,8 @@ nix profile history | grep "packages.$system.default: ∅ -> 1.0" nix profile diff-closures | grep 'env-manifest.nix: ε → ∅' # Test XDG Base Directories support - export NIX_CONFIG="use-xdg-base-directories = true" -nix profile remove 1 +nix profile remove flake1 nix profile install $flake1Dir [[ $($TEST_HOME/.local/state/nix/profile/bin/hello) = "Hello World" ]] unset NIX_CONFIG @@ -68,7 +67,7 @@ unset NIX_CONFIG # Test upgrading a package. printf NixOS > $flake1Dir/who printf 2.0 > $flake1Dir/version -nix profile upgrade 1 +nix profile upgrade flake1 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello NixOS" ]] nix profile history | grep "packages.$system.default: 1.0, 1.0-man -> 2.0, 2.0-man" @@ -89,7 +88,7 @@ nix profile diff-closures | grep 'Version 3 -> 4' # Test installing a non-flake package. nix profile install --file ./simple.nix '' [[ $(cat $TEST_HOME/.nix-profile/hello) = "Hello World!" ]] -nix profile remove 1 +nix profile remove simple nix profile install $(nix-build --no-out-link ./simple.nix) [[ $(cat $TEST_HOME/.nix-profile/hello) = "Hello World!" ]] @@ -107,7 +106,7 @@ nix profile wipe-history # Test upgrade to CA package. printf true > $flake1Dir/ca.nix printf 3.0 > $flake1Dir/version -nix profile upgrade 0 +nix profile upgrade flake1 nix profile history | grep "packages.$system.default: 1.0, 1.0-man -> 3.0, 3.0-man" # Test new install of CA package. From 6268a45b650f563bae2360e0540920a2959bdd40 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 22 Dec 2023 16:11:25 +0100 Subject: [PATCH 06/28] nix profile: Make profile element names stable The profile manifest is now an object keyed on the name returned by getNameFromURL() at installation time, instead of an array. This ensures that the names of profile elements don't change when other elements are added/removed. --- src/nix/profile.cc | 140 ++++++++++++++++---------------- tests/functional/nix-profile.sh | 17 ++-- 2 files changed, 80 insertions(+), 77 deletions(-) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 517693cd4..8b3918b80 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -45,7 +45,6 @@ const int defaultPriority = 5; struct ProfileElement { StorePathSet storePaths; - std::string name; std::optional source; bool active = true; int priority = defaultPriority; @@ -82,11 +81,6 @@ struct ProfileElement return showVersions(versions); } - bool operator < (const ProfileElement & other) const - { - return std::tuple(identifier(), storePaths) < std::tuple(other.identifier(), other.storePaths); - } - void updateStorePaths( ref evalStore, ref store, @@ -109,7 +103,9 @@ struct ProfileElement struct ProfileManifest { - std::vector elements; + using ProfileElementName = std::string; + + std::map elements; ProfileManifest() { } @@ -119,8 +115,6 @@ struct ProfileManifest if (pathExists(manifestPath)) { auto json = nlohmann::json::parse(readFile(manifestPath)); - /* Keep track of already found names to allow preventing duplicates. */ - std::set foundNames; auto version = json.value("version", 0); std::string sUrl; @@ -131,6 +125,7 @@ struct ProfileManifest sOriginalUrl = "originalUri"; break; case 2: + case 3: sUrl = "url"; sOriginalUrl = "originalUrl"; break; @@ -138,7 +133,9 @@ struct ProfileManifest throw Error("profile manifest '%s' has unsupported version %d", manifestPath, version); } - for (auto & e : json["elements"]) { + auto elems = json["elements"]; + for (auto & elem : elems.items()) { + auto & e = elem.value(); ProfileElement element; for (auto & p : e["storePaths"]) element.storePaths.insert(state.store->parseStorePath((std::string) p)); @@ -155,25 +152,16 @@ struct ProfileManifest }; } - std::string nameCandidate = element.identifier(); - if (e.contains("name")) { - nameCandidate = e["name"]; - } - else if (element.source) { - auto url = parseURL(element.source->to_string()); - auto name = getNameFromURL(url); - if (name) - nameCandidate = *name; - } + std::string name = + elems.is_object() + ? elem.key() + : e.contains("name") + ? (std::string) e["name"] + : element.source + ? getNameFromURL(parseURL(element.source->to_string())).value_or(element.identifier()) + : element.identifier(); - auto finalName = nameCandidate; - for (int i = 1; foundNames.contains(finalName); ++i) { - finalName = nameCandidate + std::to_string(i); - } - element.name = finalName; - foundNames.insert(element.name); - - elements.emplace_back(std::move(element)); + addElement(name, std::move(element)); } } @@ -187,16 +175,34 @@ struct ProfileManifest for (auto & drvInfo : drvInfos) { ProfileElement element; element.storePaths = {drvInfo.queryOutPath()}; - element.name = element.identifier(); - elements.emplace_back(std::move(element)); + addElement(std::move(element)); } } } + void addElement(std::string_view nameCandidate, ProfileElement element) + { + std::string finalName(nameCandidate); + for (int i = 1; elements.contains(finalName); ++i) + finalName = nameCandidate + "-" + std::to_string(i); + + elements.insert_or_assign(finalName, std::move(element)); + } + + void addElement(ProfileElement element) + { + auto name = + element.source + ? getNameFromURL(parseURL(element.source->to_string())) + : std::nullopt; + auto name2 = name ? *name : element.identifier(); + addElement(name2, std::move(element)); + } + nlohmann::json toJSON(Store & store) const { - auto array = nlohmann::json::array(); - for (auto & element : elements) { + auto es = nlohmann::json::object(); + for (auto & [name, element] : elements) { auto paths = nlohmann::json::array(); for (auto & path : element.storePaths) paths.push_back(store.printStorePath(path)); @@ -210,11 +216,11 @@ struct ProfileManifest obj["attrPath"] = element.source->attrPath; obj["outputs"] = element.source->outputs; } - array.push_back(obj); + es[name] = obj; } nlohmann::json json; - json["version"] = 2; - json["elements"] = array; + json["version"] = 3; + json["elements"] = es; return json; } @@ -225,7 +231,7 @@ struct ProfileManifest StorePathSet references; Packages pkgs; - for (auto & element : elements) { + for (auto & [name, element] : elements) { for (auto & path : element.storePaths) { if (element.active) pkgs.emplace_back(store->printStorePath(path), true, element.priority); @@ -267,33 +273,27 @@ struct ProfileManifest static void printDiff(const ProfileManifest & prev, const ProfileManifest & cur, std::string_view indent) { - auto prevElems = prev.elements; - std::sort(prevElems.begin(), prevElems.end()); - - auto curElems = cur.elements; - std::sort(curElems.begin(), curElems.end()); - - auto i = prevElems.begin(); - auto j = curElems.begin(); + auto i = prev.elements.begin(); + auto j = cur.elements.begin(); bool changes = false; - while (i != prevElems.end() || j != curElems.end()) { - if (j != curElems.end() && (i == prevElems.end() || i->identifier() > j->identifier())) { - logger->cout("%s%s: ∅ -> %s", indent, j->identifier(), j->versions()); + while (i != prev.elements.end() || j != cur.elements.end()) { + if (j != cur.elements.end() && (i == prev.elements.end() || i->first > j->first)) { + logger->cout("%s%s: ∅ -> %s", indent, j->second.identifier(), j->second.versions()); changes = true; ++j; } - else if (i != prevElems.end() && (j == curElems.end() || i->identifier() < j->identifier())) { - logger->cout("%s%s: %s -> ∅", indent, i->identifier(), i->versions()); + else if (i != prev.elements.end() && (j == cur.elements.end() || i->first < j->first)) { + logger->cout("%s%s: %s -> ∅", indent, i->second.identifier(), i->second.versions()); changes = true; ++i; } else { - auto v1 = i->versions(); - auto v2 = j->versions(); + auto v1 = i->second.versions(); + auto v2 = j->second.versions(); if (v1 != v2) { - logger->cout("%s%s: %s -> %s", indent, i->identifier(), v1, v2); + logger->cout("%s%s: %s -> %s", indent, i->second.identifier(), v1, v2); changes = true; } ++i; @@ -392,7 +392,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile element.updateStorePaths(getEvalStore(), store, res); - manifest.elements.push_back(std::move(element)); + manifest.addElement(std::move(element)); } try { @@ -402,7 +402,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile // See https://github.com/NixOS/nix/compare/3efa476c5439f8f6c1968a6ba20a31d1239c2f04..1fe5d172ece51a619e879c4b86f603d9495cc102 auto findRefByFilePath = [&](Iterator begin, Iterator end) { for (auto it = begin; it != end; it++) { - auto profileElement = *it; + auto & profileElement = it->second; for (auto & storePath : profileElement.storePaths) { if (conflictError.fileA.starts_with(store->printStorePath(storePath))) { return std::pair(conflictError.fileA, profileElement.toInstallables(*store)); @@ -488,13 +488,17 @@ public: return res; } - bool matches(const Store & store, const ProfileElement & element, const std::vector & matchers) + bool matches( + const Store & store, + const std::string & name, + const ProfileElement & element, + const std::vector & matchers) { for (auto & matcher : matchers) { if (auto path = std::get_if(&matcher)) { if (element.storePaths.count(store.parseStorePath(*path))) return true; } else if (auto regex = std::get_if(&matcher)) { - if (std::regex_match(element.name, regex->reg)) + if (std::regex_match(name, regex->reg)) return true; } } @@ -525,10 +529,9 @@ struct CmdProfileRemove : virtual EvalCommand, MixDefaultProfile, MixProfileElem ProfileManifest newManifest; - for (size_t i = 0; i < oldManifest.elements.size(); ++i) { - auto & element(oldManifest.elements[i]); - if (!matches(*store, element, matchers)) { - newManifest.elements.push_back(std::move(element)); + for (auto & [name, element] : oldManifest.elements) { + if (!matches(*store, name, element, matchers)) { + newManifest.elements.insert_or_assign(name, std::move(element)); } else { notice("removing '%s'", element.identifier()); } @@ -574,14 +577,13 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf auto matchers = getMatchers(store); Installables installables; - std::vector indices; + std::vector elems; auto matchedCount = 0; auto upgradedCount = 0; - for (size_t i = 0; i < manifest.elements.size(); ++i) { - auto & element(manifest.elements[i]); - if (!matches(*store, element, matchers)) { + for (auto & [name, element] : manifest.elements) { + if (!matches(*store, name, element, matchers)) { continue; } @@ -637,7 +639,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf }; installables.push_back(installable); - indices.push_back(i); + elems.push_back(&element); } if (upgradedCount == 0) { @@ -661,7 +663,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf for (size_t i = 0; i < installables.size(); ++i) { auto & installable = installables.at(i); - auto & element = manifest.elements[indices.at(i)]; + auto & element = *elems.at(i); element.updateStorePaths( getEvalStore(), store, @@ -693,11 +695,11 @@ struct CmdProfileList : virtual EvalCommand, virtual StoreCommand, MixDefaultPro if (json) { std::cout << manifest.toJSON(*store).dump() << "\n"; } else { - for (size_t i = 0; i < manifest.elements.size(); ++i) { - auto & element(manifest.elements[i]); + for (const auto & [i, e] : enumerate(manifest.elements)) { + auto & [name, element] = e; if (i) logger->cout(""); logger->cout("Name: " ANSI_BOLD "%s" ANSI_NORMAL "%s", - element.name, + name, element.active ? "" : " " ANSI_RED "(inactive)" ANSI_NORMAL); if (element.source) { logger->cout("Flake attribute: %s%s", element.source->attrPath, element.source->outputs.to_string()); diff --git a/tests/functional/nix-profile.sh b/tests/functional/nix-profile.sh index 618b6241d..003af5174 100644 --- a/tests/functional/nix-profile.sh +++ b/tests/functional/nix-profile.sh @@ -59,7 +59,7 @@ nix profile diff-closures | grep 'env-manifest.nix: ε → ∅' # Test XDG Base Directories support export NIX_CONFIG="use-xdg-base-directories = true" -nix profile remove flake1 +nix profile remove flake1 2>&1 | grep 'removed 1 packages' nix profile install $flake1Dir [[ $($TEST_HOME/.local/state/nix/profile/bin/hello) = "Hello World" ]] unset NIX_CONFIG @@ -80,7 +80,7 @@ nix profile rollback # Test uninstall. [ -e $TEST_HOME/.nix-profile/bin/foo ] -nix profile remove foo +nix profile remove foo 2>&1 | grep 'removed 1 packages' (! [ -e $TEST_HOME/.nix-profile/bin/foo ]) nix profile history | grep 'foo: 1.0 -> ∅' nix profile diff-closures | grep 'Version 3 -> 4' @@ -88,7 +88,7 @@ nix profile diff-closures | grep 'Version 3 -> 4' # Test installing a non-flake package. nix profile install --file ./simple.nix '' [[ $(cat $TEST_HOME/.nix-profile/hello) = "Hello World!" ]] -nix profile remove simple +nix profile remove simple 2>&1 | grep 'removed 1 packages' nix profile install $(nix-build --no-out-link ./simple.nix) [[ $(cat $TEST_HOME/.nix-profile/hello) = "Hello World!" ]] @@ -96,8 +96,9 @@ nix profile install $(nix-build --no-out-link ./simple.nix) mkdir $TEST_ROOT/simple-too cp ./simple.nix ./config.nix simple.builder.sh $TEST_ROOT/simple-too nix profile install --file $TEST_ROOT/simple-too/simple.nix '' -nix profile list | grep -A4 'Name:.*simple' | grep 'Name:.*simple1' -nix profile remove simple1 +nix profile list | grep -A4 'Name:.*simple' | grep 'Name:.*simple-1' +nix profile remove simple 2>&1 | grep 'removed 1 packages' +nix profile remove simple-1 2>&1 | grep 'removed 1 packages' # Test wipe-history. nix profile wipe-history @@ -110,7 +111,7 @@ nix profile upgrade flake1 nix profile history | grep "packages.$system.default: 1.0, 1.0-man -> 3.0, 3.0-man" # Test new install of CA package. -nix profile remove flake1 +nix profile remove flake1 2>&1 | grep 'removed 1 packages' printf 4.0 > $flake1Dir/version printf Utrecht > $flake1Dir/who nix profile install $flake1Dir @@ -131,14 +132,14 @@ nix profile upgrade flake1 [ -e $TEST_HOME/.nix-profile/share/man ] [ -e $TEST_HOME/.nix-profile/include ] -nix profile remove flake1 +nix profile remove flake1 2>&1 | grep 'removed 1 packages' nix profile install "$flake1Dir^man" (! [ -e $TEST_HOME/.nix-profile/bin/hello ]) [ -e $TEST_HOME/.nix-profile/share/man ] (! [ -e $TEST_HOME/.nix-profile/include ]) # test priority -nix profile remove flake1 +nix profile remove flake1 2>&1 | grep 'removed 1 packages' # Make another flake. flake2Dir=$TEST_ROOT/flake2 From a748e88bf4cca0fdc6ce75188e88017a7899d16b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 22 Dec 2023 16:27:31 +0100 Subject: [PATCH 07/28] nix profile: Remove check for "name" attribute in manifests AFAIK, we've never emitted this attribute. --- src/nix/profile.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 8b3918b80..1b0c333bd 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -155,8 +155,6 @@ struct ProfileManifest std::string name = elems.is_object() ? elem.key() - : e.contains("name") - ? (std::string) e["name"] : element.source ? getNameFromURL(parseURL(element.source->to_string())).value_or(element.identifier()) : element.identifier(); From c9125603a535f82cc9a53f47533f0a3d174e7008 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Mon, 18 Dec 2023 10:34:19 -0800 Subject: [PATCH 08/28] Unindent `print.hh` declarations --- src/libexpr/print.hh | 82 +++++++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh index 3b72ae201..abf830864 100644 --- a/src/libexpr/print.hh +++ b/src/libexpr/print.hh @@ -10,45 +10,47 @@ #include namespace nix { - /** - * Print a string as a Nix string literal. - * - * Quotes and fairly minimal escaping are added. - * - * @param s The logical string - */ - std::ostream & printLiteralString(std::ostream & o, std::string_view s); - inline std::ostream & printLiteralString(std::ostream & o, const char * s) { - return printLiteralString(o, std::string_view(s)); - } - inline std::ostream & printLiteralString(std::ostream & o, const std::string & s) { - return printLiteralString(o, std::string_view(s)); - } - /** Print `true` or `false`. */ - std::ostream & printLiteralBool(std::ostream & o, bool b); - - /** - * Print a string as an attribute name in the Nix expression language syntax. - * - * Prints a quoted string if necessary. - */ - std::ostream & printAttributeName(std::ostream & o, std::string_view s); - - /** - * Returns `true' is a string is a reserved keyword which requires quotation - * when printing attribute set field names. - */ - bool isReservedKeyword(const std::string_view str); - - /** - * Print a string as an identifier in the Nix expression language syntax. - * - * FIXME: "identifier" is ambiguous. Identifiers do not have a single - * textual representation. They can be used in variable references, - * let bindings, left-hand sides or attribute names in a select - * expression, or something else entirely, like JSON. Use one of the - * `print*` functions instead. - */ - std::ostream & printIdentifier(std::ostream & o, std::string_view s); +/** + * Print a string as a Nix string literal. + * + * Quotes and fairly minimal escaping are added. + * + * @param s The logical string + */ +std::ostream & printLiteralString(std::ostream & o, std::string_view s); +inline std::ostream & printLiteralString(std::ostream & o, const char * s) { + return printLiteralString(o, std::string_view(s)); +} +inline std::ostream & printLiteralString(std::ostream & o, const std::string & s) { + return printLiteralString(o, std::string_view(s)); +} + +/** Print `true` or `false`. */ +std::ostream & printLiteralBool(std::ostream & o, bool b); + +/** + * Print a string as an attribute name in the Nix expression language syntax. + * + * Prints a quoted string if necessary. + */ +std::ostream & printAttributeName(std::ostream & o, std::string_view s); + +/** + * Returns `true' is a string is a reserved keyword which requires quotation + * when printing attribute set field names. + */ +bool isReservedKeyword(const std::string_view str); + +/** + * Print a string as an identifier in the Nix expression language syntax. + * + * FIXME: "identifier" is ambiguous. Identifiers do not have a single + * textual representation. They can be used in variable references, + * let bindings, left-hand sides or attribute names in a select + * expression, or something else entirely, like JSON. Use one of the + * `print*` functions instead. + */ +std::ostream & printIdentifier(std::ostream & o, std::string_view s); + } From 0fa08b451682fb3311fe58112ff05c4fe5bee3a4 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Tue, 12 Dec 2023 13:57:36 -0800 Subject: [PATCH 09/28] Unify and refactor value printing Previously, there were two mostly-identical value printers -- one in `libexpr/eval.cc` (which didn't force values) and one in `libcmd/repl.cc` (which did force values and also printed ANSI color codes). This PR unifies both of these printers into `print.cc` and provides a `PrintOptions` struct for controlling the output, which allows for toggling whether values are forced, whether repeated values are tracked, and whether ANSI color codes are displayed. Additionally, `PrintOptions` allows tuning the maximum number of attributes, list items, and bytes in a string that will be displayed; this makes it ideal for contexts where printing too much output (e.g. all of Nixpkgs) is distracting. (As requested by @roberth in https://github.com/NixOS/nix/pull/9554#issuecomment-1845095735) Please read the tests for example output. Future work: - It would be nice to provide this function as a builtin, perhaps `builtins.toStringDebug` -- a printing function that never fails would be useful when debugging Nix code. - It would be nice to support customizing `PrintOptions` members on the command line, e.g. `--option to-string-max-attrs 1000`. --- src/libcmd/repl.cc | 158 +---- src/libexpr/eval.cc | 126 +--- src/libexpr/eval.hh | 4 +- src/libexpr/print-options.hh | 52 ++ src/libexpr/print.cc | 416 +++++++++++- src/libexpr/print.hh | 6 + src/libexpr/value.hh | 17 +- src/libutil/english.cc | 18 + src/libutil/english.hh | 18 + src/nix-env/user-env.cc | 5 +- src/nix-instantiate/nix-instantiate.cc | 2 +- tests/functional/lang/eval-okay-print.err.exp | 2 +- tests/functional/lang/eval-okay-print.exp | 2 +- .../lang/eval-okay-repeated-empty-attrs.exp | 1 + .../lang/eval-okay-repeated-empty-attrs.nix | 2 + .../lang/eval-okay-repeated-empty-list.exp | 1 + .../lang/eval-okay-repeated-empty-list.nix | 1 + tests/unit/libexpr/value/print.cc | 621 +++++++++++++++++- 18 files changed, 1174 insertions(+), 278 deletions(-) create mode 100644 src/libexpr/print-options.hh create mode 100644 src/libutil/english.cc create mode 100644 src/libutil/english.hh create mode 100644 tests/functional/lang/eval-okay-repeated-empty-attrs.exp create mode 100644 tests/functional/lang/eval-okay-repeated-empty-attrs.nix create mode 100644 tests/functional/lang/eval-okay-repeated-empty-list.exp create mode 100644 tests/functional/lang/eval-okay-repeated-empty-list.nix diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 7a1df74ef..72e3559df 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -93,9 +93,17 @@ struct NixRepl void evalString(std::string s, Value & v); void loadDebugTraceEnv(DebugTrace & dt); - typedef std::set ValuesSeen; - std::ostream & printValue(std::ostream & str, Value & v, unsigned int maxDepth); - std::ostream & printValue(std::ostream & str, Value & v, unsigned int maxDepth, ValuesSeen & seen); + void printValue(std::ostream & str, + Value & v, + unsigned int maxDepth = std::numeric_limits::max()) + { + ::nix::printValue(*state, str, v, PrintOptions { + .ansiColors = true, + .force = true, + .derivationPaths = true, + .maxDepth = maxDepth + }); + } }; std::string removeWhitespace(std::string s) @@ -708,7 +716,8 @@ bool NixRepl::processLine(std::string line) else if (command == ":p" || command == ":print") { Value v; evalString(arg, v); - printValue(std::cout, v, 1000000000) << std::endl; + printValue(std::cout, v); + std::cout << std::endl; } else if (command == ":q" || command == ":quit") { @@ -770,7 +779,8 @@ bool NixRepl::processLine(std::string line) } else { Value v; evalString(line, v); - printValue(std::cout, v, 1) << std::endl; + printValue(std::cout, v, 1); + std::cout << std::endl; } } @@ -892,144 +902,6 @@ void NixRepl::evalString(std::string s, Value & v) } -std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int maxDepth) -{ - ValuesSeen seen; - return printValue(str, v, maxDepth, seen); -} - - - - -// FIXME: lot of cut&paste from Nix's eval.cc. -std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int maxDepth, ValuesSeen & seen) -{ - str.flush(); - checkInterrupt(); - - state->forceValue(v, v.determinePos(noPos)); - - switch (v.type()) { - - case nInt: - str << ANSI_CYAN << v.integer << ANSI_NORMAL; - break; - - case nBool: - str << ANSI_CYAN; - printLiteralBool(str, v.boolean); - str << ANSI_NORMAL; - break; - - case nString: - str << ANSI_WARNING; - printLiteralString(str, v.string_view()); - str << ANSI_NORMAL; - break; - - case nPath: - str << ANSI_GREEN << v.path().to_string() << ANSI_NORMAL; // !!! escaping? - break; - - case nNull: - str << ANSI_CYAN "null" ANSI_NORMAL; - break; - - case nAttrs: { - seen.insert(&v); - - bool isDrv = state->isDerivation(v); - - if (isDrv) { - str << "«derivation "; - Bindings::iterator i = v.attrs->find(state->sDrvPath); - NixStringContext context; - if (i != v.attrs->end()) - str << state->store->printStorePath(state->coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation")); - else - str << "???"; - str << "»"; - } - - else if (maxDepth > 0) { - str << "{ "; - - typedef std::map Sorted; - Sorted sorted; - for (auto & i : *v.attrs) - sorted.emplace(state->symbols[i.name], i.value); - - for (auto & i : sorted) { - printAttributeName(str, i.first); - str << " = "; - if (seen.count(i.second)) - str << "«repeated»"; - else - try { - printValue(str, *i.second, maxDepth - 1, seen); - } catch (AssertionError & e) { - str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; - } - str << "; "; - } - - str << "}"; - } else - str << "{ ... }"; - - break; - } - - case nList: - seen.insert(&v); - - str << "[ "; - if (maxDepth > 0) - for (auto elem : v.listItems()) { - if (seen.count(elem)) - str << "«repeated»"; - else - try { - printValue(str, *elem, maxDepth - 1, seen); - } catch (AssertionError & e) { - str << ANSI_RED "«error: " << e.msg() << "»" ANSI_NORMAL; - } - str << " "; - } - else - str << "... "; - str << "]"; - break; - - case nFunction: - if (v.isLambda()) { - std::ostringstream s; - s << state->positions[v.lambda.fun->pos]; - str << ANSI_BLUE "«lambda @ " << filterANSIEscapes(s.str()) << "»" ANSI_NORMAL; - } else if (v.isPrimOp()) { - str << ANSI_MAGENTA "«primop»" ANSI_NORMAL; - } else if (v.isPrimOpApp()) { - str << ANSI_BLUE "«primop-app»" ANSI_NORMAL; - } else { - abort(); - } - break; - - case nFloat: - str << v.fpoint; - break; - - case nThunk: - case nExternal: - default: - str << ANSI_RED "«unknown»" ANSI_NORMAL; - break; - } - - return str; -} - - std::unique_ptr AbstractNixRepl::create( const SearchPath & searchPath, nix::ref store, ref state, std::function getValues) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index d408f1adc..0659a2173 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -105,117 +105,23 @@ RootValue allocRootValue(Value * v) #endif } -void Value::print(const SymbolTable &symbols, std::ostream &str, - std::set *seen, int depth) const - -{ - checkInterrupt(); - - if (depth <= 0) { - str << "«too deep»"; - return; - } - switch (internalType) { - case tInt: - str << integer; - break; - case tBool: - printLiteralBool(str, boolean); - break; - case tString: - printLiteralString(str, string_view()); - break; - case tPath: - str << path().to_string(); // !!! escaping? - break; - case tNull: - str << "null"; - break; - case tAttrs: { - if (seen && !attrs->empty() && !seen->insert(attrs).second) - str << "«repeated»"; - else { - str << "{ "; - for (auto & i : attrs->lexicographicOrder(symbols)) { - str << symbols[i->name] << " = "; - i->value->print(symbols, str, seen, depth - 1); - str << "; "; - } - str << "}"; - } - break; - } - case tList1: - case tList2: - case tListN: - if (seen && listSize() && !seen->insert(listElems()).second) - str << "«repeated»"; - else { - str << "[ "; - for (auto v2 : listItems()) { - if (v2) - v2->print(symbols, str, seen, depth - 1); - else - str << "(nullptr)"; - str << " "; - } - str << "]"; - } - break; - case tThunk: - case tApp: - if (!isBlackhole()) { - str << ""; - } else { - // Although we know for sure that it's going to be an infinite recursion - // when this value is accessed _in the current context_, it's likely - // that the user will misinterpret a simpler «infinite recursion» output - // as a definitive statement about the value, while in fact it may be - // a valid value after `builtins.trace` and perhaps some other steps - // have completed. - str << "«potential infinite recursion»"; - } - break; - case tLambda: - str << ""; - break; - case tPrimOp: - str << ""; - break; - case tPrimOpApp: - str << ""; - break; - case tExternal: - str << *external; - break; - case tFloat: - str << fpoint; - break; - default: - printError("Nix evaluator internal error: Value::print(): invalid value type %1%", internalType); - abort(); - } -} - -void Value::print(const SymbolTable &symbols, std::ostream &str, - bool showRepeated, int depth) const { - std::set seen; - print(symbols, str, showRepeated ? nullptr : &seen, depth); -} - // Pretty print types for assertion errors std::ostream & operator << (std::ostream & os, const ValueType t) { os << showType(t); return os; } -std::string printValue(const EvalState & state, const Value & v) +std::string printValue(EvalState & state, Value & v) { std::ostringstream out; - v.print(state.symbols, out); + v.print(state, out); return out.str(); } +void Value::print(EvalState & state, std::ostream & str, PrintOptions options) +{ + printValue(state, str, *this, options); +} const Value * getPrimOp(const Value &v) { const Value * primOp = &v; @@ -710,6 +616,26 @@ void PrimOp::check() } +std::ostream & operator<<(std::ostream & output, PrimOp & primOp) +{ + output << "primop " << primOp.name; + return output; +} + + +PrimOp * Value::primOpAppPrimOp() const +{ + Value * left = primOpApp.left; + while (left && !left->isPrimOp()) { + left = left->primOpApp.left; + } + + if (!left) + return nullptr; + return left->primOp; +} + + void Value::mkPrimOp(PrimOp * p) { p->check(); diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 5e0f1886d..9141156b1 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -84,6 +84,8 @@ struct PrimOp void check(); }; +std::ostream & operator<<(std::ostream & output, PrimOp & primOp); + /** * Info about a constant */ @@ -127,7 +129,7 @@ std::unique_ptr mapStaticEnvBindings(const SymbolTable & st, const Stati void copyContext(const Value & v, NixStringContext & context); -std::string printValue(const EvalState & state, const Value & v); +std::string printValue(EvalState & state, Value & v); std::ostream & operator << (std::ostream & os, const ValueType t); diff --git a/src/libexpr/print-options.hh b/src/libexpr/print-options.hh new file mode 100644 index 000000000..11ff9ae87 --- /dev/null +++ b/src/libexpr/print-options.hh @@ -0,0 +1,52 @@ +#pragma once +/** + * @file + * @brief Options for printing Nix values. + */ + +#include + +namespace nix { + +/** + * Options for printing Nix values. + */ +struct PrintOptions +{ + /** + * If true, output ANSI color sequences. + */ + bool ansiColors = false; + /** + * If true, force values. + */ + bool force = false; + /** + * If true and `force` is set, print derivations as + * `«derivation /nix/store/...»` instead of as attribute sets. + */ + bool derivationPaths = false; + /** + * If true, track which values have been printed and skip them on + * subsequent encounters. Useful for self-referential values. + */ + bool trackRepeated = true; + /** + * Maximum depth to evaluate to. + */ + size_t maxDepth = std::numeric_limits::max(); + /** + * Maximum number of attributes in an attribute set to print. + */ + size_t maxAttrs = std::numeric_limits::max(); + /** + * Maximum number of list items to print. + */ + size_t maxListItems = std::numeric_limits::max(); + /** + * Maximum string length to print. + */ + size_t maxStringLength = std::numeric_limits::max(); +}; + +} diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 53ba70bdd..db26ed4c2 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -1,24 +1,66 @@ -#include "print.hh" +#include #include +#include "print.hh" +#include "ansicolor.hh" +#include "signals.hh" +#include "store-api.hh" +#include "terminal.hh" +#include "english.hh" + namespace nix { -std::ostream & -printLiteralString(std::ostream & str, const std::string_view string) +void printElided( + std::ostream & output, + unsigned int value, + const std::string_view single, + const std::string_view plural, + bool ansiColors) { + if (ansiColors) + output << ANSI_FAINT; + output << " «"; + pluralize(output, value, single, plural); + output << " elided»"; + if (ansiColors) + output << ANSI_NORMAL; +} + + +std::ostream & +printLiteralString(std::ostream & str, const std::string_view string, size_t maxLength, bool ansiColors) +{ + size_t charsPrinted = 0; + if (ansiColors) + str << ANSI_MAGENTA; str << "\""; for (auto i = string.begin(); i != string.end(); ++i) { + if (charsPrinted >= maxLength) { + str << "\""; + printElided(str, string.length() - charsPrinted, "byte", "bytes", ansiColors); + return str; + } + if (*i == '\"' || *i == '\\') str << "\\" << *i; else if (*i == '\n') str << "\\n"; else if (*i == '\r') str << "\\r"; else if (*i == '\t') str << "\\t"; else if (*i == '$' && *(i+1) == '{') str << "\\" << *i; else str << *i; + charsPrinted++; } str << "\""; + if (ansiColors) + str << ANSI_NORMAL; return str; } +std::ostream & +printLiteralString(std::ostream & str, const std::string_view string) +{ + return printLiteralString(str, string, std::numeric_limits::max(), false); +} + std::ostream & printLiteralBool(std::ostream & str, bool boolean) { @@ -90,5 +132,373 @@ printAttributeName(std::ostream & str, std::string_view name) { return str; } +bool isImportantAttrName(const std::string& attrName) +{ + return attrName == "type" || attrName == "_type"; +} + +typedef std::pair AttrPair; + +struct ImportantFirstAttrNameCmp +{ + + bool operator()(const AttrPair& lhs, const AttrPair& rhs) const + { + auto lhsIsImportant = isImportantAttrName(lhs.first); + auto rhsIsImportant = isImportantAttrName(rhs.first); + return std::forward_as_tuple(!lhsIsImportant, lhs.first) + < std::forward_as_tuple(!rhsIsImportant, rhs.first); + } +}; + +typedef std::set ValuesSeen; + +class Printer +{ +private: + std::ostream & output; + EvalState & state; + PrintOptions options; + std::optional seen; + + void printRepeated() + { + if (options.ansiColors) + output << ANSI_MAGENTA; + output << "«repeated»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printNullptr() + { + if (options.ansiColors) + output << ANSI_MAGENTA; + output << "«nullptr»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printElided(unsigned int value, const std::string_view single, const std::string_view plural) + { + ::nix::printElided(output, value, single, plural, options.ansiColors); + } + + void printInt(Value & v) + { + if (options.ansiColors) + output << ANSI_CYAN; + output << v.integer; + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printFloat(Value & v) + { + if (options.ansiColors) + output << ANSI_CYAN; + output << v.fpoint; + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printBool(Value & v) + { + if (options.ansiColors) + output << ANSI_CYAN; + printLiteralBool(output, v.boolean); + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printString(Value & v) + { + printLiteralString(output, v.string_view(), options.maxStringLength, options.ansiColors); + } + + void printPath(Value & v) + { + if (options.ansiColors) + output << ANSI_GREEN; + output << v.path().to_string(); // !!! escaping? + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printNull() + { + if (options.ansiColors) + output << ANSI_CYAN; + output << "null"; + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printDerivation(Value & v) + { + try { + Bindings::iterator i = v.attrs->find(state.sDrvPath); + NixStringContext context; + std::string storePath; + if (i != v.attrs->end()) + storePath = state.store->printStorePath(state.coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation")); + + if (options.ansiColors) + output << ANSI_GREEN; + output << "«derivation"; + if (!storePath.empty()) { + output << " " << storePath; + } + output << "»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } catch (BaseError & e) { + printError_(e); + } + } + + void printAttrs(Value & v, size_t depth) + { + if (seen && !seen->insert(&v).second) { + printRepeated(); + return; + } + + if (options.force && options.derivationPaths && state.isDerivation(v)) { + printDerivation(v); + } else if (depth < options.maxDepth) { + output << "{ "; + + std::vector> sorted; + for (auto & i : *v.attrs) + sorted.emplace_back(std::pair(state.symbols[i.name], i.value)); + + if (options.maxAttrs == std::numeric_limits::max()) + std::sort(sorted.begin(), sorted.end()); + else + std::sort(sorted.begin(), sorted.end(), ImportantFirstAttrNameCmp()); + + size_t attrsPrinted = 0; + for (auto & i : sorted) { + if (attrsPrinted >= options.maxAttrs) { + printElided(sorted.size() - attrsPrinted, "attribute", "attributes"); + break; + } + + printAttributeName(output, i.first); + output << " = "; + print(*i.second, depth + 1); + output << "; "; + attrsPrinted++; + } + + output << "}"; + } else + output << "{ ... }"; + } + + void printList(Value & v, size_t depth) + { + if (seen && v.listSize() && !seen->insert(&v).second) { + printRepeated(); + return; + } + + output << "[ "; + if (depth < options.maxDepth) { + size_t listItemsPrinted = 0; + for (auto elem : v.listItems()) { + if (listItemsPrinted >= options.maxListItems) { + printElided(v.listSize() - listItemsPrinted, "item", "items"); + break; + } + + if (elem) { + print(*elem, depth + 1); + } else { + printNullptr(); + } + output << " "; + listItemsPrinted++; + } + } + else + output << "... "; + output << "]"; + } + + void printFunction(Value & v) + { + if (options.ansiColors) + output << ANSI_BLUE; + output << "«"; + + if (v.isLambda()) { + output << "lambda"; + if (v.lambda.fun) { + if (v.lambda.fun->name) { + output << " " << state.symbols[v.lambda.fun->name]; + } + + std::ostringstream s; + s << state.positions[v.lambda.fun->pos]; + output << " @ " << filterANSIEscapes(s.str()); + } + } else if (v.isPrimOp()) { + if (v.primOp) + output << *v.primOp; + else + output << "primop"; + } else if (v.isPrimOpApp()) { + output << "partially applied "; + auto primOp = v.primOpAppPrimOp(); + if (primOp) + output << *primOp; + else + output << "primop"; + } else { + abort(); + } + + output << "»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printThunk(Value & v) + { + if (v.isBlackhole()) { + // Although we know for sure that it's going to be an infinite recursion + // when this value is accessed _in the current context_, it's likely + // that the user will misinterpret a simpler «infinite recursion» output + // as a definitive statement about the value, while in fact it may be + // a valid value after `builtins.trace` and perhaps some other steps + // have completed. + if (options.ansiColors) + output << ANSI_RED; + output << "«potential infinite recursion»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } else if (v.isThunk() || v.isApp()) { + if (options.ansiColors) + output << ANSI_MAGENTA; + output << "«thunk»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } else { + abort(); + } + } + + void printExternal(Value & v) + { + v.external->print(output); + } + + void printUnknown() + { + if (options.ansiColors) + output << ANSI_RED; + output << "«unknown»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void printError_(BaseError & e) + { + if (options.ansiColors) + output << ANSI_RED; + output << "«" << e.msg() << "»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } + + void print(Value & v, size_t depth) + { + output.flush(); + checkInterrupt(); + + if (options.force) { + try { + state.forceValue(v, v.determinePos(noPos)); + } catch (BaseError & e) { + printError_(e); + return; + } + } + + switch (v.type()) { + + case nInt: + printInt(v); + break; + + case nFloat: + printFloat(v); + break; + + case nBool: + printBool(v); + break; + + case nString: + printString(v); + break; + + case nPath: + printPath(v); + break; + + case nNull: + printNull(); + break; + + case nAttrs: + printAttrs(v, depth); + break; + + case nList: + printList(v, depth); + break; + + case nFunction: + printFunction(v); + break; + + case nThunk: + printThunk(v); + break; + + case nExternal: + printExternal(v); + break; + + default: + printUnknown(); + break; + } + } + +public: + Printer(std::ostream & output, EvalState & state, PrintOptions options) + : output(output), state(state), options(options) { } + + void print(Value & v) + { + if (options.trackRepeated) { + seen.emplace(); + } else { + seen.reset(); + } + + ValuesSeen seen; + print(v, 0); + } +}; + +void printValue(EvalState & state, std::ostream & output, Value & v, PrintOptions options) +{ + Printer(output, state, options).print(v); +} } diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh index abf830864..40207d777 100644 --- a/src/libexpr/print.hh +++ b/src/libexpr/print.hh @@ -9,6 +9,9 @@ #include +#include "eval.hh" +#include "print-options.hh" + namespace nix { /** @@ -16,6 +19,7 @@ namespace nix { * * Quotes and fairly minimal escaping are added. * + * @param o The output stream to print to * @param s The logical string */ std::ostream & printLiteralString(std::ostream & o, std::string_view s); @@ -53,4 +57,6 @@ bool isReservedKeyword(const std::string_view str); */ std::ostream & printIdentifier(std::ostream & o, std::string_view s); +void printValue(EvalState & state, std::ostream & str, Value & v, PrintOptions options = PrintOptions {}); + } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index c65b336b0..214d52271 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -9,6 +9,7 @@ #include "value/context.hh" #include "input-accessor.hh" #include "source-path.hh" +#include "print-options.hh" #if HAVE_BOEHMGC #include @@ -70,7 +71,7 @@ struct Pos; class StorePath; class EvalState; class XMLWriter; - +class Printer; typedef int64_t NixInt; typedef double NixFloat; @@ -82,6 +83,7 @@ typedef double NixFloat; class ExternalValueBase { friend std::ostream & operator << (std::ostream & str, const ExternalValueBase & v); + friend class Printer; protected: /** * Print out the value @@ -139,11 +141,9 @@ private: friend std::string showType(const Value & v); - void print(const SymbolTable &symbols, std::ostream &str, std::set *seen, int depth) const; - public: - void print(const SymbolTable &symbols, std::ostream &str, bool showRepeated = false, int depth = INT_MAX) const; + void print(EvalState &state, std::ostream &str, PrintOptions options = PrintOptions {}); // Functions needed to distinguish the type // These should be removed eventually, by putting the functionality that's @@ -364,10 +364,15 @@ public: inline void mkPrimOpApp(Value * l, Value * r) { internalType = tPrimOpApp; - app.left = l; - app.right = r; + primOpApp.left = l; + primOpApp.right = r; } + /** + * For a `tPrimOpApp` value, get the original `PrimOp` value. + */ + PrimOp * primOpAppPrimOp() const; + inline void mkExternal(ExternalValueBase * e) { clearValue(); diff --git a/src/libutil/english.cc b/src/libutil/english.cc new file mode 100644 index 000000000..8c93c9156 --- /dev/null +++ b/src/libutil/english.cc @@ -0,0 +1,18 @@ +#include "english.hh" + +namespace nix { + +std::ostream & pluralize( + std::ostream & output, + unsigned int count, + const std::string_view single, + const std::string_view plural) +{ + if (count == 1) + output << "1 " << single; + else + output << count << " " << plural; + return output; +} + +} diff --git a/src/libutil/english.hh b/src/libutil/english.hh new file mode 100644 index 000000000..9c6c93571 --- /dev/null +++ b/src/libutil/english.hh @@ -0,0 +1,18 @@ +#pragma once + +#include + +namespace nix { + +/** + * Pluralize a given value. + * + * If `count == 1`, prints `1 {single}` to `output`, otherwise prints `{count} {plural}`. + */ +std::ostream & pluralize( + std::ostream & output, + unsigned int count, + const std::string_view single, + const std::string_view plural); + +} diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index 9f4d063d2..3d07cab7a 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -8,6 +8,8 @@ #include "eval.hh" #include "eval-inline.hh" #include "profiles.hh" +#include "print-ambiguous.hh" +#include namespace nix { @@ -106,7 +108,8 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, environment. */ auto manifestFile = ({ std::ostringstream str; - manifest.print(state.symbols, str, true); + std::set seen; + printAmbiguous(manifest, state.symbols, str, &seen, std::numeric_limits::max()); // TODO with C++20 we can use str.view() instead and avoid copy. std::string str2 = str.str(); StringSource source { str2 }; diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index ab590b3a6..9b36dccc6 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -56,7 +56,7 @@ void processExpr(EvalState & state, const Strings & attrPaths, std::cout << std::endl; } else { if (strict) state.forceValueDeep(vRes); - vRes.print(state.symbols, std::cout); + vRes.print(state, std::cout); std::cout << std::endl; } } else { diff --git a/tests/functional/lang/eval-okay-print.err.exp b/tests/functional/lang/eval-okay-print.err.exp index 3fc99be3e..80aa17c6e 100644 --- a/tests/functional/lang/eval-okay-print.err.exp +++ b/tests/functional/lang/eval-okay-print.err.exp @@ -1 +1 @@ -trace: [ ] +trace: [ «thunk» ] diff --git a/tests/functional/lang/eval-okay-print.exp b/tests/functional/lang/eval-okay-print.exp index 0d960fb70..aa1b2379e 100644 --- a/tests/functional/lang/eval-okay-print.exp +++ b/tests/functional/lang/eval-okay-print.exp @@ -1 +1 @@ -[ null [ [ «repeated» ] ] ] +[ null «primop toString» «partially applied primop deepSeq» «lambda @ /pwd/lang/eval-okay-print.nix:1:61» [ [ «repeated» ] ] ] diff --git a/tests/functional/lang/eval-okay-repeated-empty-attrs.exp b/tests/functional/lang/eval-okay-repeated-empty-attrs.exp new file mode 100644 index 000000000..d21e6db6b --- /dev/null +++ b/tests/functional/lang/eval-okay-repeated-empty-attrs.exp @@ -0,0 +1 @@ +[ { } { } ] diff --git a/tests/functional/lang/eval-okay-repeated-empty-attrs.nix b/tests/functional/lang/eval-okay-repeated-empty-attrs.nix new file mode 100644 index 000000000..030a3b85c --- /dev/null +++ b/tests/functional/lang/eval-okay-repeated-empty-attrs.nix @@ -0,0 +1,2 @@ +# Tests that empty attribute sets are not printed as `«repeated»`. +[ {} {} ] diff --git a/tests/functional/lang/eval-okay-repeated-empty-list.exp b/tests/functional/lang/eval-okay-repeated-empty-list.exp new file mode 100644 index 000000000..701fc7e20 --- /dev/null +++ b/tests/functional/lang/eval-okay-repeated-empty-list.exp @@ -0,0 +1 @@ +[ [ ] [ ] ] diff --git a/tests/functional/lang/eval-okay-repeated-empty-list.nix b/tests/functional/lang/eval-okay-repeated-empty-list.nix new file mode 100644 index 000000000..376c51be8 --- /dev/null +++ b/tests/functional/lang/eval-okay-repeated-empty-list.nix @@ -0,0 +1 @@ +[ [] [] ] diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc index a4f6fc014..98131112e 100644 --- a/tests/unit/libexpr/value/print.cc +++ b/tests/unit/libexpr/value/print.cc @@ -1,6 +1,7 @@ #include "tests/libexpr.hh" #include "value.hh" +#include "print.hh" namespace nix { @@ -12,7 +13,7 @@ struct ValuePrintingTests : LibExprTest void test(Value v, std::string_view expected, A... args) { std::stringstream out; - v.print(state.symbols, out, args...); + v.print(state, out, args...); ASSERT_EQ(out.str(), expected); } }; @@ -84,7 +85,7 @@ TEST_F(ValuePrintingTests, tList) vList.bigList.elems[1] = &vTwo; vList.bigList.size = 3; - test(vList, "[ 1 2 (nullptr) ]"); + test(vList, "[ 1 2 «nullptr» ]"); } TEST_F(ValuePrintingTests, vThunk) @@ -92,7 +93,7 @@ TEST_F(ValuePrintingTests, vThunk) Value vThunk; vThunk.mkThunk(nullptr, nullptr); - test(vThunk, ""); + test(vThunk, "«thunk»"); } TEST_F(ValuePrintingTests, vApp) @@ -100,32 +101,55 @@ TEST_F(ValuePrintingTests, vApp) Value vApp; vApp.mkApp(nullptr, nullptr); - test(vApp, ""); + test(vApp, "«thunk»"); } TEST_F(ValuePrintingTests, vLambda) { - Value vLambda; - vLambda.mkLambda(nullptr, nullptr); + Env env { + .up = nullptr, + .values = { } + }; + PosTable::Origin origin((std::monostate())); + auto posIdx = state.positions.add(origin, 1, 1); + auto body = ExprInt(0); + auto formals = Formals {}; - test(vLambda, ""); + ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body); + + Value vLambda; + vLambda.mkLambda(&env, &eLambda); + + test(vLambda, "«lambda @ «none»:1:1»"); + + eLambda.setName(createSymbol("puppy")); + + test(vLambda, "«lambda puppy @ «none»:1:1»"); } TEST_F(ValuePrintingTests, vPrimOp) { Value vPrimOp; - PrimOp primOp{}; + PrimOp primOp{ + .name = "puppy" + }; vPrimOp.mkPrimOp(&primOp); - test(vPrimOp, ""); + test(vPrimOp, "«primop puppy»"); } TEST_F(ValuePrintingTests, vPrimOpApp) { - Value vPrimOpApp; - vPrimOpApp.mkPrimOpApp(nullptr, nullptr); + PrimOp primOp{ + .name = "puppy" + }; + Value vPrimOp; + vPrimOp.mkPrimOp(&primOp); - test(vPrimOpApp, ""); + Value vPrimOpApp; + vPrimOpApp.mkPrimOpApp(&vPrimOp, nullptr); + + test(vPrimOpApp, "«partially applied primop puppy»"); } TEST_F(ValuePrintingTests, vExternal) @@ -176,9 +200,14 @@ TEST_F(ValuePrintingTests, depthAttrs) Value vTwo; vTwo.mkInt(2); + BindingsBuilder builderEmpty(state, state.allocBindings(0)); + Value vAttrsEmpty; + vAttrsEmpty.mkAttrs(builderEmpty.finish()); + BindingsBuilder builder(state, state.allocBindings(10)); builder.insert(state.symbols.create("one"), &vOne); builder.insert(state.symbols.create("two"), &vTwo); + builder.insert(state.symbols.create("nested"), &vAttrsEmpty); Value vAttrs; vAttrs.mkAttrs(builder.finish()); @@ -191,10 +220,10 @@ TEST_F(ValuePrintingTests, depthAttrs) Value vNested; vNested.mkAttrs(builder2.finish()); - test(vNested, "{ nested = «too deep»; one = «too deep»; two = «too deep»; }", false, 1); - test(vNested, "{ nested = { one = «too deep»; two = «too deep»; }; one = 1; two = 2; }", false, 2); - test(vNested, "{ nested = { one = 1; two = 2; }; one = 1; two = 2; }", false, 3); - test(vNested, "{ nested = { one = 1; two = 2; }; one = 1; two = 2; }", false, 4); + test(vNested, "{ nested = { ... }; one = 1; two = 2; }", PrintOptions { .maxDepth = 1 }); + test(vNested, "{ nested = { nested = { ... }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 2 }); + test(vNested, "{ nested = { nested = { }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 3 }); + test(vNested, "{ nested = { nested = { }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 4 }); } TEST_F(ValuePrintingTests, depthList) @@ -227,11 +256,561 @@ TEST_F(ValuePrintingTests, depthList) vList.bigList.elems[2] = &vNested; vList.bigList.size = 3; - test(vList, "[ «too deep» «too deep» «too deep» ]", false, 1); - test(vList, "[ 1 2 { nested = «too deep»; one = «too deep»; two = «too deep»; } ]", false, 2); - test(vList, "[ 1 2 { nested = { one = «too deep»; two = «too deep»; }; one = 1; two = 2; } ]", false, 3); - test(vList, "[ 1 2 { nested = { one = 1; two = 2; }; one = 1; two = 2; } ]", false, 4); - test(vList, "[ 1 2 { nested = { one = 1; two = 2; }; one = 1; two = 2; } ]", false, 5); + test(vList, "[ 1 2 { ... } ]", PrintOptions { .maxDepth = 1 }); + test(vList, "[ 1 2 { nested = { ... }; one = 1; two = 2; } ]", PrintOptions { .maxDepth = 2 }); + test(vList, "[ 1 2 { nested = { one = 1; two = 2; }; one = 1; two = 2; } ]", PrintOptions { .maxDepth = 3 }); + test(vList, "[ 1 2 { nested = { one = 1; two = 2; }; one = 1; two = 2; } ]", PrintOptions { .maxDepth = 4 }); + test(vList, "[ 1 2 { nested = { one = 1; two = 2; }; one = 1; two = 2; } ]", PrintOptions { .maxDepth = 5 }); +} + +struct StringPrintingTests : LibExprTest +{ + template + void test(std::string_view literal, std::string_view expected, unsigned int maxLength, A... args) + { + Value v; + v.mkString(literal); + + std::stringstream out; + printValue(state, out, v, PrintOptions { + .maxStringLength = maxLength + }); + ASSERT_EQ(out.str(), expected); + } +}; + +TEST_F(StringPrintingTests, maxLengthTruncation) +{ + test("abcdefghi", "\"abcdefghi\"", 10); + test("abcdefghij", "\"abcdefghij\"", 10); + test("abcdefghijk", "\"abcdefghij\" «1 byte elided»", 10); + test("abcdefghijkl", "\"abcdefghij\" «2 bytes elided»", 10); + test("abcdefghijklm", "\"abcdefghij\" «3 bytes elided»", 10); +} + +// Check that printing an attrset shows 'important' attributes like `type` +// first, but only reorder the attrs when we have a maxAttrs budget. +TEST_F(ValuePrintingTests, attrsTypeFirst) +{ + Value vType; + vType.mkString("puppy"); + + Value vApple; + vApple.mkString("apple"); + + BindingsBuilder builder(state, state.allocBindings(10)); + builder.insert(state.symbols.create("type"), &vType); + builder.insert(state.symbols.create("apple"), &vApple); + + Value vAttrs; + vAttrs.mkAttrs(builder.finish()); + + test(vAttrs, + "{ type = \"puppy\"; apple = \"apple\"; }", + PrintOptions { + .maxAttrs = 100 + }); + + test(vAttrs, + "{ apple = \"apple\"; type = \"puppy\"; }", + PrintOptions { }); +} + +TEST_F(ValuePrintingTests, ansiColorsInt) +{ + Value v; + v.mkInt(10); + + test(v, + ANSI_CYAN "10" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsFloat) +{ + Value v; + v.mkFloat(1.6); + + test(v, + ANSI_CYAN "1.6" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsBool) +{ + Value v; + v.mkBool(true); + + test(v, + ANSI_CYAN "true" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsString) +{ + Value v; + v.mkString("puppy"); + + test(v, + ANSI_MAGENTA "\"puppy\"" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsStringElided) +{ + Value v; + v.mkString("puppy"); + + test(v, + ANSI_MAGENTA "\"pup\"" ANSI_FAINT " «2 bytes elided»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true, + .maxStringLength = 3 + }); +} + +TEST_F(ValuePrintingTests, ansiColorsPath) +{ + Value v; + v.mkPath(state.rootPath(CanonPath("puppy"))); + + test(v, + ANSI_GREEN "/puppy" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsNull) +{ + Value v; + v.mkNull(); + + test(v, + ANSI_CYAN "null" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsAttrs) +{ + Value vOne; + vOne.mkInt(1); + + Value vTwo; + vTwo.mkInt(2); + + BindingsBuilder builder(state, state.allocBindings(10)); + builder.insert(state.symbols.create("one"), &vOne); + builder.insert(state.symbols.create("two"), &vTwo); + + Value vAttrs; + vAttrs.mkAttrs(builder.finish()); + + test(vAttrs, + "{ one = " ANSI_CYAN "1" ANSI_NORMAL "; two = " ANSI_CYAN "2" ANSI_NORMAL "; }", + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsDerivation) +{ + Value vDerivation; + vDerivation.mkString("derivation"); + + BindingsBuilder builder(state, state.allocBindings(10)); + builder.insert(state.sType, &vDerivation); + + Value vAttrs; + vAttrs.mkAttrs(builder.finish()); + + test(vAttrs, + ANSI_GREEN "«derivation»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true, + .force = true, + .derivationPaths = true + }); + + test(vAttrs, + "{ type = " ANSI_MAGENTA "\"derivation\"" ANSI_NORMAL "; }", + PrintOptions { + .ansiColors = true, + .force = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsError) +{ + Value throw_ = state.getBuiltin("throw"); + Value message; + message.mkString("uh oh!"); + Value vError; + vError.mkApp(&throw_, &message); + + test(vError, + ANSI_RED + "«" + ANSI_RED + "error:" + ANSI_NORMAL + "\n … while calling the '" + ANSI_MAGENTA + "throw" + ANSI_NORMAL + "' builtin\n\n " + ANSI_RED + "error:" + ANSI_NORMAL + " uh oh!»" + ANSI_NORMAL, + PrintOptions { + .ansiColors = true, + .force = true, + }); +} + +TEST_F(ValuePrintingTests, ansiColorsDerivationError) +{ + Value throw_ = state.getBuiltin("throw"); + Value message; + message.mkString("uh oh!"); + Value vError; + vError.mkApp(&throw_, &message); + + Value vDerivation; + vDerivation.mkString("derivation"); + + BindingsBuilder builder(state, state.allocBindings(10)); + builder.insert(state.sType, &vDerivation); + builder.insert(state.sDrvPath, &vError); + + Value vAttrs; + vAttrs.mkAttrs(builder.finish()); + + test(vAttrs, + "{ drvPath = " + ANSI_RED + "«" + ANSI_RED + "error:" + ANSI_NORMAL + "\n … while calling the '" + ANSI_MAGENTA + "throw" + ANSI_NORMAL + "' builtin\n\n " + ANSI_RED + "error:" + ANSI_NORMAL + " uh oh!»" + ANSI_NORMAL + "; type = " + ANSI_MAGENTA + "\"derivation\"" + ANSI_NORMAL + "; }", + PrintOptions { + .ansiColors = true, + .force = true + }); + + test(vAttrs, + ANSI_RED + "«" + ANSI_RED + "error:" + ANSI_NORMAL + "\n … while calling the '" + ANSI_MAGENTA + "throw" + ANSI_NORMAL + "' builtin\n\n " + ANSI_RED + "error:" + ANSI_NORMAL + " uh oh!»" + ANSI_NORMAL, + PrintOptions { + .ansiColors = true, + .force = true, + .derivationPaths = true, + }); +} + +TEST_F(ValuePrintingTests, ansiColorsAssert) +{ + ExprVar eFalse(state.symbols.create("false")); + eFalse.bindVars(state, state.staticBaseEnv); + ExprInt eInt(1); + + ExprAssert expr(noPos, &eFalse, &eInt); + + Value v; + state.mkThunk_(v, &expr); + + test(v, + ANSI_RED "«" ANSI_RED "error:" ANSI_NORMAL " assertion '" ANSI_MAGENTA "false" ANSI_NORMAL "' failed»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true, + .force = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsList) +{ + Value vOne; + vOne.mkInt(1); + + Value vTwo; + vTwo.mkInt(2); + + Value vList; + state.mkList(vList, 5); + vList.bigList.elems[0] = &vOne; + vList.bigList.elems[1] = &vTwo; + vList.bigList.size = 3; + + test(vList, + "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_CYAN "2" ANSI_NORMAL " " ANSI_MAGENTA "«nullptr»" ANSI_NORMAL " ]", + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsLambda) +{ + Env env { + .up = nullptr, + .values = { } + }; + PosTable::Origin origin((std::monostate())); + auto posIdx = state.positions.add(origin, 1, 1); + auto body = ExprInt(0); + auto formals = Formals {}; + + ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body); + + Value vLambda; + vLambda.mkLambda(&env, &eLambda); + + test(vLambda, + ANSI_BLUE "«lambda @ «none»:1:1»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true, + .force = true + }); + + eLambda.setName(createSymbol("puppy")); + + test(vLambda, + ANSI_BLUE "«lambda puppy @ «none»:1:1»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true, + .force = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsPrimOp) +{ + PrimOp primOp{ + .name = "puppy" + }; + Value v; + v.mkPrimOp(&primOp); + + test(v, + ANSI_BLUE "«primop puppy»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsPrimOpApp) +{ + PrimOp primOp{ + .name = "puppy" + }; + Value vPrimOp; + vPrimOp.mkPrimOp(&primOp); + + Value v; + v.mkPrimOpApp(&vPrimOp, nullptr); + + test(v, + ANSI_BLUE "«partially applied primop puppy»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsThunk) +{ + Value v; + v.mkThunk(nullptr, nullptr); + + test(v, + ANSI_MAGENTA "«thunk»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsBlackhole) +{ + Value v; + v.mkBlackhole(); + + test(v, + ANSI_RED "«potential infinite recursion»" ANSI_NORMAL, + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated) +{ + BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + + Value vEmpty; + vEmpty.mkAttrs(emptyBuilder.finish()); + + BindingsBuilder builder(state, state.allocBindings(10)); + builder.insert(state.symbols.create("a"), &vEmpty); + builder.insert(state.symbols.create("b"), &vEmpty); + + Value vAttrs; + vAttrs.mkAttrs(builder.finish()); + + test(vAttrs, + "{ a = { }; b = " ANSI_MAGENTA "«repeated»" ANSI_NORMAL "; }", + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, ansiColorsListRepeated) +{ + BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + + Value vEmpty; + vEmpty.mkAttrs(emptyBuilder.finish()); + + Value vList; + state.mkList(vList, 3); + vList.bigList.elems[0] = &vEmpty; + vList.bigList.elems[1] = &vEmpty; + vList.bigList.size = 2; + + test(vList, + "[ { } " ANSI_MAGENTA "«repeated»" ANSI_NORMAL " ]", + PrintOptions { + .ansiColors = true + }); +} + +TEST_F(ValuePrintingTests, listRepeated) +{ + BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + + Value vEmpty; + vEmpty.mkAttrs(emptyBuilder.finish()); + + Value vList; + state.mkList(vList, 3); + vList.bigList.elems[0] = &vEmpty; + vList.bigList.elems[1] = &vEmpty; + vList.bigList.size = 2; + + test(vList, "[ { } «repeated» ]", PrintOptions { }); + test(vList, + "[ { } { } ]", + PrintOptions { + .trackRepeated = false + }); +} + +TEST_F(ValuePrintingTests, ansiColorsAttrsElided) +{ + Value vOne; + vOne.mkInt(1); + + Value vTwo; + vTwo.mkInt(2); + + BindingsBuilder builder(state, state.allocBindings(10)); + builder.insert(state.symbols.create("one"), &vOne); + builder.insert(state.symbols.create("two"), &vTwo); + + Value vAttrs; + vAttrs.mkAttrs(builder.finish()); + + test(vAttrs, + "{ one = " ANSI_CYAN "1" ANSI_NORMAL "; " ANSI_FAINT " «1 attribute elided»" ANSI_NORMAL "}", + PrintOptions { + .ansiColors = true, + .maxAttrs = 1 + }); + + Value vThree; + vThree.mkInt(3); + + builder.insert(state.symbols.create("three"), &vThree); + vAttrs.mkAttrs(builder.finish()); + + test(vAttrs, + "{ one = " ANSI_CYAN "1" ANSI_NORMAL "; " ANSI_FAINT " «2 attributes elided»" ANSI_NORMAL "}", + PrintOptions { + .ansiColors = true, + .maxAttrs = 1 + }); +} + +TEST_F(ValuePrintingTests, ansiColorsListElided) +{ + BindingsBuilder emptyBuilder(state, state.allocBindings(1)); + + Value vOne; + vOne.mkInt(1); + + Value vTwo; + vTwo.mkInt(2); + + Value vList; + state.mkList(vList, 4); + vList.bigList.elems[0] = &vOne; + vList.bigList.elems[1] = &vTwo; + vList.bigList.size = 2; + + test(vList, + "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_FAINT " «1 item elided»" ANSI_NORMAL "]", + PrintOptions { + .ansiColors = true, + .maxListItems = 1 + }); + + Value vThree; + vThree.mkInt(3); + + vList.bigList.elems[2] = &vThree; + vList.bigList.size = 3; + + test(vList, + "[ " ANSI_CYAN "1" ANSI_NORMAL " " ANSI_FAINT " «2 items elided»" ANSI_NORMAL "]", + PrintOptions { + .ansiColors = true, + .maxListItems = 1 + }); } } // namespace nix From df84dd4d8dd3fd6381ac2ca3064432ab31a16b79 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Tue, 9 Jan 2024 11:13:45 -0800 Subject: [PATCH 10/28] Restore ambiguous value printer for `nix-instantiate` The Nix team has requested that this output format remain unchanged. I've added a warning to the man page explaining that `nix-instantiate --eval` output will not parse correctly in many situations. --- doc/manual/src/command-ref/nix-instantiate.md | 80 ++++++++++---- src/libexpr/print-ambiguous.cc | 100 ++++++++++++++++++ src/libexpr/print-ambiguous.hh | 24 +++++ src/nix-env/user-env.cc | 3 +- src/nix-instantiate/nix-instantiate.cc | 6 +- tests/functional/lang/eval-okay-print.exp | 2 +- 6 files changed, 189 insertions(+), 26 deletions(-) create mode 100644 src/libexpr/print-ambiguous.cc create mode 100644 src/libexpr/print-ambiguous.hh diff --git a/doc/manual/src/command-ref/nix-instantiate.md b/doc/manual/src/command-ref/nix-instantiate.md index e1b4a3e80..483150aa8 100644 --- a/doc/manual/src/command-ref/nix-instantiate.md +++ b/doc/manual/src/command-ref/nix-instantiate.md @@ -35,13 +35,50 @@ standard input. - `--parse`\ Just parse the input files, and print their abstract syntax trees on - standard output in ATerm format. + standard output as a Nix expression. - `--eval`\ Just parse and evaluate the input files, and print the resulting values on standard output. No instantiation of store derivations takes place. + > **Warning** + > + > This option produces ambiguous output which is not suitable for machine + > consumption. For example, these two Nix expressions print the same result + > despite having different types: + > + > ```console + > $ nix-instantiate --eval --expr '{ a = {}; }' + > { a = ; } + > $ nix-instantiate --eval --expr '{ a = ; }' + > { a = ; } + > ``` + > + > For human-readable output, `nix eval` (experimental) is more informative: + > + > ```console + > $ nix-instantiate --eval --expr 'a: a' + > + > $ nix eval --expr 'a: a' + > «lambda @ «string»:1:1» + > ``` + > + > For machine-readable output, the `--xml` option produces unambiguous + > output: + > + > ```console + > $ nix-instantiate --eval --xml --expr '{ foo = ; }' + > + > + > + > + > + > + > + > + > ``` + - `--find-file`\ Look up the given files in Nix’s search path (as specified by the `NIX_PATH` environment variable). If found, print the corresponding @@ -61,11 +98,11 @@ standard input. - `--json`\ When used with `--eval`, print the resulting value as an JSON - representation of the abstract syntax tree rather than as an ATerm. + representation of the abstract syntax tree rather than as a Nix expression. - `--xml`\ When used with `--eval`, print the resulting value as an XML - representation of the abstract syntax tree rather than as an ATerm. + representation of the abstract syntax tree rather than as a Nix expression. The schema is the same as that used by the [`toXML` built-in](../language/builtins.md). @@ -133,28 +170,29 @@ $ nix-instantiate --eval --xml --expr '1 + 2' The difference between non-strict and strict evaluation: ```console -$ nix-instantiate --eval --xml --expr 'rec { x = "foo"; y = x; }' -... - - - - - - -... +$ nix-instantiate --eval --xml --expr '{ x = {}; }' + + + + + + + + ``` Note that `y` is left unevaluated (the XML representation doesn’t attempt to show non-normal forms). ```console -$ nix-instantiate --eval --xml --strict --expr 'rec { x = "foo"; y = x; }' -... - - - - - - -... +$ nix-instantiate --eval --xml --strict --expr '{ x = {}; }' + + + + + + + + + ``` diff --git a/src/libexpr/print-ambiguous.cc b/src/libexpr/print-ambiguous.cc new file mode 100644 index 000000000..07c398dd2 --- /dev/null +++ b/src/libexpr/print-ambiguous.cc @@ -0,0 +1,100 @@ +#include "print-ambiguous.hh" +#include "print.hh" +#include "signals.hh" + +namespace nix { + +// See: https://github.com/NixOS/nix/issues/9730 +void printAmbiguous( + Value &v, + const SymbolTable &symbols, + std::ostream &str, + std::set *seen, + int depth) +{ + checkInterrupt(); + + if (depth <= 0) { + str << "«too deep»"; + return; + } + switch (v.type()) { + case nInt: + str << v.integer; + break; + case nBool: + printLiteralBool(str, v.boolean); + break; + case nString: + printLiteralString(str, v.string_view()); + break; + case nPath: + str << v.path().to_string(); // !!! escaping? + break; + case nNull: + str << "null"; + break; + case nAttrs: { + if (seen && !v.attrs->empty() && !seen->insert(v.attrs).second) + str << "«repeated»"; + else { + str << "{ "; + for (auto & i : v.attrs->lexicographicOrder(symbols)) { + str << symbols[i->name] << " = "; + printAmbiguous(*i->value, symbols, str, seen, depth - 1); + str << "; "; + } + str << "}"; + } + break; + } + case nList: + if (seen && v.listSize() && !seen->insert(v.listElems()).second) + str << "«repeated»"; + else { + str << "[ "; + for (auto v2 : v.listItems()) { + if (v2) + printAmbiguous(*v2, symbols, str, seen, depth - 1); + else + str << "(nullptr)"; + str << " "; + } + str << "]"; + } + break; + case nThunk: + if (!v.isBlackhole()) { + str << ""; + } else { + // Although we know for sure that it's going to be an infinite recursion + // when this value is accessed _in the current context_, it's likely + // that the user will misinterpret a simpler «infinite recursion» output + // as a definitive statement about the value, while in fact it may be + // a valid value after `builtins.trace` and perhaps some other steps + // have completed. + str << "«potential infinite recursion»"; + } + break; + case nFunction: + if (v.isLambda()) { + str << ""; + } else if (v.isPrimOp()) { + str << ""; + } else if (v.isPrimOpApp()) { + str << ""; + } + break; + case nExternal: + str << *v.external; + break; + case nFloat: + str << v.fpoint; + break; + default: + printError("Nix evaluator internal error: printAmbiguous: invalid value type"); + abort(); + } +} + +} diff --git a/src/libexpr/print-ambiguous.hh b/src/libexpr/print-ambiguous.hh new file mode 100644 index 000000000..50c260a9b --- /dev/null +++ b/src/libexpr/print-ambiguous.hh @@ -0,0 +1,24 @@ +#pragma once + +#include "value.hh" + +namespace nix { + +/** + * Print a value in the deprecated format used by `nix-instantiate --eval` and + * `nix-env` (for manifests). + * + * This output can't be changed because it's part of the `nix-instantiate` API, + * but it produces ambiguous output; unevaluated thunks and lambdas (and a few + * other types) are printed as Nix path syntax like ``. + * + * See: https://github.com/NixOS/nix/issues/9730 + */ +void printAmbiguous( + Value &v, + const SymbolTable &symbols, + std::ostream &str, + std::set *seen, + int depth); + +} diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index 3d07cab7a..973b6ee2b 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -108,8 +108,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, environment. */ auto manifestFile = ({ std::ostringstream str; - std::set seen; - printAmbiguous(manifest, state.symbols, str, &seen, std::numeric_limits::max()); + printAmbiguous(manifest, state.symbols, str, nullptr, std::numeric_limits::max()); // TODO with C++20 we can use str.view() instead and avoid copy. std::string str2 = str.str(); StringSource source { str2 }; diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index 9b36dccc6..87bc986e8 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -1,9 +1,11 @@ #include "globals.hh" +#include "print-ambiguous.hh" #include "shared.hh" #include "eval.hh" #include "eval-inline.hh" #include "get-drvs.hh" #include "attr-path.hh" +#include "signals.hh" #include "value-to-xml.hh" #include "value-to-json.hh" #include "store-api.hh" @@ -24,7 +26,6 @@ static int rootNr = 0; enum OutputKind { okPlain, okXML, okJSON }; - void processExpr(EvalState & state, const Strings & attrPaths, bool parseOnly, bool strict, Bindings & autoArgs, bool evalOnly, OutputKind output, bool location, Expr * e) @@ -56,7 +57,8 @@ void processExpr(EvalState & state, const Strings & attrPaths, std::cout << std::endl; } else { if (strict) state.forceValueDeep(vRes); - vRes.print(state, std::cout); + std::set seen; + printAmbiguous(vRes, state.symbols, std::cout, &seen, std::numeric_limits::max()); std::cout << std::endl; } } else { diff --git a/tests/functional/lang/eval-okay-print.exp b/tests/functional/lang/eval-okay-print.exp index aa1b2379e..0d960fb70 100644 --- a/tests/functional/lang/eval-okay-print.exp +++ b/tests/functional/lang/eval-okay-print.exp @@ -1 +1 @@ -[ null «primop toString» «partially applied primop deepSeq» «lambda @ /pwd/lang/eval-okay-print.nix:1:61» [ [ «repeated» ] ] ] +[ null [ [ «repeated» ] ] ] From 7c6f093abcb68a2d07cd6450672c120f33ab96d6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Jan 2024 13:00:53 +0100 Subject: [PATCH 11/28] .data() -> .c_str() to be on the safe side --- src/nix/develop.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 5e25833eb..1f2891378 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -603,7 +603,7 @@ struct CmdDevelop : Common, MixEnvironment setEnviron(); // prevent garbage collection until shell exits - setenv("NIX_GCROOT", gcroot.data(), 1); + setenv("NIX_GCROOT", gcroot.c_str(), 1); Path shell = "bash"; @@ -648,7 +648,7 @@ struct CmdDevelop : Common, MixEnvironment // Override SHELL with the one chosen for this environment. // This is to make sure the system shell doesn't leak into the build environment. - setenv("SHELL", shell.data(), 1); + setenv("SHELL", shell.c_str(), 1); // If running a phase or single command, don't want an interactive shell running after // Ctrl-C, so don't pass --rcfile From 8c7e2ed77c3c14f8a7c82ab6ab7b20ebcfb943a0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Jan 2024 16:21:07 +0100 Subject: [PATCH 12/28] Update release notes --- doc/manual/rl-next/nix-profile-names.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/manual/rl-next/nix-profile-names.md b/doc/manual/rl-next/nix-profile-names.md index f5953bd72..b7ad4b5d7 100644 --- a/doc/manual/rl-next/nix-profile-names.md +++ b/doc/manual/rl-next/nix-profile-names.md @@ -3,4 +3,6 @@ synopsis: "`nix profile` now allows referring to elements by human-readable name prs: 8678 --- -[`nix profile`](@docroot@/command-ref/new-cli/nix3-profile.md) now uses names to refer to installed packages when running [`list`](@docroot@/command-ref/new-cli/nix3-profile-list.md), [`remove`](@docroot@/command-ref/new-cli/nix3-profile-remove.md) or [`upgrade`](@docroot@/command-ref/new-cli/nix3-profile-upgrade.md) as opposed to indices. Indices are deprecated and will be removed in a future version. +[`nix profile`](@docroot@/command-ref/new-cli/nix3-profile.md) now uses names to refer to installed packages when running [`list`](@docroot@/command-ref/new-cli/nix3-profile-list.md), [`remove`](@docroot@/command-ref/new-cli/nix3-profile-remove.md) or [`upgrade`](@docroot@/command-ref/new-cli/nix3-profile-upgrade.md) as opposed to indices. Profile element names are generated when a package is installed and remain the same until the package is removed. + +**Warning**: The `manifest.nix` file used to record the contents of profiles has changed. Nix will automatically upgrade profiles to the new version when you modify the profile. After that, the profile can no longer be used by older versions of Nix. From 72560f7bbef2ab3c02b8ca040fe084328bdd5fbe Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Jan 2024 16:33:15 +0100 Subject: [PATCH 13/28] Add profile migration test --- tests/functional/nix-profile.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/functional/nix-profile.sh b/tests/functional/nix-profile.sh index 003af5174..6f304bd9a 100644 --- a/tests/functional/nix-profile.sh +++ b/tests/functional/nix-profile.sh @@ -193,3 +193,12 @@ nix profile install $flake2Dir --priority 0 clearProfiles nix profile install $(nix build $flake1Dir --no-link --print-out-paths) expect 1 nix profile install --impure --expr "(builtins.getFlake ''$flake2Dir'').packages.$system.default" + +# Test upgrading from profile version 2. +clearProfiles +mkdir -p $TEST_ROOT/import-profile +outPath=$(nix build --no-link --print-out-paths $flake1Dir/flake.nix^out) +printf '{ "version": 2, "elements": [ { "active": true, "attrPath": "legacyPackages.x86_64-linux.hello", "originalUrl": "flake:nixpkgs", "outputs": null, "priority": 5, "storePaths": [ "%s" ], "url": "github:NixOS/nixpkgs/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" } ] }' "$outPath" > $TEST_ROOT/import-profile/manifest.json +nix build --profile $TEST_HOME/.nix-profile $(nix store add-path $TEST_ROOT/import-profile) +nix profile list | grep -A4 'Name:.*hello' | grep "Store paths:.*$outPath" +nix profile remove hello 2>&1 | grep 'removed 1 packages, kept 0 packages' From 25c889baacd6a8b9b66ce4776ec729a40e39cf77 Mon Sep 17 00:00:00 2001 From: Mel Zuser Date: Thu, 11 Jan 2024 14:40:54 -0800 Subject: [PATCH 14/28] Fix performance of builtins.substring for empty substrings When returning a 0-length substring, avoid calling coerceToString, since it returns a string_view with the string's length, which is expensive to compute for large strings. --- src/libexpr/primops.cc | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index ee07e5568..c08aea898 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3712,9 +3712,6 @@ static RegisterPrimOp primop_toString({ static void prim_substring(EvalState & state, const PosIdx pos, Value * * args, Value & v) { int start = state.forceInt(*args[0], pos, "while evaluating the first argument (the start offset) passed to builtins.substring"); - int len = state.forceInt(*args[1], pos, "while evaluating the second argument (the substring length) passed to builtins.substring"); - NixStringContext context; - auto s = state.coerceToString(pos, *args[2], context, "while evaluating the third argument (the string) passed to builtins.substring"); if (start < 0) state.debugThrowLastTrace(EvalError({ @@ -3722,6 +3719,22 @@ static void prim_substring(EvalState & state, const PosIdx pos, Value * * args, .errPos = state.positions[pos] })); + + int len = state.forceInt(*args[1], pos, "while evaluating the second argument (the substring length) passed to builtins.substring"); + + // Special-case on empty substring to avoid O(n) strlen + // This allows for the use of empty substrings to efficently capture string context + if (len == 0) { + state.forceValue(*args[2], pos); + if (args[2]->type() == nString) { + v.mkString("", args[2]->context()); + return; + } + } + + NixStringContext context; + auto s = state.coerceToString(pos, *args[2], context, "while evaluating the third argument (the string) passed to builtins.substring"); + v.mkString((unsigned int) start >= s->size() ? "" : s->substr(start, len), context); } From 6208ca72093a0e05c56561dab349423f9bff069b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 1 Dec 2023 17:03:28 -0500 Subject: [PATCH 15/28] Separate `SystemError` from `SysError` Most of this is a `catch SysError` -> `catch SystemError` sed. This is a rather pure-churn change I would like to get out of the way. **The intersting part is `src/libutil/error.hh`.** On Unix, we will only throw the `SysError` concrete class, which has the same constructors that `SystemError` used to have. On Windows, we will throw `WinError` *and* `SysError`. `WinError` (which will be created in a later PR), will use a `DWORD` instead of `int` error value, and `GetLastError()`, which is the Windows equivalent of the `errno` machinery. Windows will *also* use `SysError` because Window's "libc" (MSVCRT) implements the POSIX interface, and we use it too. As the docs describe, while we *throw* one of the 3 choices above (2 concrete classes or the alias), we should always *catch* `SystemError`. This ensures no matter how the implementation changes for Windows (e.g. between `SysError` and `WinError`) the catching logic stays the same and stays correct. Co-Authored-By volth Co-Authored-By Eugene Butler --- src/libcmd/repl.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 4 +- src/libstore/gc.cc | 2 +- src/libstore/globals.cc | 2 +- src/libstore/keys.cc | 2 +- src/libstore/local-store.cc | 2 +- src/libstore/optimise-store.cc | 2 +- src/libstore/remote-fs-accessor.cc | 4 +- src/libutil/args.cc | 2 +- src/libutil/cgroup.cc | 2 +- src/libutil/config.cc | 2 +- src/libutil/error.hh | 42 ++++++++++++++++++--- src/libutil/file-descriptor.cc | 2 +- src/libutil/logging.cc | 2 +- src/libutil/serialise.cc | 2 +- src/libutil/util.cc | 2 +- src/nix-build/nix-build.cc | 2 +- src/nix/config-check.cc | 2 +- tests/unit/libutil/logging.cc | 6 +-- 19 files changed, 59 insertions(+), 27 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 72e3559df..918b2e53a 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -254,7 +254,7 @@ void NixRepl::mainLoop() rl_readline_name = "nix-repl"; try { createDirs(dirOf(historyFile)); - } catch (SysError & e) { + } catch (SystemError & e) { logWarning(e.info()); } #ifndef USE_READLINE diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index b01d9e237..f85301950 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1495,7 +1495,7 @@ void LocalDerivationGoal::startDaemon() daemon::processConnection(store, from, to, NotTrusted, daemon::Recursive); debug("terminated daemon connection"); - } catch (SysError &) { + } catch (SystemError &) { ignoreException(); } }); @@ -1707,7 +1707,7 @@ void LocalDerivationGoal::runChild() try { if (drv->isBuiltin() && drv->builder == "builtin:fetchurl") netrcData = readFile(settings.netrcFile); - } catch (SysError &) { } + } catch (SystemError &) { } #if __linux__ if (useChroot) { diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 2bd3a2edc..5cbce0748 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -413,7 +413,7 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor) auto env_end = std::sregex_iterator{}; for (auto i = std::sregex_iterator{envString.begin(), envString.end(), storePathRegex}; i != env_end; ++i) unchecked[i->str()].emplace(envFile); - } catch (SysError & e) { + } catch (SystemError & e) { if (errno == ENOENT || errno == EACCES || errno == ESRCH) continue; throw; diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 50584e06c..d22ae4ca0 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -118,7 +118,7 @@ void loadConfFile() try { std::string contents = readFile(path); globalConfig.applyConfig(contents, path); - } catch (SysError &) { } + } catch (SystemError &) { } }; applyConfigFile(settings.nixConfDir + "/nix.conf"); diff --git a/src/libstore/keys.cc b/src/libstore/keys.cc index 2cc50970f..70478e7ad 100644 --- a/src/libstore/keys.cc +++ b/src/libstore/keys.cc @@ -19,7 +19,7 @@ PublicKeys getDefaultPublicKeys() try { SecretKey secretKey(readFile(secretKeyFile)); publicKeys.emplace(secretKey.name, secretKey.toPublicKey()); - } catch (SysError & e) { + } catch (SystemError & e) { /* Ignore unreadable key files. That's normal in a multi-user installation. */ } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 0f3c37c8a..5a399c8be 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -276,7 +276,7 @@ LocalStore::LocalStore(const Params & params) [[gnu::unused]] auto res2 = ftruncate(fd.get(), settings.reservedSize); } } - } catch (SysError & e) { /* don't care about errors */ + } catch (SystemError & e) { /* don't care about errors */ } /* Acquire the big fat lock in shared mode to make sure that no diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index a494e6ecc..78e4f6d86 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -242,7 +242,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, /* Atomically replace the old file with the new hard link. */ try { renameFile(tempLink, path); - } catch (SysError & e) { + } catch (SystemError & e) { if (unlink(tempLink.c_str()) == -1) printError("unable to unlink '%1%'", tempLink); if (errno == EMLINK) { diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc index 03e57a565..b44edfe89 100644 --- a/src/libstore/remote-fs-accessor.cc +++ b/src/libstore/remote-fs-accessor.cc @@ -87,13 +87,13 @@ std::pair, CanonPath> RemoteFSAccessor::fetch(const CanonPat nars.emplace(storePath.hashPart(), narAccessor); return {narAccessor, restPath}; - } catch (SysError &) { } + } catch (SystemError &) { } try { auto narAccessor = makeNarAccessor(nix::readFile(cacheFile)); nars.emplace(storePath.hashPart(), narAccessor); return {narAccessor, restPath}; - } catch (SysError &) { } + } catch (SystemError &) { } } StringSink sink; diff --git a/src/libutil/args.cc b/src/libutil/args.cc index e2668c673..5187e7396 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -304,7 +304,7 @@ void RootArgs::parseCmdline(const Strings & _cmdline, bool allowShebang) for (auto pos = savedArgs.begin(); pos != savedArgs.end();pos++) cmdline.push_back(*pos); } - } catch (SysError &) { } + } catch (SystemError &) { } } for (auto pos = cmdline.begin(); pos != cmdline.end(); ) { diff --git a/src/libutil/cgroup.cc b/src/libutil/cgroup.cc index 4c2bf31ff..de83b5ad1 100644 --- a/src/libutil/cgroup.cc +++ b/src/libutil/cgroup.cc @@ -95,7 +95,7 @@ static CgroupStats destroyCgroup(const Path & cgroup, bool returnStats) using namespace std::string_literals; warn("killing stray builder process %d (%s)...", pid, trim(replaceStrings(cmdline, "\0"s, " "))); - } catch (SysError &) { + } catch (SystemError &) { } } // FIXME: pid wraparound diff --git a/src/libutil/config.cc b/src/libutil/config.cc index a3310f4ec..37f5b50c7 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -124,7 +124,7 @@ static void applyConfigInner(const std::string & contents, const std::string & p try { std::string includedContents = readFile(path); applyConfigInner(includedContents, p, parsedContents); - } catch (SysError &) { + } catch (SystemError &) { // TODO: Do we actually want to ignore this? Or is it better to fail? } } else if (!ignoreMissing) { diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 234cbe1f6..764fac1ce 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -178,20 +178,50 @@ MakeError(Error, BaseError); MakeError(UsageError, Error); MakeError(UnimplementedError, Error); -class SysError : public Error +/** + * To use in catch-blocks. + */ +MakeError(SystemError, Error); + +/** + * POSIX system error, created using `errno`, `strerror` friends. + * + * Throw this, but prefer not to catch this, and catch `SystemError` + * instead. This allows implementations to freely switch between this + * and `WinError` without breaking catch blocks. + * + * However, it is permissible to catch this and rethrow so long as + * certain conditions are not met (e.g. to catch only if `errNo = + * EFooBar`). In that case, try to also catch the equivalent `WinError` + * code. + * + * @todo Rename this to `PosixError` or similar. At this point Windows + * support is too WIP to justify the code churn, but if it is finished + * then a better identifier becomes moe worth it. + */ +class SysError : public SystemError { public: int errNo; + /** + * Construct using the explicitly-provided error number. `strerror` + * will be used to try to add additional information to the message. + */ template - SysError(int errNo_, const Args & ... args) - : Error("") + SysError(int errNo, const Args & ... args) + : SystemError(""), errNo(errNo) { - errNo = errNo_; auto hf = hintfmt(args...); err.msg = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo)); } + /** + * Construct using the ambient `errno`. + * + * Be sure to not perform another `errno`-modifying operation before + * calling this constructor! + */ template SysError(const Args & ... args) : SysError(errno, args ...) @@ -199,7 +229,9 @@ public: } }; -/** Throw an exception for the purpose of checking that exception handling works; see 'initLibUtil()'. +/** + * Throw an exception for the purpose of checking that exception + * handling works; see 'initLibUtil()'. */ void throwExceptionSelfCheck(); diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index 38dd70c8e..692be3383 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -231,7 +231,7 @@ void closeMostFDs(const std::set & exceptions) } } return; - } catch (SysError &) { + } catch (SystemError &) { } #endif diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 183aee2dc..d68ddacc0 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -116,7 +116,7 @@ void writeToStderr(std::string_view s) { try { writeFull(STDERR_FILENO, s, false); - } catch (SysError & e) { + } catch (SystemError & e) { /* Ignore failing writes to stderr. We need to ignore write errors to ensure that cleanup code that logs to stderr runs to completion if the other side of stderr has been closed diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 76b378e18..316105603 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -53,7 +53,7 @@ void FdSink::writeUnbuffered(std::string_view data) written += data.size(); try { writeFull(fd, data); - } catch (SysError & e) { + } catch (SystemError & e) { _good = false; throw; } diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 7b4b1d031..b23362b5c 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -20,7 +20,7 @@ void initLibUtil() { // When exception handling fails, the message tends to be printed by the // C++ runtime, followed by an abort. // For example on macOS we might see an error such as - // libc++abi: terminating with uncaught exception of type nix::SysError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded. + // libc++abi: terminating with uncaught exception of type nix::SystemError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded. bool caught = false; try { throwExceptionSelfCheck(); diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index ee2addb72..1ad4b387c 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -148,7 +148,7 @@ static void main_nix_build(int argc, char * * argv) args.push_back(word); } } - } catch (SysError &) { } + } catch (SystemError &) { } } struct MyArgs : LegacyArgs, MixEvalArgs diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index 410feca2f..8d4717e15 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -107,7 +107,7 @@ struct CmdConfigCheck : StoreCommand if (profileDir.find("/profiles/") == std::string::npos) dirs.insert(dir); } - } catch (SysError &) {} + } catch (SystemError &) {} } if (!dirs.empty()) { diff --git a/tests/unit/libutil/logging.cc b/tests/unit/libutil/logging.cc index c6dfe63d3..8950a26d4 100644 --- a/tests/unit/libutil/logging.cc +++ b/tests/unit/libutil/logging.cc @@ -73,7 +73,7 @@ namespace nix { } - TEST(logEI, picksUpSysErrorExitCode) { + TEST(logEI, picksUpSystemErrorExitCode) { MakeError(TestError, Error); ErrorInfo::programName = std::optional("error-unit-test"); @@ -81,12 +81,12 @@ namespace nix { try { auto x = readFile(-1); } - catch (SysError &e) { + catch (SystemError &e) { testing::internal::CaptureStderr(); logError(e.info()); auto str = testing::internal::GetCapturedStderr(); - ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n"); + ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SystemError --- error-unit-test\x1B[0m\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n"); } } From 1996105e91d8d2022869c4e66c0a0734e363052b Mon Sep 17 00:00:00 2001 From: Mel Zuser Date: Fri, 12 Jan 2024 08:57:08 -0800 Subject: [PATCH 16/28] added test for empty substring special case --- tests/functional/lang/eval-okay-substring-context.exp | 1 + tests/functional/lang/eval-okay-substring-context.nix | 11 +++++++++++ 2 files changed, 12 insertions(+) create mode 100644 tests/functional/lang/eval-okay-substring-context.exp create mode 100644 tests/functional/lang/eval-okay-substring-context.nix diff --git a/tests/functional/lang/eval-okay-substring-context.exp b/tests/functional/lang/eval-okay-substring-context.exp new file mode 100644 index 000000000..2fe7f71fa --- /dev/null +++ b/tests/functional/lang/eval-okay-substring-context.exp @@ -0,0 +1 @@ +"okay" diff --git a/tests/functional/lang/eval-okay-substring-context.nix b/tests/functional/lang/eval-okay-substring-context.nix new file mode 100644 index 000000000..d0ef70d4e --- /dev/null +++ b/tests/functional/lang/eval-okay-substring-context.nix @@ -0,0 +1,11 @@ +with builtins; + +let + + s = "${builtins.derivation { name = "test"; builder = "/bin/sh"; system = "x86_64-linux"; }}"; + +in + +if getContext s == getContext "${substring 0 0 s + unsafeDiscardStringContext s}" +then "okay" +else throw "empty substring should preserve context" From b29be1ff57e6e358b2925012a13d7d4a0312560e Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Fri, 12 Jan 2024 10:01:55 -0800 Subject: [PATCH 17/28] Document unit tests in hacking.md --- doc/manual/src/contributing/hacking.md | 5 ++++- doc/manual/src/contributing/testing.md | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/manual/src/contributing/hacking.md b/doc/manual/src/contributing/hacking.md index 9a03ac9b6..0fa59e891 100644 --- a/doc/manual/src/contributing/hacking.md +++ b/doc/manual/src/contributing/hacking.md @@ -51,11 +51,14 @@ To install it in `$(pwd)/outputs` and test it: ```console [nix-shell]$ make install -[nix-shell]$ make installcheck -j $NIX_BUILD_CORES +[nix-shell]$ make installcheck check -j $NIX_BUILD_CORES [nix-shell]$ nix --version nix (Nix) 2.12 ``` +For more information on running and filtering tests, see +[`testing.md`](./testing.md). + To build a release version of Nix for the current operating system and CPU architecture: ```console diff --git a/doc/manual/src/contributing/testing.md b/doc/manual/src/contributing/testing.md index d8d162379..31c39c16c 100644 --- a/doc/manual/src/contributing/testing.md +++ b/doc/manual/src/contributing/testing.md @@ -77,7 +77,7 @@ there is no risk of any build-system wildcards for the library accidentally pick ### Running tests You can run the whole testsuite with `make check`, or the tests for a specific component with `make libfoo-tests_RUN`. -Finer-grained filtering is also possible using the [--gtest_filter](https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests) command-line option, or the `GTEST_FILTER` environment variable. +Finer-grained filtering is also possible using the [--gtest_filter](https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests) command-line option, or the `GTEST_FILTER` environment variable, e.g. `GTEST_FILTER='ErrorTraceTest.*' make check`. ### Characterisation testing { #characaterisation-testing-unit } From dd7e7b0a30a0564741c40e70f33cbf1cd6130106 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 11 Jan 2024 11:26:03 -0500 Subject: [PATCH 18/28] Newer Nixpkgs, get `readline` on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now `nix repl` an, in principle, work on that platform too. Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/2c9c58e98243930f8cb70387934daa4bc8b00373' (2023-12-31) → 'github:NixOS/nixpkgs/86501af7f1d51915e6c335f90f2cab73d7704ef3' (2024-01-11) --- flake.lock | 6 +++--- package.nix | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/flake.lock b/flake.lock index ae98d789a..65e468e8b 100644 --- a/flake.lock +++ b/flake.lock @@ -34,11 +34,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1704018918, - "narHash": "sha256-erjg/HrpC9liEfm7oLqb8GXCqsxaFwIIPqCsknW5aFY=", + "lastModified": 1704982786, + "narHash": "sha256-w62+4HyaHafLWjvrC2Eto7bSnSJQtia8oqs3//mkpCU=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "2c9c58e98243930f8cb70387934daa4bc8b00373", + "rev": "86501af7f1d51915e6c335f90f2cab73d7704ef3", "type": "github" }, "original": { diff --git a/package.nix b/package.nix index 37410dc2f..a632fd6ec 100644 --- a/package.nix +++ b/package.nix @@ -236,7 +236,6 @@ in { openssl sqlite xz - ] ++ lib.optionals (!stdenv.hostPlatform.isWindows) [ ({ inherit readline editline; }.${readlineFlavor}) ] ++ lib.optionals enableMarkdown [ lowdown From e739a5002dab199a6cf207e6e62b394fa77f8cb2 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 12 Jan 2024 19:46:48 -0500 Subject: [PATCH 19/28] Avoid Windows macros in the parser and lexer `FLOAT`, `INT`, and `IN` are identifers taken by macros. The name `IN_KW` is chosen to match `OR_KW`, which is presumably named that way for the same reason of dodging macros. --- src/libexpr/lexer.l | 6 +++--- src/libexpr/parser.y | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l index df2cbd06f..9addb3ae8 100644 --- a/src/libexpr/lexer.l +++ b/src/libexpr/lexer.l @@ -130,7 +130,7 @@ else { return ELSE; } assert { return ASSERT; } with { return WITH; } let { return LET; } -in { return IN; } +in { return IN_KW; } rec { return REC; } inherit { return INHERIT; } or { return OR_KW; } @@ -156,7 +156,7 @@ or { return OR_KW; } .errPos = data->state.positions[CUR_POS], }); } - return INT; + return INT_LIT; } {FLOAT} { errno = 0; yylval->nf = strtod(yytext, 0); @@ -165,7 +165,7 @@ or { return OR_KW; } .msg = hintfmt("invalid float '%1%'", yytext), .errPos = data->state.positions[CUR_POS], }); - return FLOAT; + return FLOAT_LIT; } \$\{ { PUSH_STATE(DEFAULT); return DOLLAR_CURLY; } diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index b331776f0..60bcfebf9 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -365,11 +365,11 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err %type attr %token ID %token STR IND_STR -%token INT -%token FLOAT +%token INT_LIT +%token FLOAT_LIT %token PATH HPATH SPATH PATH_END %token URI -%token IF THEN ELSE ASSERT WITH LET IN REC INHERIT EQ NEQ AND OR IMPL OR_KW +%token IF THEN ELSE ASSERT WITH LET IN_KW REC INHERIT EQ NEQ AND OR IMPL OR_KW %token DOLLAR_CURLY /* == ${ */ %token IND_STRING_OPEN IND_STRING_CLOSE %token ELLIPSIS @@ -412,7 +412,7 @@ expr_function { $$ = new ExprAssert(CUR_POS, $2, $4); } | WITH expr ';' expr_function { $$ = new ExprWith(CUR_POS, $2, $4); } - | LET binds IN expr_function + | LET binds IN_KW expr_function { if (!$2->dynamicAttrs.empty()) throw ParseError({ .msg = hintfmt("dynamic attributes not allowed in let"), @@ -482,8 +482,8 @@ expr_simple else $$ = new ExprVar(CUR_POS, data->symbols.create($1)); } - | INT { $$ = new ExprInt($1); } - | FLOAT { $$ = new ExprFloat($1); } + | INT_LIT { $$ = new ExprInt($1); } + | FLOAT_LIT { $$ = new ExprFloat($1); } | '"' string_parts '"' { $$ = $2; } | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { $$ = stripIndentation(CUR_POS, data->symbols, std::move(*$2)); From cbd5553d57ebf5d0532047165a2d81825424bd76 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Sat, 13 Jan 2024 04:20:08 -0700 Subject: [PATCH 20/28] doc: provide context in glossary definitions (#9378) --- doc/manual/src/glossary.md | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/doc/manual/src/glossary.md b/doc/manual/src/glossary.md index 07891175a..1fdb8b4dd 100644 --- a/doc/manual/src/glossary.md +++ b/doc/manual/src/glossary.md @@ -3,10 +3,10 @@ - [derivation]{#gloss-derivation} A description of a build task. The result of a derivation is a - store object. Derivations are typically specified in Nix expressions + store object. Derivations declared in Nix expressions are specified using the [`derivation` primitive](./language/derivations.md). These are translated into low-level *store derivations* (implicitly by - `nix-env` and `nix-build`, or explicitly by `nix-instantiate`). + `nix-build`, or explicitly by `nix-instantiate`). [derivation]: #gloss-derivation @@ -14,6 +14,7 @@ A [derivation] represented as a `.drv` file in the [store]. It has a [store path], like any [store object]. + It is the [instantiated][instantiate] form of a derivation. Example: `/nix/store/g946hcz4c8mdvq2g8vxx42z51qb71rvp-git-2.38.1.drv` @@ -23,9 +24,9 @@ - [instantiate]{#gloss-instantiate}, instantiation - Translate a [derivation] into a [store derivation]. + Save an evaluated [derivation] as a [store derivation] in the Nix [store]. - See [`nix-instantiate`](./command-ref/nix-instantiate.md). + See [`nix-instantiate`](./command-ref/nix-instantiate.md), which produces a store derivation from a Nix expression that evaluates to a derivation. [instantiate]: #gloss-instantiate @@ -66,7 +67,7 @@ From the perspective of the location where Nix is invoked, the Nix store can be referred to _local_ or _remote_. Only a [local store]{#gloss-local-store} exposes a location in the file system of the machine where Nix is invoked that allows access to store objects, typically `/nix/store`. - Local stores can be used for building [derivations](#derivation). + Local stores can be used for building [derivations](#gloss-derivation). See [Local Store](@docroot@/command-ref/new-cli/nix3-help-stores.md#local-store) for details. [store]: #gloss-store @@ -168,9 +169,10 @@ A high-level description of software packages and compositions thereof. Deploying software using Nix entails writing Nix - expressions for your packages. Nix expressions are translated to - derivations that are stored in the Nix store. These derivations can - then be built. + expressions for your packages. Nix expressions specify [derivations][derivation], + which are [instantiated][instantiate] into the Nix store as [store derivations][store derivation]. + These derivations can then be [realised][realise] to produce + [outputs][output]. - [reference]{#gloss-reference} @@ -222,6 +224,9 @@ The [store derivation] that produced an [output path]. + The deriver for an output path can be queried with the `--deriver` option to + [`nix-store --query`](@docroot@/command-ref/nix-store/query.md). + - [validity]{#gloss-validity} A store path is valid if all [store object]s in its [closure] can be read from the [store]. From f61d951909a619b7a430d8d8aa739e310c7bf472 Mon Sep 17 00:00:00 2001 From: Las Safin Date: Sat, 13 Jan 2024 19:27:20 +0000 Subject: [PATCH 21/28] Avoid unnecessary copy of goal log The data was (accidentally?) copied into a std::string, even though the string is immediately converted into a std::string_view. The code has been changed to construct a std::string_view directly, such that one copy less happens. --- src/libstore/build/worker.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 399ad47fd..974a9f510 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -449,7 +449,7 @@ void Worker::waitForInput() } else { printMsg(lvlVomit, "%1%: read %2% bytes", goal->getName(), rd); - std::string data((char *) buffer.data(), rd); + std::string_view data((char *) buffer.data(), rd); j->lastOutput = after; goal->handleChildOutput(k, data); } From 03a6ca9b253c35b33e041dce595239968224e0d3 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 14 Jan 2024 15:25:24 -0500 Subject: [PATCH 22/28] `tests/functional/nix-profile.sh`: Add missing `--no-link` Otherwise we get a stray `tests/functional/result`, which can cause spurious failures later. (I got a failure because the test temp dir effecting the store dir changed. This caused a test later because Nix didn't want to remove the old `result` because it wasn't pointing inside the new Nix store.) --- tests/functional/nix-profile.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/nix-profile.sh b/tests/functional/nix-profile.sh index 6f304bd9a..35a62fbe2 100644 --- a/tests/functional/nix-profile.sh +++ b/tests/functional/nix-profile.sh @@ -199,6 +199,6 @@ clearProfiles mkdir -p $TEST_ROOT/import-profile outPath=$(nix build --no-link --print-out-paths $flake1Dir/flake.nix^out) printf '{ "version": 2, "elements": [ { "active": true, "attrPath": "legacyPackages.x86_64-linux.hello", "originalUrl": "flake:nixpkgs", "outputs": null, "priority": 5, "storePaths": [ "%s" ], "url": "github:NixOS/nixpkgs/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" } ] }' "$outPath" > $TEST_ROOT/import-profile/manifest.json -nix build --profile $TEST_HOME/.nix-profile $(nix store add-path $TEST_ROOT/import-profile) +nix build --profile $TEST_HOME/.nix-profile $(nix store add-path $TEST_ROOT/import-profile) --no-link nix profile list | grep -A4 'Name:.*hello' | grep "Store paths:.*$outPath" nix profile remove hello 2>&1 | grep 'removed 1 packages, kept 0 packages' From dd42a4e3e9ec6d76d393e24f449f161b62579dc5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 15 Jan 2024 08:04:46 -0500 Subject: [PATCH 23/28] 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/86501af7f1d51915e6c335f90f2cab73d7704ef3' (2024-01-11) → 'github:NixOS/nixpkgs/a1982c92d8980a0114372973cbdfe0a307f1bdea' (2024-01-12) --- flake.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake.lock b/flake.lock index 65e468e8b..f0efb4036 100644 --- a/flake.lock +++ b/flake.lock @@ -34,11 +34,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1704982786, - "narHash": "sha256-w62+4HyaHafLWjvrC2Eto7bSnSJQtia8oqs3//mkpCU=", + "lastModified": 1705033721, + "narHash": "sha256-K5eJHmL1/kev6WuqyqqbS1cdNnSidIZ3jeqJ7GbrYnQ=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "86501af7f1d51915e6c335f90f2cab73d7704ef3", + "rev": "a1982c92d8980a0114372973cbdfe0a307f1bdea", "type": "github" }, "original": { From 9b9ecdee3424056cb854bc8f1aa49fe330c08c83 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 11 Jan 2024 23:50:03 -0500 Subject: [PATCH 24/28] Simplify RapidCheck configure No more `RAPIDCHECK_HEADERS`! --- Makefile.config.in | 1 - configure.ac | 21 +-------------------- doc/internal-api/doxygen.cfg.in | 2 +- package.nix | 2 +- 4 files changed, 3 insertions(+), 23 deletions(-) diff --git a/Makefile.config.in b/Makefile.config.in index 21a9f41ec..d5c382630 100644 --- a/Makefile.config.in +++ b/Makefile.config.in @@ -29,7 +29,6 @@ LOWDOWN_LIBS = @LOWDOWN_LIBS@ OPENSSL_LIBS = @OPENSSL_LIBS@ PACKAGE_NAME = @PACKAGE_NAME@ PACKAGE_VERSION = @PACKAGE_VERSION@ -RAPIDCHECK_HEADERS = @RAPIDCHECK_HEADERS@ SHELL = @bash@ SODIUM_LIBS = @SODIUM_LIBS@ SQLITE3_LIBS = @SQLITE3_LIBS@ diff --git a/configure.ac b/configure.ac index 2594544ab..f46cff732 100644 --- a/configure.ac +++ b/configure.ac @@ -353,27 +353,8 @@ AS_IF([test "$ENABLE_UNIT_TESTS" == "yes"],[ # Look for gtest. PKG_CHECK_MODULES([GTEST], [gtest_main]) - # Look for rapidcheck. -AC_ARG_VAR([RAPIDCHECK_HEADERS], [include path of gtest headers shipped by RAPIDCHECK]) -# No pkg-config yet, https://github.com/emil-e/rapidcheck/issues/302 -AC_LANG_PUSH(C++) -AC_SUBST(RAPIDCHECK_HEADERS) -[CXXFLAGS="-I $RAPIDCHECK_HEADERS $CXXFLAGS"] -[LIBS="-lrapidcheck -lgtest $LIBS"] -AC_CHECK_HEADERS([rapidcheck/gtest.h], [], [], [#include ]) -dnl AC_CHECK_LIB doesn't work for C++ libs with mangled symbols -AC_LINK_IFELSE([ - AC_LANG_PROGRAM([[ - #include - #include - ]], [[ - return RUN_ALL_TESTS(); - ]]) - ], - [], - [AC_MSG_ERROR([librapidcheck is not found.])]) -AC_LANG_POP(C++) +PKG_CHECK_MODULES([RAPIDCHECK], [rapidcheck rapidcheck_gtest]) ]) diff --git a/doc/internal-api/doxygen.cfg.in b/doc/internal-api/doxygen.cfg.in index ad5af97e6..6c6c325bd 100644 --- a/doc/internal-api/doxygen.cfg.in +++ b/doc/internal-api/doxygen.cfg.in @@ -81,7 +81,7 @@ EXPAND_ONLY_PREDEF = YES # RECURSIVE has no effect here. # This tag requires that the tag SEARCH_INCLUDES is set to YES. -INCLUDE_PATH = @RAPIDCHECK_HEADERS@ +INCLUDE_PATH = # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this # tag can be used to specify a list of macro names that should be expanded. The diff --git a/package.nix b/package.nix index a632fd6ec..a1188ba9c 100644 --- a/package.nix +++ b/package.nix @@ -309,7 +309,7 @@ in { ] ++ lib.optional (doBuild && stdenv.isLinux && !(stdenv.hostPlatform.isStatic && stdenv.system == "aarch64-linux")) "LDFLAGS=-fuse-ld=gold" ++ lib.optional (doBuild && stdenv.hostPlatform.isStatic) "--enable-embedded-sandbox-shell" - ++ lib.optional buildUnitTests "RAPIDCHECK_HEADERS=${lib.getDev rapidcheck}/extras/gtest/include"; + ; enableParallelBuilding = true; From beed00c04e136e8d685905e4b2b1116ecdf42f63 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 13 Jan 2024 13:08:38 -0500 Subject: [PATCH 25/28] `absPath`: just take a `std::string_view` 1. Slightly more efficient 2. Easier to call Co-authored-by: Cole Helbling --- src/libutil/canon-path.cc | 6 +++--- src/libutil/file-system.cc | 14 +++++++++++--- src/libutil/file-system.hh | 2 +- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/libutil/canon-path.cc b/src/libutil/canon-path.cc index 1e465f1f6..0a0f96a05 100644 --- a/src/libutil/canon-path.cc +++ b/src/libutil/canon-path.cc @@ -6,11 +6,11 @@ namespace nix { CanonPath CanonPath::root = CanonPath("/"); CanonPath::CanonPath(std::string_view raw) - : path(absPath((Path) raw, "/")) + : path(absPath(raw, "/")) { } CanonPath::CanonPath(std::string_view raw, const CanonPath & root) - : path(absPath((Path) raw, root.abs())) + : path(absPath(raw, root.abs())) { } CanonPath::CanonPath(const std::vector & elems) @@ -22,7 +22,7 @@ CanonPath::CanonPath(const std::vector & elems) CanonPath CanonPath::fromCwd(std::string_view path) { - return CanonPath(unchecked_t(), absPath((Path) path)); + return CanonPath(unchecked_t(), absPath(path)); } std::optional CanonPath::parent() const diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 4cac35ace..ab8d32275 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -21,9 +21,16 @@ namespace fs = std::filesystem; namespace nix { -Path absPath(Path path, std::optional dir, bool resolveSymlinks) +Path absPath(PathView path, std::optional dir, bool resolveSymlinks) { + std::string scratch; + if (path[0] != '/') { + // In this case we need to call `canonPath` on a newly-created + // string. We set `scratch` to that string first, and then set + // `path` to `scratch`. This ensures the newly-created string + // lives long enough for the call to `canonPath`, and allows us + // to just accept a `std::string_view`. if (!dir) { #ifdef __GNU__ /* GNU (aka. GNU/Hurd) doesn't have any limitation on path @@ -35,12 +42,13 @@ Path absPath(Path path, std::optional dir, bool resolveSymlinks) if (!getcwd(buf, sizeof(buf))) #endif throw SysError("cannot get cwd"); - path = concatStrings(buf, "/", path); + scratch = concatStrings(buf, "/", path); #ifdef __GNU__ free(buf); #endif } else - path = concatStrings(*dir, "/", path); + scratch = concatStrings(*dir, "/", path); + path = scratch; } return canonPath(path, resolveSymlinks); } diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 4637507b3..464efc242 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -41,7 +41,7 @@ struct Source; * specified directory, or the current directory otherwise. The path * is also canonicalised. */ -Path absPath(Path path, +Path absPath(PathView path, std::optional dir = {}, bool resolveSymlinks = false); From e0a76430861efbcfaf14c8b3691a091e6e72a8ed Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Jan 2024 22:35:12 +0000 Subject: [PATCH 26/28] Bump cachix/install-nix-action from 24 to 25 Bumps [cachix/install-nix-action](https://github.com/cachix/install-nix-action) from 24 to 25. - [Release notes](https://github.com/cachix/install-nix-action/releases) - [Commits](https://github.com/cachix/install-nix-action/compare/v24...v25) --- 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 aa2551424..8d88de4b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: cachix/install-nix-action@v24 + - uses: cachix/install-nix-action@v25 with: # The sandbox would otherwise be disabled by default on Darwin extra_nix_config: "sandbox = true" @@ -62,7 +62,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@v24 + - uses: cachix/install-nix-action@v25 with: install_url: https://releases.nixos.org/nix/nix-2.13.3/install - uses: cachix/cachix-action@v13 @@ -84,7 +84,7 @@ jobs: steps: - uses: actions/checkout@v4 - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV - - uses: cachix/install-nix-action@v24 + - uses: cachix/install-nix-action@v25 with: install_url: '${{needs.installer.outputs.installerURL}}' install_options: "--tarball-url-prefix https://${{ env.CACHIX_NAME }}.cachix.org/serve" @@ -114,7 +114,7 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: cachix/install-nix-action@v24 + - uses: cachix/install-nix-action@v25 with: install_url: https://releases.nixos.org/nix/nix-2.13.3/install - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV From bf7754c0991c33146da9c339a71d661615afc93a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Jan 2024 22:35:15 +0000 Subject: [PATCH 27/28] Bump cachix/cachix-action from 13 to 14 Bumps [cachix/cachix-action](https://github.com/cachix/cachix-action) from 13 to 14. - [Release notes](https://github.com/cachix/cachix-action/releases) - [Commits](https://github.com/cachix/cachix-action/compare/v13...v14) --- updated-dependencies: - dependency-name: cachix/cachix-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa2551424..878720acc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,7 +25,7 @@ jobs: # The sandbox would otherwise be disabled by default on Darwin extra_nix_config: "sandbox = true" - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV - - uses: cachix/cachix-action@v13 + - uses: cachix/cachix-action@v14 if: needs.check_secrets.outputs.cachix == 'true' with: name: '${{ env.CACHIX_NAME }}' @@ -65,7 +65,7 @@ jobs: - uses: cachix/install-nix-action@v24 with: install_url: https://releases.nixos.org/nix/nix-2.13.3/install - - uses: cachix/cachix-action@v13 + - uses: cachix/cachix-action@v14 with: name: '${{ env.CACHIX_NAME }}' signingKey: '${{ secrets.CACHIX_SIGNING_KEY }}' @@ -119,7 +119,7 @@ jobs: install_url: https://releases.nixos.org/nix/nix-2.13.3/install - run: echo CACHIX_NAME="$(echo $GITHUB_REPOSITORY-install-tests | tr "[A-Z]/" "[a-z]-")" >> $GITHUB_ENV - run: echo NIX_VERSION="$(nix --experimental-features 'nix-command flakes' eval .\#default.version | tr -d \")" >> $GITHUB_ENV - - uses: cachix/cachix-action@v13 + - uses: cachix/cachix-action@v14 if: needs.check_secrets.outputs.cachix == 'true' with: name: '${{ env.CACHIX_NAME }}' From cbc319e9be3b29e3eb29a6e4cf08db1e0363d7bd Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 16 Jan 2024 12:18:02 +0100 Subject: [PATCH 28/28] tests/functional/lang: Test substring with negative length --- tests/functional/lang/eval-okay-substring.exp | 2 +- tests/functional/lang/eval-okay-substring.nix | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/lang/eval-okay-substring.exp b/tests/functional/lang/eval-okay-substring.exp index 6aace04b0..f48b4623a 100644 --- a/tests/functional/lang/eval-okay-substring.exp +++ b/tests/functional/lang/eval-okay-substring.exp @@ -1 +1 @@ -"ooxfoobarybarzobaabbc" +"ooxfoobarybarzobaabbc_bad" diff --git a/tests/functional/lang/eval-okay-substring.nix b/tests/functional/lang/eval-okay-substring.nix index 424af00d9..54c97e162 100644 --- a/tests/functional/lang/eval-okay-substring.nix +++ b/tests/functional/lang/eval-okay-substring.nix @@ -19,3 +19,5 @@ substring 1 2 s + substring 3 1 s + "c" + substring 5 10 "perl" ++ "_" ++ substring 3 (-1) "tebbad"