Don't undo require decorations made by other gems

[deivid-rodriguez]

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

If a gem decorates require, depending on the order things are loaded, Bundler may end up not only removing RubyGems require decorations, but also those extra decorations.

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

My fix is, instead of overwriting require with the original version, let RubyGems require be aware of Bundler having taken over, and delegate to standard require in that case.

Fixes #5263.

Make sure the following tasks are checked

• Describe the problem / feature
• Write tests for features and bug fixes
• Write code to solve the problem
• Make sure you follow the current code style and write meaningful commit messages without tags

[simi]

The change itself looks ok to me. If I understand it well, instead of defining custom require back and forth, it is currently no-op in RubyGems if Bundler is loaded thanks to the new configuration being set super early. Btw. what about standalone? Wouldn't it benefit from this as well?

But I do struggle to follow the testing changes. Part of the test/rubygems/test_gem.rb was moved into test/rubygems/bundler_test_gem.rb to not be part of rake test anymore and t.libs << "bundler/lib" was removed as well. But at the same time, this bundler_test_gem.rb file is run in rake test:isolated (explicitly) and there is no problem to keep bundler/lib in lib path for all isolated test file runs aka sh Gem.ruby, "-Ilib:test:bundler/lib", file.

I did quick testing locally, and tests are passing even when file is renamed back to be under test_ prefix, but there are some warnings (caused probably by double bundler require).

@deivid-rodriguez would you mind to explain a little why this change is needed? Is rake test:isolated the only way for now to run all RubyGems tests? Would it make sense to mention that in CONTRIBUTING.md?

[deivid-rodriguez]

Good point about standalone, I'll update to include it!

The change is needed to play nicer with gems that decorate require, you can see the full story in the issue this is closing, and the attached spec as an example.

I should've commented on test changes before setting this as ready. The problem is that RubyGems test suite, as it works now, loads bundler. This is problematic since Bundler monkeypatches RubyGems, so we end up testing Bundler, not RubyGems. See for example, d20b4d1, and that PR in general where I tried to improve the situation.

In this case, it's biting us again because during RubyGems test execution, Bundler is loaded and reverts the require monkeypatch and then depending on order, the tests checking the "automatic activation of gems on require" start failing since the behavior is no longer there. You can see the failures without test changes here: https://github.com/rubygems/rubygems/actions/runs/4007644021/jobs/6880765362.

The solution is exactly what you observed: ensure that our main suite does not load Bundler at all by moving the specs that load Bundler to a separate file that does not get run by default. We still get the functionality that loads Bundler tested through rake test:isolated where the side effects of it does not cause any problems because they run... in isolation. I think this is a good change regardless of this patch, and I can propose it as a separate PR if necessary.

I'm not sure why this didn't bite us with the previous approach, but I think this kind of isolation is helpful regardless so I'd rather not look deeper into that.

I do not think it's worth mentioning this in CONTRIBUTING because these few tests exercise a very particular functionality (auto require of bundler/setup when RUBYGEMS_GEMDEPS env variable is set) that hardly ever changes, and doesn't usually break when changing RubyGems. But I can mention it if you feel strongly. We also have many CI checks not run by default with rake test that could be mentioned but in general I feel it would be distracting and not helpful for most cases.

[deivid-rodriguez]

I added the change also to standalone! I believe this is ready, but I believe this is technically a new feature in RubyGems and a bug fix in Bundler that leverages the new feature. So let me split the PR so that each one get their proper entry in the respective changelogs.

[simi]

Feel free to merge once ready.

[deivid-rodriguez]

I extracted #6319 and will leave this one for the fix in Bundler.