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

Make ASDF_FORCE_PREPEND consistently default to "yes" on all platforms #1579

Open
gabrielsoldani opened this issue Jun 16, 2023 · 7 comments

Comments

@gabrielsoldani
Copy link

gabrielsoldani commented Jun 16, 2023

Is your feature request related to a problem? Please describe

Before 0.12.0, sourcing asdf.sh on Zsh and Bash always prepended the asdf entries to the PATH. It also looked to see if the entries were already present, and if so, removed them (this isn't strictly necessary since duplicate PATH entries are fine, but it's nice).

On 0.12.0, the behavior changed with the addition of the ASDF_FORCE_PREPEND setting (#1560). The idea is that by setting it to "no", sourcing asdf.sh will only prepend asdf's PATH entries if they aren't already present in the PATH.

"no" was chosen as the default, but this broke the behavior on macOS (#1550), where path_helper reorders the inherited PATH. So "yes" was chosen as the default for macOS.

In my understanding, this setting was included in order to let users have more fine-grained control over their PATH and to align Zsh/Bash/POSIX shell behavior with other shells supported by asdf.

This meant that 0.12.0 introduced a breaking change, but only to non-macOS users.

Describe the proposed solution

I propose that the default value of ASDF_FORCE_PREPEND should be "yes" on all platforms.

  • The user still maintains control over their PATH by virtue of choosing when to source asdf.sh in their shell profile.
  • The user could still source asdf.sh with ASDF_FORCE_PREPEND=no if they want more fine-grained control.
  • Different defaults for the same shell on different platforms diverts a very reasonable expectation: that sourcing asdf.sh on a POSIX shell does the same thing to their PATH across all POSIX-compatible systems.
  • Sourcing asdf.sh interactively consistently restores asdf's PATH precedence. This is probably what the user expects, and when it doesn't work, they don't need to search the documentation to find the ASDF_FORCE_PREPEND variable and realize they need to run ASDF_FORCE_PREPENT=yes . $HOME/.asdf/asdf.sh instead, but only if they're not on macOS.
  • Reverts the default behavior of versions prior to the 0.12.0 release.

I'd be happy to contribute the changes to the code and the documentation.

@gabrielsoldani
Copy link
Author

cc @hyperupcall

@hyperupcall
Copy link
Contributor

It seems we have different expectations on what is reasonable behavior. All other supported shells do not automatically prepend, so this is in line with other shells. But, changing it back would mean inconsistency with other shells, but consistency across platforms.

I view my changes more intuitive because you'll generally get the same behavior on the same machine. And the exception, macOS, exists due to inherent problems of macOS (namely, the broken path_helper that every single version manager must work around). I read your proposed solution, and I understand why they could be preferred, but I still disagree.

Unless there is something specific that breaks (like #1550), then I'm inclined to leave things how they currently are. Our behavior is currently aligned with all popular tools like other version managers that modify the PATH.

@gabrielsoldani
Copy link
Author

I've taken a look at what other version managers did (or didn't) do about this:

  • rbenv chose to allow duplicate PATH entries and prepend itself unconditionally. They seem to have considered detecting duplicate PATH entries, ran into the path_helper bug, and reverted the change.
  • pyenv prepends itself by default when evaluating pyenv init -. This behavior can be disabled with pyenv init --no-push-path. This is equivalent in spirit to having ASDF_FORCE_PREPEND=yes by default, allowing users to set it to "no". I assume they're aware of the problems with path_helper.
  • nvm detects if there's a "system binary PATH" entry (/bin, /usr/bin or /usr/local/bin) before its entry. If so, it prepends itself. This is, in my opinion, too clever.
  • jenv takes the same approach as rbenv and prepends its entry even if it creates duplicate entries. It doesn't seem like they've ran into the path_helper problem because they've chosen the simplest approach of them all.

Once path_helper is fixed, I agree that the default could and should be "no". But "yes" also makes sense: version managers always prepending the PATH seems to predate the path_helper problem, and avoids it completely.

I personally haven't used any version managers that went with a platform-specific workaround for path_helper. So this behavior was quite unexpected for me as a user! But I understand if our expectations are different due to different experiences :)

@hyperupcall
Copy link
Contributor

hyperupcall commented Jul 7, 2023

@gabrielsoldani Thank you for your comment and detailed links. I may have looked at reference code too quickly and it seems that automatically prepending is the more common route, so I was mistaken.

But to fix this, I would like things to remain consistent across all shells - so all shells automatically prepend instead of just Bash/Zsh/dash. Not all of the code exists for that so it's not as simple as defaulting ASDF_FORCE_PREPEND differently.

@hyperupcall
Copy link
Contributor

So this was the commit that changed things. My intentions are to fix this.

@gabrielsoldani
Copy link
Author

@hyperupcall No problem! :) Thanks for re-evaluating this decision.

I haven't used the non-Unix shells yet, but I'll gladly take a look and contribute this change, if you prefer. It might be a good first contribution and a chance to try these shells out.

@Stratus3D
Copy link
Member

I think @gabrielsoldani makes some very good points about this. While I don't feel to strongly one way or the other, I think I'm more in favor of prepending by default.

The user still maintains control over their PATH by virtue of choosing when to source asdf.sh in their shell profile.

Is this true with ASDF_FORCE_PREPEND=yes? If so, I feel like there really isn't any downside at all to defaulting to yes.

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

No branches or pull requests

3 participants