-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
Clean up Ruby patch release overrides #496
Conversation
337a8f1
to
969d7f9
Compare
@@ -1,6 +1,7 @@ | |||
FROM ghcr.io/rake-compiler/rake-compiler-dock-image:1.8.0-mri-aarch64-linux | |||
# Ensure this version matches the rack-compiler-version in Gemfile | |||
FROM ghcr.io/rake-compiler/rake-compiler-dock-image:1.9.1-mri-aarch64-linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we template these files?
UPDATE: I see Dependabot is configured to update both the Gemfile
and Dockerfiles
separately. Perhaps we can just live with merging separate pull requests at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One downside is that dependabot has a limit of PRs it will open, and it can be hard to know if all dockerfiles were updated properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand your concern correctly: Dependabot now supports grouped updates so you can open one PR for multiple dependencies, rather than one per.
Gemfile
Outdated
@@ -8,6 +8,7 @@ gemspec path: "examples/rust_reverse" | |||
gem "rake", "~> 13.0" | |||
gem "minitest", "5.15.0" | |||
gem "rake-compiler", "~> 1.2.5" # Small bug in 1.2.4 that breaks Ruby 2.5 | |||
gem "rake-compiler-dock", "~> 1.9.1" # This should match the versions used in docker/Dockerfile.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, but shouldn't this be
gem "rake-compiler-dock", "~> 1.9.1" # This should match the versions used in docker/Dockerfile.* | |
gem "rake-compiler-dock", "1.9.1" # This should match the versions used in docker/Dockerfile.* |
that way the version is absolutely pinned to the Dockerfile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good point, thanks.
`RakeCompilerDock.cross_rubies` supplies the exact patch release needed. This is available in rack-compiler-dock v1.9.1: * rake-compiler/rake-compiler-dock#149 * https://github.com/rake-compiler/rake-compiler-dock/releases/tag/v1.9.1 This simplifies the `Dockerfile`, but it does mean we have to keep the rake-compiler-dock version in the `Gemfile` in sync with all the images in `docker/Dockerfile.*`.
969d7f9
to
99c1aee
Compare
✅ Ty ty! We can fix the static build in a follow up |
As I explained in #497, I believe the static build will be fixed by oxidize-rb/actions#52. I think we just need someone to release a new version? |
RakeCompilerDock.cross_rubies
supplies the exact patch release needed. This is available in rack-compiler-dock v1.9.1:This simplifies the
Dockerfile
, but it does mean we have to keep the rake-compiler-dock version in theGemfile
in sync with all the images indocker/Dockerfile.*
.