Skip cask link when symlink already correct - #23428
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request updates Cask artifact symlink handling so that attempting to link a cask binary does not error when the destination already contains the correct symlink, making repeated brew upgrade/retries idempotent in that scenario.
Changes:
- Treat an already-correct existing symlink at the target path as a no-op (log and skip linking) instead of raising a conflict error.
- Add an RSpec example to cover the “already correctly linked” symlink case for
Cask::Artifact::Binary.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Library/Homebrew/cask/artifact/symlinked.rb | Adds early-return path to skip relinking when the existing target symlink resolves to the same source. |
| Library/Homebrew/test/cask/artifact/binary_spec.rb | Adds a regression test asserting the correct-symlink case is handled as a no-op and logs accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- A plain `brew upgrade` raised "It seems there is already a Binary at ..." even when the existing symlink resolved to the exact source about to be linked, because the realpath check sat behind `force`/`adopt`. The revert left the symlink in place so every retry failed identically until manual intervention. - Treat an already-correct symlink as a no-op: log and skip the link. Genuine conflicts, such as a real file at the target or a symlink elsewhere, still require `--force` or `--adopt`. - Rescue errors resolving the target, like `conflicting_formula` does, so unreadable symlinks fall back to the existing conflict and error handling instead of raising `Errno` exceptions. - Fixes #23426.
MikeMcQuaid
force-pushed
the
symlink-target-check
branch
from
August 4, 2026 12:04
2b9adf0 to
0376eda
Compare
krehel
approved these changes
Aug 4, 2026
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.
brew upgraderaised "It seems there is already a Binary at ..." even when the existing symlink resolved to the exact source about to be linked, because the realpath check sat behindforce/adopt. The revert left the symlink in place so every retry failed identically until manual intervention.--forceor--adopt.Fixes #23426.
brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Fable 5 max with local review, iterating and testing.