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

Support ruby 3.4 #19752

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Support ruby 3.4 #19752

wants to merge 15 commits into from

Conversation

JasonLunn
Copy link
Contributor

@JasonLunn JasonLunn commented Dec 19, 2024

#test-continuous

@JasonLunn JasonLunn requested a review from a team as a code owner December 19, 2024 23:32
@JasonLunn JasonLunn requested review from ericsalo and removed request for a team December 19, 2024 23:32
@JasonLunn JasonLunn marked this pull request as draft December 19, 2024 23:32
ruby/google-protobuf.gemspec Outdated Show resolved Hide resolved
@JasonLunn JasonLunn force-pushed the support_ruby_3.4 branch 2 times, most recently from f3e3efe to de83a11 Compare December 20, 2024 06:01
@PikachuEXE
Copy link

@@ -32,6 +32,8 @@ Gem::Specification.new do |s|
s.add_development_dependency "rake-compiler-dock", "= 1.2.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

rake-compiler and rake-compiler-dock may need an update to support 3.4.0 as well:

https://github.com/rake-compiler/rake-compiler-dock/releases/tag/v1.7.0

Copy link
Contributor

Choose a reason for hiding this comment

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

This 1.2.1 version doesn't even support 3.3 so perhaps it's already outdated or not particularly relied upon for official builds, but probably good to update nonetheless.

@chadlwilson
Copy link
Contributor

chadlwilson commented Dec 31, 2024

Not sure if it's being discussed elsewhere, but the test failures here seem to imply that the test docker image being used (us-docker.pkg.dev/protobuf-build/containers/test/linux/ruby:7.1.2-ruby-3.4.1-90d207f4e749b54c8792bbe974dfc70323b6566e) might have an old bundler, which vendors a thor version prior to 1.2.0 with this fix. The failure seems to imply it's using bundler < 2.3.1 where this was fixed upstream.

These DidYouMean constants were removed in Ruby 3.4 after deprecation in 3.1.

 /usr/local/rvm/rubies/ruby-3.4.1/bin/ruby: No such file or directory -- bundler/exe/bundler (LoadError)
