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

Allow pre-compiled binaries for ruby 3.1.0 #9566

Merged
merged 15 commits into from Mar 17, 2022

Conversation

Maaarcocr
Copy link
Contributor

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 rather x64-mingw-ucrt as explained here

Also x64-mingw-ucrt only supports 3.1+ whereas x64-mingw32 only supports anything up to 3.0 so a little if statement is needed.

@elharo elharo requested a review from haberman March 2, 2022 12:34
@elharo elharo added the ruby label Mar 2, 2022
@Maaarcocr
Copy link
Contributor Author

Maaarcocr commented Mar 2, 2022

@elharo (or anyone else who can) I saw you added the ruby label, could you please also add release notes: yes as a label? (I think it makes sense to add the PR description to the next release notes, as it would let people now that this package now ships with 3.1 compatible compiled shims).

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'

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?

Copy link
Contributor Author

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)

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.

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"

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?

Copy link
Contributor Author

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)

@Maaarcocr Maaarcocr changed the title Allow pre-compiled binaries for ruby 3.1.1 Allow pre-compiled binaries for ruby 3.1.0 Mar 2, 2022
@Maaarcocr
Copy link
Contributor Author

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.

@rochlefebvre
Copy link

Looking at the logs for MacOSRuby Release I see this bit, toward the end:

installing bundled gems:            /Users/kbuilder/.rake-compiler/ruby/x86_64-darwin/ruby-3.1.0/lib/ruby/gems/3.1.0
                                    minitest 5.15.0
                                    power_assert 2.0.1
                                    rake 13.0.6
                                    test-unit 3.5.3
                                    rexml 3.2.5
                                    rss 0.2.9
                                    net-ftp 0.1.3
                                    net-imap 0.2.2
                                    net-pop 0.1.1
                                    net-smtp 0.3.1
                                    matrix 0.4.2
                                    prime 0.1.2
                                    rbs 2.0.0
Building native extensions. This could take a while...
/Users/kbuilder/.rake-compiler/sources/ruby-3.1.0/lib/rubygems/ext/builder.rb:95:in `run': ERROR: Failed to build gem native extension. (Gem::Ext::BuildError)

    current directory: /Users/kbuilder/.rake-compiler/ruby/x86_64-darwin/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/rbs-2.0.0/ext/rbs_extension
/Users/kbuilder/.rvm/rubies/ruby-2.7.0/bin/ruby --disable\\=gems -I/Users/kbuilder/.rake-compiler/builds/x86_64-darwin/ruby-3.1.0 -I /Users/kbuilder/.rake-compiler/sources/ruby-3.1.0/lib -r ./siteconf20220302-86972-osxhy4.rb extconf.rb
/Users/kbuilder/.rake-compiler/builds/x86_64-darwin/ruby-3.1.0/rbconfig.rb:13:in `<module:RbConfig>': ruby lib version (3.1.0) doesn't match executable version (2.7.0) (RuntimeError)
	from /Users/kbuilder/.rake-compiler/builds/x86_64-darwin/ruby-3.1.0/rbconfig.rb:11:in `<top (required)>'
	from /Users/kbuilder/.rake-compiler/ruby/x86_64-darwin/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/rbs-2.0.0/ext/rbs_extension/siteconf20220302-86972-osxhy4.rb:1:in `require'
	from /Users/kbuilder/.rake-compiler/ruby/x86_64-darwin/ruby-3.1.0/lib/ruby/gems/3.1.0/gems/rbs-2.0.0/ext/rbs_extension/siteconf20220302-86972-osxhy4.rb:1:in `<top (required)>'
	from extconf.rb:in `require'

extconf failed, exit code 1

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?

@Maaarcocr
Copy link
Contributor Author

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

@Maaarcocr
Copy link
Contributor Author

I think that maybe I was overthinking the ucrt changes. I simplified the code to get rid of any logic specific to that and also made sure to run the 3.1 build for macos under ruby 3.1

@acozzette
Copy link
Member

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.

@acozzette
Copy link
Member

@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!

@acozzette acozzette self-requested a review March 5, 2022 16:22
Copy link
Member

@acozzette acozzette left a 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.

@rochlefebvre
Copy link

In the meantime, I was trying to figure out how to fix the linux release build, as maven was failing to run. I made a pr to rake-compiler-dock, as their current jruby docker image contains a version of maven that does not work with the version of jdk they use. rake-compiler/rake-compiler-dock#64

Hopefully I can get this merged soon and then I'll probably have to update this PR to use the new version.

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.

@Maaarcocr
Copy link
Contributor Author

