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

Legacy nix-shell with a path prefers shell.nix over default.nix #11699

Closed
K900 opened this issue Oct 15, 2024 · 8 comments
Closed

Legacy nix-shell with a path prefers shell.nix over default.nix #11699

K900 opened this issue Oct 15, 2024 · 8 comments
Labels

Comments

@K900
Copy link
Contributor

K900 commented Oct 15, 2024

Describe the bug

~/test
❯ cat shell.nix
throw "i'm shell.nix"

~/test
❯ cat default.nix
throw "i'm default.nix"

~/test
❯ nix shell nixpkgs#nixVersions.nix_2_24 --command nix-shell . --attr hello
error:
       … while calling the 'throw' builtin
         at /home/k900/test/shell.nix:1:1:
            1| throw "i'm shell.nix"
             | ^
            2|

       error: i'm shell.nix

~/test
❯ nix shell nixpkgs#nixVersions.nix_2_18 --command nix-shell . --attr hello
error:
       … while calling the 'throw' builtin

         at /home/k900/test/default.nix:1:1:

            1| throw "i'm default.nix"
             | ^
            2|

       error: i'm default.nix

This also breaks a common pattern of nix-shell path/to/nixpkgs -A foo.

Steps To Reproduce

  1. Get a nixpkgs checkout
  2. nix-shell . -A hello
  3. See error

Expected behavior

A shell.

nix-env --version output

nix-env (Nix) 2.24.9

Additional context

2.18 is not affected, neither is Lix, may bisect later.

Priorities

Add 👍 to issues you find important.

@K900 K900 added the bug label Oct 15, 2024
@K900 K900 changed the title Legacy nix-shell attribute selection broken on 2.24 Legacy nix-shell with a path prefers shell.nix over default.nix with -A. Oct 15, 2024
@K900 K900 changed the title Legacy nix-shell with a path prefers shell.nix over default.nix with -A. Legacy nix-shell with a path prefers shell.nix over default.nix Oct 15, 2024
@K900
Copy link
Contributor Author

K900 commented Oct 18, 2024

Never mind, it's an intentional change: cfe3ee3

Whether that was a good idea or not is to be discussed, but this is not a bug.

@roberth
Copy link
Member

roberth commented Oct 31, 2024

Hi @K900, don't hesitate to ping me next time

to be discussed

Do you think it should still look for default.nix when -A/--attr is passed?
I think that would still solve the problem in issues such as #496, without breaking this use case.

@roberth roberth reopened this Oct 31, 2024
@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

It seems sort of odd to me for the meaning of . in nix-shell . … to change if an -A follows.

@roberth
Copy link
Member

roberth commented Nov 2, 2024

I agree it seems like an ad hoc extra rule on the face of it, but we could think of it as only changing the shell rule:

nix-shell resolves to shell.nix when no attribute is specified.

It's not great, I have to admit, but it's still a manageable rule, and least it wouldn't regress a significant Nixpkgs use case.

@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

FWIW in discussion with @K900 and @alyssais on Matrix I think we considered that it might be reasonable to simply re‐export the entire package set from Nixpkgs’ shell.nix, which is kind of gross but IIRC we ultimately decided might be nicer than changing Nix behaviour here. Sadly it was a little while ago now so I don’t have a pointer to the discussion or recollection or the trade‐offs we perceived.

@roberth
Copy link
Member

roberth commented Nov 2, 2024

@emilazy I just came up with the same thoughts! NixOS/nixpkgs#353240

@K900
Copy link
Contributor Author

K900 commented Nov 3, 2024

Yeah, I think the nixpkgs change is the way to go now. At least that way all the Nix versions work as intended.

@roberth
Copy link
Member

roberth commented Nov 26, 2024

Complicating the rules to accomodate this usage of nix-shell would make its behavior incomprehensible, and probably cause even more bug reports than those that led to #11057
Closing again.

@roberth roberth closed this as not planned Won't fix, can't repro, duplicate, stale Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants