Merge pull request #15746 from apainintheneck/install-cmd-should-upgr…

…ade-existing-casks cmd/install: upgrade already installed casks
Homebrew · Jul 25, 2023 · a78173b · a78173b
2 parents 5d83900 + c9dea04
commit a78173b
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 49 deletions.
diff --git a/Library/Homebrew/cask/exceptions.rb b/Library/Homebrew/cask/exceptions.rb
@@ -132,21 +132,6 @@ def to_s
     end
   end
 
-  # Error when a cask is already installed.
-  #
-  # @api private
-  class CaskAlreadyInstalledError < AbstractCaskErrorWithToken
-    sig { returns(String) }
-    def to_s
-      <<~EOS
-        Cask '#{token}' is already installed.
-
-        To re-install #{token}, run:
-          #{Formatter.identifier("brew reinstall --cask #{token}")}
-      EOS
-    end
-  end
-
   # Error when there is a cyclic cask dependency.
   #
   # @api private

diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb
@@ -91,11 +91,6 @@ def install
       Migrator.migrate_if_needed(@cask)
 
       old_config = @cask.config
-      if @cask.installed? && !force? && !reinstall? && !upgrade?
-        return if quiet?
-
-        raise CaskAlreadyInstalledError, @cask
-      end
       predecessor = @cask if reinstall? && @cask.installed?
 
       check_conflicts

diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb
@@ -228,18 +228,34 @@ def self.install
 
       require "cask/installer"
 
-      casks.each do |cask|
-        Cask::Installer.new(cask,
-                            binaries:       args.binaries?,
-                            verbose:        args.verbose?,
-                            force:          args.force?,
-                            adopt:          args.adopt?,
-                            require_sha:    args.require_sha?,
-                            skip_cask_deps: args.skip_cask_deps?,
-                            quarantine:     args.quarantine?,
-                            quiet:          args.quiet?).install
-      rescue Cask::CaskAlreadyInstalledError => e
-        opoo e.message
+      installed_casks, new_casks = casks.partition(&:installed?)
+
+      new_casks.each do |cask|
+        Cask::Installer.new(
+          cask,
+          binaries:       args.binaries?,
+          verbose:        args.verbose?,
+          force:          args.force?,
+          adopt:          args.adopt?,
+          require_sha:    args.require_sha?,
+          skip_cask_deps: args.skip_cask_deps?,
+          quarantine:     args.quarantine?,
+          quiet:          args.quiet?,
+        ).install
+      end
+
+      if !Homebrew::EnvConfig.no_install_upgrade? && installed_casks.any?
+        Cask::Upgrade.upgrade_casks(
+          *installed_casks,
+          force:          args.force?,
+          dry_run:        args.dry_run?,
+          binaries:       args.binaries?,
+          quarantine:     args.quarantine?,
+          require_sha:    args.require_sha?,
+          skip_cask_deps: args.skip_cask_deps?,
+          verbose:        args.verbose?,
+          args:           args,
+        )
       end
     end
 

diff --git a/Library/Homebrew/env_config.rb b/Library/Homebrew/env_config.rb
@@ -315,7 +315,7 @@ module EnvConfig
         boolean:     true,
       },
       HOMEBREW_NO_INSTALL_UPGRADE:               {
-        description: "If set, `brew install` <formula> will not upgrade <formula> if it is installed but " \
+        description: "If set, `brew install` <formula/cask> will not upgrade <formula/cask> if it is installed but " \
                      "outdated.",
         boolean:     true,
       },

diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb
@@ -141,21 +141,6 @@
       end.not_to raise_error
     end
 
-    # unlike the CLI, the internal interface throws exception on double-install
-    it "installer method raises an exception when already-installed Casks are attempted" do
-      transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
-
-      expect(transmission).not_to be_installed
-
-      installer = described_class.new(transmission)
-
-      installer.install
-
-      expect do
-        installer.install
-      end.to raise_error(Cask::CaskAlreadyInstalledError)
-    end
-
     it "allows already-installed Casks to be installed if force is provided" do
       transmission = Cask::CaskLoader.load(cask_path("local-transmission"))
 
@@ -313,7 +298,7 @@
         caffeine = Cask::CaskLoader.load(path)
         expect(caffeine).to receive(:loaded_from_api?).twice.and_return(true)
         expect(caffeine).to receive(:caskfile_only?).twice.and_return(true)
-        expect(caffeine).to receive(:installed_caskfile).twice.and_return(invalid_path)
+        expect(caffeine).to receive(:installed_caskfile).once.and_return(invalid_path)
 
         described_class.new(caffeine).install
         expect(Cask::CaskLoader.load(path)).to be_installed