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

Fix crash caused by RubyGems require gem activation logic running before Bundler can properly register its own monkeypatches #7647

Merged
merged 2 commits into from May 13, 2024

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented May 10, 2024

What was the end-user or developer problem that led to this PR?

In #7603, we added a monkeypatch to Gem::StubSpecification#data so that when we fail to read the underlying gemspec due to not being readable, we can print a friendly error message. In order to do that, we reused a Bundler helper that knows how to rescue filesystem errors and re-raise them in way that Bundler knows how to handle in a friendly way.

This usage of Bundler::SharedHelpers was added without an explicit require because Bundler is supposed to setup an autoload for Bundler::SharedHelpers. However, the autoload does not seem to be triggering under some situations, such as when loading our own Rakefile. But that also does not happen always, but only when you have two different versions of psych installed in your system.

This is why:

  • Our Rakefile first loads rdoc/task which activates rdoc, which depends on psych >= 4.0.
  • Because there are multiple matching psych versions in the system, RubyGems require does not automatically activate psych but adds it to Gem::Specification.unresolved_deps and then further require calls will check in the potential paths for those unresolved_deps to see if any unresolved dependency can be activated. This enables logic that ends up using Gem::StubSpecification#data.
  • Then bundler/gem_tasks is required that eventually requires bundler/rubygems_ext and enables the monkeypatch to Gem::StubSpecification#data.
  • Then, before bundler/gem_tasks eventually loads bundler.rb and sets up an autoload for Bundler::SharedHelpers, other require calls are run that eventually use Gem::Specification#data for gem activation without Bundler::SharedHelpers being available yet.

What is your fix for the problem, implemented in this PR?

The fix to require "bundler/shared_helpers" before using it. However, that's not enough because that require itself ends up running into the same error. So on top of that, I made sure no standard require calls are used during loading of bundler/shared_helpers.

Closes #7638.

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/fix-missing-constant-error branch from 1b7fedc to 32e2be2 Compare May 10, 2024 13:40
We should make sure Bundler does not trigger RubyGems require logic for
gem activation until it had the chance to register its own monkeypatches
to RubyGems.
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/fix-missing-constant-error branch from 32e2be2 to fbd2ff8 Compare May 10, 2024 14:39
@deivid-rodriguez deivid-rodriguez changed the title Fix Rakefile failing to load shared helpers Fix crash caused by RubyGems require gem activation logic running before Bundler can properly register its own monkeypatches May 10, 2024
@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review May 10, 2024 16:21
@martinemde
Copy link
Member

Thanks for the thorough solution! I'll close my quick patch.

@deivid-rodriguez deivid-rodriguez merged commit 2198e12 into master May 13, 2024
83 checks passed
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/fix-missing-constant-error branch May 13, 2024 09:08
deivid-rodriguez added a commit that referenced this pull request May 16, 2024
…onstant-error

Fix crash caused by RubyGems `require` gem activation logic running before Bundler can properly register its own monkeypatches

(cherry picked from commit 2198e12)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants