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 rake runtime dependency for ruby #15203

Closed
wants to merge 3 commits into from
Closed

Add rake runtime dependency for ruby #15203

wants to merge 3 commits into from

Conversation

ntkme
Copy link
Contributor

@ntkme ntkme commented Dec 27, 2023

We have a rake based extension ext/google/protobuf_c/Rakefile (added since 3.25.0), which depends on rake. However, when gem is installed by bundle command, rake is not guaranteed to be activated by bundle (e.g. when BUNDLE_PATH is not default and rake is not already installed in BUNDLE_PATH). Therefore, we have to explicit declare a runtime dependency on rake for the extension, or otherwise the installation will fail with can't find gem rake (>= 0.a) with executable rake (Gem::GemNotFoundException).

Here is a reproduction:

root@4d9cb6ba41b1:/build# cat Gemfile
source 'https://rubygems.org'
gem 'google-protobuf'


root@4d9cb6ba41b1:/build# bundle config --local path vendor/bundle


root@4d9cb6ba41b1:/build# bundle
Fetching gem metadata from https://rubygems.org/...........
Resolving dependencies...
Installing google-protobuf 3.25.1 with native extensions
Gem::Ext::BuildError: ERROR: Failed to build gem native extension.

    current directory: /build/vendor/bundle/ruby/3.3.0/gems/google-protobuf-3.25.1/ext/google/protobuf_c
rake RUBYARCHDIR\=/build/vendor/bundle/ruby/3.3.0/extensions/aarch64-linux/3.3.0/google-protobuf-3.25.1 RUBYLIBDIR\=/build/vendor/bundle/ruby/3.3.0/extensions/aarch64-linux/3.3.0/google-protobuf-3.25.1
/usr/local/lib/ruby/3.3.0/rubygems.rb:259:in `find_spec_for_exe': can't find gem rake (>= 0.a) with executable rake (Gem::GemNotFoundException)
	from /usr/local/lib/ruby/3.3.0/rubygems.rb:278:in `activate_bin_path'
	from /usr/local/bin/rake:25:in `<main>'

rake failed, exit code 1

Gem files will remain installed in /build/vendor/bundle/ruby/3.3.0/gems/google-protobuf-3.25.1 for inspection.
Results logged to /build/vendor/bundle/ruby/3.3.0/extensions/aarch64-linux/3.3.0/google-protobuf-3.25.1/gem_make.out

  /usr/local/lib/ruby/3.3.0/rubygems/ext/builder.rb:125:in `run'
  /usr/local/lib/ruby/3.3.0/rubygems/ext/rake_builder.rb:30:in `build'
  /usr/local/lib/ruby/3.3.0/rubygems/ext/builder.rb:193:in `build_extension'
  /usr/local/lib/ruby/3.3.0/rubygems/ext/builder.rb:227:in `block in build_extensions'
  /usr/local/lib/ruby/3.3.0/rubygems/ext/builder.rb:224:in `each'
  /usr/local/lib/ruby/3.3.0/rubygems/ext/builder.rb:224:in `build_extensions'
  /usr/local/lib/ruby/3.3.0/rubygems/installer.rb:852:in `build_extensions'
  /usr/local/lib/ruby/3.3.0/bundler/rubygems_gem_installer.rb:76:in `build_extensions'
  /usr/local/lib/ruby/3.3.0/bundler/rubygems_gem_installer.rb:28:in `install'
  /usr/local/lib/ruby/3.3.0/bundler/source/rubygems.rb:205:in `install'
  /usr/local/lib/ruby/3.3.0/bundler/installer/gem_installer.rb:54:in `install'
  /usr/local/lib/ruby/3.3.0/bundler/installer/gem_installer.rb:16:in `install_from_spec'
  /usr/local/lib/ruby/3.3.0/bundler/installer/parallel_installer.rb:132:in `do_install'
  /usr/local/lib/ruby/3.3.0/bundler/installer/parallel_installer.rb:123:in `block in worker_pool'
  /usr/local/lib/ruby/3.3.0/bundler/worker.rb:62:in `apply_func'
  /usr/local/lib/ruby/3.3.0/bundler/worker.rb:57:in `block in process_queue'
  <internal:kernel>:187:in `loop'
  /usr/local/lib/ruby/3.3.0/bundler/worker.rb:54:in `process_queue'
  /usr/local/lib/ruby/3.3.0/bundler/worker.rb:90:in `block (2 levels) in create_threads'

An error occurred while installing google-protobuf (3.25.1), and Bundler cannot continue.

In Gemfile:
  google-protobuf

@ntkme ntkme requested a review from a team as a code owner December 27, 2023 09:37
@ntkme ntkme requested review from ericsalo and removed request for a team December 27, 2023 09:37
@ntkme
Copy link
Contributor Author

ntkme commented Jan 2, 2024

@JasonLunn @haberman @ericsalo Would you mind take a look at this issue when you get a chance? Thank you.

@JasonLunn
Copy link
Contributor

