Skip to content

Commit 774ef54

Browse files
committed
Improve Download Queue output
Update the naming, presence and values for various download queue methods to improve the output for users while making the internal code a little easier to follow. While we're here, also ensure that a single formula download still displays the download queue output and indirectly fix an issue with bottle manifests being named incorrectly.
1 parent 9e4beda commit 774ef54

File tree

10 files changed

+48
-70
lines changed

10 files changed

+48
-70
lines changed

Library/Homebrew/api/json_download.rb

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,7 @@ def downloader
4444
end
4545

4646
sig { override.returns(String) }
47-
def name = download_name
48-
49-
sig { override.returns(String) }
50-
def download_type = "JSON API"
47+
def download_queue_type = "JSON API"
5148
end
5249
end
5350
end

Library/Homebrew/api/source_download.rb

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,7 @@ def downloader
3737
end
3838

3939
sig { override.returns(String) }
40-
def name = download_name
41-
42-
sig { override.returns(String) }
43-
def download_type = "API Source"
40+
def download_queue_type = "API Source"
4441

4542
sig { override.returns(Pathname) }
4643
def cache

Library/Homebrew/bottle.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,10 @@ def github_packages_manifest_resource
186186
end
187187

188188
sig { override.returns(String) }
189-
def download_type = "Bottle"
189+
def download_queue_type = "Bottle"
190+
191+
sig { override.returns(String) }
192+
def download_queue_name = "#{name} (#{resource.version})"
190193

191194
private
192195

Library/Homebrew/cask/download.rb

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,6 @@ def initialize(cask, quarantine: nil)
2424
@quarantine = quarantine
2525
end
2626

27-
sig { override.returns(String) }
28-
def name
29-
cask.token
30-
end
31-
3227
sig { override.returns(T.nilable(::URL)) }
3328
def url
3429
return if (cask_url = cask.url).nil?
@@ -95,10 +90,10 @@ def verify_download_integrity(filename)
9590
end
9691

9792
sig { override.returns(String) }
98-
def download_name = cask.token
93+
def download_queue_name = "#{cask.token} (#{version})"
9994

10095
sig { override.returns(String) }
101-
def download_type = "Cask"
96+
def download_queue_type = "Cask"
10297

10398
private
10499

Library/Homebrew/download_queue.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ def enqueue(downloadable)
3232
def fetch
3333
return if downloads.empty?
3434

35-
if concurrency == 1 || downloads.one?
35+
if concurrency == 1
3636
downloads.each do |downloadable, promise|
3737
promise.wait!
3838
rescue ChecksumMismatchError => e
39-
opoo "#{downloadable.download_type} reports different checksum: #{e.expected}"
39+
opoo "#{downloadable.download_queue_type} reports different checksum: #{e.expected}"
4040
Homebrew.failed = true if downloadable.is_a?(Resource::Patch)
4141
rescue => e
4242
raise e unless bottle_manifest_error?(downloadable, e)
@@ -73,7 +73,7 @@ def fetch
7373
exception = future.reason if future.rejected?
7474
next 1 if bottle_manifest_error?(downloadable, exception)
7575

76-
message = "#{downloadable.download_type} #{downloadable.name}"
76+
message = "#{downloadable.download_queue_type} #{downloadable.download_queue_name}"
7777
if tty
7878
stdout_print_and_flush "#{status} #{message}#{"\n" unless last}"
7979
elsif status
@@ -82,7 +82,7 @@ def fetch
8282

8383
if future.rejected?
8484
if exception.is_a?(ChecksumMismatchError)
85-
opoo "#{downloadable.download_type} reports different checksum: #{exception.expected}"
85+
opoo "#{downloadable.download_queue_type} reports different checksum: #{exception.expected}"
8686
Homebrew.failed = true if downloadable.is_a?(Resource::Patch)
8787
next 2
8888
else

Library/Homebrew/downloadable.rb

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,11 @@ def freeze
4848
super
4949
end
5050

51-
sig { abstract.returns(String) }
52-
def name; end
53-
5451
sig { returns(String) }
55-
def download_type
56-
class_name = T.let(self.class, T::Class[Downloadable]).name&.split("::")&.last
57-
T.must(class_name).gsub(/([[:lower:]])([[:upper:]])/, '\1 \2').downcase
58-
end
52+
def download_queue_name = download_name
53+
54+
sig { abstract.returns(String) }
55+
def download_queue_type; end
5956

6057
sig(:final) { returns(T::Boolean) }
6158
def downloaded?
@@ -135,13 +132,13 @@ def verify_download_integrity(filename)
135132
EOS
136133
end
137134

135+
private
136+
138137
sig { overridable.returns(String) }
139138
def download_name
140139
@download_name ||= File.basename(determine_url.to_s).freeze
141140
end
142141

143-
private
144-
145142
sig { overridable.returns(T::Boolean) }
146143
def silence_checksum_missing_error?
147144
false

Library/Homebrew/exceptions.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ class DownloadError < RuntimeError
632632

633633
def initialize(downloadable, cause)
634634
super <<~EOS
635-
Failed to download resource #{downloadable.download_name.inspect}
635+
Failed to download resource #{downloadable.download_queue_name.inspect}
636636
#{cause.message}
637637
EOS
638638
@cause = cause

