From 33d3a5d7284c30f326d0f9006415d2a47ff6acf9 Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Wed, 9 Nov 2022 13:21:49 +1100 Subject: [PATCH 01/10] cask/artifiact/abstract_uninstall: allow wildcard entries for launchctl --- .../cask/artifact/abstract_uninstall.rb | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 19015c00d1..15427c9e6f 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -90,7 +90,22 @@ module Cask # :launchctl must come before :quit/:signal for cases where app would instantly re-launch def uninstall_launchctl(*services, command: nil, **_) booleans = [false, true] + + all_services = [] + + # if launchctl item contains a wildcard, find matching process(es) services.each do |service| + all_services.push(service) + next unless /\*/.match?(service) + + find_launchctl_with_wildcard(service) + .map { |_, _, id| id } + .each do |match| + all_services.push(match) + end + end + + all_services.each do |service| ohai "Removing launchctl service #{service}" booleans.each do |with_sudo| plist_status = command.run( @@ -268,6 +283,16 @@ module Cask end end + def find_launchctl_with_wildcard(search) + system_command!("/bin/launchctl", args: ["list"]) + .stdout.lines.drop(1) + .map { |line| line.chomp.split("\t") } + .map { |pid, state, id| [pid.to_i, state.to_i, id] } + .select do |(pid, _, id)| + pid.nonzero? && /\A#{Regexp.escape(search).gsub("\\*", ".*")}\Z/.match?(id) + end + end + # :kext should be unloaded before attempting to delete the relevant file def uninstall_kext(*kexts, command: nil, **_) kexts.each do |kext| From 836efe621fc842ee5b7ff1a42f9b8bcba5020a86 Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Mon, 5 Dec 2022 15:08:23 +1100 Subject: [PATCH 02/10] cask/artifiact/abstract_uninstall: allow wildcard entries for launchctl --- .../cask/artifact/abstract_uninstall.rb | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 15427c9e6f..50f4aab94a 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -95,14 +95,10 @@ module Cask # if launchctl item contains a wildcard, find matching process(es) services.each do |service| - all_services.push(service) - next unless /\*/.match?(service) + all_services << service + next unless service.include?("*") - find_launchctl_with_wildcard(service) - .map { |_, _, id| id } - .each do |match| - all_services.push(match) - end + all_services += find_launchctl_with_wildcard(service) end all_services.each do |service| @@ -284,13 +280,13 @@ module Cask end def find_launchctl_with_wildcard(search) + regex = Regexp.escape(search).gsub("\\*", ".*") system_command!("/bin/launchctl", args: ["list"]) .stdout.lines.drop(1) - .map { |line| line.chomp.split("\t") } - .map { |pid, state, id| [pid.to_i, state.to_i, id] } - .select do |(pid, _, id)| - pid.nonzero? && /\A#{Regexp.escape(search).gsub("\\*", ".*")}\Z/.match?(id) - end + .map do |line| + pid, _state, id = line.chomp.split("\t") + id if pid.to_i.nonzero? && id.match?(regex) + end.compact end # :kext should be unloaded before attempting to delete the relevant file From e8e6ee30b48a7f9df0bda5adc959fb0eaaaae13a Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Tue, 13 Dec 2022 00:00:43 +1100 Subject: [PATCH 03/10] add initial tests --- .../cask/artifact/abstract_uninstall.rb | 2 +- .../artifact/shared_examples/uninstall_zap.rb | 25 ++++++++++++++----- .../with-uninstall-launchctl-wildcard.rb | 11 ++++++++ .../cask/Casks/with-zap-launchctl-wildcard.rb | 11 ++++++++ 4 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-launchctl-wildcard.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-launchctl-wildcard.rb diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 50f4aab94a..c6750d27ef 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -95,7 +95,7 @@ module Cask # if launchctl item contains a wildcard, find matching process(es) services.each do |service| - all_services << service + all_services << service unless service.include?("*") next unless service.include?("*") all_services += find_launchctl_with_wildcard(service) 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 3d97672cc4..de031170b9 100644 --- a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb +++ b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb @@ -61,6 +61,19 @@ shared_examples "#uninstall_phase or #zap_phase" do end end + context "using launchctl with regex" do + let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-launchctl-wildcard")) } + let(:launchctl_list_cmd) { %w[/bin/launchctl list] } + + it "searches running launchctl items" do + allow(fake_system_command).to receive(:run) + .with("/bin/launchctl", args: ["list"], print_stderr: false, sudo: false) + .and_return(instance_double(SystemCommand::Result)) + + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) + end + end + context "using :pkgutil" do let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-pkgutil")) } @@ -117,9 +130,9 @@ shared_examples "#uninstall_phase or #zap_phase" do allow(User.current).to receive(:gui?).and_return false allow(subject).to receive(:running?).with(bundle_id).and_return(true) - expect { + expect do subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) - }.to output(/Not logged into a GUI; skipping quitting application ID 'my.fancy.package.app'\./).to_stderr + end.to output(/Not logged into a GUI; skipping quitting application ID 'my.fancy.package.app'\./).to_stderr end it "quits a running application" do @@ -130,9 +143,9 @@ shared_examples "#uninstall_phase or #zap_phase" do .and_return(instance_double("SystemCommand::Result", success?: true)) expect(subject).to receive(:running?).with(bundle_id).ordered.and_return(false) - expect { + expect do subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) - }.to output(/Application 'my.fancy.package.app' quit successfully\./).to_stdout + end.to output(/Application 'my.fancy.package.app' quit successfully\./).to_stdout end it "tries to quit the application for 10 seconds" do @@ -143,9 +156,9 @@ shared_examples "#uninstall_phase or #zap_phase" do .and_return(instance_double("SystemCommand::Result", success?: false)) time = Benchmark.measure do - expect { + expect do subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) - }.to output(/Application 'my.fancy.package.app' did not quit\./).to_stderr + end.to output(/Application 'my.fancy.package.app' did not quit\./).to_stderr end expect(time.real).to be_within(3).of(10) diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-launchctl-wildcard.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-launchctl-wildcard.rb new file mode 100644 index 0000000000..89d14231b6 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-launchctl-wildcard.rb @@ -0,0 +1,11 @@ +cask "with-uninstall-launchctl-wildcard" do + version "1.2.3" + sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" + + url "file://#{TEST_FIXTURE_DIR}/cask/MyFancyApp.zip" + homepage "https://brew.sh/fancy" + + app "Fancy.app" + + uninstall launchctl: "my.fancy.package.service.*" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-launchctl-wildcard.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-launchctl-wildcard.rb new file mode 100644 index 0000000000..598dc549a7 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-zap-launchctl-wildcard.rb @@ -0,0 +1,11 @@ +cask "with-zap-launchctl-wildcard" do + version "1.2.3" + sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" + + url "file://#{TEST_FIXTURE_DIR}/cask/MyFancyApp.zip" + homepage "https://brew.sh/fancy" + + app "Fancy.app" + + zap launchctl: "my.fancy.package.service.*" +end From 1ddd130ef9c475b69a7896774c8a86172cd86b99 Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Mon, 19 Dec 2022 14:35:20 +1100 Subject: [PATCH 04/10] add comment to .drop(), don't add blank lines to services array --- Library/Homebrew/cask/artifact/abstract_uninstall.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index c6750d27ef..6b0097d5ff 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -98,7 +98,10 @@ module Cask all_services << service unless service.include?("*") next unless service.include?("*") - all_services += find_launchctl_with_wildcard(service) + found_services = find_launchctl_with_wildcard(service) + next if found_services.blank? + + found_services.each { |service| all_services += service} end all_services.each do |service| @@ -282,6 +285,7 @@ module Cask def find_launchctl_with_wildcard(search) regex = Regexp.escape(search).gsub("\\*", ".*") system_command!("/bin/launchctl", args: ["list"]) + # skip stdout column headers .stdout.lines.drop(1) .map do |line| pid, _state, id = line.chomp.split("\t") From 01f865f93a04b305d668121c7e8c570849017a8b Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Mon, 19 Dec 2022 14:35:31 +1100 Subject: [PATCH 05/10] update test --- .../test/cask/artifact/shared_examples/uninstall_zap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 de031170b9..1858133668 100644 --- a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb +++ b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb @@ -66,7 +66,7 @@ shared_examples "#uninstall_phase or #zap_phase" do let(:launchctl_list_cmd) { %w[/bin/launchctl list] } it "searches running launchctl items" do - allow(fake_system_command).to receive(:run) + expect(fake_system_command).to receive(:run) .with("/bin/launchctl", args: ["list"], print_stderr: false, sudo: false) .and_return(instance_double(SystemCommand::Result)) From 84f288be79b0a8bd043fa74e02a643f154ea62e8 Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Mon, 19 Dec 2022 14:39:48 +1100 Subject: [PATCH 06/10] fix style --- Library/Homebrew/cask/artifact/abstract_uninstall.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 6b0097d5ff..961c1701a1 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -285,8 +285,7 @@ module Cask def find_launchctl_with_wildcard(search) regex = Regexp.escape(search).gsub("\\*", ".*") system_command!("/bin/launchctl", args: ["list"]) - # skip stdout column headers - .stdout.lines.drop(1) + .stdout.lines.drop(1) # skip stdout column headers .map do |line| pid, _state, id = line.chomp.split("\t") id if pid.to_i.nonzero? && id.match?(regex) From 2614a294198aaeff743798f0830942da0a308f4a Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Mon, 19 Dec 2022 14:43:16 +1100 Subject: [PATCH 07/10] fix style --- Library/Homebrew/cask/artifact/abstract_uninstall.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 961c1701a1..af7e6b4ddf 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -101,7 +101,7 @@ module Cask found_services = find_launchctl_with_wildcard(service) next if found_services.blank? - found_services.each { |service| all_services += service} + found_services.each { |found_service| all_services += found_service } end all_services.each do |service| From db9d48a7eca6bec74b73d5e575b55360b6be8e3f Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Wed, 28 Dec 2022 12:50:16 +1100 Subject: [PATCH 08/10] fix tests --- .../cask/artifact/abstract_uninstall.rb | 2 +- .../artifact/shared_examples/uninstall_zap.rb | 36 ++++++++++++++++--- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index af7e6b4ddf..5a92511b18 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -101,7 +101,7 @@ module Cask found_services = find_launchctl_with_wildcard(service) next if found_services.blank? - found_services.each { |found_service| all_services += found_service } + found_services.each { |found_service| all_services << found_service } end all_services.each do |service| 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 1858133668..dd6d0c0a7e 100644 --- a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb +++ b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb @@ -61,13 +61,39 @@ shared_examples "#uninstall_phase or #zap_phase" do end end - context "using launchctl with regex" do + context "using :launchctl with regex wildcard" do let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-launchctl-wildcard")) } - let(:launchctl_list_cmd) { %w[/bin/launchctl list] } + let(:launchctl_regex) { "my.fancy.package.service.*" } + let(:unknown_response) { "launchctl list returned unknown response\n" } + let(:service_info) do + <<~EOS + { + "LimitLoadToSessionType" = "Aqua"; + "Label" = "my.fancy.package.service.12345"; + "TimeOut" = 30; + "OnDemand" = true; + "LastExitStatus" = 0; + "ProgramArguments" = ( + "argument"; + ); + }; + EOS + end - it "searches running launchctl items" do - expect(fake_system_command).to receive(:run) - .with("/bin/launchctl", args: ["list"], print_stderr: false, sudo: false) + it "searches installed launchctl items" do + expect(subject).to receive(:find_launchctl_with_wildcard) + .with(launchctl_regex) + .and_return(["my.fancy.package.service.12345"]) + + allow(fake_system_command).to receive(:run) + .with("/bin/launchctl", args: ["list", "my.fancy.package.service.12345"], print_stderr: false, sudo: false) + .and_return(instance_double(SystemCommand::Result, stdout: unknown_response)) + allow(fake_system_command).to receive(:run) + .with("/bin/launchctl", args: ["list", "my.fancy.package.service.12345"], print_stderr: false, sudo: true) + .and_return(instance_double(SystemCommand::Result, stdout: service_info)) + + expect(fake_system_command).to receive(:run!) + .with("/bin/launchctl", args: ["remove", "my.fancy.package.service.12345"], sudo: true) .and_return(instance_double(SystemCommand::Result)) subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) From 3bdab156c298572cd540f81e4b2efad1dbcdbdd2 Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Thu, 29 Dec 2022 12:54:21 +1100 Subject: [PATCH 09/10] add additional test --- .../artifact/shared_examples/uninstall_zap.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) 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 dd6d0c0a7e..06bacb96e0 100644 --- a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb +++ b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb @@ -79,6 +79,15 @@ shared_examples "#uninstall_phase or #zap_phase" do }; EOS end + let(:launchctl_list) do + <<~EOS + PID Status Label + 1111 0 my.fancy.package.service.12345 + - 0 com.apple.SafariHistoryServiceAgent + - 0 com.apple.progressd + 555 0 my.fancy.package.service.test + EOS + end it "searches installed launchctl items" do expect(subject).to receive(:find_launchctl_with_wildcard) @@ -98,6 +107,16 @@ shared_examples "#uninstall_phase or #zap_phase" do subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end + + it "returns the matching launchctl services" do + expect(subject).to receive(:system_command!) + .with("/bin/launchctl", args: ["list"]) + .and_return(instance_double(SystemCommand::Result, stdout: launchctl_list)) + + expect(subject.send(:find_launchctl_with_wildcard, + "my.fancy.package.service.*")).to eq(["my.fancy.package.service.12345", + "my.fancy.package.service.test"]) + end end context "using :pkgutil" do From 3869bf156cfb4d39572283a9c5ec6714ff7b0c18 Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Thu, 29 Dec 2022 12:54:59 +1100 Subject: [PATCH 10/10] change line splitting method --- .../cask/artifact/abstract_uninstall.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 5a92511b18..5c2fab1884 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -145,6 +145,16 @@ module Cask end end + def find_launchctl_with_wildcard(search) + regex = Regexp.escape(search).gsub("\\*", ".*") + system_command!("/bin/launchctl", args: ["list"]) + .stdout.lines.drop(1) # skip stdout column headers + .map do |line| + pid, _state, id = line.chomp.split(/\s+/) + id if pid.to_i.nonzero? && id.match?(regex) + end.compact + end + sig { returns(String) } def automation_access_instructions <<~EOS @@ -282,16 +292,6 @@ module Cask end end - def find_launchctl_with_wildcard(search) - regex = Regexp.escape(search).gsub("\\*", ".*") - system_command!("/bin/launchctl", args: ["list"]) - .stdout.lines.drop(1) # skip stdout column headers - .map do |line| - pid, _state, id = line.chomp.split("\t") - id if pid.to_i.nonzero? && id.match?(regex) - end.compact - end - # :kext should be unloaded before attempting to delete the relevant file def uninstall_kext(*kexts, command: nil, **_) kexts.each do |kext|