From 6268a45b650f563bae2360e0540920a2959bdd40 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 22 Dec 2023 16:11:25 +0100 Subject: [PATCH] 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