Merge pull request #20245 from Homebrew/download_queue_install

Optionally use `download_queue` for `brew install`

Library/Homebrew/dependency.rb

@@ -51,13 +51,13 @@ class Dependency
     end
     return false unless formula
 
+    # If the opt prefix doesn't exist: we likely have an incomplete installation.
+    return false unless formula.opt_prefix.exist?
+
     return true if formula.latest_version_installed?
 
     return false if minimum_version.blank?
 
-    # If the opt prefix doesn't exist: we likely have an incomplete installation.
-    return false unless formula.opt_prefix.exist?
-
     installed_keg = formula.any_installed_keg
     return false unless installed_keg
 

Library/Homebrew/download_queue.rb

@@ -8,12 +8,13 @@ require "retryable_download"
 
 module Homebrew
   class DownloadQueue
-    sig { params(retries: Integer, force: T::Boolean).void }
+    sig { params(retries: Integer, force: T::Boolean, pour: T::Boolean).void }
-    def initialize(retries: 0, force: false)
+    def initialize(retries: 0, force: false, pour: false)
       @concurrency = T.let(EnvConfig.download_concurrency, Integer)
       @quiet = T.let(@concurrency > 1, T::Boolean)
       @tries = T.let(retries + 1, Integer)
       @force = force
+      @pour = pour
       @pool = T.let(Concurrent::FixedThreadPool.new(concurrency), Concurrent::FixedThreadPool)
     end
 
@@ -24,6 +25,10 @@ module Homebrew
       ) do |download, force, quiet|
         download.clear_cache if force
         download.fetch(quiet:)
+        if pour && download.bottle?
+          UnpackStrategy.detect(download.cached_download, prioritize_extension: true)
+                        .extract_nestedly(to: HOMEBREW_CELLAR)
+        end
       end
     end
 
@@ -168,6 +173,9 @@ module Homebrew
     sig { returns(T::Boolean) }
     attr_reader :quiet
 
+    sig { returns(T::Boolean) }
+    attr_reader :pour
+
     sig { returns(T::Hash[Downloadable, Concurrent::Promises::Future]) }
     def downloads
       @downloads ||= T.let({}, T.nilable(T::Hash[Downloadable, Concurrent::Promises::Future]))

Library/Homebrew/download_strategy.rb

@@ -30,19 +30,6 @@ class AbstractDownloadStrategy
 
   abstract!
 
-  # Extension for bottle downloads.
-  module Pourable
-    extend T::Helpers
-
-    requires_ancestor { AbstractDownloadStrategy }
-
-    sig { params(block: T.nilable(T.proc.params(arg0: String).returns(T.anything))).returns(T.nilable(T.anything)) }
-    def stage(&block)
-      ohai "Pouring #{basename}"
-      super
-    end
-  end
-
   # The download URL.
   #
   # @api public
@@ -74,7 +61,6 @@ class AbstractDownloadStrategy
     @cache = T.let(meta.fetch(:cache, HOMEBREW_CACHE), Pathname)
     @meta = T.let(meta, T::Hash[Symbol, T.untyped])
     @quiet = T.let(false, T.nilable(T::Boolean))
-    extend Pourable if meta[:bottle]
   end
 
   # Download and cache the resource at {#cached_location}.
@@ -826,7 +812,6 @@ class LocalBottleDownloadStrategy < AbstractFileDownloadStrategy
   sig { params(path: Pathname).void }
   def initialize(path)
     @cached_location = T.let(path, Pathname)
-    extend Pourable
   end
   # rubocop:enable Lint/MissingSuper
 

Library/Homebrew/formula_installer.rb

@@ -44,6 +44,9 @@ class FormulaInstaller
   sig { returns(T::Boolean) }
   attr_accessor :link_keg
 
+  sig { returns(T.nilable(Homebrew::DownloadQueue)) }
+  attr_accessor :download_queue
+
   sig {
     params(
       formula:                    Formula,
@@ -136,9 +139,12 @@ class FormulaInstaller
     @hold_locks = T.let(false, T::Boolean)
     @show_summary_heading = T.let(false, T::Boolean)
     @etc_var_preinstall = T.let([], T::Array[Pathname])
+    @download_queue = T.let(nil, T.nilable(Homebrew::DownloadQueue))
 
     # Take the original formula instance, which might have been swapped from an API instance to a source instance
     @formula = T.let(T.must(previously_fetched_formula), Formula) if previously_fetched_formula
+
+    @ran_prelude_fetch = T.let(false, T::Boolean)
   end
 
   sig { returns(T::Boolean) }
@@ -294,7 +300,7 @@ class FormulaInstaller
   end
 
   sig { void }
-  def prelude
+  def prelude_fetch
     deprecate_disable_type = DeprecateDisable.type(formula)
     if deprecate_disable_type.present?
       message = "#{formula.full_name} has been #{DeprecateDisable.message(formula)}"
@@ -312,8 +318,24 @@ class FormulaInstaller
       end
     end
 
+    # Needs to be done before expand_dependencies for compute_dependencies
+    fetch_bottle_tab if pour_bottle?
+
+    @ran_prelude_fetch = true
+  end
+
+  sig { void }
+  def prelude
+    prelude_fetch unless @ran_prelude_fetch
+
     Tab.clear_cache
 
+    # Setup bottle_tab_runtime_dependencies for compute_dependencies
+    @bottle_tab_runtime_dependencies = formula.bottle_tab_attributes
+                                              .fetch("runtime_dependencies", []).then { |deps| deps || [] }
+                                              .each_with_object({}) { |dep, h| h[dep["full_name"]] = dep }
+                                              .freeze
+
     verify_deps_exist unless ignore_deps?
 
     forbidden_license_check
@@ -778,13 +800,15 @@ on_request: installed_on_request?, options:)
     if deps.empty? && only_deps?
       puts "All dependencies for #{formula.full_name} are satisfied."
     elsif !deps.empty?
+      if deps.length > 1
         oh1 "Installing dependencies for #{formula.full_name}: " \
             "#{deps.map(&:first).map { Formatter.identifier(_1) }.to_sentence}",
             truncate: false
+      end
       deps.each { |dep, options| install_dependency(dep, options) }
     end
 
-    @show_header = true unless deps.empty?
+    @show_header = true if deps.length > 1
   end
 
   sig { params(dep: Dependency).void }
@@ -808,6 +832,7 @@ on_request: installed_on_request?, options:)
       quiet:                      quiet?,
       verbose:                    verbose?,
     )
