Add RUBY_BUILD_TARBALL_OVERRIDE to override the ruby tarball URL #2258

eregon · 2023-09-21T14:37:20Z

And add RUBY_BUILD_IGNORE_CHECKSUM when it is expected the alternative URL has different contents.

This is useful for instance when testing a truffleruby release which is not published yet.
Then I need to use a custom URL, like:

RUBY_BUILD_MIRROR_PACKAGE_URL=https://github.com/my/url.tar.gz ruby-build truffleruby-23.1.0 ~/.rubies/truffleruby-test

I also want this to try dev builds at a different URL, hence this change.
(and those might have a different checksum too, see below)

And finally I would also like a way to skip the checksum check, for instance when trying a variant of a release which therefore has a different URL and contents but yet I want to reuse the existing definition.
For example this can be useful if I want to try the CE build instead of GFTC build of truffleruby 23.1.0, like this:

RUBY_BUILD_IGNORE_CHECKSUM=1 RUBY_BUILD_MIRROR_PACKAGE_URL=https://github.com/oracle/truffleruby/releases/download/graal-23.1.0/truffleruby-community-23.1.0-linux-amd64.tar.gz ruby-build truffleruby-23.1.0 ~/.rubies/truffleruby-test

mislav

I'm skeptical of this feature because I already wasn't a fan of the original RUBY_BUILD_MIRROR_PACKAGE_URL feature, and this adds even more features around it #1793

And finally I would also like a way to skip the checksum check, for instance when trying a variant of a release which therefore has a different URL and contents but yet I want to reuse the existing definition.

From everything you described so far, it sounds like what you're doing isn't at all mirroring existing releases, but dynamically modifying the target that a build definition downloads and installs. If you need to modify a truffleruby definition slightly, would it be an option for you to create a custom definition and provide ruby-build with a path to your custom definition?

mislav · 2023-09-22T11:00:58Z

bin/ruby-build

@@ -1194,7 +1198,7 @@ isolated_gem_install() {
 apply_ruby_patch() {
  local patchfile
  case "$1" in
-  ruby-* | jruby-* | rubinius-* | truffleruby-* )
+  ruby-* | jruby-* | mruby-* | picoruby-* | truffleruby-* | truffleruby+graalvm-* )


This feels like it belongs in a separate PR

It's just a cleanup I noticed while working on this and already its own commit, but sure I can split it up.

The problem with this change being bundled here, apart from being wholly unrelated, is that it won't get its own changelog entry automatically upon cutting a release with script/release (nor when creating a release using the web interface and choosing "Generate release notes"). I believe that all feature changes deserve a changelog entry.

Also, rubinius- should stay (for backwards compatibility).

In the updated PR it's fully related and necessary.

eregon · 2023-09-25T10:22:48Z

