stdenv for nix-shell is broken on master

[domenkozar]

$ nix-shell -p jq --run jq
/nix/store/fpqvm0gishkdjcyq607gi3h9jvabadvv-stdenv-darwin/setup: line 34: local: -n: invalid option
local: usage: local name[=value] ...
/nix/store/fpqvm0gishkdjcyq607gi3h9jvabadvv-stdenv-darwin/setup: line 34: local: -n: invalid option
local: usage: local name[=value] ...
/nix/store/fpqvm0gishkdjcyq607gi3h9jvabadvv-stdenv-darwin/setup: line 86: failureHooks: command not found

Using 0eb4f6fd2547442d8c51f73d4fe11225951ee663, tracked it down to https://github.com/NixOS/nixpkgs/commit/a14cf0618219a6f135e786e69e78eb0b866248f8

Final question: why wasn't this caught with tests?

cc @NixOS/darwin-maintainers

[LnL7]

This is only a problem for nix-shell, if you don't have a newer version of bash in your path.

[grahamc]

^ to be real specific, while we fix this out of the box, installing a new version of bash with nix fixes this.

[domenkozar]

Workaround: nix-env -iA nixpkgs.bashInteractive OR NIX_BUILD_SHELL=$(nix-build -A bash '<nixpkgs>')/bin/bash nix-shell -p jq --run "jq"

[copumpkin]

Can we just have the Nix bash code be explicit about which bash it wants (e.g., in the shebang) when it wants to use recent features? I hate stuff (and it seems anti-Nix-ish) that implicitly depends on stuff being set up correctly in the environment.

cc @Ericson2314

[copumpkin]

Also, this was called out explicitly in the PR review: https://github.com/NixOS/nixpkgs/pull/27215#issuecomment-313727998

[domenkozar]

I propose we fix this long-term by teaching nix-shell about pkgs.bashInteractive.

2% of nixpkgs source code is bash and most of our infrastructure tests that, while we impurely depend on system bash for nix-shell.

[LnL7]

I can't really think of a reason not to use bash from nixpkgs, nix-shell already knows about pkgs.runCommand.

[domenkozar]

@edolstra thoughts?

[domenkozar]

Implemented in https://github.com/NixOS/nix/commit/c94f3d5575d7af5403274d1e9e2f3c9d72989751

Added a warning to not rename pkgs.bashInteractive in e8c72a4

[edolstra]

Thanks @domenkozar!

[copumpkin]

Awesome, thank you!

[Ericson2314]

Is there anything more to do here? It's my breakage so tell me what I can do to help---sorry I missed the discussion so far to begin with.

[copumpkin]

This also breaks on CentOS 7 (which does have bash 4 apparently). I could also imagine this causing some oddness with Ubuntu, which uses dash for /bin/sh, even though I haven't tried it.

[copumpkin]

@Ericson2314 can we just undo the local -n stuff for now, until we can guarantee which version of bash is going to be consuming it?

[copumpkin]

Removed darwin tag because this is definitely breaking some of my (non-NixOS) Linux machines

[copumpkin]

The CentOS failure is because local -n was introduced in Bash 4.3 and CentOS 7 ships with 4.2.46.

[Ericson2314]

@edolstra What I proposed to @copumpkin in ##nix-darwin was:

1. Immediately add notice to setup.sh aborting with nice message ("Upgrade Nix or upgrade Bash", etc) if Bash is too old. Then people won't get blindsided by nixpkgs-unstable.

2. Release 1.11.13 with @domenkozar's change---and also a fix for https://github.com/NixOS/nixpkgs/pull/27427 I'll write. (Not opposed to making it use nixpkgs's build-env, but I can crank out the whitespace flexibility I proposed quicker as a stop gap).

I don't think either of those will take much longer, if at all, than a revert. I do wish all this never happened---but if the latency is indeed comparable, I rather do that which I deem the better solution. Also, FWIW while what's currently there can bet written without namerefs, https://github.com/NixOS/nixpkgs/pull/26805 will require them or copious eval. Besides that being sketchy, it's harder for me to debug too.

[domenkozar]

+1 for releasing 1.11.14 with 2)

[Ericson2314]

https://github.com/NixOS/nix/pull/1483 WIP backport of @domenkozar's patch.

[domenkozar]

Fixed in https://github.com/NixOS/nixpkgs/pull/27618