Skip to content

Validate local collection symlink targets - #3116

Closed
OskarEichler wants to merge 1 commit into
ruby:masterfrom
OskarEichler:codex/local-collection-target
Closed

Validate local collection symlink targets#3116
OskarEichler wants to merge 1 commit into
ruby:masterfrom
OskarEichler:codex/local-collection-target

Conversation

@OskarEichler

Copy link
Copy Markdown
Contributor

Summary: require installed local collection symlinks to resolve to the configured source, rather than accepting any existing stale target.

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 commented Sep 7, 2026

Copy link
Copy Markdown
Member

Thank you for looking into this, and sorry for the slow reply. I have decided to close this one without merging.

The gap you found is real. I reproduced it locally: with a lockfile whose Sources::Local entry points at one directory and an installed symlink left over from another, check_rbs_availability! accepts it today, and your patch turns it into CollectionNotAvailable. It also does not produce a false positive on a correct symlink. So the diagnosis and the patch are both sound.

Two reasons I would still rather not take it.

First, nothing is going wrong in practice. A stale symlink only survives until the next rbs collection install, and we have had no report of anyone being bitten by it. Without a concrete failure to point at, I do not want to spend more review, test and maintenance effort on this path.

Second, the new check also rejects a symlink whose target has simply gone away, for example when the directory a local source points at has been deleted or moved. File.identical? returns false there, so we raise CollectionNotAvailable, and its message tells the user to run rbs collection install. That will fail for the same reason, so we would be replacing a quiet situation with an error that gives misleading advice. That is not a trade I want to make for a problem nobody has hit.

One more thing worth writing down in case we come back to this area. Sources::Local#install decides whether an installed symlink is current with gem_dir.readlink == from, a string comparison, while this patch uses File.identical?. The two can disagree, for instance when the same directory is reached through /tmp and /private/tmp on macOS: install would unlink and recreate the symlink while the check considers it fine. If we do revisit this, I would want both sites to share a single predicate rather than grow a second, subtly different one.

Thanks again for the careful work, and sorry that this one does not land.

@pocke pocke closed this Sep 7, 2026
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