'brew cask upgrade' will continue upgrading casks after a failure

'cask upgrade' command collects all exceptions thrown
from individual casks during the upgrade process. If
there were more than one cask that raised exceptions
during the upgrade process, a MultipleCaskErrors
exception will be thrown.

Issue #5203

Library/Homebrew/cask/cmd/upgrade.rb

@@ -31,72 +31,74 @@ module Cask
 
         ohai "Casks with `auto_updates` or `version :latest` will not be upgraded" if args.empty? && !greedy?
         oh1 "Upgrading #{outdated_casks.count} #{"outdated package".pluralize(outdated_casks.count)}:"
-        cask_upgrades = outdated_casks.map do |cask|
+        caught_exceptions = []
-          if cask.installed_caskfile.nil?
+        outdated_casks.each do |cask|
-            "#{cask.full_name} #{cask.version}"
+          begin
-          else
+            old_cask = CaskLoader.load(cask.installed_caskfile)
-            "#{cask.full_name} #{CaskLoader.load(cask.installed_caskfile).version} -> #{cask.version}"
+            puts "#{cask.full_name} #{old_cask.version} -> #{cask.version}"
+            upgrade_cask(old_cask)
+          rescue CaskError => e
+            caught_exceptions << e
+            next
           end
         end
-        puts cask_upgrades.join(", ")
+        return if caught_exceptions.empty?
+        raise MultipleCaskErrors, caught_exceptions if caught_exceptions.count > 1
+        raise caught_exceptions.first if caught_exceptions.count == 1
+      end
 
-        outdated_casks.each do |old_cask|
+      def upgrade_cask(old_cask)
-          odebug "Started upgrade process for Cask #{old_cask}"
+        odebug "Started upgrade process for Cask #{old_cask}"
-          raise CaskUnavailableError.new(old_cask, "The Caskfile is missing!") if old_cask.installed_caskfile.nil?
+        old_config = old_cask.config
 
-          old_cask = CaskLoader.load(old_cask.installed_caskfile)
+        old_cask_installer =
+          Installer.new(old_cask, binaries: binaries?,
+                                  verbose:  verbose?,
+                                  force:    force?,
+                                  upgrade:  true)
 
-          old_config = old_cask.config
+        new_cask = CaskLoader.load(old_cask.token)
 
-          old_cask_installer =
+        new_cask.config = Config.global.merge(old_config)
-            Installer.new(old_cask, binaries: binaries?,
-                                    verbose:  verbose?,
-                                    force:    force?,
-                                    upgrade:  true)
 
-          new_cask = CaskLoader.load(old_cask.token)
+        new_cask_installer =
+          Installer.new(new_cask, binaries:       binaries?,
+                                  verbose:        verbose?,
+                                  force:          force?,
+                                  skip_cask_deps: skip_cask_deps?,
+                                  require_sha:    require_sha?,
+                                  upgrade:        true,
+                                  quarantine:     quarantine?)
 
-          new_cask.config = Config.global.merge(old_config)
+        started_upgrade = false
+        new_artifacts_installed = false
 
-          new_cask_installer =
+        begin
-            Installer.new(new_cask, binaries:       binaries?,
+          # Start new Cask's installation steps
-                                    verbose:        verbose?,
+          new_cask_installer.check_conflicts
-                                    force:          force?,
-                                    skip_cask_deps: skip_cask_deps?,
-                                    require_sha:    require_sha?,
-                                    upgrade:        true,
-                                    quarantine:     quarantine?)
 
-          started_upgrade = false
+          puts new_cask_installer.caveats
-          new_artifacts_installed = false
 
-          begin
+          new_cask_installer.fetch
-            # Start new Cask's installation steps
-            new_cask_installer.check_conflicts
 
-            puts new_cask_installer.caveats
+          # 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
 
-            new_cask_installer.fetch
+          # Install the new Cask
+          new_cask_installer.stage
 
-            # Move the old Cask's artifacts back to staging
+          new_cask_installer.install_artifacts
-            old_cask_installer.start_upgrade
+          new_artifacts_installed = true
-            # And flag it so in case of error
-            started_upgrade = true
 
