Merge pull request #10166 from bobvanderlinden/profile-regex-all

profile: introduce --regex and --all

doc/manual/rl-next/profile-regex-all.md

@@ -0,0 +1,35 @@
+---
+synopsis: Introduction of `--regex` and `--all` in `nix profile remove` and `nix profile upgrade`
+prs: 10166
+---
+
+Previously the command-line arguments for `nix profile remove` and `nix profile upgrade` matched the package entries using regular expression.
+For instance:
+
+```
+nix profile remove '.*vim.*'
+```
+
+This would remove all packages that contain `vim` in their name.
+
+In most cases, only singular package names were used to remove and upgrade packages. Mixing this with regular expressions sometimes lead to unintended behavior. For instance, `python3.1` could match `python311`.
+
+To avoid unintended behavior, the arguments are now only matching exact names.
+
+Matching using regular expressions is still possible by using the new `--regex` flag:
+
+```
+nix profile remove --regex '.*vim.*'
+```
+
+One of the most useful cases for using regular expressions was to upgrade all packages. This was previously accomplished by:
+
+```
+nix profile upgrade '.*'
+```
+
+With the introduction of the `--all` flag, this now becomes more straightforward:
+
+```
+nix profile upgrade --all
+```

src/nix/profile-remove.md

@@ -11,9 +11,16 @@ R""(
 * Remove all packages:
 
   ```console
-  # nix profile remove '.*'
+  # nix profile remove --all
   ```
 
+* Remove packages by regular expression:
+
+  ```console
+  # nix profile remove --regex '.*vim.*'
+  ```
+
+
 * Remove a package by store path:
 
   ```console

src/nix/profile-upgrade.md

@@ -6,7 +6,7 @@ R""(
   reference:
 
   ```console
-  # nix profile upgrade '.*'
+  # nix profile upgrade --all
   ```
 
 * Upgrade a specific package by name:
@@ -15,6 +15,12 @@ R""(
   # nix profile upgrade hello
   ```
 
+* Upgrade all packages that include 'vim' in their name:
+
+  ```console
+  # nix profile upgrade --regex '.*vim.*'
+  ```
+
 # Description
 
 This command upgrades a previously installed package in a Nix profile,

src/nix/profile.cc

@@ -479,55 +479,150 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile
     }
 };
 
