Skip to content

Commit

Permalink
cask/uninstall: skip quit and signal directives when upgrading or rei…
Browse files Browse the repository at this point in the history
…nstalling
  • Loading branch information
bevanjkay committed Jan 19, 2024
1 parent 1976ead commit 82383eb
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 7 deletions.
19 changes: 14 additions & 5 deletions Library/Homebrew/cask/artifact/uninstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,26 @@

require "cask/artifact/abstract_uninstall"

UPGRADE_REINSTALL_SKIP_DIRECTIVES = [:quit, :signal].freeze

module Cask
module Artifact
# Artifact corresponding to the `uninstall` stanza.
#
# @api private
class Uninstall < AbstractUninstall
def uninstall_phase(**options)
ORDERED_DIRECTIVES.reject { |directive_sym| directive_sym == :rmdir }
.each do |directive_sym|
dispatch_uninstall_directive(directive_sym, **options)
end
def uninstall_phase(upgrade: false, reinstall: false, **options)
filtered_directives = ORDERED_DIRECTIVES.filter do |directive_sym|

Check warning on line 15 in Library/Homebrew/cask/artifact/uninstall.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/artifact/uninstall.rb#L15

Added line #L15 was not covered by tests
next false if directive_sym == :rmdir

next false if (upgrade || reinstall) && UPGRADE_REINSTALL_SKIP_DIRECTIVES.include?(directive_sym)

true

Check warning on line 20 in Library/Homebrew/cask/artifact/uninstall.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/artifact/uninstall.rb#L20

Added line #L20 was not covered by tests
end

filtered_directives.each do |directive_sym|
dispatch_uninstall_directive(directive_sym, **options)

Check warning on line 24 in Library/Homebrew/cask/artifact/uninstall.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/artifact/uninstall.rb#L23-L24

Added lines #L23 - L24 were not covered by tests
end
end

def post_uninstall_phase(**options)
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/cask/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,8 @@ def uninstall_artifacts(clear: false, successor: nil)
skip: clear,
force: force?,
successor: successor,
upgrade: upgrade?,
reinstall: reinstall?,
)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,18 @@
end.to output(/Application 'my.fancy.package.app' quit successfully\./).to_stdout
end

it "does not attempt to quit when upgrading or reinstalling" do
allow(User.current).to receive(:gui?).and_return true

expect(subject).not_to receive(:running?)
expect(subject).not_to receive(:quit)

expect do
subject.public_send(:"#{artifact_dsl_key}_phase", upgrade: true, command: fake_system_command)
subject.public_send(:"#{artifact_dsl_key}_phase", reinstall: true, command: fake_system_command)
end
end

it "tries to quit the application for 10 seconds" do
allow(User.current).to receive(:gui?).and_return true

Expand Down Expand Up @@ -261,6 +273,18 @@

subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
end

it "does not send signal when upgrading or reinstalling" do
allow(subject).to receive(:running_processes).with(bundle_id)
.and_return(unix_pids.map { |pid| [pid, 0, bundle_id] })

signals.each do |_signal|
expect(Process).not_to receive(:kill)
end

subject.public_send(:"#{artifact_dsl_key}_phase", upgrade: true, command: fake_system_command)
subject.public_send(:"#{artifact_dsl_key}_phase", reinstall: true, command: fake_system_command)
end
end

[:delete, :trash].each do |directive|
Expand Down
4 changes: 2 additions & 2 deletions docs/Cask-Cookbook.md
Original file line number Diff line number Diff line change
Expand Up @@ -761,8 +761,8 @@ Since `pkg` installers can do arbitrary things, different techniques are needed

* **`early_script:`** (string or hash) - like [`script:`](#uninstall-script), but runs early (for special cases, best avoided)
* [`launchctl:`](#uninstall-launchctl) (string or array) - IDs of `launchd` jobs to remove
* [`quit:`](#uninstall-quit) (string or array) - bundle IDs of running applications to quit
* [`signal:`](#uninstall-signal) (array of arrays) - signal numbers and bundle IDs of running applications to send a Unix signal to (for when `quit:` does not work)
* [`quit:`](#uninstall-quit) (string or array) - bundle IDs of running applications to quit (does not run when uninstall is initiated by `brew upgrade` or `brew reinstall`)
* [`signal:`](#uninstall-signal) (array of arrays) - signal numbers and bundle IDs of running applications to send a Unix signal to - for when `quit:` does not work (does not run when uninstall is initiated by `brew upgrade` or `brew reinstall`)
* [`login_item:`](#uninstall-login_item) (string or array) - names of login items to remove
* [`kext:`](#uninstall-kext) (string or array) - bundle IDs of kexts to unload from the system
* [`script:`](#uninstall-script) (string or hash) - relative path to an uninstall script to be run via sudo; use hash if args are needed
Expand Down

0 comments on commit 82383eb

Please sign in to comment.