Skip to content

Preserve absent Gemfile lock paths - #3118

Open
OskarEichler wants to merge 1 commit into
ruby:masterfrom
OskarEichler:codex/nil-gemfile-lock-path
Open

Preserve absent Gemfile lock paths#3118
OskarEichler wants to merge 1 commit into
ruby:masterfrom
OskarEichler:codex/nil-gemfile-lock-path

Conversation

@OskarEichler

Copy link
Copy Markdown
Contributor

Summary: keep a missing Gemfile lock path absent during collection lockfile serialization instead of raising by calling to_s on nil.

Verification: focused baseline/fixed reproduction, combined RBS 4.2.0 consumer models, and RuboCop (738 files, zero offenses). No tests are added in this PR.

Compatibility: no public API removal or dependency/version change.

@pocke pocke self-assigned this Sep 7, 2026
@pocke
pocke force-pushed the codex/nil-gemfile-lock-path branch from f5df358 to f3abf4b Compare September 7, 2026 09:41
@pocke

pocke commented Sep 7, 2026

Copy link
Copy Markdown
Member

Thank you, the change itself is right and I am planning to merge it. One correction to the description, for the record.

nil.to_s does not raise; it returns "". So nothing blows up at serialization time. What actually goes wrong is quieter, and it happens on the way back in:

  1. to_lockfile writes gemfile_lock_path: '' when gemfile_lock_path is nil.
  2. Lockfile.from_lockfile tests if p = data["gemfile_lock_path"], and "" is truthy in Ruby, so it builds Pathname("") instead of leaving the path absent.
  3. gemfile_lock_fullpath then returns lockfile_dir + Pathname(""), which is lockfile_dir itself.
  4. LockfileGenerator#validate_gemfile_lock_path! compares that directory against the real Gemfile.lock and raises GemfileLockMismatchError.

So the bug is a wrong value round-tripping through the lockfile rather than a NoMethodError, and &.to_s is the right fix because lockfile_data already declares "gemfile_lock_path" => String?.

It is also worth noting that LockfileGenerator always passes definition.lockfile.relative_path_from(lockfile_dir), so rbs collection itself never reaches this today. It matters for callers that build a Lockfile directly, and for a lockfile written before we added the key.

I have rebased the branch onto the current master. The test (head, stdlib_test rubocop) failure was an unrelated Net::HTTP test on ruby-head and is fixed there now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants