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

Prefer using local over local -n #423

Closed
wants to merge 1 commit into from
Closed

Prefer using local over local -n #423

wants to merge 1 commit into from

Conversation

ipetkov
Copy link

@ipetkov ipetkov commented Nov 29, 2023

  • zsh on macOS (the default shell) apparently does not support local -n and will error out if used

* zsh on macOS (the default shell) apparently does not support `local
  -n` and will error out if used
@Mic92
Copy link
Member

Mic92 commented Nov 29, 2023

direnv should also use bash. How do you get this sourced in zsh?

@kingarrrt
Copy link
Contributor

This would break it. local -n makes a named reference used by the caller.

@bbenne10
Copy link
Contributor

bbenne10 commented Nov 29, 2023

Using nix-direnv directly in zsh is not supported. I'm going to close this, but I think the discussion is worth continuing even so (namely "How did this get sourced in zsh?" and "If this is because of confusion over installation procedures, can we clarify installation procedures at all to help with that?")

@bbenne10 bbenne10 closed this Nov 29, 2023
@ipetkov
Copy link
Author

ipetkov commented Nov 29, 2023

My mistake, I had assumed it was due to using zsh but that's not the actual issue: macOS's provided version of bash itself doesn't appear to support local -n:

Without direnv enabled:

$ which bash
/bin/bash
$ bash --version
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin23)
Copyright (C) 2007 Free Software Foundation, Inc.

I think it's still valuable to avoid using local -n to support older shells. We use direnv to activate a Nix environment which does pull in bash 5.2, but we can't do that until direnv evaluates/activates so we're stuck using whatever version macOS distributes

Could we reopen this?

@ipetkov
Copy link
Author

ipetkov commented Nov 29, 2023

This would break it. local -n makes a named reference used by the caller.

Not super familiar with this feature, is there a different way to write the implementation to not need this?

@ipetkov
Copy link
Author

ipetkov commented Nov 29, 2023

More context: I can reproduce this with direnv installed via homebrew

@bbenne10
Copy link
Contributor

This is explicitly noted in the README already. We ask users to install a modern bash version into their profile. You can do this with homebrew or nix or any other packaging tool for macOS. If you're installing direnv from homebrew, is it not reasonable to also install bash from there? The version shipped with macOS is ancient and I'm afraid that this is not the only thing we'll run afoul of on a version that old.

I'll reopen if there's a change to the source branch that fixes for 3.2 without a bunch more complexity for modern bash versions, but I'm hesitant to say that we'll continue to support ancient bash versions on proprietary systems (even as a user of macOS + nix-direnv myself!)

@Mic92
Copy link
Member

Mic92 commented Nov 29, 2023

We heavily depend on bash's regex feature. Is this even part of 3.2?

@ipetkov
Copy link
Author

ipetkov commented Nov 29, 2023

Thanks for the context here! A bunch of colleagues hit this at work and I was hopeful there could be an easy solution but looks like there isn't. I'll suggest to them to install a newer version of bash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants