Fix Gem::TSort handling on old system RubyGems and restore its CI coverage#9684
Open
hsbt wants to merge 5 commits into
Open
Fix Gem::TSort handling on old system RubyGems and restore its CI coverage#9684hsbt wants to merge 5 commits into
hsbt wants to merge 5 commits into
Conversation
RubyGems older than 3.6 has no rubygems/vendored_tsort, and its own Gem::TSort is often already loaded through rubygems/request_set by the time Bundler runs, e.g. from Gem.activate_bin_path in binstubs. The unconditional reassignment printed an already-initialized-constant warning to stderr, breaking specs that assert empty stderr against system RubyGems. The old vendored Gem::TSort is API-compatible, so keep it when present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Since RGV started defaulting to ".", the system-rubygems-bundler job has been silently testing the repo's RubyGems instead of the one shipped with each Ruby, because switch_rubygems is injected into every spec-spawned process. Add an explicit RGV=system mode and set it on the job's test step. The harness itself keeps running on the checked out RubyGems even in this mode. The flattened lib/ contains both Bundler and RubyGems, so it is unavoidably on the harness load path and would get mixed into an older RubyGems otherwise. Only processes spawned by specs boot system RubyGems untouched, which is the configuration under test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RubyGems 3.4 and 3.5 ship vendored tsort under its pre-3.6 name rubygems/tsort, so prefer that when rubygems/vendored_tsort is missing. Requiring the real tsort activated the tsort default gem, breaking the guarantee that bundler/setup activates no gems. This mirrors the fallback chain already used by vendored_net_http. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Bundler’s Gem::TSort loading behavior when running against older system RubyGems (pre-3.6), avoiding both constant redefinition warnings and unintended activation of the tsort default gem. It also restores CI coverage for that scenario by making the system RubyGems Bundler job explicitly run in a “system RubyGems” mode.
Changes:
- Update
lib/bundler/vendored_tsort.rbto keep an already-definedGem::TSortand preferrubygems/tsort(pre-3.6 name) before falling back to stdlibtsort. - Add an explicit
RGV=systemmode for specs so spec-spawned processes boot system RubyGems untouched, and configure the system-rubygems-bundler CI job to use it. - Document
RGVbehavior and the newRGV=systemmode in GETTING_STARTED.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/support/switch_rubygems.rb | Introduces RGV=system handling while keeping the harness on the checked-out RubyGems. |
| spec/support/helpers.rb | Adjusts subprocess RUBYOPT handling so spec children can boot system RubyGems when RGV=system. |
| lib/bundler/vendored_tsort.rb | Fixes fallback chain and avoids Gem::TSort constant redefinition / default-gem activation. |
| doc/GETTING_STARTED.md | Documents default RGV behavior and how to run with RGV=system. |
| .github/workflows/system-rubygems-bundler.yml | Ensures the CI job actually exercises system RubyGems via RGV=system. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Gem::Installer#remove_stale_plugins only exists since RubyGems 4.1, so installing with no_install_plugin set on an older system RubyGems raised NameError. Reimplement it in Bundler's installer, like generate_plugins already is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Under RGV=system the harness runs on the checked out RubyGems while spec-spawned processes run system RubyGems, so rubygems-tagged filters and expectations interpolating Gem::VERSION were checking the wrong version. Capture the system RubyGems version before switching and use it for the rubygems: exclusion filter, version-dependent expectations, and the global gem cache location. The cached git gemspec comparison now serializes with the RubyGems that wrote the file, since Specification#to_ruby output differs across versions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since #9647 made Bundler reuse RubyGems' vendored tsort, running Bundler on RubyGems older than 3.6 emitted
warning: already initialized constant Gem::TSort, because binstubs preload the old vendoredGem::TSortthroughrubygems/request_setviaGem.activate_bin_pathbefore Bundler's fallback assigns the constant. Falling back to the realtsorthad a second problem. It activated the tsort default gem, breaking the guarantee thatbundler/setupactivates no gems.lib/bundler/vendored_tsort.rbnow keeps an already definedGem::TSortand otherwise prefersrubygems/tsort, the pre-3.6 name of the same vendored copy shipped by RubyGems 3.4 and 3.5, mirroring the fallback chain ofvendored_net_http. This matches the fix already applied on therelease/4.0.16branch, where the system RubyGems job caught the warning as about 100 spec failures.Master CI missed all of this because the system-rubygems-bundler job silently stopped testing system RubyGems when
RGVstarted defaulting to., sinceswitch_rubygemsis injected into every spec-spawned process. This PR adds an explicitRGV=systemmode and sets it on the job's test step. In this mode processes spawned by specs boot system RubyGems untouched, which is the configuration under test, while the harness itself keeps running on the checked out RubyGems because the flattenedlib/contains both Bundler and RubyGems and is unavoidably on the harness load path.Verified locally with ruby 3.2.11 (RubyGems 3.4.19) and ruby 3.3.0 (RubyGems 3.5.22). A bundler gem built from this branch installs cleanly and
bundle installfrom a binstub prints no warnings,bundler/setupdoes not activate the tsort default gem, andRGV=system bin/rspecgenuinely runs spec children on system RubyGems, failing with the exact CI failure mode when thevendored_tsortfix is reverted and passing with it applied.