Output of print-dev-env has a syntax error

[iFreilicht]

Describe the bug

Using a very simple flake that contains nothing but an empty devShell, running nix print-dev-env will output a bash script that contains a syntax error. Trying to run the bash script will result in the following error:

bash: line 1604: syntax error near unexpected token `;'
bash: line 1604: `            ;&'

Steps To Reproduce

1. Create a new flake with nix flake new asdf
2. cd asdf
3. Replace the content of flake.nix with:

   {
     inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
   
     outputs = { self, nixpkgs }:
       let
         pkgs = nixpkgs.legacyPackages.aarch64-darwin;
       in
       {
         devShell.aarch64-darwin = pkgs.mkShellNoCC { };
       };
   }

4. Run git add .
5. Run nix run github:NixOS/nix#nix print-dev-env | bash to see the error:

   warning: Git tree '/Users/feuh/repos/advent_of_code' is dirty
   warning: creating lock file '/Users/feuh/repos/advent_of_code/gleam/asdf/flake.lock': 
   • Added input 'nixpkgs':
       'github:NixOS/nixpkgs/c75037bbf9093a2acb617804ee46320d6d1fea5a?narHash=sha256-rL5LSYd85kplL5othxK5lmAtjyMOBg390sGBTb3LRMM%3D' (2024-03-16)
   warning: Git tree '/Users/feuh/repos/advent_of_code' is dirty
   bash: line 1604: syntax error near unexpected token `;'
   bash: line 1604: `            ;&'
   

Expected behavior

A clear and concise description of what you expected to happen.

nix-env --version output

$ nix shell github:NixOS/nix#nix -c nix-env -- --version
nix-env (Nix) 2.22.0pre20240317_5c8983b

Additional context

This is only an issue with bash 3.x. 4.0+ works fine. So this only affects darwin users, and only some other tool tries to source the file with the system bash (like it happened for me, see NixOS/nixpkgs#296936)

The full output is here: faulty.sh

The relevant context around line 1604 is this:

        case "$1" in 
            --replace)
                if ! "$_substituteStream_has_warned_replace_deprecation"; then
                    echo "substituteStream(): WARNING: '--replace' is deprecated, use --replace-{fail,warn,quiet}. ($description)" 1>&2;
                    _substituteStream_has_warned_replace_deprecation=true;
                fi;
                replace_mode='--replace-warn'
            ;&
            --replace-quiet | --replace-warn | --replace-fail)
                pattern="$2";

Nixpkgs is aware that the script is only compatible with bash 4.0+, and so setup.sh specifically has this check to prevent exactly the error we see above:

if [[ -n "${BASH_VERSINFO-}" && "${BASH_VERSINFO-}" -lt 4 ]]; then
    echo "Detected Bash version that isn't supported by Nixpkgs (${BASH_VERSION})"
    echo "Please install Bash 4 or greater to continue."
    exit 1
fi

But for some reason, it's not included in the output of print-dev-env. I assume there would have to be a hook function defined so that nix can find it in the intermediate environment and include (and call it) in the output.

Priorities

Add 👍 to issues you find important.

[thufschmitt]

Nixpkgs is aware that the script is only compatible with bash 4.0+, and so setup.sh specifically has this check to prevent exactly the error we see above [...] But for some reason, it's not included in the output of print-dev-env. I assume there would have to be a hook function defined so that nix can find it in the intermediate environment and include (and call it) in the output.

Yeah, Nix does some funky bash magic to only dump the declared environment variables and functions (and not the whole stdenv script). So that doesn't include the checking snippet that you link (that snippet does run internally, but using the bash that is declared as the derivation's builder, not the ambient one).

Few ideas to solve this:

1. Hardcode a dependency in bash>=4 in the Nix get-env script. Makes a good error-message for new Nixpkgs, but breaks the old ones for no reason
2. Remove the dependency to bash<4 in Nixpkgs. Fixes things, but in a very ugly way
3. Expose the dependency on the bash version in a way that Nix can interpret, like having stdenv set MIN_BASH_VERSION=4. That's a bit ugly because it increases the coupling between nix develop and stdenv, but it could allow Nix to gently fail when needed.

Option 3. seems to be the most reasonable one. Thoughts?

[iFreilicht]

Yes, I agree, something like option 3 seems reasonable.

I was thinking that maybe Nix could look for a special check_env_hook() function and insert (and execute) that at the top of the output of print-dev-env.
This would reduce coupling and be more flexible, so it could be used by potential other package collections or packages to check whether they have what they expect in their environment independent of bash (for example, if they built everything with zsh or go or whatever else).

But then the syntax of that would have to be compatible with absolutely everything, and if it is defined multiple times, that has to be handled, it could be abused for something the derivations are intended for, it just opens a whole can of worms for something that is already an edge case.

Specifying something like NIX_ENV_MIN_BASH_VERSION clearly in the Nix docs (and inserting a check at the top of the output of print-dev-env if it was defined) is a reusable mechanism, even if it is only defined for nixpkgs initially.

[domenkozar]

If we can avoid using bash 4 features, it is preferable to use (3) to avoid the need for workarounds that may confuse newcomers.

[bbenne10]

I understand where @domenkozar is coming from: MacOS still ships Bash 3.2 and unless you've installed bash from either Brew or Nix, you'll still get that version in your profile.

That being said: Bash 3.2 is from 2006. I think increasing the supported revision to bash4 and explicitly stating that dependency is a good option. It isn't like MacOS users can't upgrade. We just might want to make upgrade instructions clear and targeted in the error message (as I cannot believe that there's a lot of Linux distributions shipping bash3 by default any more).

[thufschmitt]

@iFreilicht : Can you handle the Nixpkgs part of this? Once that's done, I can do the Nix-side bits

[iFreilicht]

@thufschmitt Done! See NixOS/nixpkgs#299490

[fricklerhandwerk]

To be fully honest, I think none of that should even exist in Nix. See #4702 (comment) and #7501. If Nixpkgs maintainers or users of stdenv need that command, why not ship it with Nixpkgs?

[iFreilicht]

To me, nix develop and nix print-dev-env are reliable interfaces that have nothing to do with nixpkgs. nix develop is not only relevant to nixpkgs maintainers, but everyone who writes, builds and debugs their own flakes. nix print-dev-env is just a bit of a plumbing command that allows other tools (like direnv in this case) to hook into the same logic and supply useful functionality on top of that.

I do agree that more of the logic could move into nixpkgs here, but right now I feel improving the error message here matters most. I'm completely fine with all this getting ripped out again if we ever find a good abstraction that allows nix develop to contain as little magic as possible.

[thufschmitt]

Thanks @iFreilicht ! I've opened #10359 for the Nix side

@fricklerhandwerk I tend to agree, but it's here right now, so we definitely shouldn't keep it broken

[fricklerhandwerk]

it's here right now, so we definitely shouldn't keep it broken

I surely won't block, this is just my humble opinion, but we totally can decidedly ignore the issue – arguing that we'd rather prioritise stable API quality, testing, contributor experience etc., re-iterating that experimental commands are called that for a reason, and hand over the responsibility to those who care enough about this particular feature to implement it where it would be suited better anyway. Not doing that would, also in my humble opinion, be inconsistent with what we already have in writing; especially since we have a track record of not actually focusing on what we claimed we wanted to.