Analyzing: 49 targets (170 packages loaded, 1720 targets configured)
[6 / 8] Copying files; 0s remote-cache, processwrapper-sandbox ... (2 actions running)
/workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/vendor/thor/lib/thor/error.rb:105:in '<class:Thor>': uninitialized constant DidYouMean::SPELL_CHECKERS (NameError)

    DidYouMean::SPELL_CHECKERS.merge!(
              ^^^^^^^^^^^^^^^^
Did you mean?  DidYouMean::SpellChecker
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/vendor/thor/lib/thor/error.rb:1:in '<top (required)>'
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/vendor/thor/lib/thor/base.rb:3:in 'Kernel#require_relative'
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/vendor/thor/lib/thor/base.rb:3:in '<top (required)>'
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/vendor/thor/lib/thor.rb:2:in 'Kernel#require_relative'
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/vendor/thor/lib/thor.rb:2:in '<top (required)>'
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/vendored_thor.rb:8:in 'Kernel#require_relative'
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/vendored_thor.rb:8:in '<top (required)>'
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/friendly_errors.rb:3:in 'Kernel#require_relative'
	from /workspace/_build/out/external/protobuf_bundle/bundler/lib/bundler/friendly_errors.rb:3:in '<top (required)>'
	from /workspace/_build/out/external/protobuf_bundle/bundler/exe/bundle:29:in 'Kernel#require_relative'
	from /workspace/_build/out/external/protobuf_bundle/bundler/exe/bundle:29:in '<top (required)>'
	from bundler/exe/bundler:4:in 'Kernel#load'
	from bundler/exe/bundler:4:in '<main>'

@bquorning
Copy link

Is there any hope of getting this Ruby 3.4 support backported to a v3.x version of this gem?

@JasonLunn
Copy link
Contributor Author

Is there any hope of getting this Ruby 3.4 support backported to a v3.x version of this gem?

Is there a reason updating to 4.x is not a better solution? We don't usually make back ports to releases that old unless there is a security issue, and 3.x is on track to be dropped from support altogether at the end of Q1. See https://protobuf.dev/support/version-support/#ruby

@JasonLunn
Copy link
Contributor Author

Not sure if it's being discussed elsewhere, but the test failures here seem to imply that the test docker image being used (us-docker.pkg.dev/protobuf-build/containers/test/linux/ruby:7.1.2-ruby-3.4.1-90d207f4e749b54c8792bbe974dfc70323b6566e) might have an old bundler, which vendors a thor version prior to 1.2.0 with this fix. The failure seems to imply it's using bundler < 2.3.1 where this was fixed downstream.

Thanks for sharing these observations, @chadlwilson

I'm working on updates to the relevant Docker and Bazel infrastructure which are not all contained in this repository.

@chadlwilson
Copy link
Contributor

Is there any hope of getting this Ruby 3.4 support backported to a v3.x version of this gem?

Is there a reason updating to 4.x is not a better solution? We don't usually make back ports to releases that old unless there is a security issue, and 3.x is on track to be dropped from support altogether at the end of Q1. See https://protobuf.dev/support/version-support/#ruby

I'm not the original one to ask in this thread, but it'd be nice for me too. I may personally be an unusual case, but I have had a persistent regression/change in behaviour in grpc 1.65+ (specifically on Ruby 3.3 / Ubuntu) that I have not had the ability/time as a maintainer and non-expert to track down or solve, which has kept me at grpc 1.64 for a ruby-based OSS testing tool I maintain.

Since there were breaking changes in protobuf ruby 4.26.x they require grpc 1.65 (since grpc also dont backport such changes far). While this grpc problem is unlikely a protobuf problem, that leaves me (and users of this tool) stuck on protobuf 3.25.x for a while due to the grpc problem.

Probably not the right place for the discussion, but it does feel a little bit hardcore to be dropping support within a year or so after the breaking changes that came for many language eocsystems with v26, especially when things like grpc only added v26 support several months later (Jun 29 2024).

Arguing my own counterpoint, we're only talking about pre-compiled gems here, and I suppose the truly enthusiastic can compile from source, so perhaps not the end of the world from your perspective.

@bquorning
Copy link

I have a situation similar to @chadlwilson. Upgrading past protoc v26 has proven to be a bit problematic (but something we will attempt to do soon-ish). On the other hand only a couple of gems (google-protobuf and grpc) are currently blocking us from upgrading to Ruby 3.4.

@JasonLunn
Copy link
Contributor Author

Is there any hope of getting this Ruby 3.4 support backported to a v3.x version of this gem?

Is there a reason updating to 4.x is not a better solution? We don't usually make back ports to releases that old unless there is a security issue, and 3.x is on track to be dropped from support altogether at the end of Q1. See https://protobuf.dev/support/version-support/#ruby

I'm not the original one to ask in this thread, but it'd be nice for me too. I may personally be an unusual case, but I have had a persistent regression/change in behaviour in grpc 1.65+ (specifically on Ruby 3.3 / Ubuntu) that I have not had the ability/time as a maintainer and non-expert to track down or solve, which has kept me at grpc 1.64 for a ruby-based OSS testing tool I maintain.

Since there were breaking changes in protobuf ruby 4.26.x they require grpc 1.65 (since grpc also dont backport such changes far). While this grpc problem is unlikely a protobuf problem, that leaves me (and users of this tool) stuck on protobuf 3.25.x for a while due to the grpc problem.

Probably not the right place for the discussion, but it does feel a little bit hardcore to be dropping support within a year or so after the breaking changes that came for many language eocsystems with v26, especially when things like grpc only added v26 support several months later (Jun 29 2024).

Arguing my own counterpoint, we're only talking about pre-compiled gems here, and I suppose the truly enthusiastic can compile from source, so perhaps not the end of the world from your perspective.

Is there an open issue on grpc Ruby I can take a look at?

@JasonLunn
Copy link
Contributor Author

I have a situation similar to @chadlwilson. Upgrading past protoc v26 has proven to be a bit problematic (but something we will attempt to do soon-ish). On the other hand only a couple of gems (google-protobuf and grpc) are currently blocking us from upgrading to Ruby 3.4.

Can you open a new issue to provide details on what has been problematic?

@chadlwilson
Copy link
Contributor

Is there an open issue on grpc Ruby I can take a look at?

Unfortunately it reliably fails in a more complex functional regression test that I haven't had the time to replicate locally in a place easy to debug, to thus isolate and even rule out a problem on my side.

In a GitHub search I cannot find other folks limiting grpc versions in their gemfiles/gemspecs so it makes me think it's exposing some earlier problem in our code or the test. Unfortunately it's a project I inherited maintenance of in my spare time, and so I probably dont have the depth required on hand without investing a lot of time.

Again, none of that is your problem - definitely a skill/time issue on my part 😅 just illustrating some of the challenges here with the version compatibilities and the sheer rate of change in this area of the ruby ecosystem, esp with native extensions.

@JasonLunn JasonLunn requested review from mkruskal-google and removed request for ericsalo January 10, 2025 21:44
@JasonLunn JasonLunn added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🅰️ safe for tests Mark a commit as safe to run presubmits over ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants