Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement concurrent downloads in brew fetch. #17756

Merged
merged 28 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
4c29d4c
Vendor Gems.
reitermarkus Jul 14, 2024
b297be7
Implement concurrent downloads.
reitermarkus Jul 14, 2024
b6d529d
Convert `Downloadable` to a module.
reitermarkus Jul 15, 2024
f41d251
Fix type signatures.
reitermarkus Jul 15, 2024
b55528b
Color status.
reitermarkus Jul 15, 2024
601134b
Fix `--force` flag.
reitermarkus Jul 15, 2024
b353e9c
Don't require everything.
reitermarkus Jul 15, 2024
3a51f55
Move `require`.
reitermarkus Jul 15, 2024
6434533
Remove `Whirly` and show concurrent downloads.
reitermarkus Jul 15, 2024
3b5b669
Improve output.
reitermarkus Jul 15, 2024
a114575
Fix wrong terminal size.
reitermarkus Jul 16, 2024
786f5d3
Remove unneeded method.
reitermarkus Jul 16, 2024
0cafa83
Simplify `Tty` methods.
reitermarkus Jul 16, 2024
0b50f06
Fix code style.
reitermarkus Jul 16, 2024
d6ebb04
Improve output.
reitermarkus Jul 16, 2024
e7c9049
Use futures instead of promises.
reitermarkus Jul 16, 2024
404176a
Implement `Downloadable` for more types.
reitermarkus Aug 14, 2024
984cde1
Move escape codes to `Tty`.
reitermarkus Aug 14, 2024
9fd4c97
Add method for moving cursor up and to the beginning.
reitermarkus Aug 21, 2024
231ac72
Add FIXME comment.
reitermarkus Sep 4, 2024
fb0bf3b
Fix wrong `stage` method being called.
reitermarkus Sep 4, 2024
c61713b
Fix handling for `--retry` flag.
reitermarkus Sep 5, 2024
123a2ac
Comment deprecations.
reitermarkus Sep 5, 2024
846cf25
Fix wrong argument passing.
reitermarkus Sep 5, 2024
a970065
Fix wrong argument passing.
reitermarkus Sep 5, 2024
60cb810
Readd `Pourable` to `LocalBottleDownloadStrategy`.
reitermarkus Sep 5, 2024
69a04bd
Fix code style.
reitermarkus Sep 5, 2024
b68ee41
Ensure thread-pool shutdown.
reitermarkus Sep 7, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Library/Homebrew/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ end

# vendored gems (no group)
gem "addressable"
gem "concurrent-ruby"
gem "patchelf"
gem "plist"
gem "ruby-macho"
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ GEM
coderay (1.1.3)
commander (5.0.0)
highline (~> 3.0.0)
concurrent-ruby (1.3.4)
diff-lcs (1.5.1)
docile (1.4.1)
elftools (1.3.1)
Expand Down Expand Up @@ -161,6 +162,7 @@ PLATFORMS

DEPENDENCIES
addressable
concurrent-ruby
json_schemer
kramdown
method_source
Expand Down
21 changes: 19 additions & 2 deletions Library/Homebrew/api/download.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
end
end

class Download < Downloadable
class Download
include Downloadable

sig {
params(
url: String,
Expand All @@ -29,14 +31,29 @@
@cache = cache
end

sig { override.returns(API::DownloadStrategy) }
def downloader
T.cast(super, API::DownloadStrategy)

Check warning on line 36 in Library/Homebrew/api/download.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/api/download.rb#L36

Added line #L36 was not covered by tests
end

sig { override.returns(String) }
def name
download_name

Check warning on line 41 in Library/Homebrew/api/download.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/api/download.rb#L41

Added line #L41 was not covered by tests
end

sig { override.returns(String) }
def download_type
"API source"

Check warning on line 46 in Library/Homebrew/api/download.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/api/download.rb#L46

Added line #L46 was not covered by tests
end

sig { override.returns(Pathname) }
def cache
@cache || super
end

sig { returns(Pathname) }
def symlink_location
T.cast(downloader, API::DownloadStrategy).symlink_location
downloader.symlink_location

Check warning on line 56 in Library/Homebrew/api/download.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/api/download.rb#L56

Added line #L56 was not covered by tests
end
end
end
Expand Down
14 changes: 13 additions & 1 deletion Library/Homebrew/cask/download.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

module Cask
# A download corresponding to a {Cask}.
class Download < ::Downloadable
class Download
include Downloadable

include Context

attr_reader :cask
Expand All @@ -20,6 +22,11 @@
@quarantine = quarantine
end

sig { override.returns(String) }
def name
cask.token

Check warning on line 27 in Library/Homebrew/cask/download.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/download.rb#L27

Added line #L27 was not covered by tests
end

sig { override.returns(T.nilable(::URL)) }
def url
return if cask.url.nil?
Expand Down Expand Up @@ -88,6 +95,11 @@
cask.token
end

sig { override.returns(String) }
def download_type
"cask"

Check warning on line 100 in Library/Homebrew/cask/download.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/download.rb#L100

Added line #L100 was not covered by tests
end

private

def quarantine(path)
Expand Down
210 changes: 144 additions & 66 deletions Library/Homebrew/cmd/fetch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require "formula"
require "fetch"
require "cask/download"
require "retryable_download"

module Homebrew
module Cmd
Expand All @@ -25,6 +26,7 @@
"(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
MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved
switch "--HEAD",
description: "Fetch HEAD version instead of stable version."
switch "-f", "--force",
Expand Down Expand Up @@ -67,6 +69,49 @@
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

class Spinner
FRAMES = [
"⠋",
"⠙",
"⠚",
"⠞",
"⠖",
"⠦",
"⠴",
"⠲",
"⠳",
"⠓",
].freeze

sig { void }
def initialize
@start = Time.now
@i = 0

Check warning on line 100 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L99-L100

Added lines #L99 - L100 were not covered by tests
end

sig { returns(String) }
def to_s
now = Time.now

Check warning on line 105 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L105

Added line #L105 was not covered by tests
if @start + 0.1 < now
@start = now
@i = (@i + 1) % FRAMES.count

Check warning on line 108 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L108

Added line #L108 was not covered by tests
end

FRAMES.fetch(@i)

Check warning on line 111 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L111

Added line #L111 was not covered by tests
end
end

sig { override.void }
def run
Formulary.enable_factory_cache!
Expand Down Expand Up @@ -125,13 +170,10 @@
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)

Check warning on line 176 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L176

Added line #L176 was not covered by tests
rescue Interrupt
raise
rescue => e
Expand All @@ -147,14 +189,14 @@

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)

Check warning on line 195 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L195

Added line #L195 was not covered by tests
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
Expand All @@ -176,81 +218,117 @@
quarantine = true if quarantine.nil?

download = Cask::Download.new(cask, quarantine:)
fetch_cask(download)
fetch_downloadable(download)

Check warning on line 221 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L221

Added line #L221 was not covered by tests
end
end
end
end
end

private
if concurrency == 1
downloads.each do |downloadable, promise|
promise.wait!
rescue ChecksumMismatchError => e
opoo "#{downloadable.download_type.capitalize} reports different checksum: #{e.expected}"

Check warning on line 231 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L231

Added line #L231 was not covered by tests
Homebrew.failed = true if downloadable.is_a?(Resource::Patch)
end
else
spinner = Spinner.new
remaining_downloads = downloads.dup
previous_pending_line_count = 0

Check warning on line 237 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L236-L237

Added lines #L236 - L237 were not covered by tests

begin
$stdout.print Tty.hide_cursor
$stdout.flush

Check warning on line 241 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L240-L241

Added lines #L240 - L241 were not covered by tests

output_message = lambda do |downloadable, future|
status = case future.state

Check warning on line 244 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L243-L244

Added lines #L243 - L244 were not covered by tests
when :fulfilled
"#{Tty.green}✔︎#{Tty.reset}"
when :rejected
"#{Tty.red}✘#{Tty.reset}"
when :pending, :processing
"#{Tty.blue}#{spinner}#{Tty.reset}"
else
raise future.state.to_s
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
message = "#{downloadable.download_type.capitalize} #{downloadable.name}"
$stdout.puts "#{status} #{message}"
$stdout.flush

Check warning on line 257 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L255-L257

Added lines #L255 - L257 were not covered by tests

