diff --git a/Library/Homebrew/Gemfile b/Library/Homebrew/Gemfile index 7ea0f6689f2c47..4bd8cfd1b628a7 100644 --- a/Library/Homebrew/Gemfile +++ b/Library/Homebrew/Gemfile @@ -81,4 +81,4 @@ gem "plist" gem "ruby-macho" gem "sorbet-runtime" gem "warning" -gem 'whirly' +gem "whirly" diff --git a/Library/Homebrew/cask/download.rb b/Library/Homebrew/cask/download.rb index cf71a16fa2766f..b48444b9020eaa 100644 --- a/Library/Homebrew/cask/download.rb +++ b/Library/Homebrew/cask/download.rb @@ -8,7 +8,7 @@ module Cask # A download corresponding to a {Cask}. - class Download < ::Downloadable + class Download < Downloadable include Context attr_reader :cask @@ -20,6 +20,11 @@ def initialize(cask, quarantine: nil) @quarantine = quarantine end + sig { override.returns(String) } + def name + cask.token + end + sig { override.returns(T.nilable(::URL)) } def url return if cask.url.nil? @@ -88,6 +93,11 @@ def download_name cask.token end + sig { override.returns(String) } + def download_type + "cask" + end + private def quarantine(path) diff --git a/Library/Homebrew/cmd/fetch.rb b/Library/Homebrew/cmd/fetch.rb index 168c671d4deb48..a182b34928fd75 100644 --- a/Library/Homebrew/cmd/fetch.rb +++ b/Library/Homebrew/cmd/fetch.rb @@ -5,6 +5,8 @@ require "formula" require "fetch" require "cask/download" +require "retryable_download" +require "whirly" module Homebrew module Cmd @@ -25,6 +27,7 @@ class FetchCmd < AbstractCommand "(Pass `all` to download for all architectures.)" flag "--bottle-tag=", description: "Download a bottle for given tag." + flag "--concurrency=", description: "Number of concurrent downloads.", hidden: true switch "--HEAD", description: "Fetch HEAD version instead of stable version." switch "-f", "--force", @@ -67,10 +70,23 @@ class FetchCmd < AbstractCommand named_args [:formula, :cask], min: 1 end + def concurrency + @concurrency ||= args.concurrency&.to_i || 1 + end + + def download_queue + @download_queue ||= begin + require "download_queue" + DownloadQueue.new(concurrency) + end + end + sig { override.void } def run Formulary.enable_factory_cache! + Homebrew.install_gem! "concurrent-ruby" + bucket = if args.deps? args.named.to_formulae_and_casks.flat_map do |formula_or_cask| case formula_or_cask @@ -125,13 +141,10 @@ def run next end - begin - bottle.fetch_tab - rescue DownloadError - retry if retry_fetch?(bottle) - raise + if (manifest_resource = bottle.github_packages_manifest_resource) + fetch_downloadable(manifest_resource) end - fetch_formula(bottle) + fetch_downloadable(bottle) rescue Interrupt raise rescue => e @@ -147,14 +160,14 @@ def run next if fetched_bottle - fetch_formula(formula) + fetch_downloadable(formula.resource) formula.resources.each do |r| - fetch_resource(r) - r.patches.each { |p| fetch_patch(p) if p.external? } + fetch_downloadable(r) + r.patches.each { |patch| fetch_downloadable(patch.resource) if patch.external? } end - formula.patchlist.each { |p| fetch_patch(p) if p.external? } + formula.patchlist.each { |patch| fetch_downloadable(patch.resource) if patch.external? } end end else @@ -176,81 +189,43 @@ def run quarantine = true if quarantine.nil? download = Cask::Download.new(cask, quarantine:) - fetch_cask(download) + fetch_downloadable(download) end end end end - end - private + downloads.each do |downloadable, promise| + message = "#{downloadable.download_type.capitalize} #{downloadable.name}" + if concurrency > 1 + Whirly.start spinner: "arc", status: message + else + puts message + end - def fetch_resource(resource) - puts "Resource: #{resource.name}" - fetch_fetchable resource - rescue ChecksumMismatchError => e - retry if retry_fetch?(resource) - opoo "Resource #{resource.name} reports different sha256: #{e.expected}" - end + promise.wait! - def fetch_formula(formula) - fetch_fetchable(formula) - rescue ChecksumMismatchError => e - retry if retry_fetch?(formula) - opoo "Formula reports different sha256: #{e.expected}" - end + Whirly.configure stop: "✔︎" + Whirly.stop if args.concurrency + rescue ChecksumMismatchError => e + Whirly.configure stop: "✘" + Whirly.stop if args.concurrency - def fetch_cask(cask_download) - fetch_fetchable(cask_download) - rescue ChecksumMismatchError => e - retry if retry_fetch?(cask_download) - opoo "Cask reports different sha256: #{e.expected}" - end + opoo "#{downloadable.download_type.capitalize} reports different checksum: #{e.expected}" + Homebrew.failed = true if downloadable.is_a?(Resource::Patch) + end - def fetch_patch(patch) - fetch_fetchable(patch) - rescue ChecksumMismatchError => e - opoo "Patch reports different sha256: #{e.expected}" - Homebrew.failed = true + download_queue.shutdown end - def retry_fetch?(formula) - @fetch_tries ||= Hash.new { |h, k| h[k] = 1 } - if args.retry? && (@fetch_tries[formula] < FETCH_MAX_TRIES) - wait = 2 ** @fetch_tries[formula] - remaining = FETCH_MAX_TRIES - @fetch_tries[formula] - what = Utils.pluralize("tr", remaining, plural: "ies", singular: "y") - - ohai "Retrying download in #{wait}s... (#{remaining} #{what} left)" - sleep wait + private - formula.clear_cache - @fetch_tries[formula] += 1 - true - else - Homebrew.failed = true - false - end + def downloads + @downloads ||= {} end - def fetch_fetchable(formula) - formula.clear_cache if args.force? - - already_fetched = formula.cached_download.exist? - - begin - download = formula.fetch(verify_download_integrity: false) - rescue DownloadError - retry if retry_fetch?(formula) - raise - end - - return unless download.file? - - puts "Downloaded to: #{download}" unless already_fetched - puts "SHA256: #{download.sha256}" - - formula.verify_download_integrity(download) + def fetch_downloadable(downloadable) + downloads[downloadable] ||= download_queue.enqueue(RetryableDownload.new(downloadable)) end end end diff --git a/Library/Homebrew/download_queue.rb b/Library/Homebrew/download_queue.rb new file mode 100644 index 00000000000000..9a5a7205b8f02f --- /dev/null +++ b/Library/Homebrew/download_queue.rb @@ -0,0 +1,28 @@ +# typed: true +# frozen_string_literal: true + +require "downloadable" +require "concurrent" + +module Homebrew + class DownloadQueue + private attr_reader :pool + + sig { params(size: Integer).void } + def initialize(size = 1) + @pool = Concurrent::FixedThreadPool.new(size) + end + + sig { params(downloadable: Downloadable).returns(Concurrent::Promise) } + def enqueue(downloadable) + Concurrent::Promise.execute(executor: pool) do + downloadable.fetch(quiet: pool.max_length > 1) + end + end + + sig { void } + def shutdown + pool.shutdown + end + end +end diff --git a/Library/Homebrew/downloadable.rb b/Library/Homebrew/downloadable.rb index 8c03630621f82b..5d3a7e7ce85178 100644 --- a/Library/Homebrew/downloadable.rb +++ b/Library/Homebrew/downloadable.rb @@ -39,6 +39,16 @@ def freeze super end + sig { returns(String) } + def name + "" + end + + sig { returns(String) } + def download_type + self.class.name.split("::").last.gsub(/([[:lower:]])([[:upper:]])/, '\1 \2').downcase + end + sig { returns(T::Boolean) } def downloaded? cached_download.exist? @@ -78,11 +88,15 @@ def downloader end end - sig { params(verify_download_integrity: T::Boolean, timeout: T.nilable(T.any(Integer, Float))).returns(Pathname) } - def fetch(verify_download_integrity: true, timeout: nil) + sig { + params(verify_download_integrity: T::Boolean, timeout: T.nilable(T.any(Integer, Float)), + quiet: T::Boolean).returns(Pathname) + } + def fetch(verify_download_integrity: true, timeout: nil, quiet: false) cache.mkpath begin + downloader.quiet! if quiet downloader.fetch(timeout:) rescue ErrorDuringExecution, CurlDownloadStrategyError => e raise DownloadError.new(self, e) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 99ba30e834c1be..b32992eb986924 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -564,7 +564,7 @@ def synced_with_other_formulae? params(name: String, klass: T.class_of(Resource), block: T.nilable(T.proc.bind(Resource).void)) .returns(T.nilable(Resource)) } - def resource(name, klass = Resource, &block) = active_spec.resource(name, klass, &block) + def resource(name = T.unsafe(nil), klass = T.unsafe(nil), &block) = active_spec.resource(*name, *klass, &block) # Old names for the formula. # @@ -2750,8 +2750,12 @@ def on_system_blocks_exist? self.class.on_system_blocks_exist? || @on_system_blocks_exist end - def fetch(verify_download_integrity: true) - active_spec.fetch(verify_download_integrity:) + sig { + params(verify_download_integrity: T::Boolean, timeout: T.nilable(T.any(Integer, Float)), + quiet: T::Boolean).returns(Pathname) + } + def fetch(verify_download_integrity: true, timeout: nil, quiet: false) + active_spec.fetch(verify_download_integrity:, timeout:, quiet:) end def verify_download_integrity(filename) diff --git a/Library/Homebrew/patch.rb b/Library/Homebrew/patch.rb index 2f7cceae958c53..d88a59ca1bf076 100644 --- a/Library/Homebrew/patch.rb +++ b/Library/Homebrew/patch.rb @@ -106,7 +106,7 @@ class ExternalPatch def initialize(strip, &block) @strip = strip - @resource = Resource::PatchResource.new(&block) + @resource = Resource::Patch.new(&block) end sig { returns(T::Boolean) } diff --git a/Library/Homebrew/resource.rb b/Library/Homebrew/resource.rb index 7c4d6e773e3eae..5402514fa6a6cb 100644 --- a/Library/Homebrew/resource.rb +++ b/Library/Homebrew/resource.rb @@ -140,7 +140,15 @@ def files(*files) Partial.new(self, files) end - def fetch(verify_download_integrity: true) + sig { + override + .params( + verify_download_integrity: T::Boolean, + timeout: T.nilable(T.any(Integer, Float)), + quiet: T.nilable(T::Boolean), + ).returns(Pathname) + } + def fetch(verify_download_integrity: true, timeout: nil, quiet: false) fetch_patches super @@ -260,6 +268,13 @@ def determine_url_mirrors [*extra_urls, *super].uniq end + # A resource for a formula. + class Formula < Resource + def name + super || owner&.name + end + end + # A resource containing a Go package. class Go < Resource def stage(target, &block) @@ -320,7 +335,7 @@ def tab end # A resource containing a patch. - class PatchResource < Resource + class Patch < Resource attr_reader :patch_files def initialize(&block) diff --git a/Library/Homebrew/retryable_download.rb b/Library/Homebrew/retryable_download.rb new file mode 100644 index 00000000000000..dde7be6e00e81d --- /dev/null +++ b/Library/Homebrew/retryable_download.rb @@ -0,0 +1,68 @@ +# typed: true +# frozen_string_literal: true + +module Homebrew + class RetryableDownload < Downloadable + private attr_reader :downloadable + + sig { params(downloadable: Downloadable, tries: Integer).void } + def initialize(downloadable, tries: 3) + super() + + @downloadable = downloadable + @try = 0 + @tries = tries + end + + sig { override.returns(String) } + def name = downloadable.name + + sig { override.returns(String) } + def download_type = downloadable.download_type + + sig { override.returns(T::Boolean) } + def downloaded? = downloadable.downloaded? + + sig { override.returns(Pathname) } + def cached_download = downloadable.cached_download + + sig { + override.params( + verify_download_integrity: T::Boolean, + timeout: T.nilable(T.any(Integer, Float)), + quiet: T::Boolean, + ).returns(Pathname) + } + def fetch(verify_download_integrity: true, timeout: nil, quiet: false) + @try += 1 + + already_downloaded = downloadable.downloaded? + + download = downloadable.fetch(verify_download_integrity: false, timeout:, quiet:) + + return unless download.file? + + unless quiet + puts "Downloaded to: #{download}" unless already_downloaded + puts "SHA256: #{download.sha256}" + end + + downloadable.verify_download_integrity(download) if verify_download_integrity + + download + rescue DownloadError, ChecksumMismatchError + tries_remaining = @tries - @try + raise if tries_remaining.zero? + + wait = 2 ** @try + unless quiet + what = Utils.pluralize("tr", tries_remaining, plural: "ies", singular: "y") + ohai "Retrying download in #{wait}s... (#{tries_remaining} #{what} left)" + end + sleep wait + + downloadable.clear_cache + retry + end + end +end diff --git a/Library/Homebrew/software_spec.rb b/Library/Homebrew/software_spec.rb index 63e9e1b89778c7..448b9ebe964527 100644 --- a/Library/Homebrew/software_spec.rb +++ b/Library/Homebrew/software_spec.rb @@ -15,7 +15,7 @@ require "macos_version" require "extend/on_system" -class SoftwareSpec +class SoftwareSpec < Downloadable extend Forwardable include OnSystem::MacOSAndLinux @@ -34,8 +34,10 @@ class SoftwareSpec def_delegators :@resource, :sha256 def initialize(flags: []) + super() + # Ensure this is synced with `initialize_dup` and `freeze` (excluding simple objects like integers and booleans) - @resource = Resource.new + @resource = Resource::Formula.new @resources = {} @dependency_collector = DependencyCollector.new @bottle_specification = BottleSpecification.new @@ -78,6 +80,11 @@ def freeze super end + sig { override.returns(String) } + def download_type + "formula" + end + def owner=(owner) @name = owner.name @full_name = owner.full_name @@ -127,8 +134,9 @@ def resource_defined?(name) params(name: String, klass: T.class_of(Resource), block: T.nilable(T.proc.bind(Resource).void)) .returns(T.nilable(Resource)) } - def resource(name, klass = Resource, &block) + def resource(name = T.unsafe(nil), klass = Resource, &block) if block + raise ArgumentError, "Resource must have a name." if name.nil? raise DuplicateResourceError, name if resource_defined?(name) res = klass.new(name, &block) @@ -138,6 +146,8 @@ def resource(name, klass = Resource, &block) dependency_collector.add(res) res else + return @resource if name.nil? + resources.fetch(name) { raise ResourceMissingError.new(owner, name) } end end @@ -285,7 +295,7 @@ def verify_download_integrity(_filename) end end -class Bottle +class Bottle < Downloadable class Filename attr_reader :name, :version, :tag, :rebuild @@ -342,6 +352,8 @@ def extname def_delegators :resource, :cached_download def initialize(formula, spec, tag = nil) + super() + @name = formula.name @resource = Resource.new @resource.owner = formula @@ -361,8 +373,15 @@ def initialize(formula, spec, tag = nil) root_url(spec.root_url, spec.root_url_specs) end - def fetch(verify_download_integrity: true) - @resource.fetch(verify_download_integrity:) + sig { + override.params( + verify_download_integrity: T::Boolean, + timeout: T.nilable(T.any(Integer, Float)), + quiet: T.nilable(T::Boolean), + ).returns(Pathname) + } + def fetch(verify_download_integrity: true, timeout: nil, quiet: false) + resource.fetch(verify_download_integrity:, timeout:, quiet:) rescue DownloadError raise unless fallback_on_error @@ -370,6 +389,7 @@ def fetch(verify_download_integrity: true) retry end + sig { override.void } def clear_cache @resource.clear_cache github_packages_manifest_resource&.clear_cache @@ -389,26 +409,30 @@ def stage resource.downloader.stage end - def fetch_tab - return if github_packages_manifest_resource.blank? + def fetch_tab(timeout: nil, quiet: false) + return unless (resource = github_packages_manifest_resource) - github_packages_manifest_resource.fetch - rescue DownloadError - raise unless fallback_on_error + begin + resource.fetch(timeout:, quiet:) + rescue DownloadError + raise unless fallback_on_error - retry - rescue ArgumentError - raise if @fetch_tab_retried + retry + rescue ArgumentError + raise if @fetch_tab_retried - @fetch_tab_retried = true - github_packages_manifest_resource.clear_cache - retry + @fetch_tab_retried = true + resource.clear_cache + retry + end end def tab_attributes - return {} unless github_packages_manifest_resource&.downloaded? + if (resource = github_packages_manifest_resource) && resource.downloaded? + return resource.tab + end - github_packages_manifest_resource.tab + {} end sig { returns(Filename) } @@ -416,8 +440,7 @@ def filename Filename.create(resource.owner, @tag, @spec.rebuild) end - private - + sig { returns(T.nilable(Resource::BottleManifest)) } def github_packages_manifest_resource return if @resource.download_strategy != CurlGitHubPackagesDownloadStrategy @@ -440,6 +463,8 @@ def github_packages_manifest_resource end end + private + def select_download_strategy(specs) specs[:using] ||= DownloadStrategyDetector.detect(@root_url) specs[:bottle] = true diff --git a/Library/Homebrew/test/patch_spec.rb b/Library/Homebrew/test/patch_spec.rb index f9c7b5c5818ee6..c546f520a1e802 100644 --- a/Library/Homebrew/test/patch_spec.rb +++ b/Library/Homebrew/test/patch_spec.rb @@ -55,7 +55,7 @@ subject(:patch) { described_class.create(:p2, nil) } context "when the patch is empty" do - it(:resource) { expect(patch.resource).to be_a Resource::PatchResource } + it(:resource) { expect(patch.resource).to be_a Resource::Patch } it { expect(patch.patch_files).to eq(patch.resource.patch_files) } it { expect(patch.patch_files).to eq([]) } end