@ntkme - does this issue reproduce for you under Ruby 3.2 or lower, or only under 3.3?

@ntkme
Copy link
Contributor Author

ntkme commented Jan 2, 2024

Ruby 3.2 and 3.3, but not super old 2.6, so likely something with modern versions of bundler. The key is bundle config --local path vendor/bundle which sets a different installation path than the system default, and rake is not already installed there. Plus, we have to be installing the platform ruby or java gem, as the prebuilt native gem does not need to run Rakefile.

@ntkme
Copy link
Contributor Author

ntkme commented Jan 2, 2024

Here is a script for reproduction:

# make sure google-protobuf is not already installed at system level
gem uninstall -a google-protobuf

# create an empty directory
mkdir test
cd test

# write a Gemfile
tee Gemfile <<'EOF'
source 'https://rubygems.org'
gem 'google-protobuf'
EOF

# configure a differen bundle installation path
bundle config --local path vendor/bundle

# note: only the platform ruby and platform java gem has this problem
# therefore setting force_ruby_platform for reliable reproduction on any platform
# for a platform that does not have prebuilt native gem (e.g. aarch64-linux) the issue would always happen 
bundle config --local force_ruby_platform true

# install and the issue will reproduce
bundle install

@ntkme
Copy link
Contributor Author

ntkme commented Jan 2, 2024

@JasonLunn ^ I provided a script that you can reproduce this reliably on any Ruby version.

@ntkme
Copy link
Contributor Author

ntkme commented Jan 2, 2024

Rubygems actually prints a warning about this when running gem build command:

https://github.com/rubygems/rubygems/blob/v3.2.0/lib/rubygems/specification_policy.rb#L458-L468

You have specified rake based extension, but rake is not added as dependency. It is recommended to add rake as a dependency in gemspec since there's no guarantee rake will be already installed.

@ntkme
Copy link
Contributor Author

ntkme commented Jan 9, 2024

@JasonLunn Is there anything else you need? I think more users are now hitting this issue. E.g. If you setup GitHub Actions using setup-ruby the first time (no cached gems), it's always going to fail.

https://talk.jekyllrb.com/t/build-error-at-setup-ruby-stage-of-build-and-deploy-on-actions/8782

@ntkme
Copy link
Contributor Author

ntkme commented Jan 9, 2024

cc @acozzette I hope we can get this addressed before v26 is released.

@acozzette acozzette added 🅰️ safe for tests Mark a commit as safe to run presubmits over and removed wait for user action labels Jan 12, 2024
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 12, 2024
BinToss added a commit to HaloSPV3/halospv3.github.io that referenced this pull request Jan 17, 2024
This is a holdover until google-protobuf merges protocolbuffers/protobuf#15203
Copy link
Contributor

@JasonLunn JasonLunn left a comment

Choose a reason for hiding this comment

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

I'm not sure that this is the right approach. I think we can all agree that rake is not needed at runtime, only install time, and this change would promote rake to a runtime requirement for protobuf for everyone.

This seems like a bundler issue rather than an issue with this gem, and workarounds appear to exist. gem install google-protobuf -i vendor/bundle --platform=ruby seems to work fine. The problem also goes away if the Gemfile includes gem 'rake', right?

@ntkme
Copy link
Contributor Author

ntkme commented Jan 17, 2024

@JasonLunn You cannot expect how user install this, and it’s certainly not a bug of bundler, because gem command explicitly says:

You have specified rake based extension, but rake is not added as dependency. It is recommended to add rake as a dependency in gemspec since there's no guarantee rake will be already installed.

Here is an example that it fails with setup-ruby GitHub action just with its default setting:
https://github.com/ntkme/ntkme.github.io/actions/runs/7549908267/job/20554705751

@ntkme
Copy link
Contributor Author

ntkme commented Jan 17, 2024

Of course this might not be the best fix, as we need rake as dependency only if we need to run rake based extensions, meaning it is only needed for platform ruby and platform java, as it can be prebuilt for native gems.

@JasonLunn A better fix would be not use rake tasks here and just rewrite the rake task as part of extconf.rb. Would you mind look into that, as “workaround” isn’t a fix.

@JasonLunn
Copy link
Contributor

@JasonLunn You cannot expect how user install this, and it’s certainly not a bug of bundler, because gem command explicitly says:

You have specified rake based extension, but rake is not added as dependency. It is recommended to add rake as a dependency in gemspec since there's no guarantee rake will be already installed.

Here is an example that it fails with setup-ruby GitHub action just with its default setting: https://github.com/ntkme/ntkme.github.io/actions/runs/7549908267/job/20554705751

The gem installation process works, though - it is the bundler based process that has issues. Or are you saying that gem install google-protobuf -i vendor/bundle --platform=ruby doesn't work for you?

@JasonLunn
Copy link
Contributor

Of course this might not be the best fix, as we need rake as dependency only if we need to run rake based extensions, meaning it is only needed for platform ruby and platform java, as it can be prebuilt for native gems.