+    fi.download_queue = download_queue
     fi.prelude
     fi.fetch
   end
@@ -830,7 +855,7 @@ on_request: installed_on_request?, options:)
       installed_keg = Keg.new(df.prefix)
       tab ||= installed_keg.tab
       tmp_keg = Pathname.new("#{installed_keg}.tmp")
-      installed_keg.rename(tmp_keg)
+      installed_keg.rename(tmp_keg) unless tmp_keg.directory?
     end
 
     if df.tap.present? && tab.present? && (tab_tap = tab.source["tap"].presence) &&
@@ -867,6 +892,7 @@ on_request: installed_on_request?, options:)
       verbose:                    verbose?,
     )
     oh1 "Installing #{formula.full_name} dependency: #{Formatter.identifier(dep.name)}"
+    fi.prelude
     fi.install
     fi.finish
   # Handle all possible exceptions installing deps.
@@ -1337,9 +1363,13 @@ on_request: installed_on_request?, options:)
 
     return if deps.empty?
 
-    oh1 "Fetching dependencies for #{formula.full_name}: " \
+    unless download_queue
-        "#{deps.map(&:first).map { Formatter.identifier(_1) }.to_sentence}",
+      dependencies_string = deps.map(&:first)
+                                .map { Formatter.identifier(_1) }
+                                .to_sentence
+      oh1 "Fetching dependencies for #{formula.full_name}: #{dependencies_string}",
           truncate: false
+    end
 
     deps.each { |(dep, _options)| fetch_dependency(dep) }
   end
@@ -1360,15 +1390,18 @@ on_request: installed_on_request?, options:)
   def fetch_bottle_tab(quiet: false)
     return if @fetch_bottle_tab
 
+    if (download_queue = self.download_queue) &&
+       (bottle = formula.bottle) &&
+       (manifest_resource = bottle.github_packages_manifest_resource)
+      download_queue.enqueue(manifest_resource)
+    else
       begin
         formula.fetch_bottle_tab(quiet: quiet)
-      @bottle_tab_runtime_dependencies = formula.bottle_tab_attributes
-                                                .fetch("runtime_dependencies", []).then { |deps| deps || [] }
-                                                .each_with_object({}) { |dep, h| h[dep["full_name"]] = dep }
-                                                .freeze
       rescue DownloadError, Resource::BottleManifest::Error
         # do nothing
       end
+    end
+
     @fetch_bottle_tab = T.let(true, T.nilable(TrueClass))
   end
 
@@ -1381,7 +1414,7 @@ on_request: installed_on_request?, options:)
     return if only_deps?
     return if formula.local_bottle_path.present?
 
-    oh1 "Fetching #{Formatter.identifier(formula.full_name)}".strip
+    oh1 "Fetching #{Formatter.identifier(formula.full_name)}".strip unless download_queue
 
     downloadable_object = downloadable
     check_attestation = if pour_bottle?(output_warning: true)
@@ -1391,19 +1424,31 @@ on_request: installed_on_request?, options:)
     else
       @formula = Homebrew::API::Formula.source_download(formula) if formula.loaded_from_api?
 
+      if (download_queue = self.download_queue)
+        formula.enqueue_resources_and_patches(download_queue:)
+      else
         formula.fetch_patches
         formula.resources.each(&:fetch)
+      end
+
       downloadable_object = downloadable
 
       false
     end
+
+    if (download_queue = self.download_queue)
+      download_queue.enqueue(downloadable_object)
+    else
       downloadable_object.fetch
+    end
 
     # We skip `gh` to avoid a bootstrapping cycle, in the off-chance a user attempts
     # to explicitly `brew install gh` without already having a version for bootstrapping.
     # We also skip bottle installs from local bottle paths, as these are done in CI
     # as part of the build lifecycle before attestations are produced.
     if check_attestation &&
