Enable fallback to PATH from rbenv shims

[mislav]

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: 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

[konsolebox]

@konsolebox: Since you've previously provided helpful warnings around other potentially risky changes, does this one raise any warning flags for you? Thanks

I'm quite ok with the fallback code so far. I've been thinking about the GEM_HOME support rbenv will generously provide but ended up having no strong opinion about it, partially because it won't really affect me; another is because GEM_HOME will always affect the ruby instance anyway regardless if executables in ${GEM_HOME}/bin are executed or not. I only worry about the shim noise it will allocate but it's nothing really.

Is there a reason why ${GEM_HOME}/bin is not auto-included in PATH? Not saying it should be.

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.

[mislav]

Is there a reason why ${GEM_HOME}/bin is not auto-included in PATH? Not saying it should be.

You mean, to include it in PATH when libexec/rbenv is preparing the final PATH before dispatching other rbenv-* commands? We could do that in theory, but I'm more comfortable of scoping any GEM_HOME behavior to just the shim dispatch mechanism. If ${GEM_HOME}/bin wasn't in PATH already, I'm reluctant to automatically push it into PATH to avoid negatively affecting all rbenv commands and subprocesses (including ruby itself).

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.

Sorry, could you elaborate? I'm not sure I follow

[konsolebox]

Is there a reason why ${GEM_HOME}/bin is not auto-included in PATH? Not saying it should be.

You mean, to include it in PATH when libexec/rbenv is preparing the final PATH before dispatching other rbenv-* commands? We could do that in theory, but I'm more comfortable of scoping any GEM_HOME behavior to just the shim dispatch mechanism. If ${GEM_HOME}/bin wasn't in PATH already, I'm reluctant to automatically push it into PATH to avoid negatively affecting all rbenv commands and subprocesses (including ruby itself).

To be exact it's these lines:

if [ "$RBENV_BIN_PATH" = "${RBENV_ROOT}/versions/${RBENV_VERSION}/bin" ]; then
  export PATH="${RBENV_BIN_PATH}:${PATH}"
fi

It was originally:

if [ "$RBENV_VERSION" != "system" ]; then
...

Which seems to make sure only "${RBENV_ROOT}/versions/${RBENV_VERSION}/bin" is added to PATH and not ${GEM_HOME}/bin when RBENV_COMMAND_PATH points to an executable in it.

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.

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.

Sorry, could you elaborate? I'm not sure I follow

If let's say I have a PATH with a path that comes before ${RBENV_ROOT}/shims like the ${GEM_HOME}/bin path which I explicitly add:

/home/user/project/common-gems/bin:/home/user/.rbenv/shims:/home/user/.rbenv/bin:/home/user/bin:/home/user/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/opt/bin:

where GEM_HOME is /home/user/project/common-gems/bin:

I would prefer ${RBENV_ROOT}/versions/${RBENV_VERSION}/bin to only be added right before /home/user/.rbenv/shims and not at the very beginning of PATH. This is for the sake of consistency of runtime priorities.

Also if ${RBENV_ROOT}/versions/${RBENV_VERSION}/bin is already in PATH, I want that it doesn't add itself again. This is something that may happen on a recursive call to rbenv or one of its commands.

[mislav]

To be exact it's these lines:

Ah, I see! I can consider allowing ${GEM_HOME}/bin to be prepended here; that would make sense. The extra conditional you noticed me adding is because I do not want to prepend the path for a system fallback. For example, if executing foo was ultimately found in /usr/bin/foo, I do not want /usr/bin to be prepended to PATH.

[konsolebox]

Ah, I see! I can consider allowing ${GEM_HOME}/bin to be prepended here; that would make sense. The extra conditional you noticed me adding is because I do not want to prepend the path for a system fallback. For example, if executing foo was ultimately found in /usr/bin/foo, I do not want /usr/bin to be prepended to PATH.

I'm starting to think that ${RBENV_ROOT}/versions/${RBENV_VERSION}/bin should not be added to PATH at all since if theoretically a grand child process (likely a shell script) changes directory to a project directory with a .ruby-version that specifies a different Ruby version, that version can be ignored since the executables in the previous ${RBENV_ROOT}/versions/${RBENV_VERSION}/bin will be called first.

Was optimization the intention for adding it?

I also think ${GEM_HOME}/bin shouldn't also be added as well since some configuration may change its value in the project as well, which could be done in the grand child process shell itself, or some hooks for rbenv which alters the variables like this one:

/etc/rbenv.d/exec/environment.bash:

{
	__=${PWD}

	while [[ $__ == /* ]]; do
		if [[ -e $__/.ruby-environment ]]; then
			[[ -r $__/.ruby-environment ]] && source "$__/.ruby-environment"
			break
		fi

		__=${__%/*}
	done

	unset -v __
}

.ruby-environment:

export GEM_HOME=${BASH_SOURCE%/*}/../common-project-gems
export PATH=${GEM_HOME}/bin:${PATH}

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.

[konsolebox]

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, /home/user/.rbenv/shims:/home/user/.rbenv/bin:/sbin:/bin becomes /home/user/.rbenv/shims:/home/user/.rbenv/versions/3.0.4/bin:/home/user/.rbenv/bin:/sbin:/bin instead.

[mislav]

I'm starting to think that ${RBENV_ROOT}/versions/${RBENV_VERSION}/bin should not be added to PATH at all

I wanted to get away with that as well, but unfortunately, prepending the bin directory to PATH is what's needed to support ruby -S: #480

If shims remain in PATH, shelling out to ruby -S <command> would fail if command was found among rbenv shims, since shims are bash scripts and not ruby scripts.

ruby -S is itself a huge hack and I don't like that we have to work around this but I wasn't able to find other ways around this.