@JasonLunn A better fix would be not use rake tasks here and just rewrite the rake task as part of extconf.rb. Would you mind look into that, as “workaround” isn’t a fix.

I am not sure what you're suggesting, as there is already an extconf.rb. But you're welcome to update this PR or open an alternative one that implements what you're suggesting.

Before you invest in that approach though - can you confirm whether adding rake to your project's Gemfile solves your issue?

@ntkme
Copy link
Contributor Author

ntkme commented Jan 17, 2024

I am not sure what you're suggesting, as there is already an extconf.rb.

My point is to rewrite what the rake task does in pure ruby and put it inside extconf.rb so that it will not have any rake dependency at anytime.

can you confirm whether adding rake to your project's Gemfile solves your issue?

Yes, it works as a workaround. My point is that we should not expect everyone hitting this issue to do the workaround, because it's not a fault of gem or bundler. It is a clear problem that protobuf has incorrect assumption that rake is always available, per warning message given by gem. I know it because I'm also a gem author and I maintain a gem that had exactly the same problem in the past, which has had rake added as runtime dependency only for the purpose of installing extension:

https://github.com/sass-contrib/sass-embedded-host-ruby/blob/906dc5e03950c5c257c9635e37952d6c082f3af5/sass-embedded.gemspec#L29-L35

@JasonLunn
Copy link
Contributor

Rubygems actually prints a warning about this when running gem build command:

https://github.com/rubygems/rubygems/blob/v3.2.0/lib/rubygems/specification_policy.rb#L458-L468

You have specified rake based extension, but rake is not added as dependency. It is recommended to add rake as a dependency in gemspec since there's no guarantee rake will be already installed.

I took a deeper look into this today, and can confirm that this error message is not triggered when using add_development_dependency as we do. It is only triggered if neither add_dependency nor add_development_dependency are present.

@ntkme
Copy link
Contributor Author

ntkme commented Jan 17, 2024

I took a deeper look into this today, and can confirm that this error message is not triggered when using add_development_dependency as we do. It is only triggered if neither add_dependency nor add_development_dependency are present.

That's likely an oversight of rubygems, but the intent and the meaning of the message is clear.

@ntkme
Copy link
Contributor Author

ntkme commented Jan 17, 2024

I've submitted a PR to rubygems to fix the issue that warning is not printed when rake is added as a development dependency.

@tmcw
Copy link

tmcw commented Jan 22, 2024

Not to nag (okay, this is nagging, sorry), but this is a downstream dependency of Jekyll and this bug breaks the ability to install Jekyll with the current version of Ruby and the current version of Bundler.

@ntkme
Copy link
Contributor Author

ntkme commented Jan 22, 2024

Given the discussion on the rubygems PR, it is pretty clear what the current limitation is, and that rake shall be added.

In fact, ffi-compiler's instruction also tells you to add rake as a dependency: https://github.com/ffi/ffi-compiler/blob/master/README.md#examplegemspec

I think we should just get this merged as rubygems behavior on rake extensions definitely will not change anytime soon. If that still bothers protobuf developers, the way out is basically to remove reliance on ffi-compiler and its rake tasks, or in other words, to revert the extension compilation process to how it's done before 3.25.

@acozzette acozzette added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 23, 2024
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 23, 2024
@acozzette acozzette added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 23, 2024
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 23, 2024
@acozzette acozzette added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 23, 2024
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Jan 23, 2024
@ntkme ntkme deleted the ruby-rake-dependency branch January 23, 2024 23:34
@fernandopasik
Copy link

@ntkme would this problem be fixed then when v26 is released?

@JasonLunn
Copy link
Contributor

4.26.0.rc.1 was released on the 25th.

@ntkme
Copy link
Contributor Author

ntkme commented Jan 28, 2024

I just tested 4.26.0.rc.1 platform ruby gem, and it worked as expected:

# cat Gemfile
source 'https://rubygems.org'
gem 'google-protobuf', '= 4.26.0.rc.1'

# cat Gemfile.lock 
GEM
  remote: https://rubygems.org/
  specs:
    google-protobuf (4.26.0.rc.1)
      rake (>= 13)
    rake (13.1.0)

PLATFORMS
  ruby

DEPENDENCIES
  google-protobuf (= 4.26.0.rc.1)

BUNDLED WITH
   2.3.26

# bundle
Fetching gem metadata from https://rubygems.org/..
Fetching rake 13.1.0
Installing rake 13.1.0
Fetching google-protobuf 4.26.0.rc.1
Using bundler 2.3.26
Installing google-protobuf 4.26.0.rc.1 with native extensions
Bundle complete! 1 Gemfile dependency, 3 gems now installed.
Bundled gems are installed into `./vendor/bundle`

jvtrigueros added a commit to jvtrigueros/blog.jvtrigueros.com that referenced this pull request Feb 10, 2024
This is happening because of a bug on Jekyll and latest Ruby:
protocolbuffers/protobuf#15203
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.

5 participants