This repository has been archived by the owner on Jul 4, 2023. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 11.4k
(un)linkapps: modernize, prune: remove broken app symlinks #46549
Closed
Closed
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f796cfe
linkapps: modernize
UniqMartin 2e75a13
unlinkapps: modernize
UniqMartin aca1684
unlinkapps: add --dry-run option
UniqMartin 8f73c22
prune: handle broken app symlinks
UniqMartin 9941f0d
tests: update linkapps/unlinkapps tests
UniqMartin ff0cce5
tests: update prune --verbose test
UniqMartin File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
require "keg" | ||
require "cmd/tap" | ||
require "cmd/unlinkapps" | ||
|
||
module Homebrew | ||
def prune | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While you're here I wonder if it's worth just adding to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
end | ||
end |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,33 +1,68 @@ | ||
# Unlinks any Applications (.app) found in installed prefixes from /Applications | ||
require "keg" | ||
require "cmd/linkapps" | ||
|
||
module Homebrew | ||
def unlinkapps | ||
target_dir = ARGV.include?("--local") ? File.expand_path("~/Applications") : "/Applications" | ||
target_dir = linkapps_target(:local => ARGV.include?("--local")) | ||
|
||
return unless File.exist? target_dir | ||
unlinkapps_from_dir(target_dir, :dry_run => ARGV.dry_run?) | ||
end | ||
|
||
cellar_apps = Dir[target_dir + "/*.app"].select do |app| | ||
if File.symlink?(app) | ||
should_unlink? File.readlink(app) | ||
end | ||
private | ||
|
||
def unlinkapps_prune(opts = {}) | ||
opts = opts.merge(:prune => true) | ||
unlinkapps_from_dir(linkapps_target(:local => false), opts) | ||
unlinkapps_from_dir(linkapps_target(:local => true), opts) | ||
end | ||
|
||
def unlinkapps_from_dir(target_dir, opts = {}) | ||
return unless target_dir.directory? | ||
dry_run = opts.fetch(:dry_run, false) | ||
quiet = opts.fetch(:quiet, false) | ||
|
||
apps = Pathname.glob("#{target_dir}/*.app").select do |app| | ||
unlinkapps_unlink?(app, opts) | ||
end | ||
|
||
cellar_apps.each do |app| | ||
puts "Unlinking #{app}" | ||
system "unlink", app | ||
ObserverPathnameExtension.reset_counts! | ||
|
||
app_kind = opts.fetch(:prune, false) ? " (broken link)" : "" | ||
apps.each do |app| | ||
app.extend(ObserverPathnameExtension) | ||
if dry_run | ||
puts "Would unlink#{app_kind}: #{app}" | ||
else | ||
puts "Unlinking#{app_kind}: #{app}" unless quiet | ||
app.unlink | ||
end | ||
end | ||
|
||
puts "Finished unlinking from #{target_dir}" if cellar_apps | ||
return if dry_run | ||
|
||
if ObserverPathnameExtension.total.zero? | ||
puts "No apps unlinked from #{target_dir}" if ARGV.verbose? | ||
else | ||
n = ObserverPathnameExtension.total | ||
puts "Unlinked #{n} app#{plural(n)} from #{target_dir}" | ||
end | ||
end | ||
|
||
private | ||
UNLINKAPPS_PREFIXES = %W[ | ||
#{HOMEBREW_CELLAR}/ | ||
#{HOMEBREW_PREFIX}/opt/ | ||
].freeze | ||
|
||
def unlinkapps_unlink?(target_app, opts = {}) | ||
# Skip non-symlinks and symlinks that don't point into the Homebrew prefix. | ||
app = "#{target_app.readlink}" if target_app.symlink? | ||
return false unless app && app.start_with?(*UNLINKAPPS_PREFIXES) | ||
|
||
def should_unlink?(file) | ||
if ARGV.named.empty? | ||
file.start_with?("#{HOMEBREW_CELLAR}/", "#{HOMEBREW_PREFIX}/opt/") | ||
if opts.fetch(:prune, false) | ||
!File.exist?(app) # Remove only broken symlinks in prune mode. | ||
elsif ARGV.named.empty? | ||
true | ||
else | ||
ARGV.kegs.any? { |keg| file.start_with?("#{keg}/", "#{keg.opt_record}/") } | ||
ARGV.kegs.any? { |keg| app.start_with?("#{keg}/", "#{keg.opt_record}/") } | ||
end | ||
end | ||
end |
This file contains 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
This file contains 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
This file contains 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
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel super-strongly about this but we're trying to avoid
cmd/
includes now so if you're feeling extra lovely (can be a future PR) pulling these methods into another module would be ❤️There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right and I think it makes more sense to address this here in this PR (big chunks have been rewritten anyway) instead of deferring the code move to a new PR. I'll reshuffle things soon. Thanks for the soft nudge. 😻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