-            # Install the new Cask
+          # If successful, wipe the old Cask from staging
-            new_cask_installer.stage
+          old_cask_installer.finalize_upgrade
-
+        rescue CaskError => e
-            new_cask_installer.install_artifacts
+          new_cask_installer.uninstall_artifacts if new_artifacts_installed
-            new_artifacts_installed = true
+          new_cask_installer.purge_versioned_files
-
+          old_cask_installer.revert_upgrade if started_upgrade
-            # If successful, wipe the old Cask from staging
+          raise e
-            old_cask_installer.finalize_upgrade
-          rescue CaskError => e
-            new_cask_installer.uninstall_artifacts if new_artifacts_installed
-            new_cask_installer.purge_versioned_files
-            old_cask_installer.revert_upgrade if started_upgrade
-            raise e
-          end
         end
       end
 

Library/Homebrew/cask/exceptions.rb

@@ -1,6 +1,19 @@
 module Cask
   class CaskError < RuntimeError; end
 
+  class MultipleCaskErrors < CaskError
+    def initialize(errors)
+      @errors = errors
+    end
+
+    def to_s
+      <<~EOS
+        Problems with multiple casks:
+        #{@errors.map(&:to_s).join("\n")}
+      EOS
+    end
+  end
+
   class AbstractCaskErrorWithToken < CaskError
     attr_reader :token
     attr_reader :reason

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

@@ -222,4 +222,61 @@ describe Cask::Cmd::Upgrade, :cask do
       expect(bad_checksum.staged_path).not_to exist
     end
   end
+
+  context "multiple failures" do
+    let(:installed) {
+      [
+        "outdated/bad-checksum",
+        "outdated/local-transmission",
+        "outdated/bad-checksum2",
+      ]
+    }
+
+    before do
+      installed.each { |cask| Cask::Cmd::Install.run(cask) }
+
+      allow_any_instance_of(described_class).to receive(:verbose?).and_return(true)
+    end
+
+    it "will not end the upgrade process" do
+      bad_checksum = Cask::CaskLoader.load("bad-checksum")
+      bad_checksum_path = bad_checksum.config.appdir.join("Caffeine.app")
+
+      local_transmission = Cask::CaskLoader.load("local-transmission")
+      local_transmission_path = Cask::Config.global.appdir.join("Transmission.app")
+
+      bad_checksum_2 = Cask::CaskLoader.load("bad-checksum2")
+      bad_checksum_2_path = bad_checksum_2.config.appdir.join("container")
+
+      expect(bad_checksum).to be_installed
+      expect(bad_checksum_path).to be_a_directory
+      expect(bad_checksum.versions).to include("1.2.2")
+
+      expect(local_transmission).to be_installed
+      expect(local_transmission_path).to be_a_directory
+      expect(local_transmission.versions).to include("2.60")
+
+      expect(bad_checksum_2).to be_installed
+      expect(bad_checksum_2_path).to be_a_file
+      expect(bad_checksum_2.versions).to include("1.2.2")
+
+      expect {
+        described_class.run
+      }.to raise_error(Cask::MultipleCaskErrors)
+
+      expect(bad_checksum).to be_installed
+      expect(bad_checksum_path).to be_a_directory
+      expect(bad_checksum.versions).to include("1.2.2")
+      expect(bad_checksum.staged_path).not_to exist
+
+      expect(local_transmission).to be_installed
+      expect(local_transmission_path).to be_a_directory
+      expect(local_transmission.versions).to include("2.61")
+
+      expect(bad_checksum_2).to be_installed
+      expect(bad_checksum_2_path).to be_a_file
+      expect(bad_checksum_2.versions).to include("1.2.2")
+      expect(bad_checksum_2.staged_path).not_to exist
+    end
+  end
 end

Library/Homebrew/test/support/fixtures/cask/Casks/bad-checksum2.rb

@@ -0,0 +1,9 @@
+cask 'bad-checksum2' do
+  version '1.2.3'
+  sha256 'badbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadbadb'
+
+  url "file://#{TEST_FIXTURE_DIR}/cask/container.tar.gz"
+  homepage 'https://brew.sh/container-tar-gz'
+
+  app 'container'
+end

Library/Homebrew/test/support/fixtures/cask/Casks/outdated/bad-checksum2.rb

@@ -0,0 +1,9 @@
+cask 'bad-checksum2' do
+  version '1.2.2'
+  sha256 'fab685fabf73d5a9382581ce8698fce9408f5feaa49fa10d9bc6c510493300f5'
+
+  url "file://#{TEST_FIXTURE_DIR}/cask/container.tar.gz"
+  homepage 'https://brew.sh/container-tar-gz'
+
+  app 'container'
+end