Fixup 'rubygems: latest' #551

MSP-Greg · 2023-12-24T03:50:33Z

Fixes using 'rubygems; latest' for MRI Ruby 2.3 and later. I tried updating Ruby 2.2 and got errors for two different versions.

gem update --system will not allow a bounded version, only an exact version. So, code is somewhat brittle if new patch releases are done for older RubyGems versions.

Code works same as existing for non MRI Rubies.

One thing I wasn't sure about is whether Ruby master/head builds should not have anything done for 'rubygems; latest'. Sometime this year I think the RubyGems version was a pre-release, so latest would install an earlier version? Not.Sure.

Happy Holidays...

MSP-Greg · 2023-12-24T05:14:48Z

Added a second commit which skips gem update --system for Ruby master/head builds. Added a note to action.yml.

MSP-Greg · 2023-12-24T16:26:53Z

JFYI...

Puma separates MRI & non MRI CI.

In MRI CI, we didn't use the rubygems input, as it really didn't work. We added a step to fix RubyGems/Bundler in older Rubies. Note that Puma CI tests Bundler functionality, as it's used when restarting/reloading Puma.

I used this PR in Puma CI, and it works as expected. I removed the additional step needed for older Rubies.

Head builds are left alone, and Rubies are upgraded with the proper version of RubyGems. We test Ruby 2.4 and later on all platforms.

Happy Holidays, Greg

MSP-Greg · 2023-12-26T14:25:39Z

Note that with the release of RubyGems 3.5.x (restricted to Ruby 3.x), in current master all Ruby versions 2.x fail when the rubygems input is set to latest.

See https://github.com/MSP-Greg/setup-ruby/actions/runs/7330308381

MSP-Greg · 2023-12-27T01:13:24Z

@eregon

Any thoughts on TruffleRuby and maybe JRuby? Handle like Ruby head builds?

eregon · 2023-12-27T13:15:45Z

Indeed I noticed https://github.com/ruby/setup-ruby/actions/runs/7324250188/job/19947638571 thank you for the fix!

eregon · 2023-12-27T13:21:06Z

Any thoughts on TruffleRuby and maybe JRuby? Handle like Ruby head builds?

TruffleRuby ignores gem update --system currently so nothing special to do there.
For JRuby I don't know, but simpler is better (i.e. let's keep as-is).

eregon · 2023-12-27T13:17:20Z

rubygems.js

@@ -28,3 +28,19 @@ export async function rubygemsUpdate(rubygemsVersionInput, rubyPrefix) {

  return true
 }
