Merge pull request #20328 from Homebrew/download_queue_cask

Add Cask install/upgrade/reinstall support for download queue

Author: MikeMcQuaid

.github/workflows/tests.yml

@@ -368,7 +368,7 @@ jobs:
     env:
       HOMEBREW_TEST_BOT_ANALYTICS: 1
       HOMEBREW_ENFORCE_SBOM: 1
-      HOMEBREW_DOWNLOAD_CONCURRENCY: 4
+      HOMEBREW_DOWNLOAD_CONCURRENCY: auto
     steps:
       - name: Install Homebrew and Homebrew's dependencies
         # All other images are built from our Homebrew Dockerfile.

Library/Homebrew/api/cask.rb

@@ -28,8 +28,8 @@ def self.fetch(token)
         Homebrew::API.fetch "cask/#{token}.json"
       end
 
-      sig { params(cask: ::Cask::Cask).returns(::Cask::Cask) }
-      def self.source_download(cask)
+      sig { params(cask: ::Cask::Cask, download_queue: T.nilable(Homebrew::DownloadQueue)).returns(Homebrew::API::SourceDownload) }
+      def self.source_download(cask, download_queue: nil)
         path = cask.ruby_source_path.to_s
         sha256 = cask.ruby_source_checksum[:sha256]
         checksum = Checksum.new(sha256) if sha256
@@ -44,7 +44,20 @@ def self.source_download(cask)
           ],
           cache:   HOMEBREW_CACHE_API_SOURCE/"#{tap}/#{git_head}/Cask",
         )
-        download.fetch
+
+        if download_queue
+          download_queue.enqueue(download)
+        elsif !download.cache.exist?
+          download.fetch
+        end
+
+        download
+      end
+
+      sig { params(cask: ::Cask::Cask).returns(::Cask::Cask) }
+      def self.source_download_cask(cask)
+        download = source_download(cask)
+
         ::Cask::CaskLoader::FromPathLoader.new(download.symlink_location)
                                           .load(config: cask.config)
       end

Library/Homebrew/cask/installer.rb

@@ -21,14 +21,14 @@ class Installer
         skip_cask_deps: T::Boolean, binaries: T::Boolean, verbose: T::Boolean, zap: T::Boolean,
         require_sha: T::Boolean, upgrade: T::Boolean, reinstall: T::Boolean, installed_as_dependency: T::Boolean,
         installed_on_request: T::Boolean, quarantine: T::Boolean, verify_download_integrity: T::Boolean,
-        quiet: T::Boolean
+        quiet: T::Boolean, download_queue: T.nilable(Homebrew::DownloadQueue)
       ).void
     }
     def initialize(cask, command: SystemCommand, force: false, adopt: false,
                    skip_cask_deps: false, binaries: true, verbose: false,
                    zap: false, require_sha: false, upgrade: false, reinstall: false,
                    installed_as_dependency: false, installed_on_request: true,
-                   quarantine: true, verify_download_integrity: true, quiet: false)
+                   quarantine: true, verify_download_integrity: true, quiet: false, download_queue: nil)
       @cask = cask
       @command = command
       @force = force
@@ -45,6 +45,7 @@ def initialize(cask, command: SystemCommand, force: false, adopt: false,
       @quarantine = quarantine
       @verify_download_integrity = verify_download_integrity
       @quiet = quiet
+      @download_queue = download_queue
     end
 
     sig { returns(T::Boolean) }
@@ -104,14 +105,14 @@ def self.caveats(cask)
     def fetch(quiet: nil, timeout: nil)
       odebug "Cask::Installer#fetch"
 
-      load_cask_from_source_api! if @cask.loaded_from_api? && @cask.caskfile_only?
+      load_cask_from_source_api! if cask_from_source_api?
       verify_has_sha if require_sha? && !force?
       check_requirements
 
       forbidden_tap_check
       forbidden_cask_and_formula_check
 
-      download(quiet:, timeout:)
+      download(quiet:, timeout:) if @download_queue.nil?
 
       satisfy_cask_and_formula_dependencies
     end
@@ -790,9 +791,20 @@ def forbidden_cask_and_formula_check
       )
     end
 
+    sig { void }
+    def enqueue_downloads
+      download_queue = @download_queue
+      return if download_queue.nil?
+
+      Homebrew::API::Cask.source_download(@cask, download_queue:) if cask_from_source_api?
+
+      download_queue.enqueue(downloader)
+    end
+
     private
 
     # load the same cask file that was used for installation, if possible
+    sig { void }
     def load_installed_caskfile!
       Migrator.migrate_if_needed(@cask)
 
@@ -807,12 +819,18 @@ def load_installed_caskfile!
         end
       end
 
-      load_cask_from_source_api! if @cask.loaded_from_api? && @cask.caskfile_only?
+      load_cask_from_source_api! if cask_from_source_api?
       # otherwise we default to the current cask
     end
 
+    sig { void }
     def load_cask_from_source_api!
