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

Install only one of rvm or rbenv for managing ruby versions #603

Open
nevans opened this issue Jun 29, 2023 · 5 comments
Open

Install only one of rvm or rbenv for managing ruby versions #603

nevans opened this issue Jun 29, 2023 · 5 comments

Comments

@nevans
Copy link

nevans commented Jun 29, 2023

Is there any benefit to installing both rvm and rbenv?

Context

rvm does so much: it installs plugins that change the behavior of rubygems and the gem command, it overrides and updates a bunch of environment variables, it updates umask, it edits the ruby binary, and it performs various other mutations on your environment whenever you load your shell or change directories.

Unfortunately, if you load rbenv or place its shims earlier in the PATH than rvm, then some truly weird situations arise. E.g: which gem reports the rbenv shim, but running gem uses the rvm installation. And rvm can "infect" the rbenv installation with its own gem plugins, and other similar clashes. This creates an unstable system with arcane error messages that can be incredibly difficult and frustrating to debug.

Even worse, the rvm documentation for how to uninstall rvm simply doesn't work inside devcontainers that use the ruby feature. After the rvm files have been removed and various /etc files have been edited, the rbenv installation might still be broken because the devcontainer-feature.json sets the GEM_HOME and GEM_PATH environment variables to rvm directories.

In situations (such as GitHub Codespaces) where the default image has both rvm and rbenv installed, this can run into trouble with user's dotfiles, which are often configured to check for rbenv and prefer it over rvm.

Workarounds

Unfortunately, setting GEM_HOME and GEM_PATH to empty strings is not the same as unsetting them. And it isn't possible to simply unset Dockerfile ENV vars. (Is it possible to unset devcontainer.json env vars?)

We can manually override those variables to something else in our own devcontainer.json files, but that still effectively breaks rbenv: rbenv uses shims to load the correct versions of ruby or gem, and those installed versions provide their own sensible dynamic defaults. This is especially useful when switching between branches with different ruby versions, when testing multiple ruby versions with a shell script that sets RBENV_VERSION, when using multiple repositories, or when multiple directories in a single repository have different .ruby-version files. Setting GEM_HOME and GEM_PATH to a static path can break all of these scenarios.

I often run into this problem on public GitHub Codespaces. My most common workaround is to simply work locally and not use a devcontainer or the Codespace. If I'm just doing a quick fix, I'll remove rbenv from my dotfiles. But when I'm using the devenv for a little bit longer, I'll workaround by first sudo rming all of the rvm dirs and then call unset GEM_HOME; unset GEM_PATH from my shell prompt.

Various proposals

The simplest approach is probably to simply not install rbenv inside the ruby feature. It's already broken, so removing it would stop wasting people's time with long, confusing, disappointing debugging sessions.

However, both anecdotally and based on github stars and forks, rbenv is more popular than rvm. It is also simpler and easier to debug. Personally, I'd rather see rbenv promoted as the default. Users should still be able to install rvm and I think it should still work, more or less. rvm breaks rbenv but rbenv doesn't break rvm. However this might not be considered backward compatible, and probably requires a major version bump.

IMO, the most important thing would be to simply remove the following lines from devcontainer-feature.json:

"GEM_PATH": "/usr/local/rvm/gems/default:/usr/local/rvm/gems/default@global",
"GEM_HOME": "/usr/local/rvm/gems/default",

Then, if a user wants to disable rvm, all they would need to do is delete or comment out /etc/profile.d/rvm.sh. And I think that a manually installed rvm could still work, so long as everything is able to load the environment from /etc/profile.d/rvm.sh.

There might be some reason that rvm is strongly preferred, e.g: installations are faster because it downloads binaries and bit-twiddles them, rather than compile them. In that case, it might be reasonable to use the "mini rvm" integration that is available for chruby: https://rvm.io/workflow/chruby. This uses mrvm to install ruby versions and chruby to switch. I believe this mrvm approach could also be made to work with rbenv, although I've never attempted this myself.

