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

Add support for Ruby 3.4 #221

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

mmizutani
Copy link

@mmizutani mmizutani commented Dec 29, 2024

What? Why?

Ruby 3.4 (3.4.0 and 3.4.1) has been released, bringing some improvement in YJIT performance, syntax ergonomics, etc.
This PR ensures that gruf gem works with Ruby 3.4.

How was it tested?

Local testing and the end-to-end suite in this repository were performed to ensure performance and grpc gem compatibility.
Although the CI container image for Ruby 3.4 is not available, at least local unit tests and e2e tests with Ruby 3.4 are passing.

Prerequisites

@phoffer
Copy link

phoffer commented Dec 31, 2024

I wonder if the the maintainers would be ok with removing the upper limit in the gemspec? Or alternatively, changing to < 4. At least then, apps can update without requiring a new gem release every January. Adding a new version to the CI matrix would give official support, but loosening the restriction would be helpful to our own app maintenance

…afety/ClassInstanceVariable in rubocop-thread_safety v0.6.0
@@ -51,26 +51,26 @@ def reload
#
# @return [::Gruf::Controllers::Autoloader]
#
# rubocop:disable ThreadSafety/InstanceVariableInClassMethod
Copy link
Author

Choose a reason for hiding this comment

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

This cop was renamed to ThreadSafety/ClassInstanceVariable in rubocop-thread_safety v0.6.0 released 2 months ago.

https://github.com/rubocop/rubocop-thread_safety/blob/master/CHANGELOG.md#060

@splittingred
Copy link
Member

splittingred commented Jan 6, 2025

Hi all! Thanks for starting on this. @phoffer one of the big reasons we pin in gruf is that Google's gRPC gem support for latest Ruby versions post-new-year every year lags considerability. There are often issues, compile problems, and distribution issues with it, and so we prefer to ensure stability in gruf by explicitly pinning the Ruby version. I'm not keen on releasing that pin (evidence for 3.4 support in grpc/grpc still continues to be an issue post-release.)

Once it hits stability - and especially, once precompiled libraries for grpc/grpc are built, as not having these can have financial impact on companies due to extended build times due to grpc needing to compile extensions every build run - we'll relax the pin.

Thanks!

@mmizutani
Copy link
Author

mmizutani commented Jan 14, 2025

Based on splittingred's explanation, I have added a comment explaining why the upper limit of the required Ruby major version is specified to gruf.gemspec so that the maintainers will not have to explain the background for similar questions every new year.

@chadlwilson
Copy link

chadlwilson commented Jan 17, 2025

If it helps, there were no code/compile changes that seemed to be needed within grpc to "support " 3.4.1 (at least none known right now), but if you wanted to avoid slow gem installs and need for users to have all the relevant compile tooling available it still may make sense to wait for the pre-compiled gems before unlocking your required ruby version.

In other words the 'issues' at the moment are specific to pre-compiling the gems across multiple target architectures, and specific to a bug in the 3.4.0 release which ruby maintainers fixed in 3.4.1.

Users stuck on non-maintained grpc versions are still going to need to compile from source for 3.4 anyway, of course (and although I have raised the linked PRs, am not sure whether grpc maintainers will backport and to which versions right now since I am just a community member trying to reduce the lag to get 3.4 pre-compiled gems, not a maintainer)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants