(un)linkapps: modernize, prune: remove broken app symlinks #46549
Conversation
| kegs = Formula.installed.map do |formula| | ||
| formula_kegs = formula.installed_kegs | ||
| next if formula_kegs.empty? | ||
| formula_kegs.detect(&:linked?) || formula_kegs.max_by(&:version) |
There was a problem hiding this comment.
We shouldn't use Formula.installed here. The thing is commands like link/unlink/switch/uninstall/linkapp should work fine with DIY installation, i.e. no formula required. So let's use rack directly instead of trying to load formula.
There was a problem hiding this comment.
Thanks for pointing this out! I'll fix it shortly. DIY seems like such an esoteric feature that I tend to always forget about it (and I never used it).
There was a problem hiding this comment.
Beside the complete DIY installation(i.e. manually compiling while set prefix to cellar), another common situation is brew install <URL/path to formula> following with cache purge. Therefore, formula file will also be gone.
|
Looks good to me 👍
That would be great. I was going to take a look but you're already closer to it 😉 |
|
I addressed the problem raised by @xu-cheng (thanks!) with the nice side-effect of being much faster now. Nothing else changed in what I just pushed.
I intended to do this anyway and as this issue of stale symlinks has resurfaced again in #46542, I decided to address it sooner than later. 😸 |
|
Updated to address a merge conflict. It's ✅ again. |
| @@ -451,6 +451,10 @@ With `--verbose` or `-v`, many commands print extra debugging information. Note | |||
|
|
|||
| If no <formulae> are provided, all linked app will be removed. | |||
|
|
|||
| * `unlinkapps --prune`: | |||
| Removes links created by `brew linkapps` that are no longer valid, e.g., | |||
| because a formula was removed or apps provided by a formula have changed. | |||
There was a problem hiding this comment.
Wondering if this behaviour should just be done by brew prune?
There was a problem hiding this comment.
This sounds like a better place for this functionality; thanks for pointing it out! Do you mean as default behavior for brew prune or as a separate mode of operation, e.g. brew prune --apps?
There was a problem hiding this comment.
Default behaviour; I don't see a reason why you'd not want to do this.
There was a problem hiding this comment.
Agreed; I'll update the code to move the new functionality into brew prune.
UniqMartin
changed the title
|
Relocated pruning of broken app symlinks into |
| @@ -47,5 +48,7 @@ def prune | |||
| print "and #{d} directories " if d > 0 | |||
| puts "from #{HOMEBREW_PREFIX}" | |||
| end unless ARGV.dry_run? | |||
|
|
|||
| unlinkapps_prune(:dry_run => ARGV.dry_run?, :quiet => true) | |||
There was a problem hiding this comment.
While you're here I wonder if it's worth just adding to uninstall too? No worries if you'd rather leave it for another PR.
There was a problem hiding this comment.
I'd prefer to do that in another PR as I expect some discussion. The reason is there are multiple places from where this likely needs to be called, though uninstall is the most obvious one.
|
A few nits but nothing preventing this being merged if you'd rather do the rest in another PR 👍 |
|
@UniqMartin thoughts on merging as-is? |
|
I was hoping to revamp things directly in here to avoid cross |
Simplify code by using `Pathname` methods as much as possible. Also avoid calling external commands for basic functionality like symlink creation, refactor code that can be shared with `brew unlinkapps`, and print a summary line at the end (if symlinks were created).
Simplify code by using `Pathname` methods as much as possible. Also avoid calling external commands for basic functionality like unlinking, reduce code duplication by using a method from `cmd/linkapps.rb`, count unlinked symlinks with `ObserverPathnameExtension`, and adjust output for consistency with `brew linkapps`.
Add `--dry-run` option as is customary for destructive commands. Update `bash` completion and man page accordingly. Also correct and update documentation for both `brew linkapps` and `brew unlinkapps` in more general terms.
Remove broken symlinks from `/Applications` and `~/Applications` that were previously created by `brew linkapps`, but are no longer valid because formulae were uninstalled or the provided apps have changed.
|
@UniqMartin Sounds good 👍 |
|
Thanks (also for the nudge)! Test failure should be fixed, too. I'll ship as soon as the bot is happy. |
|
Closing in favor of Homebrew/brew#15 as this no longer cleanly applies in the new repository. |
The first two commits are all about using
Pathnameas much as possible (bringing the code up to current Homebrew standards), reducing duplication, and avoiding external commands in favor of native methods.The third (and last) commit implements
brew unlinkapps --pruneand allows to remove broken symlinks from/Applicationsand~/Applicationsif they were previously created withbrew linkapps. This part also providesHomebrew.unlinkapps_prunethat could be used fromcmd/uninstall.rbet al. to implement automatic pruning of broken symlinks as suggested in #46542 (will be addressed in a separate PR).