From e0911eef73e36d5b42ebd2e9fa114d535ab287f7 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Wed, 1 May 2024 19:55:26 -0600 Subject: [PATCH] nix3-profile: make element names stable Based off of commit 6268a45b650f563bae2360e0540920a2959bdd40 Upstream-PR: https://github.com/NixOS/nix/pull/9656 Co-authored-by: Eelco Dolstra Change-Id: I0fcf069a8537c61ad6fc4eee1f3c193a708ea1c4 --- doc/manual/rl-next/nix-profile-names.md | 6 +- src/libcmd/cmd-profiles.cc | 110 +++++++++++++----------- src/libcmd/cmd-profiles.hh | 6 +- src/nix/profile.cc | 38 ++++---- src/nix/upgrade-nix.cc | 8 +- tests/functional/nix-profile.sh | 15 ++-- 6 files changed, 102 insertions(+), 81 deletions(-) diff --git a/doc/manual/rl-next/nix-profile-names.md b/doc/manual/rl-next/nix-profile-names.md index 2a37691e6..53dc53fa9 100644 --- a/doc/manual/rl-next/nix-profile-names.md +++ b/doc/manual/rl-next/nix-profile-names.md @@ -1,7 +1,9 @@ --- synopsis: "`nix profile` now allows referring to elements by human-readable name, and no longer accepts indices" prs: 8678 -cls: 978 +cls: [978, 980] --- -[`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 have been removed. +[`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 have been removed. 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. diff --git a/src/libcmd/cmd-profiles.cc b/src/libcmd/cmd-profiles.cc index 5fa7f902c..4b17d60c6 100644 --- a/src/libcmd/cmd-profiles.cc +++ b/src/libcmd/cmd-profiles.cc @@ -6,6 +6,7 @@ #include "logging.hh" #include "names.hh" #include "store-api.hh" +#include "url-name.hh" namespace nix { @@ -109,8 +110,6 @@ ProfileManifest::ProfileManifest(EvalState & state, const Path & profile) if (pathExists(manifestPath)) { auto json = nlohmann::json::parse(readFile(manifestPath)); - // Keep track of already found names so we can prevent duplicates. - std::set foundNames; auto version = json.value("version", 0); std::string sUrl; @@ -121,6 +120,8 @@ ProfileManifest::ProfileManifest(EvalState & state, const Path & profile) sOriginalUrl = "originalUri"; break; case 2: + [[fallthrough]]; + case 3: sUrl = "url"; sOriginalUrl = "originalUrl"; break; @@ -128,7 +129,10 @@ ProfileManifest::ProfileManifest(EvalState & state, const Path & profile) 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)); @@ -145,26 +149,17 @@ ProfileManifest::ProfileManifest(EvalState & state, const Path & profile) e["outputs"].get()}; } - std::string nameCandidate(element.identifier()); + // TODO(Qyriad): holy crap this chain of ternaries needs cleanup. + std::string name = + elems.is_object() + ? elem.key() + : e.contains("name") + ? static_cast(e["name"]) + : element.source + ? getNameFromURL(parseURL(element.source->to_string())).value_or(element.identifier()) + : element.identifier(); - if (e.contains("name")) { - nameCandidate = e["name"]; - } else if (element.source) { - auto const url = parseURL(element.source->to_string()); - auto const name = getNameFromURL(url); - if (name) { - nameCandidate = *name; - } - } - - auto finalName = nameCandidate; - for (unsigned appendedIndex = 1; foundNames.contains(finalName); ++appendedIndex) { - finalName = nameCandidate + std::to_string(appendedIndex); - } - element.name = finalName; - foundNames.insert(element.name); - - elements.emplace_back(std::move(element)); + addElement(name, std::move(element)); } } else if (pathExists(profile + "/manifest.nix")) { // FIXME: needed because of pure mode; ugly. @@ -176,16 +171,37 @@ ProfileManifest::ProfileManifest(EvalState & state, const Path & profile) 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 ProfileManifest::addElement(std::string_view nameCandidate, ProfileElement element) +{ + std::string finalName(nameCandidate); + + for (unsigned i = 1; elements.contains(finalName); ++i) { + finalName = nameCandidate + "-" + std::to_string(i); + } + + elements.insert_or_assign(finalName, std::move(element)); +} + +void ProfileManifest::addElement(ProfileElement element) +{ + auto name = + element.source + ? getNameFromURL(parseURL(element.source->to_string())) + : std::nullopt; + + auto finalName = name.value_or(element.identifier()); + addElement(finalName, std::move(element)); +} + nlohmann::json ProfileManifest::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)); @@ -200,11 +216,11 @@ nlohmann::json ProfileManifest::toJSON(Store & store) const 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; } @@ -215,7 +231,7 @@ StorePath ProfileManifest::build(ref store) 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); @@ -261,35 +277,29 @@ void ProfileManifest::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 prevElemIt = prev.elements.begin(); + auto curElemIt = 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 (prevElemIt != prev.elements.end() || curElemIt != cur.elements.end()) { + if (curElemIt != cur.elements.end() && (prevElemIt == prev.elements.end() || prevElemIt->first > curElemIt->first)) { + logger->cout("%s%s: ∅ -> %s", indent, curElemIt->second.identifier(), curElemIt->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()); + ++curElemIt; + } else if (prevElemIt != prev.elements.end() && (curElemIt == cur.elements.end() || prevElemIt->first < curElemIt->first)) { + logger->cout("%s%s: %s -> ∅", indent, prevElemIt->second.identifier(), prevElemIt->second.versions()); changes = true; - ++i; + ++prevElemIt; } else { - auto v1 = i->versions(); - auto v2 = j->versions(); + auto v1 = prevElemIt->second.versions(); + auto v2 = curElemIt->second.versions(); if (v1 != v2) { - logger->cout("%s%s: %s -> %s", indent, i->identifier(), v1, v2); + logger->cout("%s%s: %s -> %s", indent, prevElemIt->second.identifier(), v1, v2); changes = true; } - ++i; - ++j; + ++prevElemIt; + ++curElemIt; } } diff --git a/src/libcmd/cmd-profiles.hh b/src/libcmd/cmd-profiles.hh index 7f2c8a76f..2185daa34 100644 --- a/src/libcmd/cmd-profiles.hh +++ b/src/libcmd/cmd-profiles.hh @@ -35,7 +35,6 @@ constexpr int DEFAULT_PRIORITY = 5; struct ProfileElement { StorePathSet storePaths; - std::string name; std::optional source; bool active = true; int priority = DEFAULT_PRIORITY; @@ -57,7 +56,7 @@ struct ProfileElement struct ProfileManifest { - std::vector elements; + std::map elements; ProfileManifest() { } @@ -67,6 +66,9 @@ struct ProfileManifest StorePath build(ref store); + void addElement(std::string_view nameCandidate, ProfileElement element); + void addElement(ProfileElement element); + static void printDiff(const ProfileManifest & prev, const ProfileManifest & cur, std::string_view indent); }; diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 131abb258..401d5bd77 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -105,7 +105,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile element.updateStorePaths(getEvalStore(), store, res); - manifest.elements.push_back(std::move(element)); + manifest.addElement(std::move(element)); } try { @@ -115,7 +115,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)); @@ -202,13 +202,19 @@ public: return res; } - bool matches(const Store & store, const ProfileElement & element, const std::vector & matchers) + bool matches( + Store const & store, + // regex_match doesn't take a string_view lol + std::string const & name, + ProfileElement const & element, + std::vector const & 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; } } @@ -240,10 +246,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()); } @@ -289,14 +294,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; } @@ -368,7 +372,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf }; installables.push_back(installable); - indices.push_back(i); + elems.push_back(&element); } @@ -393,7 +397,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, @@ -425,14 +429,14 @@ 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 (auto const & [i, nameElemPair] : enumerate(manifest.elements)) { + auto & [name, element] = nameElemPair; 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) { diff --git a/src/nix/upgrade-nix.cc b/src/nix/upgrade-nix.cc index 31a051246..ca8080f88 100644 --- a/src/nix/upgrade-nix.cc +++ b/src/nix/upgrade-nix.cc @@ -208,7 +208,8 @@ struct CmdUpgradeNix : MixDryRun, EvalCommand // Find which profile element has Nix in it. // It should be impossible to *not* have Nix, since we grabbed this // store path by looking for things with bin/nix-env in them anyway. - auto findNix = [&](ProfileElement const & elem) -> bool { + auto findNix = [&](std::pair const & nameElemPair) -> bool { + auto const & [name, elem] = nameElemPair; for (auto const & ePath : elem.storePaths) { auto const nixEnv = store->printStorePath(ePath) + "/bin/nix-env"; if (pathExists(nixEnv)) { @@ -226,14 +227,15 @@ struct CmdUpgradeNix : MixDryRun, EvalCommand // *Should* be impossible... assert(elemWithNix != std::end(manifest.elements)); + auto const nixElemName = elemWithNix->first; + // Now create a new profile element for the new Nix version... ProfileElement elemForNewNix = { .storePaths = {newNix}, }; // ...and splork it into the manifest where the old profile element was. - // (Remember, elemWithNix is an iterator) - *elemWithNix = elemForNewNix; + manifest.elements.at(nixElemName) = elemForNewNix; // Build the new profile, and switch to it. StorePath const newProfile = manifest.build(store); diff --git a/tests/functional/nix-profile.sh b/tests/functional/nix-profile.sh index 387e73b2a..0f17e96ee 100644 --- a/tests/functional/nix-profile.sh +++ b/tests/functional/nix-profile.sh @@ -60,7 +60,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 @@ -81,7 +81,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' @@ -89,7 +89,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!" ]] @@ -97,8 +97,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 @@ -111,7 +112,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 @@ -132,7 +133,7 @@ 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 ]