-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Enable fallback to PATH from rbenv shims #1446
base: master
Are you sure you want to change the base?
Conversation
Previously, when someone activated rbenv shim `foo`, rbenv would error out if `RBENV_ROOT/versions/RBENV_VERSION/bin/foo` did not exist. This change allows `foo` to be additionally looked up in PATH as fallback. This prevents a case where one of the Ruby versions is shadowing a system-wide command of the same name, preventing the use of the global command across all other Ruby versions. rbenv used to be strict around this, allowing the fallback to avoid ever falling back to system Ruby for invocations like `bundle` if the current Ruby version did not yet install Bundler. The current approach prevents this scenario by explicitly disallowing fallback for the following executables: ruby, rake, gem, bundle, bundler, irb, rdoc, ri.
I'm quite ok with the fallback code so far. I've been thinking about the GEM_HOME support Is there a reason why I also ideally think "version bin path" should be made sure to be only added once, and also only added exactly before the shims path to avoid getting a higher PATH priority from a directory a user may not want it to. |
You mean, to include it in PATH when
Sorry, could you elaborate? I'm not sure I follow |
To be exact it's these lines:
It was originally:
Which seems to make sure only I have no definite opinion about it as well. Perhaps not adding it to PATH is safer, and maybe more consistent to instances where GEM_HOME is modified somewhere during runtime.
If let's say I have a PATH with a path that comes before
where I would prefer Also if |
Ah, I see! I can consider allowing |
I'm starting to think that Was optimization the intention for adding it? I also think
The GEM_HOME value example is mostly useful for examining the gems a project pulls in because they're placed in an isolated directory. They're easy to grep, clean up and refresh. |
If it's for the sake of having newly installed executables detected immediately at the same runtime, the paths should be added after the shims path instead. For example, |
I wanted to get away with that as well, but unfortunately, prepending the bin directory to PATH is what's needed to support If shims remain in PATH, shelling out to
|
Previously, when someone activated rbenv shim
foo
, rbenv would error out ifRBENV_ROOT/versions/RBENV_VERSION/bin/foo
did not exist.This change allows
foo
to be additionally looked up in PATH as fallback. This prevents a case where one of the Ruby versions is shadowing a system-wide command of the same name, preventing the use of the global command across all other Ruby versions.rbenv used to be strict around this: the lack of a fallback mechanism was to avoid ever falling back to system Ruby for invocations like
bundle
if the current Ruby version did not yet install Bundler. The current approach prevents this scenario by explicitly disallowing fallback for the following executables: ruby, rake, gem, bundle, bundler, irb, rdoc, ri.Fixes #187
Fixes #865 /cc @jasonkarns
Closes #1110
Additionally, this changeset is a followup to #1436 by bumping up GEM_HOME so it takes precedence over executables found in PATH.
@konsolebox: Since you've previously provided helpful warnings around other potentially risky changes, does this one raise any warning flags for you? Thanks