Defer :script uninstall, add :early_script

fixes #2254

CONTRIBUTING.md

@@ -218,14 +218,15 @@ many features to help properly remove a Cask-installed application.
 These features are utilized via a hash argument to `uninstall` with any number
 of the following keys:
 
-* `:script` (string or hash) - relative path to an uninstall script to be run via sudo; use hash if args are needed
-  - `:executable` - relative path to an uninstall script to be run via sudo (required for hash)
-  - `:args` - array of arguments to the uninstall script
-  - `:input` - array of lines of input to be sent to `stdin` of the script
+* `:early_script` (string or hash) - like `:script`, but runs early (for backward compat, best avoided)
 * `:launchctl` (string or array) - ids of launchctl services to remove
 * `:quit` (string or array) - bundle id of running applications to quit before proceeding with the uninstaller
 * `:kext` (string or array) - bundle id of kext(s) to unload from the system before proceeding with the uninstaller
 * `:pkgutil` (string or regexp) - regexp matching bundle id(s) of packages to uninstall using `pkgutil`
+* `:script` (string or hash) - relative path to an uninstall script to be run via sudo; use hash if args are needed
+  - `:executable` - relative path to an uninstall script to be run via sudo (required for hash)
+  - `:args` - array of arguments to the uninstall script
+  - `:input` - array of lines of input to be sent to `stdin` of the script
 * `:files` (array) - absolute paths of files or directories to remove
   - should only be used as a last resort, since this is the blunt force approach
 

lib/cask/artifact/pkg.rb

@@ -52,18 +52,22 @@ class Cask::Artifact::Pkg < Cask::Artifact::Base
   def manually_uninstall(uninstall_options)
     ohai "Running uninstall process for #{@cask}; your password may be necessary"
 
-    unknown_keys = uninstall_options.keys - [:script, :quit, :kext, :pkgutil, :launchctl, :files]
+    unknown_keys = uninstall_options.keys - [:early_script, :launchctl, :quit, :kext, :script, :pkgutil, :files]
     unless unknown_keys.empty?
       opoo "Unknown arguments to uninstall: #{unknown_keys.join(", ")}. Running `brew update; brew upgrade brew-cask` will likely fix it.'"
     end
 
-    if uninstall_options.key? :script
-      executable, script_arguments = self.class.read_script_arguments(uninstall_options, :script)
+    # Preserve prior functionality of script which runs first. Should rarely be needed.
+    # :early_script should not delete files, better defer that to :script.
+    # If Cask writers never need :early_script it may be removed in the future.
+    if uninstall_options.key? :early_script
+      executable, script_arguments = self.class.read_script_arguments(uninstall_options, :early_script)
       ohai "Running uninstall script #{executable}"
-      raise "Error in Cask #{@cask}: uninstall :script without :executable." if executable.nil?
+      raise "Error in Cask #{@cask}: uninstall :early_script without :executable." if executable.nil?
       @command.run!(@cask.destination_path.join(executable), script_arguments)
     end
 
+    # :launchctl must come before :quit for cases where app would instantly re-launch
     if uninstall_options.key? :launchctl
       [*uninstall_options[:launchctl]].each do |service|
         ohai "Removing launchctl service #{service}"
@@ -71,6 +75,7 @@ class Cask::Artifact::Pkg < Cask::Artifact::Base
       end
     end
 
+    # :quit must come before :kext so the kext will not be in use by the app
     if uninstall_options.key? :quit
       [*uninstall_options[:quit]].each do |id|
         ohai "Quitting application ID #{id}"
@@ -81,6 +86,7 @@ class Cask::Artifact::Pkg < Cask::Artifact::Base
       end
     end
 
+    # :kext should be unloaded before attempting to delete the relevant file
     if uninstall_options.key? :kext
       [*uninstall_options[:kext]].each do |kext|
         ohai "Unloading kernel extension #{kext}"
@@ -91,6 +97,13 @@ class Cask::Artifact::Pkg < Cask::Artifact::Base
       end
     end
 
+    # :script must come before :pkgutil or :files so that the script file is not already deleted
+    if uninstall_options.key? :script
+      executable, script_arguments = self.class.read_script_arguments(uninstall_options, :script)
+      raise "Error in Cask #{@cask}: uninstall :script without :executable." if executable.nil?
+      @command.run!(@cask.destination_path.join(executable), script_arguments)
+    end
+
     if uninstall_options.key? :pkgutil
       ohai "Removing files from pkgutil Bill-of-Materials"
       pkgs = Cask::Pkg.all_matching(uninstall_options[:pkgutil], @command)