[ruby] add back logic to avoid rb_abi_version if RUBY_PATCHLEVEL >= 0#38487
[ruby] add back logic to avoid rb_abi_version if RUBY_PATCHLEVEL >= 0#38487apolcyn wants to merge 1 commit intogrpc:masterfrom
Conversation
|
I still don't think it is needed due to the changes made in ruby 3.2.1 as documented at f78b353 and could not replicate the problem originally reported on ruby 3.2.1+ (source build) even without the check. Looking at the logs from rake-compiler-dock I also didn't think it is coming from the host version after all (it was logging different things for different target build versions) so I rejected my initial theory that the host version was involved. Nevertheless if you want to add it back, that's your call. |
|
Thanks @chadlwilson . There is one concrete case I'm thinking of that I think is still broken, without this, though: If rake-compiler-dock were to update its "host" ruby version to 3.2.1 or greater, then we'd evaluate That would still be broken AFAICS since the symbol is not retained on ruby releases < 3.2.1. This fixes it, albeit in a subtle way. That said I'm debating if this would be better done more explicitly by just blanket disabling |
|
I dont believe that will break anything, even if it does use the host version as deecribed, but haven't tested it, obviously. Looks like we can test this scenario empirically as rake-compiler-dock has changed the host ruby version to 3.4.1 with https://github.com/rake-compiler/rake-compiler-dock/releases/tag/v1.9.0 |
|
Note that the earlier PR was reverted off master and 1.70 so we'll have to resubmit anyway, so can either restore this in another PR or confirm if it's required. |
Per discussion on grpc#38487 Signed-off-by: Chad Wilson <chadw@thoughtworks.com>
…t (REDUX) (#38601) Repeat attempt of #38338 to bring Ruby 3.4 precompiled gems to grpc. Pending #38597 Brief history - #38338 merged - #38458 backported to 1.70.x and merged - Above two changes reverted in #38516 and #38515 due to release urgency. Root cause was minor artifact build timeout due to release infra having smaller hosts (fixed in #38597) - Discussion in #38487 about adding back the workaround with `RUBY_PATCHLEVEL`. Unclear if necessary. This PR - cherry-picks the original squashed PR - adds back the `RUBY_PATCHLEVEL` hack, per discussion in #38487 - further upgrades `rake-compiler-dock` to `1.9.1` which avoids coupling patch versions between grpc and rake-compiler-dock - This version changes the host Ruby version from `3.1` -> `3.4` so _technically_ we _could_ validate the theory from #38487. Will await @apolcyn advice. Needs rake-compiler-dock test image re-uploads courtesy of @apolcyn 🙏 Closes #38601 PiperOrigin-RevId: 720824180
|
This can be closed now; combined into #38601 :-) |
…t (REDUX) (grpc#38601) Repeat attempt of grpc#38338 to bring Ruby 3.4 precompiled gems to grpc. Pending grpc#38597 Brief history - grpc#38338 merged - grpc#38458 backported to 1.70.x and merged - Above two changes reverted in grpc#38516 and grpc#38515 due to release urgency. Root cause was minor artifact build timeout due to release infra having smaller hosts (fixed in grpc#38597) - Discussion in grpc#38487 about adding back the workaround with `RUBY_PATCHLEVEL`. Unclear if necessary. This PR - cherry-picks the original squashed PR - adds back the `RUBY_PATCHLEVEL` hack, per discussion in grpc#38487 - further upgrades `rake-compiler-dock` to `1.9.1` which avoids coupling patch versions between grpc and rake-compiler-dock - This version changes the host Ruby version from `3.1` -> `3.4` so _technically_ we _could_ validate the theory from grpc#38487. Will await @apolcyn advice. Needs rake-compiler-dock test image re-uploads courtesy of @apolcyn 🙏 Closes grpc#38601 PiperOrigin-RevId: 720824180 (cherry picked from commit ef13727)
…t (REDUX) (grpc#38601) Repeat attempt of grpc#38338 to bring Ruby 3.4 precompiled gems to grpc. Pending grpc#38597 Brief history - grpc#38338 merged - grpc#38458 backported to 1.70.x and merged - Above two changes reverted in grpc#38516 and grpc#38515 due to release urgency. Root cause was minor artifact build timeout due to release infra having smaller hosts (fixed in grpc#38597) - Discussion in grpc#38487 about adding back the workaround with `RUBY_PATCHLEVEL`. Unclear if necessary. This PR - cherry-picks the original squashed PR - adds back the `RUBY_PATCHLEVEL` hack, per discussion in grpc#38487 - further upgrades `rake-compiler-dock` to `1.9.1` which avoids coupling patch versions between grpc and rake-compiler-dock - This version changes the host Ruby version from `3.1` -> `3.4` so _technically_ we _could_ validate the theory from grpc#38487. Will await @apolcyn advice. Needs rake-compiler-dock test image re-uploads courtesy of @apolcyn 🙏 Closes grpc#38601 PiperOrigin-RevId: 720824180
#38338 removed this check but I think it is better to keep in, out of caution, for a couple reasons:
it's possible this might effect gems being built from source, which are not extremely well covered in our tests
During binary release builds with rake-compiler-dock, this is actually coming from the "host" ruby version rather than the
RUBY_CC_VERSIONthat we're building for i.e. the "target" ruby version. Currently it looks like rake-compiler-dock is using a "target" ruby version of 3.1. If that were to change say to 3.2, then without this check our logic inextconf.rbwould change in evaluatinghave_ruby_abi_versionfromfalsetotrue. Adding this check back in prevents that (although in a slightly more brittle way than I'd like).cc @chadlwilson FYI