From 2308f0c571c5864d771a659d16523f081d9c7e9a Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Mon, 1 Oct 2018 12:29:21 +0200 Subject: [PATCH] Skip quitting applications when not logged into GUI. --- .../cask/artifact/abstract_uninstall.rb | 8 ++++- Library/Homebrew/cask/artifact/pkg.rb | 8 ++--- Library/Homebrew/cask/caskroom.rb | 4 ++- Library/Homebrew/cask/staged.rb | 8 ++--- Library/Homebrew/cask/utils.rb | 7 ++-- Library/Homebrew/system_command.rb | 12 ++++--- .../Homebrew/test/cask/artifact/app_spec.rb | 2 +- .../spec/shared_examples/cask_staged.rb | 6 ++-- Library/Homebrew/test/utils/user_spec.rb | 36 +++++++++++++++++++ Library/Homebrew/utils/user.rb | 18 ++++++++++ 10 files changed, 84 insertions(+), 25 deletions(-) create mode 100644 Library/Homebrew/test/utils/user_spec.rb create mode 100644 Library/Homebrew/utils/user.rb diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index 679c459ec9..d325618631 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -1,5 +1,6 @@ require "timeout" +require "utils/user" require "cask/artifact/abstract_artifact" module Cask @@ -124,9 +125,14 @@ module Cask # :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| - ohai "Quitting application ID #{bundle_id}" next if running_processes(bundle_id, command: command).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) begin diff --git a/Library/Homebrew/cask/artifact/pkg.rb b/Library/Homebrew/cask/artifact/pkg.rb index 56b10d7bbe..825ad937a2 100644 --- a/Library/Homebrew/cask/artifact/pkg.rb +++ b/Library/Homebrew/cask/artifact/pkg.rb @@ -1,5 +1,6 @@ require "plist" +require "utils/user" require "cask/artifact/abstract_artifact" require "extend/hash_validator" @@ -50,11 +51,10 @@ module Cask end with_choices_file do |choices_path| args << "-applyChoiceChangesXML" << choices_path if choices_path - logged_in_user = Utils.current_user env = { - "LOGNAME" => logged_in_user, - "USER" => logged_in_user, - "USERNAME" => logged_in_user, + "LOGNAME" => User.current, + "USER" => User.current, + "USERNAME" => User.current, } command.run!("/usr/sbin/installer", sudo: true, args: args, print_stdout: true, env: env) end diff --git a/Library/Homebrew/cask/caskroom.rb b/Library/Homebrew/cask/caskroom.rb index 486456b677..73a1b0ae8d 100644 --- a/Library/Homebrew/cask/caskroom.rb +++ b/Library/Homebrew/cask/caskroom.rb @@ -1,3 +1,5 @@ +require "utils/user" + module Cask module Caskroom module_function @@ -18,7 +20,7 @@ module Cask SystemCommand.run("/bin/mkdir", args: ["-p", path], sudo: sudo) SystemCommand.run("/bin/chmod", args: ["g+rwx", path], sudo: sudo) - SystemCommand.run("/usr/sbin/chown", args: [Utils.current_user, path], sudo: sudo) + SystemCommand.run("/usr/sbin/chown", args: [User.current, path], sudo: sudo) SystemCommand.run("/usr/bin/chgrp", args: ["admin", path], sudo: sudo) end diff --git a/Library/Homebrew/cask/staged.rb b/Library/Homebrew/cask/staged.rb index a8b6ee86e7..51a2f92ce4 100644 --- a/Library/Homebrew/cask/staged.rb +++ b/Library/Homebrew/cask/staged.rb @@ -1,3 +1,5 @@ +require "utils/user" + module Cask module Staged def info_plist_file(index = 0) @@ -31,7 +33,7 @@ module Cask sudo: false) end - def set_ownership(paths, user: current_user, group: "staff") + def set_ownership(paths, user: User.current, group: "staff") full_paths = remove_nonexistent(paths) return if full_paths.empty? @@ -40,10 +42,6 @@ module Cask sudo: true) end - def current_user - Utils.current_user - end - private def remove_nonexistent(paths) diff --git a/Library/Homebrew/cask/utils.rb b/Library/Homebrew/cask/utils.rb index cbbf1c0303..d95091a516 100644 --- a/Library/Homebrew/cask/utils.rb +++ b/Library/Homebrew/cask/utils.rb @@ -1,3 +1,4 @@ +require "utils/user" require "yaml" require "open3" require "stringio" @@ -52,7 +53,7 @@ module Cask # before using sudo+chown ohai "Using sudo to gain ownership of path '#{path}'" command.run("/usr/sbin/chown", - args: command_args + ["--", current_user, path], + args: command_args + ["--", User.current, path], sudo: true) tried_ownership = true # retry chflags/chmod after chown @@ -62,10 +63,6 @@ module Cask end end - def self.current_user - Etc.getpwuid(Process.euid).name - end - def self.path_occupied?(path) File.exist?(path) || File.symlink?(path) end diff --git a/Library/Homebrew/system_command.rb b/Library/Homebrew/system_command.rb index 29f3fdfcff..2b1c222fe8 100644 --- a/Library/Homebrew/system_command.rb +++ b/Library/Homebrew/system_command.rb @@ -8,12 +8,14 @@ require "extend/hash_validator" using HashValidator require "extend/predicable" -def system_command(*args) - SystemCommand.run(*args) -end +module Kernel + def system_command(*args) + SystemCommand.run(*args) + end -def system_command!(*args) - SystemCommand.run!(*args) + def system_command!(*args) + SystemCommand.run!(*args) + end end class SystemCommand diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index 9726cbc732..8f7fcd1ccd 100644 --- a/Library/Homebrew/test/cask/artifact/app_spec.rb +++ b/Library/Homebrew/test/cask/artifact/app_spec.rb @@ -80,7 +80,7 @@ describe Cask::Artifact::App, :cask do let(:force) { true } before do - allow(Cask::Utils).to receive(:current_user).and_return("fake_user") + allow(User).to receive(:current).and_return(User.new("fake_user")) end describe "target is both writable and user-owned" do diff --git a/Library/Homebrew/test/support/helper/spec/shared_examples/cask_staged.rb b/Library/Homebrew/test/support/helper/spec/shared_examples/cask_staged.rb index cd154facab..888e76ea63 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_examples/cask_staged.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_examples/cask_staged.rb @@ -76,7 +76,7 @@ shared_examples Cask::Staged do it "can set the ownership of a file" do fake_pathname = existing_path - allow(staged).to receive(:current_user).and_return("fake_user") + allow(User).to receive(:current).and_return(User.new("fake_user")) allow(staged).to receive(:Pathname).and_return(fake_pathname) FakeSystemCommand.expects_command( @@ -89,7 +89,7 @@ shared_examples Cask::Staged do it "can set the ownership of multiple files" do fake_pathname = existing_path - allow(staged).to receive(:current_user).and_return("fake_user") + allow(User).to receive(:current).and_return(User.new("fake_user")) allow(staged).to receive(:Pathname).and_return(fake_pathname) FakeSystemCommand.expects_command( @@ -112,7 +112,7 @@ shared_examples Cask::Staged do end it "cannot set the ownership of a file that does not exist" do - allow(staged).to receive(:current_user).and_return("fake_user") + allow(User).to receive(:current).and_return(User.new("fake_user")) fake_pathname = non_existent_path allow(staged).to receive(:Pathname).and_return(fake_pathname) diff --git a/Library/Homebrew/test/utils/user_spec.rb b/Library/Homebrew/test/utils/user_spec.rb new file mode 100644 index 0000000000..50d7ee42be --- /dev/null +++ b/Library/Homebrew/test/utils/user_spec.rb @@ -0,0 +1,36 @@ +require "utils/user" + +describe User do + subject { described_class.current } + + it { is_expected.to eq ENV["USER"] } + + describe "#gui?" do + before do + allow(SystemCommand).to receive(:run).with("who") + .and_return([who_output, "", instance_double(Process::Status, success?: true)]) + end + + context "when the current user is in a console session" do + let(:who_output) { + <<~EOS + #{ENV["USER"]} console Oct 1 11:23 + #{ENV["USER"]} ttys001 Oct 1 11:25 + EOS + } + + its(:gui?) { is_expected.to be true } + end + + context "when the current user is not in a console session" do + let(:who_output) { + <<~EOS + #{ENV["USER"]} ttys001 Oct 1 11:25 + fake_user ttys002 Oct 1 11:27 + EOS + } + + its(:gui?) { is_expected.to be false } + end + end +end diff --git a/Library/Homebrew/utils/user.rb b/Library/Homebrew/utils/user.rb new file mode 100644 index 0000000000..d8658d49dc --- /dev/null +++ b/Library/Homebrew/utils/user.rb @@ -0,0 +1,18 @@ +require "delegate" +require "etc" + +require "system_command" + +class User < DelegateClass(String) + def gui? + out, _, status = system_command "who" + return false unless status.success? + out.lines + .map(&:split) + .any? { |user, type,| user == self && type == "console" } + end + + def self.current + @current ||= new(Etc.getpwuid(Process.euid).name) + end +end