Library/Homebrew/resource.rb

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,8 @@ def owner=(owner)
5757
patches.each { |p| p.owner = owner }
5858
end
5959

60-
# Removes /s from resource names; this allows Go package names
61-
# to be used as resource names without confusing software that
62-
# interacts with {download_name}, e.g. `github.com/foo/bar`.
63-
def escaped_name
64-
name.tr("/", "-")
65-
end
66-
67-
def download_name
68-
return owner.name if name.nil?
69-
return escaped_name if owner.nil?
70-
71-
"#{owner.name}--#{escaped_name}"
72-
end
60+
sig { override.returns(String) }
61+
def download_queue_type = "Resource"
7362

7463
# Verifies download and unpacks it.
7564
# The block may call `|resource, staging| staging.retain!` to retain the staging
@@ -236,9 +225,6 @@ def specs
236225
@url&.specs || {}.freeze
237226
end
238227

239-
sig { override.returns(String) }
240-
def download_type = "Resource"
241-
242228
protected
243229

244230
def stage_resource(prefix, debug_symbols: false, &block)
@@ -247,6 +233,19 @@ def stage_resource(prefix, debug_symbols: false, &block)
247233

248234
private
249235

236+
sig { override.returns(String) }
237+
def download_name
238+
return owner.name if name.nil?
239+
240+
# Removes /s from resource names; this allows Go package names
241+
# to be used as resource names without confusing software that
242+
# interacts with {download_name}, e.g. `github.com/foo/bar`.
243+
escaped_name = name.tr("/", "-")
244+
return escaped_name if owner.nil?
245+
246+
"#{owner.name}--#{escaped_name}"
247+
end
248+
250249
def determine_url_mirrors
251250
extra_urls = []
252251
url = T.must(self.url)
@@ -288,14 +287,10 @@ def initialize(path)
288287
# A resource for a formula.
289288
class Formula < Resource
290289
sig { override.returns(String) }
291-
def name
292-
T.must(owner).name
293-
end
290+
def download_queue_type = "Formula"
294291

295292
sig { override.returns(String) }
296-
def download_name
297-
name
298-
end
293+
def download_queue_name = "#{T.must(owner).name} (#{version})"
299294
end
300295

301296
# A resource containing a Go package.
@@ -344,10 +339,10 @@ def installed_size
344339
end
345340

346341
sig { override.returns(String) }
347-
def download_type = "Bottle Manifest"
342+
def download_queue_type = "Bottle Manifest"
348343

349344
sig { override.returns(String) }
350-
def name = bottle.name
345+
def download_queue_name = "#{bottle.name} (#{bottle.resource.version})"
351346

352347
private
353348

@@ -403,15 +398,15 @@ def directory(val = nil)
403398
end
404399

405400
sig { override.returns(String) }
406-
def download_type = "Patch"
401+
def download_queue_type = "Patch"
407402

408403
sig { override.returns(String) }
409-
def name
410-
if (url = self.url)
411-
url.to_s.split("/").last
412-
else
413-
super
404+
def download_queue_name
405+
if (last_url_component = url.to_s.split("/").last)
406+
return last_url_component
414407
end
408+
409+
super
415410
end
416411
end
417412
end

Library/Homebrew/retryable_download.rb

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ def initialize(downloadable, tries:, pour: false)
2828
end
2929

3030
sig { override.returns(String) }
31-
def name = downloadable.name
31+
def download_queue_name = downloadable.download_queue_name
3232

3333
sig { override.returns(String) }
34-
def download_type = downloadable.download_type
34+
def download_queue_type = downloadable.download_queue_type
3535

3636
sig { override.returns(Pathname) }
3737
def cached_download = downloadable.cached_download
@@ -103,9 +103,6 @@ def fetch(verify_download_integrity: true, timeout: nil, quiet: false)
103103
sig { override.params(filename: Pathname).void }
104104
def verify_download_integrity(filename) = downloadable.verify_download_integrity(filename)
105105

106-
sig { override.returns(String) }
107-
def download_name = downloadable.download_name
108-
109106
private
110107

111108
sig { returns(Downloadable) }

Library/Homebrew/software_spec.rb

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ class SoftwareSpec
2828
attr_reader :name, :full_name, :owner, :build, :resources, :patches, :options, :deprecated_flags,
2929
:deprecated_options, :dependency_collector, :bottle_specification, :compiler_failures
3030

31-
def_delegators :@resource, :stage, :fetch, :verify_download_integrity, :source_modified_time, :download_name,
31+
def_delegators :@resource, :stage, :fetch, :verify_download_integrity, :source_modified_time,
3232
:cached_download, :clear_cache, :checksum, :mirrors, :specs, :using, :version, :mirror,
33-
:downloader
33+
:downloader, :download_queue_name, :download_queue_type
3434

3535
def_delegators :@resource, :sha256
3636

@@ -81,9 +81,6 @@ def freeze
8181
super
8282
end
8383

84-
sig { override.returns(String) }
85-
def download_type = "Formula"
86-
8784
def owner=(owner)
8885
@name = owner.name
8986
@full_name = owner.full_name

0 commit comments

Comments
 (0)