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 Ruby with ruby-install #5356

Merged
merged 2 commits into from
Jul 12, 2022
Merged

Install Ruby with ruby-install #5356

merged 2 commits into from
Jul 12, 2022

Conversation

mattt
Copy link
Contributor

@mattt mattt commented Jul 11, 2022

Alternative to #5190.
Related to #5031 (comment).

Currently, we use the Brightbox PPA to install Ruby with apt. However, this limits our ability to update to a more recent patch release of Ruby 2.7 (or later, Ruby 3).

This PR updates this step in Dockerfile to instead use ruby-install to install Ruby versions. I used Ruby 2.7.5 to match the version installed now. As a follow-up, we should upgrade to 2.7.6.

@mattt mattt requested a review from a team as a code owner July 11, 2022 12:23
@mattt mattt mentioned this pull request Jul 11, 2022
@mattt mattt force-pushed the mattt/ruby-install branch from 7229f1a to 9f431bf Compare July 11, 2022 12:26
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Nice! 😍

@mattt mattt force-pushed the mattt/ruby-install branch from 9f431bf to be4fd82 Compare July 11, 2022 12:42
Dockerfile Show resolved Hide resolved
@jurre
Copy link
Member

jurre commented Jul 11, 2022

I think this is the right approach, thanks for taking this on, I've actually been meaning to do this for so long! I believe there's a little syntax issue with the Dockerfile that we need to patch up, but other than that looks good!

Copy link
Member

@landongrindheim landongrindheim left a comment

Choose a reason for hiding this comment

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

Great improvement 🏆

Comment on lines +77 to +78
ARG BUNDLER_V1_VERSION=1.17.3
ARG BUNDLER_V2_VERSION=2.3.13
Copy link
Member

Choose a reason for hiding this comment

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

I really like this update 🥇

💡 Seeing this change makes me wonder if we could take it further. What do you think about making these ENV variables, then pointing the references in our code (example) to BUNDER_Vx_VERSION? If you're interested, I think a followup PR would be a great fit for the change 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully recall the whole thing from my work on #4973, but I'm pretty sure if BUNDLER_VERSION is used is because RubyGems actually uses that ENV variable to activate the proper Bundler version. So if I understood what you mean, I think it would not work as expected. But if we introduced something like #4973 it wouldn't be needed anyways.

Dockerfile Show resolved Hide resolved
@mattt mattt merged commit 575bb90 into main Jul 12, 2022
@mattt mattt deleted the mattt/ruby-install branch July 12, 2022 14:47
@brrygrdn brrygrdn mentioned this pull request Jul 14, 2022
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

Successfully merging this pull request may close these issues.

4 participants