+       # TODO: support this for download queues at some point
+       download_queue.nil? &&
        Homebrew::Attestation.enabled? &&
        formula.tap&.core_tap? &&
        formula.name != "gh"
@@ -1489,7 +1534,10 @@ on_request: installed_on_request?, options:)
   sig { void }
   def pour
     HOMEBREW_CELLAR.cd do
-      downloadable.downloader.stage
+      # download queue has already done the actual staging but we'll lie about
+      # pouring now for nicer output
+      ohai "Pouring #{downloadable.downloader.basename}"
+      downloadable.downloader.stage unless download_queue
     end
 
     Tab.clear_cache

Library/Homebrew/install.rb

@@ -6,6 +6,7 @@ require "fileutils"
 require "hardware"
 require "development_tools"
 require "upgrade"
+require "download_queue"
 
 module Homebrew
   # Helper module for performing (pre-)install checks.
@@ -313,32 +314,45 @@ module Homebrew
         skip_post_install: false,
         skip_link: false
       )
-        unless dry_run
+        formulae_names_to_install = formula_installers.map { |fi| fi.formula.name }
-          formula_installers.each do |fi|
+        return if formulae_names_to_install.empty?
-            fi.prelude
-            fi.fetch
-          rescue CannotInstallFormulaError => e
-            ofail e.message
-            next
-          rescue UnsatisfiedRequirements, DownloadError, ChecksumMismatchError => e
-            ofail "#{fi.formula}: #{e}"
-            next
-          end
-        end
 
         if dry_run
-          if (formulae_name_to_install = formula_installers.map { |fi| fi.formula.name })
+          ohai "Would install #{Utils.pluralize("formula", formulae_names_to_install.count,
-            ohai "Would install #{Utils.pluralize("formula", formulae_name_to_install.count,
                                                 plural: "e", include_count: true)}:"
-            puts formulae_name_to_install.join(" ")
+          puts formulae_names_to_install.join(" ")
 
           formula_installers.each do |fi|
             print_dry_run_dependencies(fi.formula, fi.compute_dependencies, &:name)
           end
-          end
           return
         end
 
+        formula_sentence = formulae_names_to_install.map { |name| Formatter.identifier(name) }.to_sentence
+        oh1 "Fetching downloads for: #{formula_sentence}", truncate: false
+        if EnvConfig.download_concurrency > 1
+          download_queue = Homebrew::DownloadQueue.new(pour: true)
+          formula_installers.each do |fi|
+            fi.download_queue = download_queue
+          end
+        end
+        begin
+          [:prelude_fetch, :prelude, :fetch].each do |step|
+            formula_installers.each do |fi|
+              fi.public_send(step)
+            rescue UnsatisfiedRequirements, DownloadError, ChecksumMismatchError => e
+              ofail "#{fi.formula}: #{e}"
+              next
+            end
+            download_queue&.fetch
+          rescue CannotInstallFormulaError => e
+            ofail e.message
+            next
+          end
+        ensure
+          download_queue&.shutdown
+        end
+
         formula_installers.each do |fi|
           install_formula(fi)
           Cleanup.install_formula_clean!(fi.formula)

Library/Homebrew/retryable_download.rb

@@ -89,6 +89,9 @@ module Homebrew
     sig { override.returns(String) }
     def download_name = downloadable.download_name
 
+    sig { returns(T::Boolean) }
+    def bottle? = downloadable.is_a?(Bottle)
+
     private
 
     sig { returns(Downloadable) }

Library/Homebrew/test/cmd/deps_spec.rb

@@ -29,10 +29,13 @@ RSpec.describe Homebrew::Cmd::Deps do
     setup_test_formula "recommended_test"
     setup_test_formula "installed"
 
-    # Mock `Formula#any_version_installed?` by creating the tab in a plausible keg directory
+    # Mock `Formula#any_version_installed?` by creating the tab in a plausible keg directory and opt link
-    keg_dir = HOMEBREW_CELLAR/"installed"/"1.0"
+    keg_dir = HOMEBREW_CELLAR/"installed/1.0"
     keg_dir.mkpath
     touch keg_dir/AbstractTab::FILENAME
+    opt_link = HOMEBREW_PREFIX/"opt/installed"
+    opt_link.parent.mkpath
+    FileUtils.ln_sf keg_dir, opt_link
 
     expect { brew "deps", "baz", "--include-test", "--missing", "--skip-recommended" }
       .to be_a_success

Library/Homebrew/test/download_strategies/abstract_spec.rb

@@ -19,18 +19,4 @@ RSpec.describe AbstractDownloadStrategy do
       expect(strategy.source_modified_time).to eq(File.mtime("foo"))
     end
   end
-
-  context "when specs[:bottle]" do
-    let(:specs) { { bottle: true } }
-
-    it "extends Pourable" do
-      expect(strategy).to be_a(AbstractDownloadStrategy::Pourable)
-    end
-  end
-
-  context "without specs[:bottle]" do
-    it "is does not extend Pourable" do
-      expect(strategy).not_to be_a(AbstractDownloadStrategy::Pourable)
-    end
-  end
 end