From c9dea04bd4e1df57e2d4d6cb0d91fa76abbae569 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Sun, 23 Jul 2023 18:21:15 -0700 Subject: [PATCH] cmd/install: upgrade already installed casks Previously, the behavior was to warn users that a cask was already installed and then skip modifying the installed version. This is different to how we handled things with formulas. For them we would upgrade any already installed formulas. This just brings casks in line with what we already do with formulas. Changes: - cmd/install: Upgrade already installed casks if HOMEBREW_NO_INSTALL_UPGRADE is not set - env_config: Update wording of HOMEBREW_NO_INSTALL_UPGRADE to include casks - remove error that was only used to alert about already installed casks Note: - The upgrade command for casks defaults to --greedy when you pass named casks to the command which means that this will always default to that behavior since you must specify the name of the cask when installing. --- Library/Homebrew/cask/exceptions.rb | 15 -------- Library/Homebrew/cask/installer.rb | 5 --- Library/Homebrew/cmd/install.rb | 40 ++++++++++++++------ Library/Homebrew/env_config.rb | 2 +- Library/Homebrew/test/cask/installer_spec.rb | 17 +-------- 5 files changed, 30 insertions(+), 49 deletions(-) diff --git a/Library/Homebrew/cask/exceptions.rb b/Library/Homebrew/cask/exceptions.rb index e37817b6c3..05bdde818a 100644 --- a/Library/Homebrew/cask/exceptions.rb +++ b/Library/Homebrew/cask/exceptions.rb @@ -132,21 +132,6 @@ module Cask 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 index 5cac32602b..5c27a9ed29 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -91,11 +91,6 @@ module Cask 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 index 5537ae5d20..cba5cf9ba1 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -228,18 +228,34 @@ module Homebrew 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 index c039b0bb5b..5f7ec25fca 100644 --- a/Library/Homebrew/env_config.rb +++ b/Library/Homebrew/env_config.rb @@ -315,7 +315,7 @@ module Homebrew boolean: true, }, HOMEBREW_NO_INSTALL_UPGRADE: { - description: "If set, `brew install` will not upgrade if it is installed but " \ + description: "If set, `brew install` will not upgrade 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 index 983c4de983..0555cf50ff 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -141,21 +141,6 @@ describe Cask::Installer, :cask do 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 @@ describe Cask::Installer, :cask do 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