Properly upgrade Casks with version :latest

Library/Homebrew/cask/lib/hbc/artifact/moved.rb

@@ -71,7 +71,7 @@ def move_back(skip: false, force: false, command: nil, **options)
           command.run("/bin/mv", args: [target, source], sudo: true)
         end
 
-        add_altname_metadata(target, source.basename, command: command)
+        add_altname_metadata(source, target.basename, command: command)
       end
 
       def delete(target, force: false, command: nil, **_)

Library/Homebrew/cask/lib/hbc/cli/upgrade.rb

@@ -55,14 +55,14 @@ def run
 
             new_cask_installer.fetch
 
-            new_cask_installer.stage
-
             # Move the old Cask's artifacts back to staging
             old_cask_installer.start_upgrade
             # And flag it so in case of error
             started_upgrade = true
 
             # Install the new Cask
+            new_cask_installer.stage
+
             new_cask_installer.install_artifacts
             new_artifacts_installed = true
 

Library/Homebrew/cask/lib/hbc/installer.rb

@@ -31,7 +31,7 @@ def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false
       @upgrade = upgrade
     end
 
-    attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, :upgrade?, :verbose?
+    attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, :upgrade?, :verbose?, :backed_up?
 
     def self.print_caveats(cask)
       odebug "Printing caveats"
@@ -378,16 +378,30 @@ def start_upgrade
 
       disable_accessibility_access
       uninstall_artifacts
+      backup
+    end
+
+    def backup
+      @cask.staged_path.rename backup_path
+    end
+
+    def restore_backup
+      return unless backup_path.directory?
+
+      Pathname.new(@cask.staged_path).rmtree if @cask.staged_path.exist?
+
+      backup_path.rename @cask.staged_path
     end
 
     def revert_upgrade
       opoo "Reverting upgrade for Cask #{@cask}"
+      restore_backup
       install_artifacts
       enable_accessibility_access
     end
 
     def finalize_upgrade
-      purge_versioned_files
+      purge_backed_versioned_files
 
       puts summary
     end
@@ -420,19 +434,43 @@ def zap
       purge_caskroom_path
     end
 
+    def backup_path
+      return nil if @cask.staged_path.nil?
+      Pathname.new "#{@cask.staged_path}.upgrading"
+    end
+
+    def version_is_latest?
+      @cask.versions.include?("latest")
+    end
+
     def gain_permissions_remove(path)
       Utils.gain_permissions_remove(path, command: @command)
     end
 
+    def purge_backed_versioned_files
+      ohai "Purging files for version #{@cask.version} of Cask #{@cask}"
+
+      # versioned staged distribution
+      gain_permissions_remove(backup_path) if !backup_path.nil? && backup_path.exist?
+
+      # Homebrew-Cask metadata
+      purge_metadata
+    end
+
     def purge_versioned_files
       ohai "Purging files for version #{@cask.version} of Cask #{@cask}"
 
       # versioned staged distribution
       gain_permissions_remove(@cask.staged_path) if [email protected]_path.nil? && @cask.staged_path.exist?
 
       # Homebrew-Cask metadata
+      purge_metadata
+    end
+
+    def purge_metadata
       if @cask.metadata_versioned_path.respond_to?(:children) &&
-         @cask.metadata_versioned_path.exist?
+         @cask.metadata_versioned_path.exist? &&
+         !(upgrade? && version_is_latest?)
         @cask.metadata_versioned_path.children.each do |subdir|
           unless PERSISTENT_METADATA_SUBDIRS.include?(subdir.basename)
             gain_permissions_remove(subdir)

Library/Homebrew/test/cask/cli/upgrade_spec.rb

@@ -9,6 +9,7 @@
         "outdated/local-caffeine",
         "outdated/local-transmission",
         "outdated/auto-updates",
+        "outdated/version-latest",
       ]
     }
 
@@ -103,6 +104,9 @@
         auto_updates_path = Hbc.appdir.join("MyFancyApp.app")
         local_transmission = Hbc::CaskLoader.load("local-transmission")
         local_transmission_path = Hbc.appdir.join("Transmission.app")
+        version_latest = Hbc::CaskLoader.load("version-latest")
+        version_latest_path_1 = Hbc.appdir.join("Caffeine Mini.app")
+        version_latest_path_2 = Hbc.appdir.join("Caffeine Pro.app")
 
         expect(local_caffeine).to be_installed
         expect(local_caffeine_path).to be_a_directory
@@ -116,6 +120,11 @@
         expect(local_transmission_path).to be_a_directory
         expect(local_transmission.versions).to include("2.60")
 
+        expect(version_latest).to be_installed
+        expect(version_latest_path_1).to be_a_directory
+        expect(version_latest_path_2).to be_a_directory
+        expect(version_latest.versions).to include("latest")
+
         described_class.run("--greedy")
 
         expect(local_caffeine).to be_installed
@@ -129,6 +138,11 @@
         expect(local_transmission).to be_installed
         expect(local_transmission_path).to be_a_directory
         expect(local_transmission.versions).to include("2.61")
+
+        expect(version_latest).to be_installed
+        expect(version_latest_path_1).to be_a_directory
+        expect(version_latest_path_2).to be_a_directory
+        expect(version_latest.versions).to include("latest")
       end
 
       it 'does not include the Casks with "auto_updates true" when the version did not change' do

Library/Homebrew/test/support/fixtures/cask/Casks/outdated/version-latest.rb

@@ -0,0 +1,10 @@
+cask 'version-latest' do
+  version :latest
+  sha256 :no_check
+
+  url "file://#{TEST_FIXTURE_DIR}/cask/caffeines.zip"
+  homepage 'http://example.com/local-caffeine'
+
+  app 'Caffeine Mini.app'
+  app 'Caffeine Pro.app'
+end

Library/Homebrew/test/support/fixtures/cask/Casks/version-latest.rb

@@ -0,0 +1,10 @@
+cask 'version-latest' do
+  version :latest
+  sha256 :no_check
+
+  url "file://#{TEST_FIXTURE_DIR}/cask/caffeines.zip"
+  homepage 'http://example.com/local-caffeine'
+
+  app 'Caffeine Mini.app'
+  app 'Caffeine Pro.app'
+end