Merge pull request #6642 from reitermarkus/pkg-before-app

Install `pkg` before `app`.

Library/Homebrew/cask/artifact/abstract_artifact.rb

@@ -50,6 +50,11 @@ module Cask
           # depend on other artifacts still being installed.
           Uninstall,
           Installer,
+          # `pkg` should be run before `binary`, so
+          # targets are created prior to linking.
+          # `pkg` should be run before `app`, since an `app` could
+          # contain a nested installer (e.g. `wireshark`).
+          Pkg,
           [
             App,
             Suite,
@@ -67,9 +72,6 @@ module Cask
             Vst3Plugin,
             ScreenSaver,
           ],
-          # `pkg` should be run before `binary`, so
-          # targets are created prior to linking.
-          Pkg,
           Binary,
           Manpage,
           PostflightBlock,

Library/Homebrew/cask/artifact/moved.rb

@@ -54,10 +54,14 @@ module Cask
           command.run!("/bin/mv", args: [source, target], sudo: true)
         end
 
+        FileUtils.ln_sf target, source
+
         add_altname_metadata(target, source.basename, command: command)
       end
 
       def move_back(skip: false, force: false, command: nil, **options)
+        FileUtils.rm source if source.symlink? && source.dirname.join(source.readlink) == target
+
         if Utils.path_occupied?(source)
           message = "It seems there is already #{self.class.english_article} " \
                     "#{self.class.english_name} at '#{source}'"

Library/Homebrew/cask/installer.rb

@@ -94,6 +94,8 @@ module Cask
       fetch
       uninstall_existing_cask if reinstall?
 
+      backup if force? && @cask.staged_path.exist? && @cask.metadata_versioned_path.exist?
+
       oh1 "Installing Cask #{Formatter.identifier(@cask)}"
       opoo "macOS's Gatekeeper has been disabled for this Cask" unless quarantine?
       stage
@@ -104,7 +106,12 @@ module Cask
 
       ::Utils::Analytics.report_event("cask_install", @cask.token) unless @cask.tap&.private?
 
+      purge_backed_up_versioned_files
+
       puts summary
+    rescue
+      restore_backup
+      raise
     end
 
     def check_conflicts
@@ -411,6 +418,8 @@ module Cask
     end
 
     def finalize_upgrade
+      ohai "Purging files for version #{@cask.version} of Cask #{@cask}"
+
       purge_backed_up_versioned_files
 
       puts summary
@@ -471,8 +480,6 @@ module Cask
     end
 
     def purge_backed_up_versioned_files
-      ohai "Purging files for version #{@cask.version} of Cask #{@cask}"
-
       # versioned staged distribution
       gain_permissions_remove(backup_path) if backup_path&.exist?
 

Library/Homebrew/test/cask/artifact/alt_target_spec.rb

@@ -24,7 +24,7 @@ describe Cask::Artifact::App, :cask do
       install_phase
 
       expect(target_path).to be_a_directory
-      expect(source_path).not_to exist
+      expect(source_path).to be_a_symlink
     end
 
     describe "when app is in a subdirectory" do
@@ -45,7 +45,7 @@ describe Cask::Artifact::App, :cask do
         install_phase
 
         expect(target_path).to be_a_directory
-        expect(appsubdir.join("Caffeine.app")).not_to exist
+        expect(appsubdir.join("Caffeine.app")).to be_a_symlink
       end
     end
 
@@ -56,7 +56,7 @@ describe Cask::Artifact::App, :cask do
       install_phase
 
       expect(target_path).to be_a_directory
-      expect(source_path).not_to exist
+      expect(source_path).to be_a_symlink
 
       expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist
       expect(cask.staged_path.join("Caffeine Deluxe.app")).to be_a_directory

Library/Homebrew/test/cask/artifact/app_spec.rb

@@ -21,7 +21,7 @@ describe Cask::Artifact::App, :cask do
       install_phase
 
       expect(target_path).to be_a_directory
-      expect(source_path).not_to exist
+      expect(source_path).to be_a_symlink
     end
 
     describe "when app is in a subdirectory" do
@@ -42,7 +42,7 @@ describe Cask::Artifact::App, :cask do
         install_phase
 
         expect(target_path).to be_a_directory
-        expect(appsubdir.join("Caffeine.app")).not_to exist
+        expect(appsubdir.join("Caffeine.app")).to be_a_symlink
       end
     end
 
@@ -53,7 +53,7 @@ describe Cask::Artifact::App, :cask do
       install_phase
 
       expect(target_path).to be_a_directory
-      expect(source_path).not_to exist
+      expect(source_path).to be_a_symlink
 
       expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist
       expect(cask.staged_path.join("Caffeine Deluxe.app")).to exist
@@ -100,7 +100,7 @@ describe Cask::Artifact::App, :cask do
               .to output(stdout).to_stdout
               .and output(stderr).to_stderr
 
-            expect(source_path).not_to exist
+            expect(source_path).to be_a_symlink
             expect(target_path).to be_a_directory
 
             contents_path = target_path.join("Contents/Info.plist")
@@ -148,7 +148,7 @@ describe Cask::Artifact::App, :cask do
               .to output(stdout).to_stdout
               .and output(stderr).to_stderr
 
-            expect(source_path).not_to exist
+            expect(source_path).to be_a_symlink
             expect(target_path).to be_a_directory
 
             contents_path = target_path.join("Contents/Info.plist")
@@ -191,7 +191,7 @@ describe Cask::Artifact::App, :cask do
             .to output(stdout).to_stdout
             .and output(stderr).to_stderr
 
-          expect(source_path).not_to exist
+          expect(source_path).to be_a_symlink
           expect(target_path).to be_a_directory
 
           contents_path = target_path.join("Contents/Info.plist")

Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb

@@ -30,7 +30,7 @@ describe Cask::Artifact::Artifact, :cask do
     install_phase.call
 
     expect(target_path).to be_a_directory
-    expect(source_path).not_to exist
+    expect(source_path).to be_a_symlink
   end
 
   it "avoids clobbering an existing artifact" do

Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb

@@ -24,23 +24,26 @@ describe Cask::Artifact::App, :cask do
       install_phase
 
       expect(target_path_mini).to be_a_directory
-      expect(source_path_mini).not_to exist
+      expect(source_path_mini).to be_a_symlink
 
       expect(target_path_pro).to be_a_directory
-      expect(source_path_pro).not_to exist
+      expect(source_path_pro).to be_a_symlink
     end
 
     describe "when apps are in a subdirectory" do
       let(:cask) { Cask::CaskLoader.load(cask_path("with-two-apps-subdir")) }
 
+      let(:source_path_mini) { cask.staged_path.join("Caffeines", "Caffeine Mini.app") }
+      let(:source_path_pro) { cask.staged_path.join("Caffeines", "Caffeine Pro.app") }
+
       it "installs both apps using the proper target directory" do
         install_phase
 
         expect(target_path_mini).to be_a_directory
-        expect(source_path_mini).not_to exist
+        expect(source_path_mini).to be_a_symlink
 
         expect(target_path_pro).to be_a_directory
-        expect(source_path_pro).not_to exist
+        expect(source_path_pro).to be_a_symlink
       end
     end
 
@@ -50,7 +53,7 @@ describe Cask::Artifact::App, :cask do
       install_phase
 
       expect(target_path_mini).to be_a_directory
-      expect(source_path_mini).not_to exist
+      expect(source_path_mini).to be_a_symlink
 
       expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist
       expect(cask.staged_path.join("Caffeine Deluxe.app")).to exist