-
Notifications
You must be signed in to change notification settings - Fork 15.5k
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
Allow pre-compiled binaries for ruby 3.1.0 #9566
Conversation
@elharo (or anyone else who can) I saw you added the I cannot add any labels as I do not have the privileges to do so :/ |
ruby/Rakefile
Outdated
['x86-mingw32', 'x64-mingw32', 'x86_64-linux', 'x86-linux', 'x64-mingw-ucrt'].each do |plat| | ||
# x64-mingw-ucrt only supports 3.1+ whereas 'x64-mingw32' only supports up to 3.0 | ||
versions = if plat == 'x64-mingw-ucrt' | ||
'3.1.1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice that the other versions are minor versions with a .0 as the patch, e.g. 3.0.0, 2.7.0... Your Ruby 3.1 version string is written as 3.1.1 instead of 3.1.0.
Is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went for the latest version of ruby. I'm not sure if it would make any difference, I could use 3.1.0 if it also would support 3.1.1 (not sure about this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth a shot, if only for consistency.
ruby/google-protobuf.gemspec
Outdated
end | ||
s.test_files = ["tests/basic.rb", | ||
"tests/stress.rb", | ||
"tests/generated_code_test.rb"] | ||
s.required_ruby_version = '>= 2.3' | ||
s.add_development_dependency "rake-compiler", "~> 1.1.0" | ||
s.add_development_dependency "rake-compiler", "~> 1.2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rake-compiler does not have a 1.2.0 release yet: https://rubygems.org/gems/rake-compiler. Did you mean to move up to the latest release, 1.1.9?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I may have just mistakenly changed this. Fixed it back to what it was, as it also should support the latest (not that it's needed I think)
I did not noticed that more work would be needed to setup CI for both testing on ruby 31 and releasing. I've added the changes to fix this in my latest PR. |
Looking at the logs for MacOSRuby Release I see this bit, toward the end:
While building native extensions for gems bundled in Ruby 3.1, it complains that we're running a 2.7 interpreter with a 3.1 library. I see that ruby_build_environment.sh defaults to Ruby 2.7 in a few spots. I'm unsure if that is related somehow? |
yeah I'm trying to understand how we build 3.0.0 and what's missing to get 3.1.0 working |
I think that maybe I was overthinking the |
Sorry for the confusion on this, there is a Google-internal config we need to add before the 3.1 test runs are fully enabled. I can take care of this after the PR is merged. |
@Maaarcocr I rebuilt the Docker image, so that should fix most or all of the failing test runs. The change looks good to me overall but I want to look more closely at the changes to ruby_build_environment.sh on Monday. Thank you for all your work on this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me but I guess we will still need to wait for your rake-compiler-dock change to be released.
I'm very happy to see that your rake-compiler-dock fix was accepted and merged 🎉 . While you wait for confirmation on a new release, may I suggest you temporarily depend on rake-compiler-dock's head? Something like: # ruby/Gemfile
group :development do
gem 'rake-compiler-dock', git: 'https://github.com/rake-compiler/rake-compiler-dock.git'
end We'll see if this update fixes the Linux Ruby checks or if other changes are needed. |
I tried this, but the issue is that the docker image that is pulled still does not show the changes I made (I guess someone has to push a new image version). So we have to wait on a new release (of at least the docker image) |
Is this I pulled your branch and tried running |
When I look at the latest test runs, everything seems to have passed except for "Linux Ruby Release ", which failed in a JRuby step (attn: @JasonLunn ).
This seems unrelated to this PR? I'll kick it one more time in case the failure was spurious. |
@haberman I believe that last failure is supposed to be fixed by @Maaarcocr's change to rake-compiler-dock, but it sounds like we need to wait for the fix to be released. |
the changes just got released. hopefully this will fix all CIs. |
The codespell error is unrelated and I went ahead and fixed it separately. |
We should cherry-pick this into the 3.20.x branch so it makes it into the 3.20.0 release. |
* Allow pre-compiled binaries for ruby 3.1.1 * add comment * fix build and use ruby 3.1.0 * add ruby31 to build CI for tests and release * trying to fix ci * install ruby 3.1.0 in ruby_build_environment.sh * use head for rvm to install 3.1.0 * just install master version of rvm in prepare_build_macos_rc * force install of master rvm in ruby_build_environment.sh * Use coroutine=universal when compiling ruby31 * use ucontext * fix filename * fix coroutine name * use git head for rake-compiler-dock * use newest rake-compiler-dock version
* Allow pre-compiled binaries for ruby 3.1.0 (#9566) * Allow pre-compiled binaries for ruby 3.1.1 * add comment * fix build and use ruby 3.1.0 * add ruby31 to build CI for tests and release * trying to fix ci * install ruby 3.1.0 in ruby_build_environment.sh * use head for rvm to install 3.1.0 * just install master version of rvm in prepare_build_macos_rc * force install of master rvm in ruby_build_environment.sh * Use coroutine=universal when compiling ruby31 * use ucontext * fix filename * fix coroutine name * use git head for rake-compiler-dock * use newest rake-compiler-dock version * Updated CHANGES.txt for Ruby changes. * Fixed Ruby 3.1 tests by marking intersect? as unimplemented. (#9645) * Fixed Ruby 3.1 tests by marking intersect? as unimplemented. * Updated compatibility tests. Co-authored-by: Marco Concetto Rudilosso <marcoconcettorudilosso@gmail.com>
This change is creating fake arm64 ruby gems that are not actually arm64 (reporting as x86_64 Mach-O headers). What should we do about this given our pending release for 21.0? |
Reference: #9804 Have arm64 binaries on macOS previously worked, and this PR broke them? |
I have the correct way (I think) of doing the |
Why?
Fixes #9364
What are the changes?
I added 3.1.1 as a RUBY_CC_VERSION. I had to upgrade rake-compiler-dock to 1.2.0 as this is the only version of this gem that supports compilation for ruby 3.1+
Furthermore, I had to add a bit of logic (and a new platform this compiles to) as since RubyInstaller 3.1 x64 Windows is not
x64-mingw32
but ratherx64-mingw-ucrt
as explained hereAlso
x64-mingw-ucrt
only supports 3.1+ whereasx64-mingw32
only supports anything up to 3.0 so a little if statement is needed.