If you can make it work on one platform, but not break anything on the others, that seems fine, but the way your PR is now formulated, this is not the case.

If this PR requires a particular feature (the -n-flag for local), why not just do a feature test and write two implementations?

@0xABAB In that other PR I link, I need to use this a few more times. I could work around them all with eval, but numerous such fallbacks are annoying to maintain.

Also, I recall you were also interested in Nix/nixpkgs eventually supporting store paths with special characters? The eval fallbacks are usually broken with respect to that, but these aren't.

I think that given all that, asking MacOS users to nix-env -A nixpkgs.bash if they wish to use nix-shell is pretty lightweight.

Also, in a future with nix use, nix shell will become quite niche.

I think that given all that, asking MacOS users to nix-env -A nixpkgs.bash if they wish to use nix-shell is pretty lightweight.

Why not just make nix-shell depend on a Nix version of bash then on MacOSX? In that case, you don't need to educate MacOSX users.

AFAIK, stdenv already requires a newer bash than what macOS provides. (I think it's still stuck at 3.2.) So this seems fine to me.

Namerefs were introduced in bash 4.3 BTW, which is more than 3 years old.

This indeed breaks nix-shell on macOS X (version 10.11), which comes with bash-3.2. The following error messages appear:

bash: local: -n: invalid option
local: usage: local name[=value] ...
bash: local: -n: invalid option
local: usage: local name[=value] ...
bash: failureHooks: command not found

However, this is indeed fixed by installing a newer bash, running, e.g., nix-env -iA nixpkgs.bashInteractive. Installing plain nixpkgs.bash also solves the issue, but provides a rather unfriendly shell.

Maybe nix-shell itself could check the bash version and provide an error message a little bit more friendly.

Right, stdenv could do that check and show better help.

I think that given all that, asking MacOS users to nix-env -A nixpkgs.bash if they wish to use nix-shell is pretty lightweight.

I strongly disagree. It feels very counter to the Nix philosophy to require manual user intervention and expect an implicit contextual dependency to be set up before really basic Nix functionality works. The reason Nix is so appealing to me (and many others I know) is that I don't have to do stuff like that to make things work, because Nix is setting up and reasoning about explicit dependencies all the way down.

Also, it seems strange to me to merge this an hour after submitting with no explicit testing on macOS despite @Ericson2314 acknowledging that it could break macOS in the opening comment of the PR. As expected, this has now broken stdenv on master.

The darwin maintainers already have a hard time being such a minority in the community, because Linux folks inadvertently break our stuff all the time. That's frustrating but understandable because inadvertent and we don't have a good way to test Darwin from Linux; what's more frustrating is seeing an explicit understanding that this will break us and merging it anyway. I get that you requested a review from me (presumably as a Darwin representative) but I was busy and didn't get to it (nor did any other Darwin folks) and you merged it anyway, thus knowingly breaking the entire platform...

Well, as I wrote:

AFAIK, stdenv already requires a newer bash than what macOS provides. (I think it's still stuck at 3.2.) So this seems fine to me.

So it was not expected to cause additional breakage on macOS. Also "breaks nix-shell" != "breaking the entire platform".

Such regressions can occur from time to time, that's why we have a stable branch after all. We can easily revert this change.

long-term fix proposal: #27493 (comment)

@edolstra I get it, but this also means that the default nix install on CentOS is now broken too (this change made it out into the nixpkgs-unstable channel). I'd bet Ubuntu is as well [just tested, it's fine, but CentOS is still b0rked].