Right, this is indeed not exactly mirroring.
@mislav How would you feel about adding another environment variable then?
I was thinking something like RUBY_BUILD_OVERRIDE_RUBY_URL or so originally (and this would only override the URL if it's "a ruby" package, which then would use the same check as here).

(then I saw RUBY_BUILD_MIRROR_PACKAGE_URL already existed and it seemed nicer to extend it)

mislav · 2023-09-25T10:24:00Z

I take this as custom build definitions not being a suitable workaround for you?

eregon · 2023-09-25T10:26:13Z

Yeah they would be too annoying to be of any real use.
Also for the first 2 use cases, those are actually quite related to mirroring, and custom definitions there would not be testing what we want.

eregon · 2023-09-25T10:32:04Z

My immediate/main use case at hand is @nirvdrum found an issue with the latest TruffleRuby release, and I'd like to make it easy for him/others to try other builds, such as (release|dev) builds at other URLs, CE, etc. Infrastructure typically uses releases of ruby-build so it's not really convenient to ship your own definition for that and that would likely require more infra changes (based on infra details of course). While setting environment variables is quite easy.

eregon · 2023-10-02T13:13:18Z

@mislav What do you think of the above? Which way would be OK to merge?

mislav · 2023-10-04T11:08:00Z

bin/ruby-build

@@ -1194,7 +1198,7 @@ isolated_gem_install() {
 apply_ruby_patch() {
  local patchfile
  case "$1" in
-  ruby-* | jruby-* | rubinius-* | truffleruby-* )
+  ruby-* | jruby-* | mruby-* | picoruby-* | truffleruby-* | truffleruby+graalvm-* )


The problem with this change being bundled here, apart from being wholly unrelated, is that it won't get its own changelog entry automatically upon cutting a release with script/release (nor when creating a release using the web interface and choosing "Generate release notes"). I believe that all feature changes deserve a changelog entry.

Also, rubinius- should stay (for backwards compatibility).

bin/ruby-build

mislav · 2023-10-04T11:31:44Z

bin/ruby-build

-      fi
-    elif [ -n "$RUBY_BUILD_MIRROR_PACKAGE_URL" ]; then
-      mirror_url="$RUBY_BUILD_MIRROR_PACKAGE_URL"
+  if [ -n "$RUBY_BUILD_IGNORE_CHECKSUM" ]; then


I do not believe we should expose this additional option, since we would not want anyone to actually use it.

Instead, in order to support the use-case that you are describing (testing Ruby releases that do not yet have their own definition), I suggest that we introduce a new environment variable completely orthogonal to the mirror feature (since this is technically not related to mirroring, either). The new environment variable could be called something like RUBY_BUILD_TARBALL_OVERRIDE, and the value read from it would replace both the URL and checksum values in fetch_tarball. If the environment variable only supplies a URL, then checksum checks would be off anyway, but a checksum could also be optionally supplied too. How does that sound?

Yes that sounds great, I'll update the PR with it.

eregon · 2023-10-11T19:55:54Z

@mislav Updated, could you review and merge if it's OK?

eregon · 2023-10-11T19:58:32Z

bin/ruby-build

@@ -371,6 +371,10 @@ fetch_tarball() {
  local checksum
  local extracted_dir

+  if is_ruby_package "$1" && [ -n "$RUBY_BUILD_TARBALL_OVERRIDE" ]; then


The reason we need the is_ruby_package check is because fetch_tarball is also used for non-ruby packages like openssl and overriding both URLs with the same URL would make no sense.

* Update the check for whether a package is a ruby.

eregon · 2023-10-12T09:49:09Z

Rebased on top of master to include #2268, and locally shellcheck bin/ruby-build is clean.

mislav

Love the brevity of the final solution! Thank you.

eregon · 2023-10-13T22:36:46Z

Indeed it's beautiful how small it is, I was positively surprised as well :)

eregon requested a review from mislav September 21, 2023 14:37

eregon force-pushed the overridable-url branch from d5bbda7 to fe97440 Compare September 21, 2023 14:39

mislav reviewed Sep 22, 2023

View reviewed changes

mislav requested changes Oct 4, 2023

View reviewed changes

eregon force-pushed the overridable-url branch from fe97440 to 06a58c4 Compare October 11, 2023 19:52

eregon changed the title ~~Let RUBY_BUILD_MIRROR_PACKAGE_URL work even when there is no checksum~~ Add RUBY_BUILD_TARBALL_OVERRIDE to override the ruby tarball URL Oct 11, 2023

eregon force-pushed the overridable-url branch from 06a58c4 to e87c419 Compare October 11, 2023 19:55

eregon commented Oct 11, 2023

View reviewed changes

eregon force-pushed the overridable-url branch from e87c419 to 7f03f11 Compare October 11, 2023 20:03

eregon requested a review from mislav October 11, 2023 20:04

Add RUBY_BUILD_TARBALL_OVERRIDE to override the ruby tarball URL

216084d

* Update the check for whether a package is a ruby.

eregon force-pushed the overridable-url branch from 7f03f11 to 216084d Compare October 12, 2023 09:48

mislav approved these changes Oct 13, 2023

View reviewed changes

mislav merged commit 5f4cea1 into rbenv:master Oct 13, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RUBY_BUILD_TARBALL_OVERRIDE to override the ruby tarball URL #2258

Add RUBY_BUILD_TARBALL_OVERRIDE to override the ruby tarball URL #2258

eregon commented Sep 21, 2023 •

edited

Loading

mislav left a comment

mislav Sep 22, 2023

eregon Sep 25, 2023

mislav Oct 4, 2023 •

edited

Loading

eregon Oct 11, 2023

eregon commented Sep 25, 2023 •

edited

Loading

mislav commented Sep 25, 2023

eregon commented Sep 25, 2023

eregon commented Sep 25, 2023 •

edited

Loading

eregon commented Oct 2, 2023

mislav Oct 4, 2023 •

edited

Loading

mislav Oct 4, 2023

eregon Oct 11, 2023

eregon commented Oct 11, 2023

eregon Oct 11, 2023 •

edited

Loading

eregon commented Oct 12, 2023

mislav left a comment

eregon commented Oct 13, 2023

Add RUBY_BUILD_TARBALL_OVERRIDE to override the ruby tarball URL #2258

Add RUBY_BUILD_TARBALL_OVERRIDE to override the ruby tarball URL #2258

Conversation

eregon commented Sep 21, 2023 • edited Loading

mislav left a comment

Choose a reason for hiding this comment

mislav Sep 22, 2023

Choose a reason for hiding this comment

eregon Sep 25, 2023

Choose a reason for hiding this comment

mislav Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

eregon Oct 11, 2023

Choose a reason for hiding this comment

eregon commented Sep 25, 2023 • edited Loading

mislav commented Sep 25, 2023

eregon commented Sep 25, 2023

eregon commented Sep 25, 2023 • edited Loading

eregon commented Oct 2, 2023

mislav Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

mislav Oct 4, 2023

Choose a reason for hiding this comment

eregon Oct 11, 2023

Choose a reason for hiding this comment

eregon commented Oct 11, 2023

eregon Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

eregon commented Oct 12, 2023

mislav left a comment

Choose a reason for hiding this comment

eregon commented Oct 13, 2023

eregon commented Sep 21, 2023 •

edited

Loading

mislav Oct 4, 2023 •

edited

Loading

eregon commented Sep 25, 2023 •

edited

Loading

eregon commented Sep 25, 2023 •

edited

Loading

mislav Oct 4, 2023 •

edited

Loading

eregon Oct 11, 2023 •

edited

Loading