Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

profile: introduce --regex and --all #10166

Merged
merged 9 commits into from
Mar 8, 2024

Conversation

bobvanderlinden
Copy link
Member

@bobvanderlinden bobvanderlinden commented Mar 6, 2024

Motivation

Currently nix profile upgrade and nix profile remove both implicitly allow using regex by default. This usually works like matching normal package names, but sometimes works slightly different.

Related, the documentation mentions using nix profile upgrade '.*' to upgrade all packages. However, as #7487 shows, this doesn't work for store-path installed packages. Question is: should we also regex-match on store paths? I think this can lead to unintended matching of hashes. So, probably not.

Since most people will likely use the arguments as full names of packages, instead of regex, I thought I'd make this explicit:

  • nix profile upgrade vim
  • nix profile upgrade --regex '.*vim.*'
  • nix profile upgrade --all
  • nix profile upgrade /nix/store/...-vim-2.0

Same goes for remove:

  • nix profile remove vim
  • nix profile remove --regex '.*vim.*'
  • nix profile remove --all
  • nix profile remove /nix/store/...-vim-2.0

Although minor, this also avoids issues like python3.1 matching python311 and python310. Also minor: it avoids running into reserved regex characters, as #3530 describes.

Implementation

I've restructured the matching of packages a bit, so that it uses a generic matcher type with variants Name, StorePath, Regex and All matchers.

I've also introduced assertStderr in the functional test functions, so that it is easier to compare stderr output against expected output.

Context

Fixes #7487
Fixes #10162
Related: #7962
Fixes regex issue, but for nix profile: #3530

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions bot added documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Mar 6, 2024
Copy link
Contributor

@iFreilicht iFreilicht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only had a quick look for now, but so far it looks good. I'll have a closer look in the coming days, if nobody else beats me to it.

Also don't forget to add release notes!

src/nix/profile-remove.md Outdated Show resolved Hide resolved
tests/functional/nix-profile.sh Outdated Show resolved Hide resolved
@bobvanderlinden
Copy link
Member Author

@iFreilicht Thanks for the quick look! Already helps a lot 👍

Also don't forget to add release notes!

It took some time to figure out where the release notes reside. I think I've found it. CONTRIBUTING.md mentions adding it to rl-next.md, but that file is gitignored.

src/nix/profile.cc Outdated Show resolved Hide resolved
src/nix/profile.cc Outdated Show resolved Hide resolved
src/nix/profile.cc Outdated Show resolved Hide resolved
src/nix/profile.cc Outdated Show resolved Hide resolved
src/nix/profile.cc Outdated Show resolved Hide resolved
@bobvanderlinden bobvanderlinden force-pushed the profile-regex-all branch 2 times, most recently from 8dddd2a to 49b86b0 Compare March 7, 2024 20:54
@bobvanderlinden
Copy link
Member Author

Thanks for the reviews!

Since the --all exclusivity needed to distinguish all from other matchers I first introduced a type, but eventually it made more sense to just make Matcher an abstract struct to get a more conventional structure and use ptr equivalence to check for all.

src/nix/profile.cc Outdated Show resolved Hide resolved
@edolstra edolstra merged commit ae2bd46 into NixOS:master Mar 8, 2024
8 checks passed
@bobvanderlinden bobvanderlinden deleted the profile-regex-all branch March 8, 2024 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nix profile cannot intuitively upgrade all packages nix profile remove '.*' doesn't remove path installabes
3 participants