def fetch_formula(formula)
fetch_fetchable(formula)
rescue ChecksumMismatchError => e
retry if retry_fetch?(formula)
opoo "Formula reports different sha256: #{e.expected}"
end
if future.rejected? && (e = future.reason).is_a?(ChecksumMismatchError)
opoo "#{downloadable.download_type.capitalize} reports different checksum: #{e.expected}"
Homebrew.failed = true if downloadable.is_a?(Resource::Patch)
next 2

Check warning on line 262 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L262

Added line #L262 was not covered by tests
end

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
1

Check warning on line 265 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L265

Added line #L265 was not covered by tests
end

def fetch_patch(patch)
fetch_fetchable(patch)
rescue ChecksumMismatchError => e
opoo "Patch reports different sha256: #{e.expected}"
Homebrew.failed = true
end
until remaining_downloads.empty?

Check warning on line 268 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L268

Added line #L268 was not covered by tests
begin
finished_states = [:fulfilled, :rejected]

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")
finished_downloads, remaining_downloads = remaining_downloads.partition do |_, future|
finished_states.include?(future.state)

Check warning on line 273 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L272-L273

Added lines #L272 - L273 were not covered by tests
end

ohai "Retrying download in #{wait}s... (#{remaining} #{what} left)"
sleep wait
finished_downloads.each do |downloadable, future|
previous_pending_line_count -= 1
$stdout.print Tty.clear_to_end
$stdout.flush
output_message.call(downloadable, future)

Check warning on line 280 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L276-L280

Added lines #L276 - L280 were not covered by tests
end

formula.clear_cache
@fetch_tries[formula] += 1
true
else
Homebrew.failed = true
false
end
end
previous_pending_line_count = 0
remaining_downloads.each do |downloadable, future|

Check warning on line 284 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L283-L284

Added lines #L283 - L284 were not covered by tests
# FIXME: Allow printing full terminal height.
break if previous_pending_line_count >= [concurrency, (Tty.height - 1)].min

def fetch_fetchable(formula)
formula.clear_cache if args.force?
$stdout.print Tty.clear_to_end
$stdout.flush
previous_pending_line_count += output_message.call(downloadable, future)

Check warning on line 290 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L288-L290

Added lines #L288 - L290 were not covered by tests
end

already_fetched = formula.cached_download.exist?
if previous_pending_line_count.positive?
$stdout.print Tty.move_cursor_up_beginning(previous_pending_line_count)
$stdout.flush

Check warning on line 295 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L295

Added line #L295 was not covered by tests
end

sleep 0.05

Check warning on line 298 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L298

Added line #L298 was not covered by tests
rescue Interrupt
remaining_downloads.each do |_, future|

Check warning on line 300 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L300

Added line #L300 was not covered by tests
# FIXME: Implement cancellation of running downloads.
end

begin
download = formula.fetch(verify_download_integrity: false)
rescue DownloadError
retry if retry_fetch?(formula)
raise
if previous_pending_line_count.positive?
$stdout.print Tty.move_cursor_down(previous_pending_line_count - 1)
$stdout.flush

Check warning on line 306 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L306

Added line #L306 was not covered by tests
end

raise

Check warning on line 309 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L309

Added line #L309 was not covered by tests
end
end
ensure
$stdout.print Tty.show_cursor
$stdout.flush

Check warning on line 314 in Library/Homebrew/cmd/fetch.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cmd/fetch.rb#L313-L314

Added lines #L313 - L314 were not covered by tests
end
end

return unless download.file?
download_queue.shutdown
reitermarkus marked this conversation as resolved.
Show resolved Hide resolved
end

puts "Downloaded to: #{download}" unless already_fetched
puts "SHA256: #{download.sha256}"
private

formula.verify_download_integrity(download)
def downloads
@downloads ||= {}
end

def fetch_downloadable(downloadable)
downloads[downloadable] ||= begin
tries = args.retry? ? {} : { tries: 1 }
MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved
download_queue.enqueue(RetryableDownload.new(downloadable, **tries), force: args.force?)
end
end
end
end
Expand Down
1 change: 0 additions & 1 deletion Library/Homebrew/cmd/install.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ def run
end

if casks.any?

if args.dry_run?
if (casks_to_install = casks.reject(&:installed?).presence)
ohai "Would install #{::Utils.pluralize("cask", casks_to_install.count, include_count: true)}:"
Expand Down
Loading