diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 0714283c4e..64e909b666 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -94,8 +94,10 @@ module Cask command.run!("/bin/launchctl", args: ["remove", service], sudo: with_sudo) sleep 1 end - paths = ["/Library/LaunchAgents/#{service}.plist", - "/Library/LaunchDaemons/#{service}.plist"] + paths = [ + "/Library/LaunchAgents/#{service}.plist", + "/Library/LaunchDaemons/#{service}.plist", + ] paths.each { |elt| elt.prepend(ENV["HOME"]) } unless with_sudo paths = paths.map { |elt| Pathname(elt) }.select(&:exist?) paths.each do |path| @@ -105,46 +107,78 @@ module Cask next unless Pathname(service).exist? command.run!("/bin/launchctl", args: ["unload", "-w", "--", service], sudo: with_sudo) - command.run!("/bin/rm", args: ["-f", "--", service], sudo: with_sudo) + command.run!("/bin/rm", args: ["-f", "--", service], sudo: with_sudo) sleep 1 end end end - def running_processes(bundle_id, command: nil) - command.run!("/bin/launchctl", args: ["list"]).stdout.lines - .map { |line| line.chomp.split("\t") } - .map { |pid, state, id| [pid.to_i, state.to_i, id] } - .select do |(pid, _, id)| - pid.nonzero? && id.match?(/^#{Regexp.escape(bundle_id)}($|\.\d+)/) - end + def running_processes(bundle_id) + system_command!("/bin/launchctl", args: ["list"]) + .stdout.lines + .map { |line| line.chomp.split("\t") } + .map { |pid, state, id| [pid.to_i, state.to_i, id] } + .select do |(pid, _, id)| + pid.nonzero? && id.match?(/^#{Regexp.escape(bundle_id)}($|\.\d+)/) + end end # :quit/:signal must come before :kext so the kext will not be in use by a running process def uninstall_quit(*bundle_ids, command: nil, **_) bundle_ids.each do |bundle_id| - next if running_processes(bundle_id, command: command).empty? + next if running_processes(bundle_id).empty? unless User.current.gui? ohai "Not logged into a GUI; skipping quitting application ID '#{bundle_id}'." next end - ohai "Quitting application ID '#{bundle_id}'." - command.run!("/usr/bin/osascript", args: ["-e", %Q(tell application id "#{bundle_id}" to quit)], sudo: true) + ohai "Quitting application '#{bundle_id}'..." begin - Timeout.timeout(3) do + Timeout.timeout(10) do Kernel.loop do - break if running_processes(bundle_id, command: command).empty? + next unless quit(bundle_id).success? + if running_processes(bundle_id).empty? + puts "Application '#{bundle_id}' quit successfully." + break + end end end rescue Timeout::Error + opoo "Application '#{bundle_id}' did not quit." next end end end + def quit(bundle_id) + script = <<~JAVASCRIPT + 'use strict'; + + ObjC.import('stdlib') + + function run(argv) { + var app = Application(argv[0]) + + try { + app.quit() + } catch (err) { + if (app.running()) { + $.exit(1) + } + } + + $.exit(0) + } + JAVASCRIPT + + system_command "osascript", args: ["-l", "JavaScript", "-e", script, bundle_id], + print_stderr: false, + sudo: true + end + private :quit + # :signal should come after :quit so it can be used as a backup when :quit fails def uninstall_signal(*signals, command: nil, **_) signals.each do |pair| @@ -154,7 +188,7 @@ module Cask signal, bundle_id = pair ohai "Signalling '#{signal}' to application ID '#{bundle_id}'" - pids = running_processes(bundle_id, command: command).map(&:first) + pids = running_processes(bundle_id).map(&:first) next unless pids.any? # Note that unlike :quit, signals are sent from the current user (not @@ -174,13 +208,12 @@ module Cask login_items.each do |name| ohai "Removing login item #{name}" - command.run!( - "/usr/bin/osascript", + system_command!( + "osascript", args: [ "-e", %Q(tell application "System Events" to delete every login item whose name is "#{name}"), ], - sudo: false, ) sleep 1 end @@ -190,14 +223,14 @@ module Cask def uninstall_kext(*kexts, command: nil, **_) kexts.each do |kext| ohai "Unloading kernel extension #{kext}" - is_loaded = command.run!("/usr/sbin/kextstat", args: ["-l", "-b", kext], sudo: true).stdout + is_loaded = system_command!("/usr/sbin/kextstat", args: ["-l", "-b", kext], sudo: true).stdout if is_loaded.length > 1 - command.run!("/sbin/kextunload", args: ["-b", kext], sudo: true) + system_command!("/sbin/kextunload", args: ["-b", kext], sudo: true) sleep 1 end - command.run!("/usr/sbin/kextfind", args: ["-b", kext], sudo: true).stdout.chomp.lines.each do |kext_path| + system_command!("/usr/sbin/kextfind", args: ["-b", kext], sudo: true).stdout.chomp.lines.each do |kext_path| ohai "Removing kernel extension #{kext_path}" - command.run!("/bin/rm", args: ["-rf", kext_path], sudo: true) + system_command!("/bin/rm", args: ["-rf", kext_path], sudo: true) end end end @@ -287,7 +320,7 @@ module Cask end def trash_paths(*paths, command: nil, **_) - result = command.run!("/usr/bin/osascript", args: ["-e", <<~APPLESCRIPT, *paths]) + result = command.run!("osascript", args: ["-e", <<~APPLESCRIPT, *paths]) on run argv repeat with i from 1 to (count argv) set item i of argv to (item i of argv as POSIX file) diff --git a/Library/Homebrew/test/cask/artifact/uninstall_zap_shared_examples.rb b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb similarity index 59% rename from Library/Homebrew/test/cask/artifact/uninstall_zap_shared_examples.rb rename to Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb index 5593679db3..6a6a54a9eb 100644 --- a/Library/Homebrew/test/cask/artifact/uninstall_zap_shared_examples.rb +++ b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb @@ -1,10 +1,12 @@ +require "benchmark" + shared_examples "#uninstall_phase or #zap_phase" do + subject { artifact } + let(:artifact_dsl_key) { described_class.dsl_key } let(:artifact) { cask.artifacts.find { |a| a.is_a?(described_class) } } let(:fake_system_command) { FakeSystemCommand } - subject { artifact.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) } - context "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] } @@ -38,7 +40,7 @@ shared_examples "#uninstall_phase or #zap_phase" do FakeSystemCommand.expects_command(launchctl_remove_cmd) - subject + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end it "works when job is owned by system" do @@ -54,7 +56,7 @@ shared_examples "#uninstall_phase or #zap_phase" do FakeSystemCommand.expects_command(sudo(launchctl_remove_cmd)) - subject + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end end @@ -80,7 +82,7 @@ shared_examples "#uninstall_phase or #zap_phase" do expect(main_pkg).to receive(:uninstall) expect(agent_pkg).to receive(:uninstall) - subject + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end end @@ -89,43 +91,65 @@ shared_examples "#uninstall_phase or #zap_phase" do let(:kext_id) { "my.fancy.package.kernelextension" } it "is supported" do - FakeSystemCommand.stubs_command( - sudo(%W[/usr/sbin/kextstat -l -b #{kext_id}]), "loaded" - ) + allow(subject).to receive(:system_command!) + .with("/usr/sbin/kextstat", args: ["-l", "-b", kext_id], sudo: true) + .and_return(instance_double("SystemCommand::Result", stdout: "loaded")) - FakeSystemCommand.expects_command( - sudo(%W[/sbin/kextunload -b #{kext_id}]), - ) + expect(subject).to receive(:system_command!) + .with("/sbin/kextunload", args: ["-b", kext_id], sudo: true) + .and_return(instance_double("SystemCommand::Result")) - FakeSystemCommand.expects_command( - sudo(%W[/usr/sbin/kextfind -b #{kext_id}]), "/Library/Extensions/FancyPackage.kext\n" - ) + expect(subject).to receive(:system_command!) + .with("/usr/sbin/kextfind", args: ["-b", kext_id], sudo: true) + .and_return(instance_double("SystemCommand::Result", stdout: "/Library/Extensions/FancyPackage.kext\n")) - FakeSystemCommand.expects_command( - sudo(["/bin/rm", "-rf", "/Library/Extensions/FancyPackage.kext"]), - ) + expect(subject).to receive(:system_command!) + .with("/bin/rm", args: ["-rf", "/Library/Extensions/FancyPackage.kext"], sudo: true) - subject + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end end context "using :quit" do let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-quit")) } let(:bundle_id) { "my.fancy.package.app" } - let(:quit_application_script) do - %Q(tell application id "#{bundle_id}" to quit) + + it "is skipped when the user is not a GUI user" do + allow(User.current).to receive(:gui?).and_return false + allow(subject).to receive(:running_processes).with(bundle_id).and_return([[0, "", bundle_id]]) + + expect { + 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_stdout end - it "is supported" do - FakeSystemCommand.stubs_command( - %w[/bin/launchctl list], "999\t0\t#{bundle_id}\n" - ) + it "quits a running application" do + allow(User.current).to receive(:gui?).and_return true - FakeSystemCommand.stubs_command( - %w[/bin/launchctl list], - ) + expect(subject).to receive(:running_processes).with(bundle_id).ordered.and_return([[0, "", bundle_id]]) + expect(subject).to receive(:quit).with(bundle_id) + .and_return(instance_double("SystemCommand::Result", success?: true)) + expect(subject).to receive(:running_processes).with(bundle_id).ordered.and_return([]) - subject + expect { + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) + }.to output(/Application 'my.fancy.package.app' quit successfully\./).to_stdout + end + + it "tries to quit the application for 10 seconds" do + allow(User.current).to receive(:gui?).and_return true + + allow(subject).to receive(:running_processes).with(bundle_id).and_return([[0, "", bundle_id]]) + allow(subject).to receive(:quit).with(bundle_id) + .and_return(instance_double("SystemCommand::Result", success?: false)) + + time = Benchmark.measure do + expect { + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) + }.to output(/Application 'my.fancy.package.app' did not quit\./).to_stderr + end + + expect(time.real).to be_within(3).of(10) end end @@ -136,15 +160,14 @@ shared_examples "#uninstall_phase or #zap_phase" do let(:unix_pids) { [12_345, 67_890] } it "is supported" do - FakeSystemCommand.stubs_command( - %w[/bin/launchctl list], unix_pids.map { |pid| [pid, 0, bundle_id].join("\t") }.join("\n") - ) + 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).to receive(:kill).with(signal, *unix_pids) end - subject + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end end @@ -161,7 +184,7 @@ shared_examples "#uninstall_phase or #zap_phase" do let(:fake_system_command) { NeverSudoSystemCommand } let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-#{directive}")) } - around(:each) do |example| + around do |example| begin ENV["HOME"] = dir @@ -173,18 +196,21 @@ shared_examples "#uninstall_phase or #zap_phase" do end end - before(:each) do + before do + # rubocop:disable RSpec/AnyInstance allow_any_instance_of(Cask::Artifact::AbstractUninstall).to receive(:trash_paths) .and_wrap_original do |method, *args| - result = method.call(*args) - FileUtils.rm_rf result.stdout.split("\0") + method.call(*args).tap do |result| + FileUtils.rm_rf result.stdout.split("\0") + end end + # rubocop:enable RSpec/AnyInstance end it "is supported" do expect(paths).to all(exist) - subject + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) paths.each do |path| expect(path).not_to exist @@ -199,12 +225,12 @@ shared_examples "#uninstall_phase or #zap_phase" do let(:empty_directory) { Pathname.new("#{TEST_TMPDIR}/empty_directory_path") } let(:ds_store) { empty_directory.join(".DS_Store") } - before(:each) do + before do empty_directory.mkdir FileUtils.touch ds_store end - after(:each) do + after do FileUtils.rm_rf empty_directory end @@ -212,7 +238,7 @@ shared_examples "#uninstall_phase or #zap_phase" do expect(empty_directory).to exist expect(ds_store).to exist - subject + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) expect(ds_store).not_to exist expect(empty_directory).not_to exist @@ -243,7 +269,7 @@ shared_examples "#uninstall_phase or #zap_phase" do ) InstallHelper.install_without_artifacts(cask) - subject + subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command) end end end @@ -252,12 +278,14 @@ shared_examples "#uninstall_phase or #zap_phase" do let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-login-item")) } it "is supported" do - FakeSystemCommand.expects_command( - ["/usr/bin/osascript", "-e", 'tell application "System Events" to delete every login ' \ - 'item whose name is "Fancy"'], + expect(subject).to receive(:system_command!) + .with( + "osascript", + args: ["-e", 'tell application "System Events" to delete every login item whose name is "Fancy"'], ) + .and_return(instance_double("SystemCommand::Result")) - subject + subject.public_send(:"#{artifact_dsl_key}_phase", 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 febc881f83..cfee17fb85 100644 --- a/Library/Homebrew/test/cask/artifact/uninstall_spec.rb +++ b/Library/Homebrew/test/cask/artifact/uninstall_spec.rb @@ -1,4 +1,4 @@ -require_relative "uninstall_zap_shared_examples" +require_relative "shared_examples/uninstall_zap" describe Cask::Artifact::Uninstall, :cask do describe "#uninstall_phase" do diff --git a/Library/Homebrew/test/cask/artifact/zap_spec.rb b/Library/Homebrew/test/cask/artifact/zap_spec.rb index 035ec67d42..a5cef6c530 100644 --- a/Library/Homebrew/test/cask/artifact/zap_spec.rb +++ b/Library/Homebrew/test/cask/artifact/zap_spec.rb @@ -1,4 +1,4 @@ -require_relative "uninstall_zap_shared_examples" +require_relative "shared_examples/uninstall_zap" describe Cask::Artifact::Zap, :cask do describe "#zap_phase" do