Merge pull request #22394 from Homebrew/cask-shared-download-queue

Share cask upgrade download queues

Author: MikeMcQuaid

Library/Homebrew/api/cask.rb

@@ -49,28 +49,33 @@ def self.fetch_cask_json!(name)
         ).returns(Homebrew::API::SourceDownload)
       }
       def self.source_download(cask, download_queue: Homebrew.default_download_queue, enqueue: false)
+        download = source_download_for(cask)
+
+        if enqueue
+          download_queue.enqueue(download)
+        elsif !download.symlink_location.exist?
+          download.fetch
+        end
+
+        download
+      end
+
+      sig { params(cask: ::Cask::Cask).returns(Homebrew::API::SourceDownload) }
+      def self.source_download_for(cask)
         path = cask.ruby_source_path.to_s
         sha256 = cask.ruby_source_checksum[:sha256]
         checksum = Checksum.new(sha256) if sha256
         git_head = cask.tap_git_head || "HEAD"
         tap = cask.tap&.full_name || "Homebrew/homebrew-cask"
 
-        download = Homebrew::API::SourceDownload.new(
+        Homebrew::API::SourceDownload.new(
           "https://raw.githubusercontent.com/#{tap}/#{git_head}/#{path}",
           checksum,
           mirrors: [
             "#{HOMEBREW_API_DEFAULT_DOMAIN}/cask-source/#{File.basename(path)}",
           ],
           cache:   HOMEBREW_CACHE_API_SOURCE/"#{tap}/#{git_head}/Cask",
         )
-
-        if enqueue
-          download_queue.enqueue(download)
-        elsif !download.symlink_location.exist?
-          download.fetch
-        end
-
-        download
       end
 
       sig { params(cask: ::Cask::Cask).returns(::Cask::Cask) }

Library/Homebrew/cask/installer.rb

@@ -54,6 +54,8 @@ def initialize(cask, command: SystemCommand, force: false, adopt: false,
       @quiet = quiet
       @download_queue = download_queue
       @defer_fetch = defer_fetch
+      @source_download = T.let(nil, T.nilable(Homebrew::API::SourceDownload))
+      @ran_prelude_fetch = T.let(false, T::Boolean)
       @ran_prelude = T.let(false, T::Boolean)
       @cask_and_formula_dependencies = T.let(nil, T.nilable(T::Array[T.any(Formula, ::Cask::Cask)]))
     end
@@ -901,13 +903,7 @@ def forbidden_cask_artifacts_check
     def prelude
       return if @ran_prelude
 
-      check_deprecate_disable
-      check_conflicts
-      check_requirements
-      # Run the cask-self forbidden checks before loading the caskfile from the
-      # Source API so a forbidden cask never triggers a network fetch.
-      forbidden_tap_check(cask_only: true)
-      forbidden_cask_and_formula_check(cask_only: true)
+      check_prelude_requirements unless @ran_prelude_fetch
       load_cask_from_source_api! if cask_from_source_api?
       forbidden_tap_check
       forbidden_cask_and_formula_check
@@ -916,26 +912,73 @@ def prelude
       @ran_prelude = true
     end
 
+    sig { returns(T::Boolean) }
+    def source_download_requires_pre_fetch?
+      cask_from_source_api? && @cask.languages.any?
+    end
+
+    sig { params(download_queue: Homebrew::DownloadQueue).void }
+    def prelude_fetch(download_queue: @download_queue)
+      return unless (download = prelude_fetch_download)
+
+      download_queue.enqueue(download)
+    end
+
+    sig { returns(T.nilable(Homebrew::API::SourceDownload)) }
+    def prelude_fetch_download
+      return if @ran_prelude_fetch
+
+      check_prelude_requirements
+      @ran_prelude_fetch = true
+      return unless source_download_requires_pre_fetch?
+
+      if source_download.downloaded?
+        source_download.verify_download_integrity(source_download.cached_download)
+        source_download.downloader.create_symlink_to_cached_download(source_download.cached_download)
+        return
+      end
+
+      source_download
+    end
+
     sig { void }
     def enqueue_downloads
       download_queue = @download_queue
-      check_requirements
+      prelude_fetch(download_queue:) unless @ran_prelude_fetch
 
       # FIXME: We need to load Cask source before enqueuing to support
       # language-specific URLs, but this will block the main process.
-      if cask_from_source_api?
-        if @cask.languages.any?
-          load_cask_from_source_api!
-        else
-          Homebrew::API::Cask.source_download(@cask, download_queue:, enqueue: true)
-        end
+      if source_download_requires_pre_fetch?
+        load_cask_from_source_api!
+      elsif cask_from_source_api?
+        Homebrew::API::Cask.source_download(@cask, download_queue:, enqueue: true)
       end
 
+      forbidden_tap_check
+      forbidden_cask_and_formula_check
+      forbidden_cask_artifacts_check
+
       download_queue.enqueue(downloader)
     end
 
     private
 
+    sig { void }
+    def check_prelude_requirements
+      check_deprecate_disable
+      check_conflicts
+      check_requirements
+      # Run the cask-self forbidden checks before loading the caskfile from the
+      # Source API so a forbidden cask never triggers a network fetch.
+      forbidden_tap_check(cask_only: true)
+      forbidden_cask_and_formula_check(cask_only: true)
+    end
+
+    sig { returns(Homebrew::API::SourceDownload) }
+    def source_download
+      @source_download ||= Homebrew::API::Cask.source_download_for(@cask)
+    end
+
     # load the same cask file that was used for installation, if possible
     sig { void }
     def load_installed_caskfile!

Library/Homebrew/cask/reinstall.rb

@@ -2,6 +2,7 @@
 # frozen_string_literal: true
 
 require "utils/output"
+require "install"
 
 module Cask
   class Reinstall
@@ -58,11 +59,9 @@ def self.reinstall_casks(
         end
 
         unless skip_prefetch
-          cask_installers.each(&:prelude)
-
+          Homebrew::Install.enqueue_cask_installers(cask_installers, 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
       ensure

Library/Homebrew/cask/upgrade.rb

@@ -5,6 +5,7 @@
 require "cask/config"
 require "cask/quarantine"
 require "deprecate_disable"
+require "install"
 require "utils/output"
 
 module Cask
@@ -245,12 +246,10 @@ def self.upgrade_casks!(
             # rubocop:enable Style/DoubleNegation
           end
 
-          fetchable_cask_installers.each(&:prelude)
-
           fetchable_casks_sentence = fetchable_casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence
+          Homebrew::Install.enqueue_cask_installers(fetchable_cask_installers,
+                                                    download_queue: prefetch_download_queue)
           oh1 "Fetching downloads for: #{fetchable_casks_sentence}", truncate: false
-
-          fetchable_cask_installers.each(&:enqueue_downloads)
           prefetch_download_queue.fetch
         ensure
           prefetch_download_queue.shutdown if created_download_queue

Library/Homebrew/cmd/install.rb

@@ -369,7 +369,7 @@ def run
                 )
               end
 
-              Install.enqueue_cask_installers(fetch_cask_installers)
+              Install.enqueue_cask_installers(fetch_cask_installers, download_queue:)
             end
 
             download_queue.fetch

Library/Homebrew/cmd/reinstall.rb

@@ -248,7 +248,7 @@ def run
                   defer_fetch:    true,
                 )
               end
-              Install.enqueue_cask_installers(fetch_cask_installers)
+              Install.enqueue_cask_installers(fetch_cask_installers, download_queue: shared_download_queue)
               shared_download_queue.fetch
               casks_prefetched = true
               valid_formula_installers

Library/Homebrew/cmd/upgrade.rb

@@ -748,7 +748,7 @@ def prefetch_outdated_casks!(casks, download_queue:, prefetch_names: nil,
           )
         end
         cask_names = outdated_casks.map(&:full_name)
-        Install.enqueue_cask_installers(fetchable_cask_installers)
+        Install.enqueue_cask_installers(fetchable_cask_installers, download_queue:)
         prefetch_names&.replace(cask_names)
         prefetch_upgrades&.replace(
           outdated_casks.map { |cask| "#{cask.full_name} #{cask.installed_version} -> #{cask.version}" },

Library/Homebrew/install.rb

@@ -402,9 +402,17 @@ def show_combined_fetch_downloads_heading(formula_names: [], cask_names: [])
         oh1 "Fetching downloads for: #{combined_fetch_targets.to_sentence}", truncate: false
       end
 
-      sig { params(cask_installers: T::Array[T.untyped]).void }
-      def enqueue_cask_installers(cask_installers)
-        cask_installers.each(&:prelude)
+      sig { params(cask_installers: T::Array[T.untyped], download_queue: Homebrew::DownloadQueue).void }
+      def enqueue_cask_installers(cask_installers, download_queue:)
+        if cask_installers.any?(&:source_download_requires_pre_fetch?)
+          source_downloads = cask_installers.filter_map(&:prelude_fetch_download)
+          if source_downloads.any?
+            oh1 "Downloading Cask files"
+            source_downloads.each { |source_download| download_queue.enqueue(source_download) }
+            download_queue.fetch
+          end
+        end
+
         cask_installers.each(&:enqueue_downloads)
       end
 

Library/Homebrew/test/cask/installer_spec.rb

@@ -549,6 +549,40 @@ class #{Formulary.class_s(dep_name)} < Formula
     end
   end
 
+  describe "#prelude_fetch" do
+    it "enqueues source API caskfiles before the main cask download" do
+      cask = Cask::Cask.new("source-api-cask") do
+        url "file://#{TEST_FIXTURE_DIR}/cask/container.tar.gz"
+      end
+      allow(cask).to receive_messages(loaded_from_api?: true, caskfile_only?: true, languages: ["en"])
+      download_queue = instance_double(Homebrew::DownloadQueue)
+      installer = klass.new(cask, download_queue:)
+      source_download = instance_double(Homebrew::API::SourceDownload, downloaded?: false)
+
+      expect(Homebrew::API::Cask).to receive(:source_download_for).with(cask).and_return(source_download)
+      expect(download_queue).to receive(:enqueue).with(source_download)
+      expect(Homebrew::API::Cask).not_to receive(:source_download_cask)
+      expect(installer).not_to receive(:download)
+
+      installer.prelude_fetch
+    end
+
+    it "leaves source API caskfiles in the main queue when their URL is known" do
+      cask = Cask::Cask.new("source-api-cask") do
+        url "file://#{TEST_FIXTURE_DIR}/cask/container.tar.gz"
+      end
+      allow(cask).to receive_messages(loaded_from_api?: true, caskfile_only?: true, languages: [])
+      download_queue = instance_double(Homebrew::DownloadQueue)
+      installer = klass.new(cask, download_queue:)
+
+      expect(Homebrew::API::Cask).to receive(:source_download).with(cask, download_queue:, enqueue: true)
+      expect(Homebrew::API::Cask).not_to receive(:source_download_cask)
+      expect(download_queue).to receive(:enqueue).with(instance_of(Cask::Download))
+
+      installer.enqueue_downloads
+    end
+  end
+
   describe "rename operations" do
     let(:tmpdir) { mktmpdir }
     let(:staged_path) { Pathname(tmpdir) }

Library/Homebrew/test/cask/reinstall_spec.rb

@@ -69,11 +69,13 @@
 
     failing_installer = instance_double(Cask::Installer)
     allow(failing_installer).to receive(:prelude)
+    allow(failing_installer).to receive(:source_download_requires_pre_fetch?).and_return(false)
     allow(failing_installer).to receive(:enqueue_downloads)
     allow(failing_installer).to receive(:install).and_raise(Cask::CaskError.new("reinstall failed"))
 
     successful_installer = instance_double(Cask::Installer)
     allow(successful_installer).to receive(:prelude)
+    allow(successful_installer).to receive(:source_download_requires_pre_fetch?).and_return(false)
     allow(successful_installer).to receive(:enqueue_downloads)
 
     allow(Cask::Installer).to receive(:new).and_return(failing_installer, successful_installer)