Skip to content
This repository has been archived by the owner on Nov 30, 2023. It is now read-only.

Issue 704: Adding enabling and disabling of RBENV and RVM packages #1333

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

KanishkT123
Copy link
Contributor

Work Item ID

Description


Allows for RBENV and RVM tools to be enabled or disabled in the Ruby dev container.

PR Checklist


Use the check-list below to ensure your branch is ready for PR. If the item is not applicable, leave it blank.

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My code follows the code style of this project.
  • I ran the lint checks which produced no new errors nor warnings for my changes.
  • I have checked to ensure there aren't other open Pull Requests for the same update/change.

Does this introduce a breaking change?


  • Yes
  • No

If this introduces a breaking change, please describe the impact and migration path for existing applications below.

Testing


Other information or known dependencies

PR has been reviewed and approved internally by @juzuluag , @bderusha and
@dnastrain


  • Any other information or known dependencies that is important to this PR.
  1. Changes to ruby-debian.sh script that modifies the bashrc and zshrc files in a devcontainer to allow conditional enabling of rbenv and rvm.
  2. Changes to devcontainer.json to include two new ENV Variables for built containers: RBENV_ENABLED and RVM_ENABLED which are read by the bashrc and zshrc to enable/disable RBENV and RVM
  3. Default enables rbenv and rvm in base.Dockerfile in order to reduce workflow disruption.
  4. Default disables rbenv and rvm in devcontainer.json in order to reduce tooling overhead for devcontainer users.

@KanishkT123 KanishkT123 requested a review from Chuxel March 3, 2022 15:50
Copy link
Member

@Chuxel Chuxel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments on initial review.

@KanishkT123 KanishkT123 requested a review from Chuxel March 4, 2022 16:18
@Chuxel
Copy link
Member

Chuxel commented Mar 8, 2022

So reviewing this - the changes look good... but one thing that is flummoxing me is it still seems like somehow rvm is getting sourced when I set "RVM_ENABLED":"false" Are you seeing that as well? I am seeing rbenv not be found as expected - it's just rvm.

@Chuxel
Copy link
Member

Chuxel commented Mar 8, 2022

Ah - found it. It looks like rvm is installing a file at /etc/profile.d/rvm.sh that we didn't expect. This is getting picked up when VS Code starts rather than when a new terminal is created.

I suspect this will need to have the same condition added to it.

@KanishkT123
Copy link
Contributor Author

Ah - found it. It looks like rvm is installing a file at /etc/profile.d/rvm.sh that we didn't expect. This is getting picked up when VS Code starts rather than when a new terminal is created.

I suspect this will need to have the same condition added to it.

I'm not sure exactly how this works. Do you mean modifying the rvm.sh file directly, or do you mean maybe having the .bashrc file delete the rvm.sh file in profile.d?

@Chuxel
Copy link
Member

Chuxel commented Mar 15, 2022

@KanishkT123 Yeah, that is a reasonable way to handle it I think. I don't think this originally happened but got added somewhere along the way by the rvm installer.

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

Successfully merging this pull request may close these issues.

4 participants