-      @cask = Homebrew::API::Cask.source_download(@cask)
+      @cask = Homebrew::API::Cask.source_download_cask(@cask)
+    end
+
+    sig { returns(T::Boolean) }
+    def cask_from_source_api?
+      @cask.loaded_from_api? && @cask.caskfile_only?
     end
   end
 end

Library/Homebrew/cask/reinstall.rb

@@ -23,11 +23,20 @@ def self.reinstall_casks(
 
       quarantine = true if quarantine.nil?
 
-      casks.each do |cask|
-        Installer
-          .new(cask, binaries:, verbose:, force:, skip_cask_deps:, require_sha:, reinstall: true, quarantine:, zap:)
-          .install
+      download_queue = Homebrew::DownloadQueue.new(pour: true) if Homebrew::EnvConfig.download_concurrency > 1
+      cask_installers = casks.map do |cask|
+        Installer.new(cask, binaries:, verbose:, force:, skip_cask_deps:, require_sha:, reinstall: true,
+                      quarantine:, zap:, download_queue:)
       end
+
+      if download_queue
+        oh1 "Fetching downloads for: #{casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence}",
+            truncate: false
+        cask_installers.each(&:enqueue_downloads)
+        download_queue.fetch
+      end
+
+      cask_installers.each(&:install)
     end
   end
 end

Library/Homebrew/cask/upgrade.rb

@@ -98,11 +98,6 @@ def self.upgrade_casks!(
         end
       end
 
-      verb = dry_run ? "Would upgrade" : "Upgrading"
-      oh1 "#{verb} #{outdated_casks.count} outdated #{::Utils.pluralize("package", outdated_casks.count)}:"
-
-      caught_exceptions = []
-
       upgradable_casks = outdated_casks.map do |c|
         unless c.installed?
           odie <<~EOS
@@ -114,6 +109,33 @@ def self.upgrade_casks!(
         [CaskLoader.load(c.installed_caskfile), c]
       end
 
+      return false if upgradable_casks.empty?
+
+      if !dry_run && Homebrew::EnvConfig.download_concurrency > 1
+        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|
+          # 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
+
+        download_queue.fetch
+      end
+
+      verb = dry_run ? "Would upgrade" : "Upgrading"
+      oh1 "#{verb} #{upgradable_casks.count} outdated #{::Utils.pluralize("package", upgradable_casks.count)}:"
+
+      caught_exceptions = []
+
       puts upgradable_casks
         .map { |(old_cask, new_cask)| "#{new_cask.full_name} #{old_cask.version} -> #{new_cask.version}" }
         .join("\n")
@@ -123,7 +145,7 @@ def self.upgrade_casks!(
         upgrade_cask(
           old_cask, new_cask,
           binaries:, force:, skip_cask_deps:, verbose:,
-          quarantine:, require_sha:
+          quarantine:, require_sha:, download_queue:
         )
       rescue => e
         new_exception = e.exception("#{new_cask.full_name}: #{e}")
@@ -149,11 +171,12 @@ def self.upgrade_casks!(
         require_sha:    T.nilable(T::Boolean),
         skip_cask_deps: T.nilable(T::Boolean),
         verbose:        T.nilable(T::Boolean),
+        download_queue: T.nilable(Homebrew::DownloadQueue),
       ).void
     }
     def self.upgrade_cask(
       old_cask, new_cask,
-      binaries:, force:, quarantine:, require_sha:, skip_cask_deps:, verbose:
+      binaries:, force:, quarantine:, require_sha:, skip_cask_deps:, verbose:, download_queue:
     )
       require "cask/installer"
 
@@ -181,6 +204,7 @@ def self.upgrade_cask(
         require_sha:,
         upgrade:        true,
         quarantine:,
+        download_queue:,
       }.compact
 
       new_cask_installer =

Library/Homebrew/cmd/install.rb

@@ -242,6 +242,24 @@ def run
 
           installed_casks, new_casks = casks.partition(&:installed?)
 
+          download_queue = Homebrew::DownloadQueue.new(pour: true) if Homebrew::EnvConfig.download_concurrency > 1
+          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|
+              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
+
+            download_queue.fetch
+          end
+
           new_casks.each do |cask|
             Cask::Installer.new(
               cask,

Library/Homebrew/test/cask/installer_spec.rb

@@ -225,7 +225,7 @@
 
       it "installs cask" do
         source_caffeine = Cask::CaskLoader.load(path)
-        expect(Homebrew::API::Cask).to receive(:source_download).once.and_return(source_caffeine)
+        expect(Homebrew::API::Cask).to receive(:source_download_cask).once.and_return(source_caffeine)
 
         caffeine = Cask::CaskLoader.load(path)
         expect(caffeine).to receive(:loaded_from_api?).once.and_return(true)
@@ -293,7 +293,7 @@
 
       it "uninstalls cask" do
         source_caffeine = Cask::CaskLoader.load(path)
-        expect(Homebrew::API::Cask).to receive(:source_download).twice.and_return(source_caffeine)
+        expect(Homebrew::API::Cask).to receive(:source_download_cask).twice.and_return(source_caffeine)
 
         caffeine = Cask::CaskLoader.load(path)
         expect(caffeine).to receive(:loaded_from_api?).twice.and_return(true)