If any of these proposals sound like they might be acceptable, or if you have another better alternative in mind, I can create a draft PR for you to review.

@samruddhikhandale
Copy link
Member

Hi 👋

Thanks for writing a detailed issue, and providing proposals and workarounds. We greatly appreciate it! ✨

What do you think of a solution as mentioned in devcontainers/images#572 (comment) ? 👇

We should update the Ruby Feature, and add two new Feature options which controls rvm and rbenv installation. (eg: installRVM, installRbenv)

We can decide which one to install by default, or if we should disable both. In any case, I understand that we'd need to bump up the version for the Ruby Feature (which I think is alright given the confusions it would avoid)

@nevans
Copy link
Author

nevans commented Jul 2, 2023

@samruddhikhandale Thanks for reading and considering!

What do you think of a solution as mentioned in devcontainers/images#572 (comment) ?

Haha, I think it's probably better than all of my proposals. 😉

But I couldn't tell if it's possible to vary containerEnv vars based on feature options values. Is it? In addition to varying the values, can we choose which env vars to set or not set based on options (to account for the difference between null and "")?

On the other hand, I'm not sure how necessary the GEM_HOME and GEM_PATH containerEnv vars are for rvm to correctly operate. If they are necessary, perhaps we can make them unnecessary by adding a rubygems/defaults/operating_system.rb file. See the following for context:

Although I'd honestly be a little bit surprised if rvm doesn't already do this or something like this, perhaps in one of its rubygems plugins. 🤞🏻


Additionally:

IMO, it really doesn't make much sense to ever support more than one version manager. So, instead of two or more boolean options, what do you think about something like installRubyVersionManager: "rvm" | "rbenv", "chruby" | null?

And, if the default is to install none of them, then it would be nice to provide install scripts for all of them, as a convenience. When none are installed, the user could be informed of these scripts, in a /etc/motd sort of style. What do you think?

@ThomasOwens
Copy link

I'm commenting here, since this is the repo that appears to accept pull requests. This is also related to microsoft/vscode-dev-containers#704 and devcontainers/images#572.

Although this isn't currently causing me any issues, I do have some time and would like to try to contribute some improvements to the Ruby devcontainer ecosystem, but I'm not sure if there's any kind of agreement on what the solution would be. Installing two Ruby version managers is a problem, and there's already a system Ruby installed.

From what I see, the best solution would be to add a new option for version manager selection to the Ruby feature. Using a string type and an enum of ["none", "rvm", "rbenv"] would cover the current options and be extensible if someone wanted to contribute, for example, chruby or asdf in the future. The default option would be "none" to not install any version manager and only rely on the installed system ruby version.

It's out-of-scope, but I've noticed some disparities between rvm and rbenv as well, such as rvm supporting the ability to specify a list of multiple ruby versions to install. I think the incremental improvement of separating out rvm and rbenv to allow for the three use cases (system ruby users, rvm users, and rbenv users) would be a good step to allow rvm users or rbenv users to implement additional enhancements for their specific version managers.

Please let me know if this solution would be acceptable. I'd like to make sure that the approach would be correct before spending time and effort on a PR, especially since I'm no longer blocked in terms of being able to use these devcontainers for my projects.

@samruddhikhandale
Copy link
Member

Thank you so much @ThomasOwens for being willing to contribute this change, we definitely appreciate it and we are more than happy to accept the PR.

I agree that we should decide a path before you jump on to the code changes. Let me do an analysis on the existing Ruby Feature, and discuss your proposed approach with other devcontainer maintainers. I'll respond to this issue with an update soon on the next steps. Thank you for your patience.

@samruddhikhandale
Copy link
Member

We love the overall idea proposed in this issue! Thank you ✨

For the question of which ruby version manager should be installed by default, started a discussion in the #devcontainers-community slack channel. We are hoping to gather feedback from the community before we finalize it. Also, opened #757

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

No branches or pull requests

3 participants