-class MixProfileElementMatchers : virtual Args
+struct Matcher
 {
-    std::vector<std::string> _matchers;
+    virtual std::string getTitle() = 0;
+    virtual bool matches(const std::string & name, const ProfileElement & element) = 0;
+};
+
+struct RegexMatcher : public Matcher
+{
+    std::regex regex;
+    std::string pattern;
+
+    RegexMatcher(const std::string & pattern) : regex(pattern, std::regex::extended | std::regex::icase), pattern(pattern)
+    { }
+
+    std::string getTitle() override
+    {
+        return fmt("Regex '%s'", pattern);
+    }
+
+    bool matches(const std::string & name, const ProfileElement & element) override
+    {
+        return std::regex_match(element.identifier(), regex);
+    }
+};
+
+struct StorePathMatcher : public Matcher
+{
+    nix::StorePath storePath;
+
+    StorePathMatcher(const nix::StorePath & storePath) : storePath(storePath)
+    { }
+
+    std::string getTitle() override
+    {
+        return fmt("Store path '%s'", storePath.to_string());
+    }
+
+    bool matches(const std::string & name, const ProfileElement & element) override
+    {
+        return element.storePaths.count(storePath);
+    }
+};
+
+struct NameMatcher : public Matcher
+{
+    std::string name;
+
+    NameMatcher(const std::string & name) : name(name)
+    { }
+
+    std::string getTitle() override
+    {
+        return fmt("Package name '%s'", name);
+    }
+
+    bool matches(const std::string & name, const ProfileElement & element) override
+    {
+        return name == this->name;
+    }
+};
+
+struct AllMatcher : public Matcher
+{
+    std::string getTitle() override
+    {
+        return "--all";
+    }
+
+    bool matches(const std::string & name, const ProfileElement & element) override
+    {
+        return true;
+    }
+};
+
+AllMatcher all;
+
+class MixProfileElementMatchers : virtual Args, virtual StoreCommand
+{
+    std::vector<ref<Matcher>> _matchers;
 
 public:
 
     MixProfileElementMatchers()
     {
-        expectArgs("elements", &_matchers);
+        addFlag({
+            .longName = "all",
+            .description = "Match all packages in the profile.",
+            .handler = {[this]() {
+                _matchers.push_back(ref<AllMatcher>(std::shared_ptr<AllMatcher>(&all, [](AllMatcher*) {})));
+            }},
+        });
+        addFlag({
+            .longName = "regex",
+            .description = "A regular expression to match one or more packages in the profile.",
+            .labels = {"pattern"},
+            .handler = {[this](std::string arg) {
+                _matchers.push_back(make_ref<RegexMatcher>(arg));
+            }},
+        });
+        expectArgs({
+            .label = "elements",
+            .optional = true,
+            .handler = {[this](std::vector<std::string> args) {
+                for (auto & arg : args) {
+                    if (auto n = string2Int<size_t>(arg)) {
+                        throw Error("'nix profile' no longer supports indices ('%d')", *n);
+                    } else if (getStore()->isStorePath(arg)) {
+                        _matchers.push_back(make_ref<StorePathMatcher>(getStore()->parseStorePath(arg)));
+                    } else {
+                        _matchers.push_back(make_ref<NameMatcher>(arg));
+                    }
+                }
+            }}
+        });
     }
 
-    struct RegexPattern {
-        std::string pattern;
-        std::regex  reg;
-    };
-    typedef std::variant<Path, RegexPattern> Matcher;
-
-    std::vector<Matcher> getMatchers(ref<Store> store)
-    {
-        std::vector<Matcher> res;
-
-        for (auto & s : _matchers) {
-            if (auto n = string2Int<size_t>(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)});
+    std::set<std::string> getMatchingElementNames(ProfileManifest & manifest) {
+        if (_matchers.empty()) {
+            throw UsageError("No packages specified.");
         }
 
-        return res;
-    }
+        if (std::find_if(_matchers.begin(), _matchers.end(), [](const ref<Matcher> & m) { return m.dynamic_pointer_cast<AllMatcher>(); }) != _matchers.end() && _matchers.size() > 1) {
+            throw UsageError("--all cannot be used with package names or regular expressions.");
+        }
 
-    bool matches(
-        const Store & store,
-        const std::string & name,
-        const ProfileElement & element,
-        const std::vector<Matcher> & matchers)
-    {
-        for (auto & matcher : matchers) {
-            if (auto path = std::get_if<Path>(&matcher)) {
-                if (element.storePaths.count(store.parseStorePath(*path))) return true;
-            } else if (auto regex = std::get_if<RegexPattern>(&matcher)) {
-                if (std::regex_match(name, regex->reg))
-                    return true;
+        if (manifest.elements.empty()) {
+            warn("There are no packages in the profile.");
+            return {};
+        }
+
+        std::set<std::string> result;
+        for (auto & matcher : _matchers) {
+            bool foundMatch = false;
+            for (auto & [name, element] : manifest.elements) {
+                if (matcher->matches(name, element)) {
+                    result.insert(name);
+                    foundMatch = true;
+                }
+            }
+            if (!foundMatch) {
+                warn("%s does not match any packages in the profile.", matcher->getTitle());
             }
         }
-
-        return false;
+        return result;
     }
 };
 
@@ -549,16 +644,19 @@ struct CmdProfileRemove : virtual EvalCommand, MixDefaultProfile, MixProfileElem
     {
         ProfileManifest oldManifest(*getEvalState(), *profile);
 
-        auto matchers = getMatchers(store);
+        ProfileManifest newManifest = oldManifest;
 
-        ProfileManifest newManifest;
+        auto matchingElementNames = getMatchingElementNames(oldManifest);
 
-        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());
-            }
+        if (matchingElementNames.empty()) {
+            warn ("No packages to remove. Use 'nix profile list' to see the current profile.");
+            return;
+        }
+
+        for (auto & name : matchingElementNames) {
+            auto & element = oldManifest.elements[name];
+            notice("removing '%s'", element.identifier());
+            newManifest.elements.erase(name);
         }
 
         auto removedCount = oldManifest.elements.size() - newManifest.elements.size();
@@ -566,16 +664,6 @@ struct CmdProfileRemove : virtual EvalCommand, MixDefaultProfile, MixProfileElem
             removedCount,
             newManifest.elements.size());
 
-        if (removedCount == 0) {
-            for (auto matcher: matchers) {
-                if (const Path * path = std::get_if<Path>(&matcher)) {
-                    warn("'%s' does not match any paths", *path);
-                } else if (const RegexPattern * regex = std::get_if<RegexPattern>(&matcher)) {
-                    warn("'%s' does not match any packages", regex->pattern);
-                }
-            }
-            warn ("Use 'nix profile list' to see the current profile.");
-        }
         updateProfile(newManifest.build(store));
     }
 };
