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

nix profile counter-intuitively uses regexp to match packages #7962

Closed
bobvanderlinden opened this issue Mar 4, 2023 · 18 comments
Closed

nix profile counter-intuitively uses regexp to match packages #7962

bobvanderlinden opened this issue Mar 4, 2023 · 18 comments
Labels
bug new-cli Relating to the "nix" command

Comments

@bobvanderlinden
Copy link
Member

nix profile remove and upgrade accept an regexp argument to be able to match specific (or ranges of) packages. This usually results in confusion. Some common errors that people can run into:

Forgetting quotation in most shells:

$ nix profile remove .*vim.*
fish: No matches for wildcard '.*vim.*'. See `help wildcards-globbing`.
nix profile remove .*vim.*
                   ^~~~~~^
$ nix profile remove .*vim.*
zsh: no matches found: .*vim.*

Using wildcards instead of regexp result in confusing error message:

$ nix profile remove '*vim*'
error: Mismatched '(' and ')' in regular expression

Which ( and )?

Forgetting regexp and wildcards altogether:

$ nix profile remove vim
warning: 'vim' does not match any packages
warning: Use 'nix profile list' to see the current profile.

This makes it seem like vim was not installed at all, but it just wasn't matched by the regexp vim, because it should match flake:nixpkgs#legacyPackages.x86_64-linux.vim.

I'm thinking nix profile should only optionally accept regexp and it should be very explicit about it. For instance --regex "your regex". It shouldn't be the default to match packages.

@thufschmitt
Copy link
Member

That is indeed quite bad as an interface. Just jotting an extra possible point in the design space (no idea whether it's a good one, but it's the first that came to my mind while reading this): keep a regex, but don't require it to be a full match. That way nix profile remove vim would be equivalent to nix profile remove '.*vim.*' and would correctly match

@roberth roberth added the new-cli Relating to the "nix" command label Mar 16, 2023
@iFreilicht
Copy link
Contributor

@thufschmitt I really like this idea! It makes the interface much nicer without losing any power. I thought about potential edge cases, but the ones I came up with (i.e. removing gitlab, which is also a flake type) aren't actually a problem, as the regex only matches against the attribute path, so legacyPackages.aarch64-darwin.vim.

Seems this is a no-brainer with low-risk. I'll take a stab at implementing it!

@bobvanderlinden
Copy link
Member Author

bobvanderlinden commented Jun 20, 2023

Adding .* will also be very confusing. When using nix profile remove vim it'll not only remove vim, but also everything else that includes the substring vim, like vimpc or others. Still quite unintuitively, as it will now remove unintended packages.

I'm more and more leaning towards #7967 to fix these kinds of issues.

@iFreilicht
Copy link
Contributor

Yeah so I tried it out and you're right, implicit partial matches are problematic. However, we could do a complete match on the identifier that's reported by diff-closures:

# nix profile diff-closures
[...]
Version 4 -> 5:
  grc: 1.13 → ∅, -105.2 KiB

Version 5 -> 6:
  autojump: 22.5.3 → ∅, -133.7 KiB
  grc: ∅ → 1.13, +105.2 KiB

Version 6 -> 7:
  grc: 1.13 → ∅, -105.2 KiB
  profile: -54.2 KiB
[...]

This has to be done anyway for backwards-compatibility with packages installed by nix-env. The identifier is always calculated from the store path, so it is 100% stable.

@iFreilicht
Copy link
Contributor

iFreilicht commented Jun 20, 2023

Ok I digged into it a little further and I agree, we need #7967. Everything else is just a hack and bound to cause implicit problems later on.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/flakes-as-a-unified-format-for-profiles/29476/1

@sullyj3
Copy link

sullyj3 commented Aug 1, 2023

Another issue is that it's not clear what regex dialect is used (is this documented anywhere?), so I have no way of finding out how to use word boundaries, for example.

@iFreilicht
Copy link
Contributor

iFreilicht commented Aug 1, 2023

@sullyj3 Nix is using the C++ std::regex library, so it follows C++'s modified ECMAScript regex syntax.

However there is one exception: builtins.regex specifically uses POSIX extended regex. I'm not sure, but I assume this was chosen to be consistent with tools like grep as those are used in builder scripts.

This is a good point, though, we should document in general what regex syntax is used for all commands that accept them.

@sullyj3
Copy link

sullyj3 commented Aug 1, 2023

@iFreilicht Cheers! According to that page, the dialect does support \b for word boundary, any idea what I'm doing wrong here?

⮞ nix search nixpkgs '\brip'
error: Invalid escape in regular expression

@iFreilicht
Copy link
Contributor

iFreilicht commented Aug 1, 2023

@sullyj3 I looked at the actual implementation, and it seems both nix search and nix profile are also exceptions to the rule, though not documented. They use POSIX extended syntax as well.

I created an issue for this: #8768

POSIX extended syntax doesn't have a word boundary escape sequence, but there's [[:<:]] and [[:>:]] to match the beginning and end of a word, respectively.

@sullyj3
Copy link

sullyj3 commented Aug 1, 2023

Hmm, still no joy:

