Merge pull request #24194 from Homebrew/merge_zap_uninstall

cask: merge uninstall and zap directives into one pass
This commit is contained in:
Mike McQuaid authored and GitHub committed 2026-10-17 12:08:05 +00:00
commit 08e9f17c6f
12 files changed
+241 -133

No files matched your search

@@ -38,11 +38,54 @@ module Cask
:on_upgrade,
].freeze
UPGRADE_REINSTALL_SKIP_DIRECTIVES = [:signal].freeze
sig { params(cask: Cask, directives: DirectivesType).returns(AbstractUninstall) }
def self.from_args(cask, **directives)
new(cask, **directives)
end
# Dispatches the directives of one or more `uninstall`/`zap` stanzas in a single
# `ORDERED_DIRECTIVES` pass, the earlier stanza's first at each step, so that
# zapping runs `zap` directives alongside the `uninstall` stanza's. Each stanza's
# `deferred_directives` are skipped, and are the only ones run when `deferred:`.
sig {
params(
artifacts: T::Array[AbstractUninstall],
command: T.class_of(SystemCommand),
deferred: T::Boolean,
force: T::Boolean,
successor: T.nilable(Cask),
upgrade: T::Boolean,
reinstall: T::Boolean,
quit: T::Boolean,
).void
}
def self.dispatch_directives(artifacts, command:, deferred: false, force: false, successor: nil,
upgrade: false, reinstall: false, quit: true)
ORDERED_DIRECTIVES.each do |directive_sym|
next if directive_sym == :quit && !quit
artifacts.each do |artifact|
next if artifact.deferred_directives.include?(directive_sym) != deferred
if (upgrade || reinstall) &&
UPGRADE_REINSTALL_SKIP_DIRECTIVES.include?(directive_sym) &&
Array(artifact.directives[:on_upgrade]).map { |name| name.to_s.to_sym }.exclude?(directive_sym)
next
end
artifact.dispatch_uninstall_directive(directive_sym, command:, force:, successor:, upgrade:)
end
end
end
# Directives that wait until the cask's artifacts have been removed and its
# `uninstall_postflight` steps have run, so that directories which held only
# those artifacts are seen as empty.
sig { overridable.returns(T::Array[Symbol]) }
def deferred_directives = [:rmdir]
sig { returns(T::Hash[Symbol, DirectivesType]) }
attr_reader :directives
@@ -180,22 +223,7 @@ module Cask
end
end
private
sig {
params(
command: T.class_of(SystemCommand),
force: T::Boolean,
successor: T.nilable(Cask),
upgrade: T::Boolean,
).void
}
def dispatch_uninstall_directives(command:, force: false, successor: nil, upgrade: false)
ORDERED_DIRECTIVES.each do |directive_sym|
dispatch_uninstall_directive(directive_sym, command:, force:, successor:, upgrade:)
end
end
# Runs this stanza's `directive_sym` directive, if it has one.
# The `uninstall_*` methods are dispatched dynamically, so they take these
# options uniformly and ignore the ones they do not need.
sig {
@@ -216,6 +244,8 @@ module Cask
command:, force:, successor:, upgrade:)
end
private
sig { returns(Symbol) }
def stanza
self.class.dsl_key
+2 -31
View File
@@ -7,8 +7,6 @@ module Cask
module Artifact
# Artifact corresponding to the `uninstall` stanza.
class Uninstall < AbstractUninstall
UPGRADE_REINSTALL_SKIP_DIRECTIVES = [:signal].freeze
sig {
params(
command: T.class_of(SystemCommand),
@@ -23,34 +21,7 @@ module Cask
}
def uninstall_phase(command:, skip: false, force: false, verbose: false, successor: nil, upgrade: false,
reinstall: false, quit: true)
raw_on_upgrade = directives[:on_upgrade]
on_upgrade_syms =
case raw_on_upgrade
when Symbol
[raw_on_upgrade]
when Array
raw_on_upgrade.map(&:to_sym)
else
[]
end
on_upgrade_set = on_upgrade_syms.to_set
filtered_directives = ORDERED_DIRECTIVES.filter do |directive_sym|
next false if directive_sym == :rmdir
next false if directive_sym == :quit && !quit
if (upgrade || reinstall) &&
UPGRADE_REINSTALL_SKIP_DIRECTIVES.include?(directive_sym) &&
on_upgrade_set.exclude?(directive_sym)
next false
end
true
end
filtered_directives.each do |directive_sym|
dispatch_uninstall_directive(directive_sym, command:, force:, successor:, upgrade:)
end
AbstractUninstall.dispatch_directives([self], command:, force:, successor:, upgrade:, reinstall:, quit:)
end
sig {
@@ -63,7 +34,7 @@ module Cask
).void
}
def post_uninstall_phase(command:, skip: false, force: false, verbose: false, successor: nil)
dispatch_uninstall_directive(:rmdir, command:, force:, successor:)
AbstractUninstall.dispatch_directives([self], command:, deferred: true, force:, successor:)
end
end
end
+4 -10
View File
@@ -7,16 +7,10 @@ module Cask
module Artifact
# Artifact corresponding to the `zap` stanza.
class Zap < AbstractUninstall
sig {
params(
command: T.class_of(SystemCommand),
force: T::Boolean,
verbose: T::Boolean,
).void
}
def zap_phase(command:, force: false, verbose: false)
dispatch_uninstall_directives(command:, force:)
end
# Removing files also waits, as `uninstall` steps may still need them, e.g. to
# identify a keychain certificate to delete.
sig { override.returns(T::Array[Symbol]) }
def deferred_directives = [:pkgutil, :delete, :trash, :rmdir]
end
end
end
+48 -51
View File
@@ -715,67 +715,64 @@ on_request: true)
puts summary
end
sig { params(clear: T::Boolean, successor: T.nilable(Cask), quit: T::Boolean).void }
def uninstall_artifacts(clear: false, successor: nil, quit: true)
sig {
params(clear: T::Boolean, successor: T.nilable(Cask), quit: T::Boolean, zap_stanzas: T::Array[Artifact::Zap])
.void
}
def uninstall_artifacts(clear: false, successor: nil, quit: true, zap_stanzas: [])
odebug "Uninstalling artifacts"
odebug "#{::Utils.pluralize("artifact", artifacts.length, include_count: true)} defined", artifacts
artifacts.each do |artifact|
if artifact.respond_to?(:uninstall_phase)
artifact = T.cast(
artifact,
T.any(
Artifact::AbstractFlightBlock,
Artifact::GeneratedCompletion,
Artifact::KeyboardLayout,
Artifact::Moved,
Artifact::PostflightSteps,
Artifact::PreflightSteps,
Artifact::Qlplugin,
Artifact::Symlinked,
Artifact::Uninstall,
Artifact::UninstallPostflightSteps,
Artifact::UninstallPreflightSteps,
),
)
directive_artifacts = [*artifacts.grep(Artifact::Uninstall), *zap_stanzas]
odebug "Uninstalling artifact of class #{artifact.class}"
if artifact.is_a?(Artifact::Uninstall)
artifact.uninstall_phase(command: @command, verbose: verbose?, skip: clear, force: force?, successor:,
upgrade: upgrade?, reinstall: reinstall?, quit:)
else
artifact.uninstall_phase(command: @command, verbose: verbose?, skip: clear, force: force?, successor:,
upgrade: upgrade?, reinstall: reinstall?)
end
end
next unless artifact.respond_to?(:post_uninstall_phase)
artifact = T.cast(artifact, Artifact::Uninstall)
odebug "Post-uninstalling artifact of class #{artifact.class}"
artifact.post_uninstall_phase(
command: @command,
verbose: verbose?,
skip: clear,
force: force?,
successor:,
)
preflight, removable = T.cast(
artifacts.grep_v(Artifact::AbstractUninstall).select { |artifact| artifact.respond_to?(:uninstall_phase) },
T::Array[
T.any(
Artifact::AbstractFlightBlock,
Artifact::GeneratedCompletion,
Artifact::KeyboardLayout,
Artifact::Moved,
Artifact::PostflightSteps,
Artifact::PreflightSteps,
Artifact::Qlplugin,
Artifact::Symlinked,
Artifact::UninstallPostflightSteps,
Artifact::UninstallPreflightSteps,
),
],
).partition do |artifact|
order = artifact.sort_order
uninstall_order = order.fetch(Artifact::Uninstall)
order.fetch(artifact.class, uninstall_order) < uninstall_order
end
preflight.each do |artifact|
odebug "Uninstalling artifact of class #{artifact.class}"
artifact.uninstall_phase(command: @command, verbose: verbose?, skip: clear, force: force?, successor:,
upgrade: upgrade?, reinstall: reinstall?)
end
Artifact::AbstractUninstall.dispatch_directives(directive_artifacts, command: @command, force: force?,
successor:, upgrade: upgrade?, reinstall: reinstall?, quit:)
removable.each do |artifact|
odebug "Uninstalling artifact of class #{artifact.class}"
artifact.uninstall_phase(command: @command, verbose: verbose?, skip: clear, force: force?, successor:,
upgrade: upgrade?, reinstall: reinstall?)
end
Artifact::AbstractUninstall.dispatch_directives(directive_artifacts, command: @command, deferred: true,
force: force?, successor:)
end
sig { void }
def zap
load_installed_caskfile!
uninstall_artifacts
if (zap_stanzas = @cask.artifacts.grep(Artifact::Zap)).empty?
opoo "No zap stanza present for Cask '#{@cask}'"
else
ohai "Dispatching zap stanza"
zap_stanzas.each do |stanza|
stanza.zap_phase(command: @command, verbose: verbose?, force: force?)
end
end
oh1 "Zapping Cask #{Formatter.identifier(@cask)}"
zap_stanzas = @cask.artifacts.grep(Artifact::Zap)
opoo "No zap stanza present for Cask '#{@cask}'" if zap_stanzas.empty?
uninstall_artifacts(clear: true, zap_stanzas:)
ohai "Removing all staged versions of Cask '#{@cask}'"
purge_caskroom_path
end
@@ -4,7 +4,7 @@
require "benchmark"
require "services/system"
RSpec.shared_examples "#uninstall_phase or #zap_phase" do
RSpec.shared_examples "uninstall/zap directive dispatch" do
extend Test::Helper::TestEach
subject { artifact }
@@ -18,6 +18,11 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
allow(Cask::Artifact::AbstractUninstall).to receive(:ancestor_bundle_ids).and_return([])
end
def dispatch_stanza(**options)
described_class.dispatch_directives([subject], **options)
described_class.dispatch_directives([subject], deferred: true, **options)
end
context "when using :launchctl" do
let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-launchctl")) }
let(:launchctl_list_cmd) { %w[/bin/launchctl list my.fancy.package.service] }
@@ -50,7 +55,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
must_succeed: false, sudo: false, sudo_as_root: false)
.and_return(instance_double(SystemCommand::Result, success?: true))
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end
it "works when job is owned by system" do
@@ -66,7 +71,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
must_succeed: false, sudo: true, sudo_as_root: true)
.and_return(instance_double(SystemCommand::Result, success?: true))
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end
it "does not fail when sudo removal fails" do
@@ -83,7 +88,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
.and_return(instance_double(SystemCommand::Result, success?: false))
expect do
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end.not_to raise_error
end
end
@@ -135,7 +140,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
must_succeed: false, sudo: true, sudo_as_root: true)
.and_return(instance_double(SystemCommand::Result, success?: true))
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end
it "returns the matching launchctl services" do
@@ -168,7 +173,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
expect(main_pkg).to receive(:uninstall)
expect(agent_pkg).to receive(:uninstall)
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end
end
@@ -192,7 +197,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
expect(subject).to receive(:system_command!)
.with("/bin/rm", args: ["-rf", "/Library/Extensions/FancyPackage.kext"], sudo: true, sudo_as_root: true)
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end
end
@@ -205,7 +210,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
allow(subject).to receive(:running?).with(bundle_id).and_return(true)
expect do
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end.to output(/Not logged into a GUI; skipping quitting application ID 'my.fancy.package.app'\./).to_stderr
end
@@ -216,7 +221,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
expect(subject).not_to receive(:quit)
expect do
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end.to output(/Skipping quitting application 'my.fancy.package.app' as `brew` is running inside it\./)
.to_stderr
end
@@ -230,7 +235,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
expect(subject).to receive(:running?).with(bundle_id).ordered.and_return(false)
expect do
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end.to output(/Application 'my.fancy.package.app' quit successfully\./).to_stdout
end
@@ -244,7 +249,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
allow(Timeout).to receive(:timeout).and_raise(Timeout::Error)
expect do
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end.to output(/Application 'my.fancy.package.app' did not quit\./).to_stderr
end
end
@@ -267,12 +272,10 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
expect(Process).to receive(:kill).with(signal, *unix_pids).and_return(1)
end
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end
it "does not send signal when upgrading or reinstalling" do
skip "only uninstall has upgrade and reinstall phases" if artifact_dsl_key == :zap
allow(subject).to receive(:running_processes).with(bundle_id)
.and_return(unix_pids.map { |pid| [pid, 0, bundle_id] })
@@ -280,8 +283,8 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
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)
dispatch_stanza(upgrade: true, command: fake_system_command)
dispatch_stanza(reinstall: true, command: fake_system_command)
end
end
@@ -315,7 +318,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
it "is supported" do
expect(paths).to all(exist)
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
paths.each do |path|
expect(path).not_to exist
@@ -344,7 +347,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
)
InstallHelper.install_without_artifacts(cask)
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end
end
end
@@ -360,7 +363,7 @@ RSpec.shared_examples "#uninstall_phase or #zap_phase" do
)
.and_return(instance_double(SystemCommand::Result, success?: true))
subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
dispatch_stanza(command: fake_system_command)
end
end
end
@@ -9,7 +9,7 @@ RSpec.describe Cask::Artifact::Uninstall, :cask do
describe "#uninstall_phase" do
let(:fake_system_command) { NeverSudoSystemCommand }
include_examples "#uninstall_phase or #zap_phase"
include_examples "uninstall/zap directive dispatch"
context "when sudo is disabled" do
let(:cask) { Cask::CaskLoader.load(cask_path("with-uninstall-launchctl")) }
@@ -4,8 +4,8 @@
require_relative "shared_examples/uninstall_zap"
RSpec.describe Cask::Artifact::Zap, :cask do
describe "#zap_phase" do
include_examples "#uninstall_phase or #zap_phase"
describe "directive dispatch" do
include_examples "uninstall/zap directive dispatch"
context "when using :rmdir" do
subject(:artifact) { cask.artifacts.find { |a| a.is_a?(described_class) } }
@@ -29,7 +29,7 @@ RSpec.describe Cask::Artifact::Zap, :cask do
expect(empty_directory_tree).to exist
expect(ds_store).to exist
artifact.zap_phase(command: fake_system_command)
described_class.dispatch_directives([artifact], command: fake_system_command, deferred: true)
expect(ds_store).not_to exist
expect(empty_directory).not_to exist
@@ -47,7 +47,9 @@ RSpec.describe Cask::Artifact::Zap, :cask do
after { FileUtils.rm_rf target }
it "leaves the link and its target alone" do
expect { artifact.zap_phase(command: fake_system_command) }.not_to raise_error
expect do
described_class.dispatch_directives([artifact], command: fake_system_command, deferred: true)
end.not_to raise_error
expect(empty_directory).to be_a_symlink
expect(target.join("nested")).to be_a_directory
@@ -5,6 +5,8 @@ require "cask/installer"
require "install"
RSpec.describe Cask::Installer, :cask do
let(:rmdir_directory) { Pathname("#{TEST_TMPDIR}/rmdir_app_directory") }
def stub_dmg_extraction
allow(UnpackStrategy::Dmg).to receive(:can_extract?).and_return(true)
allow_any_instance_of(UnpackStrategy::Dmg).to receive(:extract_nestedly) do |_strategy, to:, **|
@@ -512,6 +514,44 @@ RSpec.describe Cask::Installer, :cask do
expect(caffeine).not_to be_installed
expect(Pathname(caffeine.config.appdir).join("Caffeine.app")).not_to be_a_symlink
end
it "zap method runs zap directives alongside uninstall ones, removing files last" do
cask = Cask::CaskLoader.load(cask_path("with-uninstall-and-zap"))
dispatched = []
allow_any_instance_of(Cask::Artifact::AbstractUninstall)
.to receive(:dispatch_uninstall_directive) do |artifact, directive, **|
dispatched << [artifact.class.dsl_key, directive] if artifact.directives[directive].present?
end
allow_any_instance_of(Cask::Artifact::UninstallPostflightSteps)
.to receive(:uninstall_phase) { dispatched << [:uninstall_postflight] }
described_class.new(cask, command: NeverSudoSystemCommand).zap
expect(dispatched).to eq [
[:uninstall, :quit], [:zap, :quit], [:zap, :script], [:uninstall, :delete],
[:uninstall_postflight], [:zap, :pkgutil], [:zap, :delete], [:zap, :trash]
]
end
it "zap method tolerates an artifact target removed by an uninstall directive" do
cask = Cask::CaskLoader.load(cask_path("with-uninstall-delete-app"))
described_class.new(cask, command: NeverSudoSystemCommand).install
expect { described_class.new(cask, command: NeverSudoSystemCommand).zap }.not_to raise_error
end
context "when an `rmdir:` directory holds an artifact" do
after { FileUtils.rm_rf rmdir_directory }
it "zap method removes directories emptied by removing artifacts" do
cask = Cask::CaskLoader.load(cask_path("with-uninstall-rmdir-app"))
described_class.new(cask).install
described_class.new(cask).zap
expect(rmdir_directory).not_to exist
end
end
end
describe "#backup" do
@@ -530,6 +570,20 @@ RSpec.describe Cask::Installer, :cask do
end
describe "uninstall" do
context "with an `uninstall rmdir:` directory holding an artifact" do
after { FileUtils.rm_rf rmdir_directory }
it "removes the directory once the artifact is removed" do
cask = Cask::CaskLoader.load(cask_path("with-uninstall-rmdir-app"))
described_class.new(cask).install
described_class.new(cask).uninstall
expect(rmdir_directory/"nested").not_to exist
expect(rmdir_directory).to exist
end
end
it "fully uninstalls a Cask" do
caffeine = Cask::CaskLoader.load(cask_path("local-caffeine"))
installer = described_class.new(caffeine)
+1 -1
View File
@@ -31,9 +31,9 @@ RSpec.describe Cask::Reinstall, :cask do
Cask::Installer.new(caffeine).install
output = Regexp.new <<~EOS
==> Zapping Cask local-caffeine
==> Backing up App 'Caffeine.app' to '.*Caffeine.app'
==> Removing App '.*Caffeine.app'
==> Dispatching zap stanza
==> Trashing files:
.*org.example.caffeine.plist
==> Removing all staged versions of Cask 'local-caffeine'
@@ -0,0 +1,27 @@
# typed: false
cask "with-uninstall-and-zap" do
version "1.2.3"
sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"
url "file://#{TEST_FIXTURE_DIR}/cask/MyFancyPkg.zip"
homepage "https://brew.sh/fancy-pkg"
pkg "MyFancyPkg/Fancy.pkg"
uninstall_postflight_steps do
remove "uninstall-postflight-marker"
end
uninstall quit: "my.fancy.package.app",
delete: "#{TEST_TMPDIR}/absolute_path"
zap quit: "my.fancy.package.app",
script: {
executable: "MyFancyPkg/FancyUninstaller.tool",
args: ["--please"],
},
pkgutil: "my.fancy.package.*",
delete: "#{TEST_TMPDIR}/zap_delete_path",
trash: "#{TEST_TMPDIR}/zap_path"
end
@@ -0,0 +1,15 @@
# typed: false
cask "with-uninstall-delete-app" do
version "1.2.3"
sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94"
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip"
homepage "https://brew.sh/"
app "Caffeine.app"
uninstall delete: "#{appdir}/Caffeine.app"
zap trash: "#{TEST_TMPDIR}/with_uninstall_delete_app_zap"
end
@@ -0,0 +1,15 @@
# typed: false
cask "with-uninstall-rmdir-app" do
version "1.2.3"
sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94"
url "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip"
homepage "https://brew.sh/"
app "Caffeine.app", target: "#{TEST_TMPDIR}/rmdir_app_directory/nested/Caffeine.app"
uninstall rmdir: "#{TEST_TMPDIR}/rmdir_app_directory/nested"
zap rmdir: "#{TEST_TMPDIR}/rmdir_app_directory"
end