admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

Add JSON API download strategy for download queue

Browse Source 
This fixes the weird/broken existing behaviour which was incorrectly
creating symlinks at download time. It also defers much more logic to
the original code.

For clarity, rename the existing `API::Download` class to
`API::SourceDownload`.

While we're here:
- add a/improve the `download_type` method on all `Downloadable`
  subclasses to improve download queue output format
- move some logic to `RetryDownload`
This commit is contained in:
Mike McQuaid 2025-07-22 17:48:32 +01:00
parent 3bec1171ce
commit ed5805e50c
No known key found for this signature in database
13 changed files with 155 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
Library/Homebrew/api.rb
View File

@ -68,7 +68,8 @@ module Homebrew
if download_queue
unless skip_download
download = Homebrew::API::Download.new(url, nil, cache: HOMEBREW_CACHE_API, require_checksum: false)
require "api/json_download"
download = Homebrew::API::JSONDownload.new(endpoint, target:, stale_seconds:)
download_queue.enqueue(download)
end
return [{}, false]

4
Library/Homebrew/api/cask.rb
View File

@ -3,7 +3,7 @@
require "cachable"
require "api"
require "api/download"
require "api/source_download"
require "download_queue"
module Homebrew
@ -36,7 +36,7 @@ module Homebrew
git_head = cask.tap_git_head || "HEAD"
tap = cask.tap&.full_name || "Homebrew/homebrew-cask"
download = Homebrew::API::Download.new(
download = Homebrew::API::SourceDownload.new(
"https://raw.githubusercontent.com/#{tap}/#{git_head}/#{path}",
checksum,
mirrors: [

69
Library/Homebrew/api/download.rb
View File

@ -1,69 +0,0 @@
# typed: strict
# frozen_string_literal: true
require "downloadable"
module Homebrew
module API
class DownloadStrategy < CurlDownloadStrategy
sig { override.returns(Pathname) }
def symlink_location
cache/name
end
end
class Download
include Downloadable
sig {
params(
url: String,
checksum: T.nilable(Checksum),
mirrors: T::Array[String],
cache: T.nilable(Pathname),
require_checksum: T::Boolean,
).void
}
def initialize(url, checksum, mirrors: [], cache: nil, require_checksum: true)
super()
@url = T.let(URL.new(url, using: API::DownloadStrategy), URL)
@checksum = checksum
@mirrors = mirrors
@cache = cache
@require_checksum = require_checksum
end
sig { override.returns(API::DownloadStrategy) }
def downloader
T.cast(super, API::DownloadStrategy)
end
sig { override.returns(String) }
def name
download_name
end
sig { override.returns(String) }
def download_type
"API source"
end
sig { override.returns(Pathname) }
def cache
@cache || super
end
sig { returns(Pathname) }
def symlink_location
downloader.symlink_location
end
private
sig { override.returns(T::Boolean) }
def silence_checksum_missing_error?
!@require_checksum
end
end
end
end

4
Library/Homebrew/api/formula.rb
View File

@ -3,7 +3,7 @@
require "cachable"
require "api"
require "api/download"
require "api/source_download"
require "download_queue"
module Homebrew
@ -34,7 +34,7 @@ module Homebrew
git_head = formula.tap_git_head || "HEAD"
tap = formula.tap&.full_name || "Homebrew/homebrew-core"
download = Homebrew::API::Download.new(
download = Homebrew::API::SourceDownload.new(
"https://raw.githubusercontent.com/#{tap}/#{git_head}/#{path}",
formula.ruby_source_checksum,
cache: HOMEBREW_CACHE_API_SOURCE/"#{tap}/#{git_head}/Formula",

53
Library/Homebrew/api/json_download.rb Normal file
View File

@ -0,0 +1,53 @@
# typed: strict
# frozen_string_literal: true
require "downloadable"
module Homebrew
module API
class JSONDownloadStrategy < AbstractDownloadStrategy
sig { params(url: String, name: String, version: T.any(NilClass, String, Version), meta: T.untyped).void }
def initialize(url, name, version, **meta)
super
@target = T.let(meta.fetch(:target), Pathname)
@stale_seconds = T.let(meta.fetch(:stale_seconds), Integer)
end
sig { override.params(timeout: T.nilable(T.any(Integer, Float))).returns(Pathname) }
def fetch(timeout: nil)
with_context quiet: quiet? do
Homebrew::API.fetch_json_api_file(url, target: cached_location, stale_seconds: meta.fetch(:stale_seconds))
end
cached_location
end
sig { override.returns(Pathname) }
def cached_location
meta.fetch(:target)
end
end
class JSONDownload
include Downloadable
sig { params(url: String, target: Pathname, stale_seconds: Integer).void }
def initialize(url, target:, stale_seconds:)
super()
@url = T.let(URL.new(url, using: API::JSONDownloadStrategy, target:, stale_seconds:), URL)
@target = target
@stale_seconds = stale_seconds
end
sig { override.returns(API::JSONDownloadStrategy) }
def downloader
T.cast(super, API::JSONDownloadStrategy)
end
sig { override.returns(String) }
def name = download_name
sig { override.returns(String) }
def download_type = "JSON API"
end
end
end

56
Library/Homebrew/api/source_download.rb Normal file
View File

@ -0,0 +1,56 @@
# typed: strict
# frozen_string_literal: true
require "downloadable"
module Homebrew
module API
class SourceDownloadStrategy < CurlDownloadStrategy
sig { override.returns(Pathname) }
def symlink_location
cache/name
end
end
class SourceDownload
include Downloadable
sig {
params(
url: String,
checksum: T.nilable(Checksum),
mirrors: T::Array[String],
cache: T.nilable(Pathname),
).void
}
def initialize(url, checksum, mirrors: [], cache: nil)
super()
@url = T.let(URL.new(url, using: API::SourceDownloadStrategy), URL)
@checksum = checksum
@mirrors = mirrors
@cache = cache
end
sig { override.returns(API::SourceDownloadStrategy) }
def downloader
T.cast(super, API::SourceDownloadStrategy)
end
sig { override.returns(String) }
def name = download_name
sig { override.returns(String) }
def download_type = "API Source"
sig { override.returns(Pathname) }
def cache
@cache || super
end
sig { returns(Pathname) }
def symlink_location
downloader.symlink_location
end
end
end
end

3
Library/Homebrew/bottle.rb
View File

@ -185,6 +185,9 @@ class Bottle
end
end
sig { override.returns(String) }
def download_type = "Bottle"
private
def select_download_strategy(specs)

8
Library/Homebrew/cask/download.rb
View File

@ -95,14 +95,10 @@ module Cask
end
sig { override.returns(String) }
def download_name
cask.token
end
def download_name = cask.token
sig { override.returns(String) }
def download_type
"cask"
end
def download_type = "Cask"
private

14
Library/Homebrew/download_queue.rb
View File

@ -21,16 +21,10 @@ module Homebrew
sig { params(downloadable: Downloadable).void }
def enqueue(downloadable)
downloads[downloadable] ||= Concurrent::Promises.future_on(
pool, RetryableDownload.new(downloadable, tries:), force, quiet
pool, RetryableDownload.new(downloadable, tries:, pour:), force, quiet
) 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)
elsif download.api?
FileUtils.touch(download.cached_download, mtime: Time.now)
end
end
end
@ -42,7 +36,7 @@ module Homebrew
downloads.each do |downloadable, promise|
promise.wait!
rescue ChecksumMismatchError => e
opoo "#{downloadable.download_type.capitalize} reports different checksum: #{e.expected}"
opoo "#{downloadable.download_type} reports different checksum: #{e.expected}"
Homebrew.failed = true if downloadable.is_a?(Resource::Patch)
end
else
@ -66,13 +60,13 @@ module Homebrew
raise future.state.to_s
end
message = "#{downloadable.download_type.capitalize} #{downloadable.name}"
message = "#{downloadable.download_type} #{downloadable.name}"
$stdout.print "#{status} #{message}#{"\n" unless last}"
$stdout.flush
if future.rejected?
if (e = future.reason).is_a?(ChecksumMismatchError)
opoo "#{downloadable.download_type.capitalize} reports different checksum: #{e.expected}"
opoo "#{downloadable.download_type} reports different checksum: #{e.expected}"
Homebrew.failed = true if downloadable.is_a?(Resource::Patch)
next 2
else

9
Library/Homebrew/resource.rb
View File

@ -235,6 +235,9 @@ class Resource
@url&.specs || {}.freeze
end
sig { override.returns(String) }
def download_type = "Resource"
protected
def stage_resource(prefix, debug_symbols: false, &block)
@ -339,6 +342,9 @@ class Resource
manifest_annotations["sh.brew.bottle.installed_size"]&.to_i
end
sig { override.returns(String) }
def download_type = "Bottle Manifest"
private
def manifest_annotations
@ -391,6 +397,9 @@ class Resource
@directory = val
end
sig { override.returns(String) }
def download_type = "Patch"
end
end

27
Library/Homebrew/retryable_download.rb
View File

@ -1,6 +1,9 @@
# typed: strict
# frozen_string_literal: true
require "bottle"
require "api/json_download"
module Homebrew
class RetryableDownload
include Downloadable
@ -14,13 +17,14 @@ module Homebrew
sig { override.returns(T::Array[String]) }
def mirrors = downloadable.mirrors
sig { params(downloadable: Downloadable, tries: Integer).void }
def initialize(downloadable, tries:)
sig { params(downloadable: Downloadable, tries: Integer, pour: T::Boolean).void }
def initialize(downloadable, tries:, pour: false)
super()
@downloadable = downloadable
@try = T.let(0, Integer)
@tries = tries
@pour = pour
end
sig { override.returns(String) }
@ -65,7 +69,15 @@ module Homebrew
puts "SHA256: #{download.sha256}"
end
downloadable.verify_download_integrity(download) if verify_download_integrity
json_download = downloadable.is_a?(API::JSONDownload)
downloadable.verify_download_integrity(download) if verify_download_integrity && !json_download
if pour && downloadable.is_a?(Bottle)
UnpackStrategy.detect(download, prioritize_extension: true)
.extract_nestedly(to: HOMEBREW_CELLAR)
elsif json_download
FileUtils.touch(download, mtime: Time.now)
end
download
rescue DownloadError, ChecksumMismatchError, Resource::BottleManifest::Error
@ -89,15 +101,12 @@ module Homebrew
sig { override.returns(String) }
def download_name = downloadable.download_name
sig { returns(T::Boolean) }
def bottle? = downloadable.is_a?(Bottle)
sig { returns(T::Boolean) }
def api? = downloadable.is_a?(API::Download)
private
sig { returns(Downloadable) }
attr_reader :downloadable
sig { returns(T::Boolean) }
attr_reader :pour
end
end

4
Library/Homebrew/software_spec.rb
View File

@ -82,9 +82,7 @@ class SoftwareSpec
end
sig { override.returns(String) }
def download_type
"formula"
end
def download_type = "Formula"
def owner=(owner)
@name = owner.name

6
Library/Homebrew/test/api/cask_spec.rb
View File

@ -55,14 +55,14 @@ RSpec.describe Homebrew::API::Cask do
before do
allow(Homebrew::API).to receive(:fetch_json_api_file).and_return([[], true])
allow_any_instance_of(Homebrew::API::Download).to receive(:fetch)
allow_any_instance_of(Homebrew::API::Download).to receive(:symlink_location).and_return(
allow_any_instance_of(Homebrew::API::SourceDownload).to receive(:fetch)
allow_any_instance_of(Homebrew::API::SourceDownload).to receive(:symlink_location).and_return(
TEST_FIXTURE_DIR/"cask/Casks/everything.rb",
)
end
it "specifies the correct URL and sha256" do
expect(Homebrew::API::Download).to receive(:new).with(
expect(Homebrew::API::SourceDownload).to receive(:new).with(
"https://raw.githubusercontent.com/Homebrew/homebrew-cask/abcdef1234567890abcdef1234567890abcdef12/Casks/everything.rb",
Checksum.new("d8d0d6b2e5ff65388eccb82236fd3aa157b4a29bb043a1f72b97f0e9b70e8320"),
any_args,