@@ -598,20 +686,20 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf
     {
         ProfileManifest manifest(*getEvalState(), *profile);
 
-        auto matchers = getMatchers(store);
-
         Installables installables;
         std::vector<ProfileElement *> elems;
 
-        auto matchedCount = 0;
         auto upgradedCount = 0;
 
-        for (auto & [name, element] : manifest.elements) {
-            if (!matches(*store, name, element, matchers)) {
-                continue;
-            }
+        auto matchingElementNames = getMatchingElementNames(manifest);
 
-            matchedCount++;
+        if (matchingElementNames.empty()) {
+            warn("No packages to upgrade. Use 'nix profile list' to see the current profile.");
+            return;
+        }
+
+        for (auto & name : matchingElementNames) {
+            auto & element = manifest.elements[name];
 
             if (!element.source) {
                 warn(
@@ -669,18 +757,8 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf
         }
 
         if (upgradedCount == 0) {
-            if (matchedCount == 0) {
-                for (auto & matcher : matchers) {
-                    if (const Path * path = std::get_if<Path>(&matcher)) {
-                        warn("'%s' does not match any paths", *path);
-                    } else if (const RegexPattern * regex = std::get_if<RegexPattern>(&matcher)) {
-                        warn("'%s' does not match any packages", regex->pattern);
-                    }
-                }
-            } else {
-                warn("Found some packages but none of them could be upgraded.");
-            }
-            warn ("Use 'nix profile list' to see the current profile.");
+            warn("Found some packages but none of them could be upgraded.");
+            return;
         }
 
         auto builtPaths = builtPathsPerInstallable(

tests/functional/common/vars-and-functions.sh.in

@@ -216,6 +216,17 @@ expectStderr() {
     return 0
 }
 
+# Run a command and check whether the stderr matches stdin.
+# Show a diff when output does not match.
+# Usage:
+#
+#   assertStderr nix profile remove nothing << EOF
+#   error: This error is expected
+#   EOF
+assertStderr() {
+    diff -u /dev/stdin <($@ 2>/dev/null 2>&1)
+}
+
 needLocalStore() {
   if [[ "$NIX_REMOTE" == "daemon" ]]; then
     skipTest "Can’t run through the daemon ($1)"

tests/functional/nix-profile.sh

@@ -74,10 +74,49 @@ 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"
 
+# Test upgrading package using regular expression.
+printf 2.1 > $flake1Dir/version
+nix profile upgrade --regex '.*'
+[[ $(readlink $TEST_HOME/.nix-profile/bin/hello) =~ .*-profile-test-2\.1/bin/hello ]]
+nix profile rollback
+
+# Test upgrading all packages
+printf 2.2 > $flake1Dir/version
+nix profile upgrade --all
+[[ $(readlink $TEST_HOME/.nix-profile/bin/hello) =~ .*-profile-test-2\.2/bin/hello ]]
+nix profile rollback
+printf 1.0 > $flake1Dir/version
+
+# Test --all exclusivity.
+assertStderr nix --offline profile upgrade --all foo << EOF
+error: --all cannot be used with package names or regular expressions.
+Try 'nix --help' for more information.
+EOF
+
+# Test matching no packages using literal package name.
+assertStderr nix --offline profile upgrade this_package_is_not_installed << EOF
+warning: Package name 'this_package_is_not_installed' does not match any packages in the profile.
+warning: No packages to upgrade. Use 'nix profile list' to see the current profile.
+EOF
+
+# Test matching no packages using regular expression.
+assertStderr nix --offline profile upgrade --regex '.*unknown_package.*' << EOF
+warning: Regex '.*unknown_package.*' does not match any packages in the profile.
+warning: No packages to upgrade. Use 'nix profile list' to see the current profile.
+EOF
+
+# Test removing all packages using regular expression.
+nix profile remove --regex '.*' 2>&1 | grep "removed 2 packages, kept 0 packages"
+nix profile rollback
+
 # Test 'history', 'diff-closures'.
 nix profile diff-closures
 
 # Test rollback.
+printf World > $flake1Dir/who
+nix profile upgrade flake1
+printf NixOS > $flake1Dir/who
+nix profile upgrade flake1
 nix profile rollback
 [[ $($TEST_HOME/.nix-profile/bin/hello) = "Hello World" ]]