From 64311c288940d2227978069744e599e06d0008e8 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 5 Aug 2025 14:38:24 +0100 Subject: [PATCH] Add Cask::Installer#prelude to check before download queueing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #20374 When using HOMEBREW_DOWNLOAD_CONCURRENCY, cask binaries were being downloaded before checking if the cask could actually be installed (e.g., disabled casks or conflict checks). This resulted in unnecessary downloads for casks that would ultimately fail to install. This change adds a `prelude` method to Cask::Installer that performs early validation checks (deprecation/disable status and conflicts) similar to Formula#prelude_fetch. The prelude method is called before enqueueing downloads in all download queue scenarios (install, reinstall, and upgrade commands), ensuring that validation failures occur before the "Fetching downloads for:" message is displayed. Key changes: - Add Cask::Installer#prelude method with @ran_prelude tracking - Call prelude before enqueueing downloads in install/reinstall/upgrade - Refactor to avoid creating installer objects multiple times - Maintain backward compatibility for non-download-queue scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- Library/Homebrew/cask/installer.rb | 14 ++++++++++++-- Library/Homebrew/cask/reinstall.rb | 2 ++ Library/Homebrew/cask/upgrade.rb | 13 ++++++++----- Library/Homebrew/cmd/install.rb | 14 +++++++++----- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index cb91c6513f..967971247c 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -46,6 +46,7 @@ module Cask @verify_download_integrity = verify_download_integrity @quiet = quiet @download_queue = download_queue + @ran_prelude = T.let(false, T::Boolean) end sig { returns(T::Boolean) } @@ -140,8 +141,7 @@ module Cask old_config = @cask.config predecessor = @cask if reinstall? && @cask.installed? - check_deprecate_disable - check_conflicts + prelude print caveats fetch @@ -791,6 +791,16 @@ on_request: true) ) end + sig { void } + def prelude + return if @ran_prelude + + check_deprecate_disable + check_conflicts + + @ran_prelude = true + end + sig { void } def enqueue_downloads download_queue = @download_queue diff --git a/Library/Homebrew/cask/reinstall.rb b/Library/Homebrew/cask/reinstall.rb index 3a6ea68e46..970a5daf6b 100644 --- a/Library/Homebrew/cask/reinstall.rb +++ b/Library/Homebrew/cask/reinstall.rb @@ -30,6 +30,8 @@ module Cask end if download_queue + cask_installers.each(&:prelude) + oh1 "Fetching downloads for: #{casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence}", truncate: false cask_installers.each(&:enqueue_downloads) diff --git a/Library/Homebrew/cask/upgrade.rb b/Library/Homebrew/cask/upgrade.rb index 91ee0090f0..7b4d1fbeab 100644 --- a/Library/Homebrew/cask/upgrade.rb +++ b/Library/Homebrew/cask/upgrade.rb @@ -115,19 +115,22 @@ module Cask download_queue = Homebrew::DownloadQueue.new(pour: true) fetchable_casks = upgradable_casks.map(&:last) - fetchable_casks_sentence = fetchable_casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence - oh1 "Fetching downloads for: #{fetchable_casks_sentence}", truncate: false - - fetchable_casks.each do |cask| + fetchable_cask_installers = fetchable_casks.map do |cask| # This is significantly easier given the weird difference in Sorbet signatures here. # rubocop:disable Style/DoubleNegation Installer.new(cask, binaries: !!binaries, verbose: !!verbose, force: !!force, skip_cask_deps: !!skip_cask_deps, require_sha: !!require_sha, upgrade: true, quarantine:, download_queue:) - .enqueue_downloads # rubocop:enable Style/DoubleNegation end + fetchable_cask_installers.each(&:prelude) + + fetchable_casks_sentence = fetchable_casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence + oh1 "Fetching downloads for: #{fetchable_casks_sentence}", truncate: false + + fetchable_cask_installers.each(&:enqueue_downloads) + download_queue.fetch end diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 55e3ca82d1..30ad5392db 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -248,17 +248,21 @@ module Homebrew fetch_casks = Homebrew::EnvConfig.no_install_upgrade? ? new_casks : casks if download_queue - fetch_casks_sentence = fetch_casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence - oh1 "Fetching downloads for: #{fetch_casks_sentence}", truncate: false - - fetch_casks.each do |cask| + fetch_cask_installers = fetch_casks.map do |cask| Cask::Installer.new(cask, binaries: args.binaries?, verbose: args.verbose?, force: args.force?, skip_cask_deps: args.skip_cask_deps?, require_sha: args.require_sha?, reinstall: true, quarantine: args.quarantine?, zap: args.zap?, download_queue:) - .enqueue_downloads end + # Run prelude checks for all casks before enqueueing downloads + fetch_cask_installers.each(&:prelude) + + fetch_casks_sentence = fetch_casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence + oh1 "Fetching downloads for: #{fetch_casks_sentence}", truncate: false + + fetch_cask_installers.each(&:enqueue_downloads) + download_queue.fetch end