Replace the rubygems-specific lockfile parser with Bundler's parser#9564
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes RubyGems’ duplicated gem.deps lockfile tokenizer/parser and instead uses Bundler::LockfileParser to read gem.deps.rb.lock, translating Bundler’s parsed sources/specs back into RubyGems resolver sets (LockSet / GitSet / VendorSet).
Changes:
- Route
Gem::RequestSet#load_gemdepslockfile loading through a newload_lockfileadapter that consumesBundler::LockfileParser. - Add
lockfile_path:kwarg toBundler::LockfileParser#initializeand test that it affects error locations/messages. - Remove the old RubyGems lockfile parser/tokenizer implementation and associated tests, and adjust test load paths/manifest.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/rubygems/request_set.rb | Replaces RubyGems lockfile parsing with a Bundler parser adapter (load_lockfile). |
| bundler/lib/bundler/lockfile_parser.rb | Adds lockfile_path: kwarg used for error reporting / checksum positions. |
| spec/bundler/lockfile_parser_spec.rb | Adds coverage for lockfile_path: behavior in errors and checksum positions. |
| test/rubygems/test_gem_request_set.rb | Adds new integration tests asserting sets are created from GEM/GIT/PATH lockfile sections. |
| lib/rubygems/request_set/lockfile.rb | Removes now-unused require_relative "lockfile/tokenizer". |
| lib/rubygems/request_set/lockfile/parser.rb | Deleted (old RubyGems lockfile parser). |
| lib/rubygems/request_set/lockfile/tokenizer.rb | Deleted (old RubyGems lockfile tokenizer). |
| test/rubygems/test_gem_request_set_lockfile_parser.rb | Deleted (tests for removed RubyGems lockfile parser). |
| test/rubygems/test_gem_request_set_lockfile_tokenizer.rb | Deleted (tests for removed RubyGems lockfile tokenizer). |
| Rakefile | Adds bundler/lib to test load path. |
| Manifest.txt | Removes deleted RubyGems lockfile parser/tokenizer entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+335
to
+340
| def load_lockfile(lock_file) # :nodoc: | ||
| require "bundler" | ||
| require "bundler/lockfile_parser" | ||
|
|
||
| parser = Bundler::LockfileParser.new(File.read(lock_file), lockfile_path: lock_file) | ||
|
|
Comment on lines
+378
to
+382
| parser.dependencies.each_value do |dep| | ||
| gem dep.name, *dep.requirement.as_list | ||
| end | ||
| end | ||
|
|
hsbt
force-pushed
the
replace-lockfile-parser-with-bundler
branch
from
295ce6d to
02336c1
Compare
The path was previously inferred from Bundler.default_lockfile via SharedHelpers, which couples the parser to a configured Bundler environment. Accepting lockfile_path: as a keyword argument lets external callers (e.g. Gem::RequestSet) instantiate the parser without setting up Bundler globals, while preserving the previous fallback.
These tests pin down the observable RequestSet state produced when load_gemdeps reads a lockfile with GEM, GIT, or PATH sections, so the upcoming switch to Bundler::LockfileParser can be evaluated against a clear behavioral baseline.
The rubygems-specific tokenizer/parser pair is replaced by Bundler's LockfileParser, with a small adapter that translates the parsed LazySpecifications and Bundler sources back into the LockSet, GitSet, and VendorSet objects that the resolver expects. This unifies lockfile parsing on a single implementation so the standalone tokenizer/parser sources and tests can be retired in subsequent commits.
Both files exercised Gem::RequestSet::Lockfile::Tokenizer and ::Parser directly, which are no longer used by load_gemdeps now that Bundler::LockfileParser handles the read path. End-to-end coverage of the GEM, GIT, and PATH sections lives in test_gem_request_set.
These files implemented the read path that Bundler::LockfileParser now covers via the adapter in Gem::RequestSet#load_lockfile. The Lockfile class itself is kept because Gem::RequestSet#install_from_gemdeps still uses Lockfile.build / Lockfile#write for emitting .lock files, which will be migrated separately.
hsbt
force-pushed
the
replace-lockfile-parser-with-bundler
branch
from
02336c1 to
a8627d7
Compare
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.
What was the end-user or developer problem that led to this PR?
lib/rubygems/request_set/lockfile/parser.rbandtokenizer.rbduplicatedBundler::LockfileParserfor thegem install -gread path.What is your fix for the problem, implemented in this PR?
This PR removes both and routes
Gem::RequestSet#load_gemdepsthrough a small adapter thattranslates Bundler's parsed sources back into the resolver's
LockSet/GitSet/VendorSet.Bundler::LockfileParser#initializegains alockfile_path:kwarg so it can be used without setting up a Bundler context.Make sure the following tasks are checked