+
+async function rubygemsLatest(gem, platform, engine, version) {


Could you add a link here to rubygems/rubygems#7329 or another appropriate issue explaining why we need to workaround this bug in RubyGems?

As mentioned, this has always been an issue, but since it's only tested here on Ruby 2.6, it failed because of the recently changed minimum Ruby version in RubyGems.

RubyGems 3.4 supported Ruby 2.6 and later, RubyGems 3.5 supports Ruby 3.0 and later.

eregon · 2023-12-27T13:19:13Z

yarn.lock

-  version "2.1.1"
-  resolved "https://registry.yarnpkg.com/@actions/http-client/-/http-client-2.1.1.tgz#a8e97699c315bed0ecaeaaeb640948470d4586a0"
-  integrity sha512-qhrkRMB40bbbLo7gF+0vu+X+UawOvQQqNAA/5Unx774RS8poaOhThDOG6BGmxvAnxhQnDp2BG/ZUm65xZILTpw==
+  version "2.2.0"


Could you undo all changes in yarn.lock?
They can cause failures, etc and updating usually makes it worse more than it helps from past experience.
In any case updating it should be done in its own commit&PR so easy to revert if any issue.

I'll split the yarn.lock changes.

eregon · 2023-12-27T13:23:40Z

rubygems.js

+
+async function rubygemsLatest(gem, platform, engine, version) {
+  if (engine === 'ruby') {
+    if (version >= '3.0') {


Comparing versions as strings feels brittle, could you use common.floatVersion() instead?

Expecting a x.10 version? Regardless, I'll update.

eregon · 2023-12-27T13:25:30Z

rubygems.js

+      console.log(`Cannot update RubyGems for Ruby version ${version}`)
+    }
+  } else {
+    exec.exec(gem, ['update', '--system'])


Missing await

Sorry, that line is only for non MRI Ruby. Given our/my lack of knowledge of JRuby, remove it and log a message that RubyGems is not updated on non MRI platforms?

Nevermind, I'll fix it as you've listed above. So, same behavior as current with non MRI.

eregon · 2023-12-27T13:27:07Z

.github/workflows/test.yml

+      fail-fast: false
+      matrix:
+        include:
+        - { ruby: '3.2', rg_vers: '3.5.3'  }


rg_vers -> rubygems_version/expected_version/expected_rubygems_version (abbreviations are an overhead on readability if not super well known ones)

eregon · 2023-12-27T13:28:51Z

.github/workflows/test.yml

        rubygems: latest
-    - run: ruby -e "exit(Gem.rubygems_version > Gem::Version.new('3.0.3'))"
+    - run: ruby -e "puts Gem::VERSION; exit(Gem.rubygems_version >= Gem::Version.new('${{ matrix.rg_vers }}'))"


Could you swap single and double quotes? It's safer to use single quotes outside for Ruby code (was already an issue before but since you're touching this line let's do it)

eregon · 2023-12-27T13:30:11Z

action.yml

@@ -12,7 +12,7 @@ inputs:
    description: |
      The version of RubyGems to use. Either 'default' (the default), 'latest', or a version number (e.g., 3.3.5).
      For 'default', no action is taken and the version of RubyGems that comes with Ruby by default is used.
-      For 'latest', `gem update --system` is run to update to the latest RubyGems version.
+      For 'latest', `gem update --system` is run to update to the proper latest RubyGems version.  Ruby head/master builds and Ruby 2.2 and earlier will not be updated.


Keep a sentence per line

eregon · 2023-12-27T13:30:25Z

action.yml

@@ -12,7 +12,7 @@ inputs:
    description: |
      The version of RubyGems to use. Either 'default' (the default), 'latest', or a version number (e.g., 3.3.5).
      For 'default', no action is taken and the version of RubyGems that comes with Ruby by default is used.
-      For 'latest', `gem update --system` is run to update to the latest RubyGems version.
+      For 'latest', `gem update --system` is run to update to the proper latest RubyGems version.  Ruby head/master builds and Ruby 2.2 and earlier will not be updated.


proper latest RubyGems -> latest compatible RubyGems

eregon · 2023-12-27T13:32:51Z

@MSP-Greg I'm curious, why do you update RubyGems in Puma's CI?
I don't recall any reason to update RubyGems in CI at the moment.

eregon

Looks great but needs some polishing (see comments)

MSP-Greg · 2023-12-27T17:06:16Z

I'm curious, why do you update RubyGems in Puma's CI?

Honestly, I can't recall. I wrote the code and included a note with required_ruby_version. Older RubyGems/Bundler were very quirky with that, especially when one throws in cross-platform and pre-compiled gems being available.

but needs some polishing

Not a good phrase to use. Conversely, I guess I shouldn't write code when I'm also doing 'pre Christmas Eve dinner prep'...

This issue has come up in the past, a few issues related. In one message you said:

I can't care less about Ruby 2.5 and old RubyGems bugs, that's the problem of anyone still using Ruby 2.5.

I've advocated using newer Ruby versions, and also using head builds in CI, but I also understand testing against older versions.

As you've stated, this is fixing 'old RubyGems bugs'. I believe current RubyGems/Bundler has fixed all the bugs (thanks to some hard work by very few people).

Although this code may need the RubyGems versions updated from time to time (so it's a burden here), this fix will benefit others that maybe aren't as familiar with all the issues involved with older RubyGems, cross-platform CI and scripting, etc, etc. For others, it may cleanup their Actions workflow scripts...

MSP-Greg · 2023-12-27T18:37:16Z

@eregon I think I fixed/updated all items.

eregon

Thank you!

eregon · 2023-12-27T19:46:56Z

Honestly, I can't recall. I wrote the code and included a note with required_ruby_version. Older RubyGems/Bundler were very quirky with that, especially when one throws in cross-platform and pre-compiled gems being available.

No worries.

but needs some polishing

Not a good phrase to use. Conversely, I guess I shouldn't write code when I'm also doing 'pre Christmas Eve dinner prep'...

What's wrong with it? I didn't mean anything bad. Is "needs some cleanup" better?
I just meant review should be addressed before merging, because it's not just trivial/stylistic things (which could be cleaned up separately later easily).
But it's fairly obvious so maybe I shouldn't have said anything.

This issue has come up in the past, a few issues related. In one message you said:

I can't care less about Ruby 2.5 and old RubyGems bugs, that's the problem of anyone still using Ruby 2.5.

Yeah in general I'm unhappy to have to workaround RubyGems/Bundler bugs, specifically I want to ensure such bugs are properly known upstream and fixed there, but I understand in cases such as this one (where we cannot fix RubyGems shipped with some old Ruby) it's warranted.
I'm still unsure whether updating RubyGems is ever necessary/a good idea or it brings more problems than it helps (that's my current impression based on a few cases of people trying to use it and asking help). But regardless of that I don't want to reopen that debate here and it does seem some people use it.
I appreciate your PR to fix this.

I've advocated using newer Ruby versions, and also using head builds in CI, but I also understand testing against older versions.

The fact this action supports Ruby 1.9 means we try to support as much as possible, but still stay reasonable and simple and e.g. not accumulate many fixes for old Rubies (e.g. if it doesn't compile anymore, we don't try to patch that Ruby, it's just not available).
IMO if there is something in the ecosystem breaking for ancient Rubies it's a good push to stop supporting that Ruby in gems (BTW I wouldn't consider 2.7 ancient yet).

dentarg · 2023-12-28T15:47:12Z

I'm curious, why do you update RubyGems in Puma's CI?
I don't recall any reason to update RubyGems in CI at the moment.

Here's an example from Sinatra CI, on Ruby 3.0 we use latest RubyGems version: https://github.com/sinatra/sinatra/blob/5d844eecdc349d32c5c7de72fe68b5ebaafdbee5/.github/workflows/test.yml#L63-L70 due to rubygems/rubygems#6490

MSP-Greg mentioned this pull request Dec 26, 2023

gem update --system does not respect RubyGems constraints on current ruby rubygems/rubygems#7329

Closed

eregon reviewed Dec 27, 2023

View reviewed changes

eregon requested changes Dec 27, 2023

View reviewed changes

MSP-Greg force-pushed the 00-rubygems-latest branch from c5d9dd9 to 52bdd5b Compare December 27, 2023 18:21

Fixup Rubygems latest

800bff9

MSP-Greg force-pushed the 00-rubygems-latest branch from 52bdd5b to 800bff9 Compare December 27, 2023 18:26

eregon approved these changes Dec 27, 2023

View reviewed changes

eregon merged commit 28f9302 into ruby:master Dec 27, 2023
166 checks passed

MSP-Greg deleted the 00-rubygems-latest branch December 27, 2023 20:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixup 'rubygems: latest' #551

Fixup 'rubygems: latest' #551

MSP-Greg commented Dec 24, 2023 •

edited

Loading

MSP-Greg commented Dec 24, 2023

MSP-Greg commented Dec 24, 2023

MSP-Greg commented Dec 26, 2023 •

edited

Loading

MSP-Greg commented Dec 27, 2023

eregon commented Dec 27, 2023 •

edited

Loading

eregon commented Dec 27, 2023 •

edited

Loading

eregon Dec 27, 2023

MSP-Greg Dec 27, 2023

eregon Dec 27, 2023 •

edited

Loading

MSP-Greg Dec 27, 2023

eregon Dec 27, 2023

MSP-Greg Dec 27, 2023

eregon Dec 27, 2023

MSP-Greg Dec 27, 2023

eregon Dec 27, 2023

eregon Dec 27, 2023

eregon Dec 27, 2023

eregon Dec 27, 2023

eregon commented Dec 27, 2023

eregon left a comment

MSP-Greg commented Dec 27, 2023 •

edited

Loading

MSP-Greg commented Dec 27, 2023

eregon left a comment

eregon commented Dec 27, 2023 •

edited

Loading

dentarg commented Dec 28, 2023

Fixup 'rubygems: latest' #551

Fixup 'rubygems: latest' #551

Conversation

MSP-Greg commented Dec 24, 2023 • edited Loading

MSP-Greg commented Dec 24, 2023

MSP-Greg commented Dec 24, 2023

MSP-Greg commented Dec 26, 2023 • edited Loading

MSP-Greg commented Dec 27, 2023

eregon commented Dec 27, 2023 • edited Loading

eregon commented Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eregon Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eregon commented Dec 27, 2023

eregon left a comment

Choose a reason for hiding this comment

MSP-Greg commented Dec 27, 2023 • edited Loading

MSP-Greg commented Dec 27, 2023

eregon left a comment

Choose a reason for hiding this comment

eregon commented Dec 27, 2023 • edited Loading

dentarg commented Dec 28, 2023

MSP-Greg commented Dec 24, 2023 •

edited

Loading

MSP-Greg commented Dec 26, 2023 •

edited

Loading

eregon commented Dec 27, 2023 •

edited

Loading

eregon commented Dec 27, 2023 •

edited

Loading

eregon Dec 27, 2023 •

edited

Loading

MSP-Greg commented Dec 27, 2023 •

edited

Loading

eregon commented Dec 27, 2023 •

edited

Loading