From 463e9241934c3bf24b07457568e859cd672510d3 Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Thu, 10 Sep 2026 10:58:27 +1000 Subject: [PATCH] cask: run zap and uninstall directives together - `brew uninstall --zap` ran the whole `zap` stanza only after every artifact had been removed, so a `zap` `script:` inside the app could never run: `mullvad-vpn` and `private-internet-access` both raise. - Run `zap` directives up to `script:` in the `uninstall` stanza's pass, before the artifacts are removed, `uninstall` arguments first. - Keep `zap` `pkgutil:`, `delete:` and `trash:` after the artifacts and `uninstall_postflight` steps, as before: `betwixt`'s postflight reads a certificate that its `zap` stanza trashes. - Run `rmdir:` for both stanzas after the artifacts are removed, so a directory that held only those artifacts is seen as empty. - Pass `clear: true` when zapping, as `brew uninstall` already does, so an artifact target removed by `uninstall` `delete:` is skipped rather than raising for casks such as `cloudnet`, `mqttfx` and `pokerstars`. - `zap` `signal:` now follows the same upgrade and reinstall rules as `uninstall` `signal:`. - Head the output with `Zapping Cask`, like `Uninstalling Cask`, as `Dispatching zap stanza` no longer marks a separate pass. --- .../cask/artifact/abstract_uninstall.rb | 62 +++++++++--- Library/Homebrew/cask/artifact/uninstall.rb | 33 +------ Library/Homebrew/cask/artifact/zap.rb | 14 +-- Library/Homebrew/cask/installer.rb | 99 +++++++++---------- .../artifact/shared_examples/uninstall_zap.rb | 41 ++++---- .../test/cask/artifact/uninstall_spec.rb | 2 +- .../Homebrew/test/cask/artifact/zap_spec.rb | 10 +- Library/Homebrew/test/cask/installer_spec.rb | 54 ++++++++++ Library/Homebrew/test/cask/reinstall_spec.rb | 2 +- .../cask/Casks/with-uninstall-and-zap.rb | 27 +++++ .../cask/Casks/with-uninstall-delete-app.rb | 15 +++ .../cask/Casks/with-uninstall-rmdir-app.rb | 15 +++ 12 files changed, 241 insertions(+), 133 deletions(-) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-and-zap.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete-app.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-rmdir-app.rb diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 6bab703063..ec5c843420 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -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 diff --git a/Library/Homebrew/cask/artifact/uninstall.rb b/Library/Homebrew/cask/artifact/uninstall.rb index 4636b07cbb..d882e07643 100644 --- a/Library/Homebrew/cask/artifact/uninstall.rb +++ b/Library/Homebrew/cask/artifact/uninstall.rb @@ -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 diff --git a/Library/Homebrew/cask/artifact/zap.rb b/Library/Homebrew/cask/artifact/zap.rb index 336312b1bb..8b0545b79f 100644 --- a/Library/Homebrew/cask/artifact/zap.rb +++ b/Library/Homebrew/cask/artifact/zap.rb @@ -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 diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index fb3ea53311..708242188f 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -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 diff --git a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb index 61801225f7..32959970c1 100644 --- a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb +++ b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb @@ -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 diff --git a/Library/Homebrew/test/cask/artifact/uninstall_spec.rb b/Library/Homebrew/test/cask/artifact/uninstall_spec.rb index 9f838a7832..ae697bb049 100644 --- a/Library/Homebrew/test/cask/artifact/uninstall_spec.rb +++ b/Library/Homebrew/test/cask/artifact/uninstall_spec.rb @@ -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")) } diff --git a/Library/Homebrew/test/cask/artifact/zap_spec.rb b/Library/Homebrew/test/cask/artifact/zap_spec.rb index 7f04af3bde..fc48dcde3b 100644 --- a/Library/Homebrew/test/cask/artifact/zap_spec.rb +++ b/Library/Homebrew/test/cask/artifact/zap_spec.rb @@ -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 diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index e673c8177d..1252897e82 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -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) diff --git a/Library/Homebrew/test/cask/reinstall_spec.rb b/Library/Homebrew/test/cask/reinstall_spec.rb index 040004ffce..e821042546 100644 --- a/Library/Homebrew/test/cask/reinstall_spec.rb +++ b/Library/Homebrew/test/cask/reinstall_spec.rb @@ -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' diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-and-zap.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-and-zap.rb new file mode 100644 index 0000000000..7df871b770 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-and-zap.rb @@ -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 diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete-app.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete-app.rb new file mode 100644 index 0000000000..8a0cbd3bb8 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-delete-app.rb @@ -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 diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-rmdir-app.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-rmdir-app.rb new file mode 100644 index 0000000000..158ee6d9b9 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-rmdir-app.rb @@ -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