Conversation
Split the download and install process of a gem (cherry picked from commit bcc4469)
Fix gem which command test isolation (cherry picked from commit 8b405fa)
Introduce a priority queue (cherry picked from commit 115228e)
fix: include owner role in `gem owner` (cherry picked from commit 0bcfe4b)
Retry git fetch without --depth for dumb HTTP transport (cherry picked from commit cb6fbe3)
…koff Add exponential backoff to bundler retries (cherry picked from commit fd31044)
fix: Ensure trailing slash is added to source URIs added via gem sources (cherry picked from commit 19debfb)
Prevent tests from modifying user's global git config (cherry picked from commit 1be513c)
Refactor Bundler tests to invoke RubyGems API directly (cherry picked from commit 9d755be)
Normalize the number of workers when performing parallel operations (cherry picked from commit be5febe)
Check the git version only **once** per `bundle install` (cherry picked from commit e3685c9)
There was a problem hiding this comment.
Pull request overview
This PR prepares the RubyGems/Bundler 4.0.9 release by bumping versions, updating release lockfiles/changelogs, and rolling up several Bundler/RubyGems behavioral and spec-suite changes (parallel download+install, priority queue, checksum locking, retry backoff, git behavior, and gem sources normalization).
Changes:
- Bump RubyGems/Bundler versions to 4.0.9 and update associated changelogs and lockfiles (including Bundler checksum entries).
- Update Bundler installation pipeline (separate download/install, prioritize native-extension installs, normalize parallelism worker counts, add retry exponential backoff, memoize git version).
- Improve RubyGems CLI behavior/tests (
gem sourcestrailing slash normalization;gem ownerdisplays role; fix test isolation).
Reviewed changes
Copilot reviewed 40 out of 49 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tool/bundler/vendor_gems.rb.lock | Update Bundler version/checksum in vendored lock. |
| tool/bundler/test_gems.rb.lock | Update Bundler version/checksum in test lock. |
| tool/bundler/standard_gems.rb.lock | Update Bundler checksum and bundled-with version. |
| tool/bundler/rubocop_gems.rb.lock | Update Bundler checksum and bundled-with version. |
| tool/bundler/release_gems.rb.lock | Update Bundler checksum and bundled-with version. |
| tool/bundler/lint_gems.rb.lock | Update Bundler checksum and bundled-with version. |
| tool/bundler/dev_gems.rb.lock | Update Bundler checksum and bundled-with version. |
| test/rubygems/test_gem_commands_which_command.rb | Remove “fails in isolation” TODO now that isolation is fixed elsewhere. |
| test/rubygems/test_gem_commands_sources_command.rb | Add/adjust tests for trailing-slash normalization and remove/add/prepend behaviors. |
| test/rubygems/test_gem_commands_owner_command.rb | Update test fixture and expectations to include owner roles. |
| lib/rubygems/commands/sources_command.rb | Normalize source URIs (trailing slash) and refactor add/append/prepend/remove source construction. |
| lib/rubygems/commands/owner_command.rb | Display role when listing gem owners. |
| lib/rubygems.rb | Bump RubyGems VERSION and update API documentation link. |
| bundler/spec/support/windows_tag_group.rb | Add new parallel installer spec to a Windows runner group. |
| bundler/spec/support/helpers.rb | Reset GitProxy memoization; replace shelling-out gem commands with in-process RubyGems APIs; add simulated platform helper. |
| bundler/spec/support/filters.rb | Add git: version filtering for specs. |
| bundler/spec/support/checksums.rb | Allow checksum helper to read gems from configurable folder (e.g. cache/). |
| bundler/spec/support/builders.rb | Use Gem::Package.build in-process instead of gem build subprocess. |
| bundler/spec/spec_helper.rb | Disable retry delays in tests; isolate git config for tests. |
| bundler/spec/realworld/fixtures/warbler/Gemfile.lock | Bump bundled-with version to 4.0.9. |
| bundler/spec/realworld/fixtures/tapioca/Gemfile.lock | Bump bundled-with version to 4.0.9. |
| bundler/spec/install/gems/compact_index_spec.rb | Switch uninstall to new in-process uninstall helper. |
| bundler/spec/commands/update_spec.rb | Add Bundler checksum expectations for lockfile checksum section tests. |
| bundler/spec/commands/clean_spec.rb | Switch gem list to in-process list helper. |
| bundler/spec/commands/check_spec.rb | Switch uninstall to in-process uninstall helper. |
| bundler/spec/bundler/worker_spec.rb | Add coverage for priority-queue ordering. |
| bundler/spec/bundler/source/git/git_proxy_spec.rb | Update tests for git version memoization and dumb HTTP fetch/clone retry behavior. |
| bundler/spec/bundler/retry_spec.rb | Add specs for exponential backoff behavior and disabling delays. |
| bundler/spec/bundler/installer/parallel_installer_spec.rb | New spec asserting priority enqueueing for native extensions. |
| bundler/spec/bundler/gem_helper_spec.rb | Switch gem list to in-process list helper. |
| bundler/spec/bundler/env_spec.rb | Update git version stubbing to match Open3-based implementation. |
| bundler/lib/bundler/worker.rb | Add a priority queue to Worker and priority-aware dequeue logic. |
| bundler/lib/bundler/version.rb | Bump Bundler version to 4.0.9. |
| bundler/lib/bundler/source/rubygems.rb | Split out download from install; cache installer objects; add synchronization around requiring installer. |
| bundler/lib/bundler/source/metadata.rb | Add checksum_store for metadata source to support Bundler self checksum locking. |
| bundler/lib/bundler/source/git/git_proxy.rb | Memoize git version checks at class level; implement fetch/clone retry without --depth for dumb HTTP. |
| bundler/lib/bundler/source.rb | Add base download hook for sources. |
| bundler/lib/bundler/settings.rb | Add installation_parallelization helper based on jobs/CPU count. |
| bundler/lib/bundler/self_manager.rb | Ensure Bundler self-install does an explicit download step before install. |
| bundler/lib/bundler/retry.rb | Add exponential backoff + jitter to retries; make base delay configurable/defaulted. |
| bundler/lib/bundler/plugin/installer.rb | Add explicit download step before installing plugin specs. |
| bundler/lib/bundler/plugin/api/source.rb | Add plugin source download hook. |
| bundler/lib/bundler/man/bundle-config.1.ronn | Update jobs documentation to mention download + install parallelism. |
| bundler/lib/bundler/man/bundle-config.1 | Regenerate manpage content for jobs wording change. |
| bundler/lib/bundler/lockfile_parser.rb | Track a metadata source to store/parse Bundler’s own checksum entry. |
| bundler/lib/bundler/lockfile_generator.rb | Append Bundler’s checksum entry to CHECKSUMS section when applicable. |
| bundler/lib/bundler/installer/parallel_installer.rb | Refactor to download then install; add installability checks and priority enqueueing. |
| bundler/lib/bundler/installer/gem_installer.rb | Add download wrapper method for sources. |
| bundler/lib/bundler/installer.rb | Use Bundler.settings.installation_parallelization for worker counts. |
| bundler/lib/bundler/fetcher/gem_remote_fetcher.rb | Set remote fetcher pool size based on parallelization setting. |
| bundler/lib/bundler/definition.rb | Merge locked metadata source checksums; use configurable worker count for git preloading. |
| bundler/lib/bundler/cli/pristine.rb | Use settings-based parallelization for pristine jobs. |
| bundler/CHANGELOG.md | Add Bundler 4.0.9 changelog entry. |
| CHANGELOG.md | Add RubyGems 4.0.9 changelog entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rescue Gem::Security::Exception => e | ||
| raise SecurityError, | ||
| "The gem #{File.basename(path, ".gem")} can't be installed because " \ | ||
| "The gem #{installer.gem} can't be installed because " \ | ||
| "the security policy didn't allow it, with the message: #{e.message}" |
There was a problem hiding this comment.
This security error message now interpolates installer.gem, which is a full file path. The previous behavior used just the gem name, avoiding leaking local filesystem paths into user-facing errors. Consider switching back to a basename (e.g. derived from spec or the gem file name) for this message.
| remote.cache_slug | ||
| end | ||
|
|
||
| # We are using a mutex to reaed and write from/to the hash. |
There was a problem hiding this comment.
Typo in comment: "reaed" should be "read".
| # We are using a mutex to reaed and write from/to the hash. | |
| # We are using a mutex to read and write from/to the hash. |
| def append_source(source_uri) # :nodoc: | ||
| check_rubygems_https source_uri | ||
|
|
||
| source = Gem::Source.new source_uri | ||
|
|
||
| check_typo_squatting(source) | ||
| source = build_new_source(source_uri) | ||
| source_uri = source.uri.to_s | ||
|
|
There was a problem hiding this comment.
Same issue as add_source: build_new_source happens before the begin/rescue, so invalid URIs can raise and bypass the intended error handling. Move build_new_source into the begin block (or add an outer rescue) so gem sources --append reports a clean error and terminates consistently.
| def remove_source(source_uri) # :nodoc: | ||
| source = Gem::Source.new source_uri | ||
| source = build_source(source_uri) | ||
| source_uri = source.uri.to_s | ||
|
|
There was a problem hiding this comment.
remove_source now calls build_source (URI parsing + normalization) without any rescue/terminate handling. For invalid URIs, this can raise and crash the command instead of printing "... is not a URI" and exiting with status 1. Consider wrapping this in the same rescue Gem::URI::Error, ArgumentError pattern used by add/append/prepend.
| # installation of a gem. | ||
| # | ||
| # @return [Boolean] Whether the download of the gem succeeded. | ||
| def download(spec, opts); end |
There was a problem hiding this comment.
The base implementation of download requires two positional arguments (spec, opts). New call sites (e.g. plugin installer/self-manager) invoke download(spec) with a single argument, which will raise ArgumentError for plugin sources that don't override this method. Make opts optional (e.g. opts = {}) and/or accept splat args to keep backwards-compatible call signatures.
| def download(spec, opts); end | |
| def download(spec, opts = {}); end |
| def add_source(source_uri) # :nodoc: | ||
| check_rubygems_https source_uri | ||
|
|
||
| source = Gem::Source.new source_uri | ||
|
|
||
| check_typo_squatting(source) | ||
| source = build_new_source(source_uri) | ||
| source_uri = source.uri.to_s | ||
|
|
There was a problem hiding this comment.
build_new_source is called outside the begin/rescue block, so Gem::Source.new / URI parsing errors (and the HTTPS/typo-squatting checks) will raise without being converted into the existing friendly "... is not a URI" + exit(1) behavior. Wrap the build_new_source call inside the begin block (or rescue around it) to preserve command behavior for invalid URIs.
| def prepend_source(source_uri) # :nodoc: | ||
| check_rubygems_https source_uri | ||
|
|
||
| source = Gem::Source.new source_uri | ||
|
|
||
| check_typo_squatting(source) | ||
| source = build_new_source(source_uri) | ||
| source_uri = source.uri.to_s | ||
|
|
There was a problem hiding this comment.
Same issue as add_source: build_new_source is executed before the begin/rescue, so invalid URIs can raise and bypass the command's error handling. Wrap build_new_source with the existing rescue so --prepend behaves consistently on bad input.
| def process_specs(installed_specs) | ||
| spec = worker_pool.deq | ||
|
|
||
| if spec.installed? | ||
| installed_specs[spec.name] = true | ||
| return | ||
| elsif spec.failed? | ||
| return | ||
| elsif spec.ready_to_install?(installed_specs) | ||
| spec.state = :installable | ||
| end | ||
|
|
||
| worker_pool.enq(spec, priority: spec.enqueue_with_priority?) | ||
| end |
There was a problem hiding this comment.
process_specs re-enqueues specs that are in :downloaded state but not yet ready_to_install?. Since the worker lambda returns the spec unchanged for unknown states, this can create a tight enqueue/dequeue loop (high CPU) until dependencies are installed. Only enqueue when there's actual work (:enqueued download or :installable install), and trigger installation enqueueing after dependency state changes (e.g., after a spec becomes :installed).
| @@ -114,7 +116,7 @@ | |||
|
|
|||
| context "with a OSX version number" do | |||
| before do | |||
| expect(git_proxy).to receive(:git_local).with("--version"). | |||
| expect(described_class).to receive(:full_version). | |||
| and_return("git version 1.2.3 (Apple Git-BS)") | |||
| end | |||
There was a problem hiding this comment.
The stubbed return value for described_class.full_version includes the "git version" prefix, but GitProxy.full_version now strips that prefix and returns just the version string. With the current stub, GitProxy.version's regex will return nil and the spec will fail. Update the stubbed values to match the stripped full_version return (e.g. "1.2.3" / "1.2.3 (Apple Git-BS)" / "1.2.3.msysgit.0").
| # Prevent tests from modifying the user's global git config. | ||
| # GIT_CONFIG_GLOBAL and GIT_CONFIG_NOSYSTEM are available since Git 2.32. | ||
| git_version = `git --version`[/(\d+\.\d+\.\d+)/, 1] | ||
| if Gem::Version.new(git_version) >= Gem::Version.new("2.32") | ||
| ENV["GIT_CONFIG_GLOBAL"] = File.join(ENV["HOME"], ".gitconfig") | ||
| ENV["GIT_CONFIG_NOSYSTEM"] = "1" | ||
| end |
There was a problem hiding this comment.
This assumes git --version returns a parsable version string. If git is missing or the regex returns nil, Gem::Version.new(git_version) will raise and break the test suite. Please guard this with Bundler.git_present? and/or handle nil (skip setting these env vars when the version can't be determined).
|
#9366 is not working with |
gem owner#9403bundle install#9406