Maaarcocr commented Mar 7, 2022

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)

@haberman
Copy link
Member

haberman commented Mar 8, 2022

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).

Is this kokoro/linux/dockerfile/test/ruby/Dockerfile?

I pulled your branch and tried running kokoro/linux/dockerfile/push_testing_images.sh, which usually updates our Docker images, but it didn't affect this one.

@haberman
Copy link
Member

haberman commented Mar 8, 2022

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 ).

[ERROR] Error executing Maven.
[ERROR] java.lang.IllegalStateException: Unable to load cache item
[ERROR] Caused by: Unable to load cache item
[ERROR] Caused by: Could not initialize class com.google.inject.internal.cglib.core.$MethodWrapper
rake aborted!
Command failed with status (1): [docker run -v /tmpfs/src/github/protobuf:/tmpfs/src/github/protobuf -e UID\=1000 -e GID\=1000 -e USER\=kbuilder -e GROUP\=kbuilder -e GEM_PRIVATE_KEY_PASSPHRASE -e ftp_proxy -e http_proxy -e https_proxy -e RCD_HOST_RUBY_PLATFORM\=x86_64-linux -e RCD_HOST_RUBY_VERSION\=2.4.1 -e RCD_IMAGE\=larskanis/rake-compiler-dock-jruby:1.2.0 -w /tmpfs/src/github/protobuf --rm -i larskanis/rake-compiler-dock-jruby:1.2.0 runas sigfw bash -c \ \ \ \ \ \ sudo\ apt-get\ install\ maven\ -y\ \&\&\ \ \ \ \ \ \ cd\ java\ \&\&\ mvn\ install\ -Dmaven.test.skip\=true\ \&\&\ cd\ ../ruby\ \&\&\ \ \ \ \ \ \ bundle\ \&\&\ \ \ \ \ \ \ IN_DOCKER\=true\ rake\ compile\ gem'
']
/usr/local/rvm/gems/ruby-2.4.1/bundler/gems/rake-compiler-dock-cd2afa24deca/lib/rake_compiler_dock/starter.rb:97:in `block in exec'
/usr/local/rvm/gems/ruby-2.4.1/bundler/gems/rake-compiler-dock-cd2afa24deca/lib/rake_compiler_dock/starter.rb:47:in `each'
/usr/local/rvm/gems/ruby-2.4.1/bundler/gems/rake-compiler-dock-cd2afa24deca/lib/rake_compiler_dock/starter.rb:47:in `exec'
/usr/local/rvm/gems/ruby-2.4.1/bundler/gems/rake-compiler-dock-cd2afa24deca/lib/rake_compiler_dock/starter.rb:14:in `sh'
/usr/local/rvm/gems/ruby-2.4.1/bundler/gems/rake-compiler-dock-cd2afa24deca/lib/rake_compiler_dock.rb:46:in `sh'
/tmpfs/src/github/protobuf/ruby/Rakefile:116:in `block in <top (required)>'
/usr/local/rvm/gems/ruby-2.4.1/gems/rake-13.0.6/exe/rake:27:in `<top (required)>'
/usr/local/rvm/gems/ruby-2.4.1/bin/ruby_executable_hooks:15:in `eval'
/usr/local/rvm/gems/ruby-2.4.1/bin/ruby_executable_hooks:15:in `<main>'
Tasks: TOP => gem:native => gem:java
(See full trace by running task with --trace)

This seems unrelated to this PR? I'll kick it one more time in case the failure was spurious.

@acozzette
Copy link
Member

@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.

@Maaarcocr
Copy link
Contributor Author

@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.

@acozzette
Copy link
Member

The codespell error is unrelated and I went ahead and fixed it separately.

@haberman haberman merged commit abdfd09 into protocolbuffers:master Mar 17, 2022
@haberman
Copy link
Member

We should cherry-pick this into the 3.20.x branch so it makes it into the 3.20.0 release.

haberman pushed a commit to haberman/protobuf that referenced this pull request Mar 21, 2022
* 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
haberman added a commit that referenced this pull request Mar 21, 2022
* 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>
@perezd
Copy link
Contributor

perezd commented Apr 20, 2022

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?

@haberman
Copy link
Member

Reference: #9804

Have arm64 binaries on macOS previously worked, and this PR broke them?

@johnnyshields
Copy link
Contributor

johnnyshields commented May 13, 2022

I have the correct way (I think) of doing the mingw-ucrt here: #9963. Please kindly merge this so we can have Ruby 3.1 on Windows, thanks!

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

Successfully merging this pull request may close these issues.

Add Ruby 3.1 compability
8 participants