⮞ nix search nixpkgs '[[:<:]]rip'
error: Invalid character class.

Possibly it's not supported by whatever library is being used?

Edit:
If I'm looking in the right place (not a C++ programmer), the C++ docs link to this page which doesn't seem to mention word boundaries. That's unfortunate.

@iFreilicht
Copy link
Contributor

That is the right place to look in, and it seems you're right :/ Sorry about that. What's your usecase for using a word-boundary in a search anyway? Maybe there's a different solution to your problem?

@sullyj3
Copy link

sullyj3 commented Aug 1, 2023

I was searching for the rip package, and nix search nixpkgs rip gives a flood of irrelevant results.

[...]
* legacyPackages.x86_64-linux.weechatScripts.weechat-grep (0.8.5)
  Search in Weechat buffers and logs (for Weechat 0.3.*)

* legacyPackages.x86_64-linux.weechatScripts.weechat-matrix (0.3.0)
  A Python plugin for Weechat that lets Weechat communicate over the Matrix protocol

* legacyPackages.x86_64-linux.weechatScripts.weechat-matrix-bridge (2018-11-19)
  A WeeChat script in Lua that implements the matrix.org chat protocol

* legacyPackages.x86_64-linux.weechatScripts.weechat-notify-send (0.9)
  A WeeChat script that sends highlight and message notifications through notify-send

* legacyPackages.x86_64-linux.weechatScripts.weechat-otr (1.9.2)
  WeeChat script for Off-the-Record messaging

* legacyPackages.x86_64-linux.weechatScripts.zncplayback (0.2.1)
  Add support for the ZNC Playback module
[...]

From grepping a cloned copy of nixpkgs it does seem like it's not in there, but that's beside the point.

@sullyj3
Copy link

sullyj3 commented Aug 1, 2023

Unfortunately piping nix search into grep or ripgrep doesn't work correctly as a workaround either, as nix's output seems to be colored regardless of whether it's connected to a pipe or a terminal. I see there's #7558 covering that. That took me a second to figure out haha.

⮞ nix search nixpkgs ripgrep | rg '\bripgrep'
⮞ # no output???
⮞ nix search nixpkgs ripgrep | less
evaluating 'legacyPackages.x86_64-linux.emacsPackages'* ESC[0;1mlegacyPackages.x86_64-linux.emacsPackages.projectile-ESC[32;1mripgrepESC[0;1mESC[0m (20221013.541)
evaluating 'legacyPackages.x86_64-linux.emacsPackages'
* ESC[0;1mlegacyPackages.x86_64-linux.emacsPackages.ESC[32;1mripgrepESC[0;1mESC[0m (20220520.1410)
evaluating 'legacyPackages.x86_64-linux'
* ESC[0;1mlegacyPackages.x86_64-linux.repgrepESC[0m (0.14.2)
  An interactive replacer for ESC[32;1mripgrepESC[0m that makes it easy to find and replace across files on the command line
evaluating 'legacyPackages.x86_64-linux'
* ESC[0;1mlegacyPackages.x86_64-linux.ESC[32;1mripgrepESC[0;1mESC[0m (13.0.0)
  A utility that combines the usability of The Silver Searcher with the raw speed of grep

* ESC[0;1mlegacyPackages.x86_64-linux.ESC[32;1mripgrepESC[0;1m-allESC[0m (0.9.6)
  ESC[32;1mRipgrepESC[0m, but also search in PDFs, E-Books, Office documents, zip, tar.gz, and more
evaluating 'legacyPackages.x86_64-linux'
* ESC[0;1mlegacyPackages.x86_64-linux.vgrepESC[0m (2.6.1)
  User-friendly pager for grep/git-grep/ESC[32;1mripgrepESC[0m
(END)
⮞ # ah.

@sullyj3
Copy link

sullyj3 commented Aug 1, 2023

This is getting a bit off topic apologies. I'll just note that #3047 seems relevant

@iFreilicht
Copy link
Contributor

iFreilicht commented Aug 1, 2023

No worries, but maybe you could collect these in a new issue similar to this one? We are aware that nix search is not very useful in its current form, but I didn't see an issue that really tracks what the current problems are. If you have any ideas on how to improve the terrible matching right now (maybe by only searching the package name by default, stuff like that), you can absolutely create feature requests as well. We can then have an extended discussion there :)

For more brain-storming kind of threads, you might also want to post on the Nix Forums, either in the #Development or #Help categories. You might find existing threads on the usability problems with nix search there as well.

Thank you for taking the time to dig into this!

@sullyj3
Copy link

sullyj3 commented Aug 1, 2023

Sure, I'll have a crack later this week.

@bobvanderlinden
Copy link
Member Author

Regexes are still used in nix profiles remove/upgrade, but we have profile entry names now. It now matches regex on those. Using '.*' might still be confusing, but at least the main use-case (nix profile remove vim) works now 🥳.

I think I'll close this. If there are still remaining cases, please open a new issue and feel free to mention #7966, I'll add it to the list 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug new-cli Relating to the "nix" command
Projects
None yet
Development

No branches or pull requests

6 participants