Merge pull request #5010 from reitermarkus/user-gui

Skip quitting applications when not logged into GUI.

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

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,
+            "LOGNAME" => User.current,
-            "USER" => logged_in_user,
+            "USER" => User.current,
-            "USERNAME" => logged_in_user,
+            "USERNAME" => User.current,
           }
           command.run!("/usr/sbin/installer", sudo: true, args: args, print_stdout: true, env: env)
         end

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
 

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)

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

Library/Homebrew/system_command.rb

@@ -8,12 +8,14 @@ require "extend/hash_validator"
 using HashValidator
 require "extend/predicable"
 
-def system_command(*args)
+module Kernel
-  SystemCommand.run(*args)
+  def system_command(*args)
-end
+    SystemCommand.run(*args)
+  end
 
-def system_command!(*args)
+  def system_command!(*args)
-  SystemCommand.run!(*args)
+    SystemCommand.run!(*args)
+  end
 end
 
 class SystemCommand

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

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)
 

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

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