LnL7 / nix-darwin

nix modules for darwin
MIT License
2.82k stars 431 forks source link

Remove nix-shell early return in /etc/{zshenv,bashrc} #1072

Closed antoineco closed 2 weeks ago

antoineco commented 2 weeks ago

The condition does not match the comment, and therefore not the original intention. It currently returns early in any type of Nix shell, not just pure ones, including 'nix develop'.

Besides being unnecessary, this check prevents Nix shells from functioning properly. For instance, it causes the initialization of the Zsh fpath to be skipped, which is critical. The fact that the user is unable to opt out of this behaviour makes this an ever bigger problem since /etc/zshenv is being loaded unconditionally by Zsh.

For reference, NixOS does not perform such check, and apparently never did based on my inspection of the commit history.

For a history, see the following commits (without comment or context unfortunately):

emilazy commented 2 weeks ago

It looks like Bash also does the same thing currently. We should probably be consistent across the shells.

A little scared to touch something that looks as load‐bearing as this, but if it works with nix develop, nix develop --pure, and the nix-shell equivalents, then this seems fine.

antoineco commented 2 weeks ago

I originally didn't want to get out of my way since I don't use Bash as my day to day shell, but I agree that we should be consistent.

AFAIK pure shells have a reasonably restricted environment and work as intended without having to resort to these kind of early returns. At least I couldn't reproduce a situation in which this would be a problem. And as I wrote, it seems like NixOS never introduced this early return at any given point, so the current behavior just causes more inconsistencies between various Nix-managed hosts.

emilazy commented 2 weeks ago

Yeah, it seems fine to do. Could you adjust the commit message to {bash,zsh}: …?