Search improvements for --file and --expr

[dramforever]

trafficstars

Motivation

The new nix search no longer works without flakes without some hacks. This PR fixes this so that this would work:

$ nix search --file '<nixpkgs>' '' hello-unfree

Still a slight mouthful, but now it's at least working as much as --file could.

Context

Discussed in: https://discourse.nixos.org/t/nix-search-broken/24447

A workaround is possible, which uses the special handling of legacyPackages to ignore evaluation errors, but it's pretty ugly and relies on a weird internal behavior:

$ nix search --impure --expr '{ legacyPackages.${builtins.currentSystem} = import <nixpkgs> {};}' '' hello-unfree

There are two main changes:

• CmdSearch now recurses into recurseForDerivations = true if file || expr. For better error messages, top-level eval errors are not ignored.
• InstallableAttrPath::getCursors now has a custom implementation that auto-calls the value and provides the full attribute path in the cursor (root->findAlongAttrPath so that is has a correct parent).

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

• [x] agreed on idea
• [ ] agreed on implementation strategy
• [ ] tests, as appropriate
  • functional tests - tests/**.sh
  • unit tests - src/*/tests
  • integration tests
• [ ] documentation in the manual
• [ ] code and comments are self-explanatory
• [ ] commit message explains why the change was made
• [ ] new feature or bug fix: updated release notes

[nixos-discourse]

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

https://discourse.nixos.org/t/nix-search-broken/24447/12

[fricklerhandwerk]

Triaged in the Nix team meeting:

We want to support the use case, and it's relevant for stabilising the new CLI. But the proposed solution is suboptimal. @Ericson2314 and @tomberek will propose an alternative approach and report back. Postponed until then.

Complete discussion

• @thufschmitt: we want the fix since it relates to stabilising the CLI
• @edolstra: the problem with this is that the syntax is nix search <flakeref> <search term> and it's not clear how it would fit with non-flakes search
  • the proposal uses a dummy argument, and that's really ugly
  • if someone comes up with an elegant way for taking singleton installables, we can adapt it
• @tomberek: the implementation may not be what we want, but we agree that we want to support the use case
• @Ericson2314, @tomberek: would like to propose a design
• idea approved, postponed; @Ericson2314 and @tomberek will report back with a proposal

[nixos-discourse]

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

https://discourse.nixos.org/t/2023-03-03-nix-team-meeting-minutes-37/25998/1

[dramforever]

I'd like to clarify that the '' isn't really dummy, it's just 'search from root'. You can search any other sub-package-set with e.g. nix search --file '<nixpkgs>' python3Packages

[thufschmitt]

@dramforever would . instead of '' work? That would probably make more explicit the fact that it's the path from the expression root and so make the command a bit more intuitive

[dramforever]

. already works. Though I want to say it's a bit of a hack, since InstallableCommands defaults to ., which means 'current directory' for flakes, and it just seems to be hacked around for the --file/--expr case to mean 'root'. I couldn't remember where in the code it's done though.

A redesign would probably still be nice. In my fork-ish nix-dram I've had it changed to:

nix search [-i INSTALLABLE] <terms ...>

Though that's because nix-dram is based on a 'default flake' (see discussion in e.g. #4438), which -i can, well, default to.

[flickerfly]

I'm coping with this by using the following alias. Hopefully, that'll help those who, like me, are looking forward to this bug being fixed.

alias nixpkgs-search="nix --extra-experimental-features nix-command --extra-experimental-features flakes search nixpkgs"

[tejing1]

I created a wrapper to implement the legacyPackages workaround, and never thought to link it here... but here it is, in case it's useful: https://github.com/tejing1/nix-search

[dramforever]

(misclicked, sorry)

[tomberek]

This is a problem to fix to make nix search usable in the non-flake case.

@dramforever would . instead of '' work? That would probably make more explicit the fact that it's the path from the expression root and so make the command a bit more intuitive

"." is better than the empty string, it is still not particularly appealing for general usage.

we had said this above, but I'm not clear on a good design yet, see below

if someone comes up with an elegant way for taking singleton installables, we can adapt it

The contention is this part of the installables semantics:

Options that change the interpretation of installables:

      · --expr expr
        
        Interpret installables as attribute paths relative to the Nix expression expr.
    
      · --file / -f file
        
        Interpret installables as attribute paths relative to the Nix expression stored in file. If file is
        the character -, then a Nix expression will be read from standard input. Implies --impure.

brainstorming:

• nix search -f '<nixpkgs>' something is the desired syntax?
• but now it becomes harder to say nix search -f '<nixpkgs>' inside-this-attr something'

This would require logic for file&&expr to be detected earlier than when the search results are processed. @dramforever this would mean I'd expect conditionals in installable.cc for this case.

conclusion

this PR seems to be a step in the right direction, supporting both "" and "." to make the workaround better.

[tomberek]

Related: https://discourse.nixos.org/t/installables-cli-design/38601