Run collection cleaner from CLI - #3127
Open
OskarEichler wants to merge 2 commits into
Open
Conversation
`rbs collection clean` built a `Collection::Cleaner` and never called `#clean` on it, so the command walked away with exit status 0 while leaving every directory in place. The bug dates back to 38fd8b2, the commit that introduced `rbs collection`, so the command had never once removed anything. `test/rbs/collection/cleaner_test.rb` exercises `Cleaner#clean` directly, which is why the missing call went unnoticed for so long. This test goes through `RBS::CLI#run` instead, so it covers the wiring between the subcommand and the cleaner. The lockfile lists `ast 2.4`. The collection directory also holds `ast 2.3` and `rainbow 3.0`, covering both reasons `Cleaner#needed?` rejects a directory: a gem locked at another version, and a gem absent from the lockfile. `ast/2.4/ast.rbs` is written so that the assertion on the version that survives shows its RBS files are left alone rather than only that a directory still exists. The collection is named with `--collection` rather than by changing the working directory. `Config.find_config_path` walks from the working directory up to the filesystem root, so a `Dir.chdir` based test stays inside its temporary directory only as long as the config file is written before `cli.run`. With `TMPDIR` pointed inside a real project, a reordering would let the search reach an ancestor's lockfile and delete that project's `.gem_rbs_collection`. `--collection` expands the path it is given and skips the search. The config file is written empty. `clean` reads `path` and `gems` from the lockfile and never opens the config, and `path` appears in both files, so giving the config a `path` of its own would suggest that the command consults it. Its one remaining job is to be what `--collection` names and what the lockfile path is derived from, which is why the lockfile is written through `Config.to_lockfile_path` instead of a hardcoded name that happens to match. The `source` block in the lockfile stays even though `Cleaner#needed?` does not read it. `Lockfile.from_lockfile` passes `gem["source"]` to `Sources.from_config_entry`, so a lockfile without it raises `NoMethodError` when read through `Lockfile` rather than `Config`. ruby#3127 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HSq3kKMB8ULhZrnL5urD7y
pocke
force-pushed
the
codex/run-collection-clean
branch
from
September 7, 2026 10:03
57c147a to
d1fdb1b
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
2 participants
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.
Summary: invoke the cleaner from the collection clean command instead of constructing it without performing cleanup.
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.