From 4b375799dfb733f39f48cf2a020245a849c5cbe7 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Mon, 3 Aug 2026 14:25:46 +0100 Subject: [PATCH] Tolerate missing bottle manifests in prefetches - After #23381, homebrew-core CI fails installing dependencies on version-bump PRs: the bumped formula's stale bottle block derives its manifest URL from the new version, whose bottle has not been published yet, and `DownloadQueue#fetch` treats every bottle manifest failure as fatal. - Before that change the default ask-mode plan fetched manifests synchronously via `Formula#fetch_bottle_tab`, which rescues download errors so dependency resolution falls back to a full install, and the installer's memoisation then kept the queue from retrying the download. - Add `DownloadQueue#fetch(allow_failures:)`: failed downloads are still reported but neither raise nor mark the fetch or run as failed. Use it for the metadata-only drains (`fetch_formulae`'s bottle manifest waits, `brew install`'s ask-mode drain and `brew upgrade`'s tab prefetch), restoring the synchronous path's tolerance. Manifest failures in fetches that pour bottles remain fatal. --- Library/Homebrew/cmd/install.rb | 3 +- Library/Homebrew/download_queue.rb | 52 +++++++++++++++++--- Library/Homebrew/install.rb | 6 ++- Library/Homebrew/test/cmd/upgrade_spec.rb | 3 +- Library/Homebrew/test/download_queue_spec.rb | 47 ++++++++++++++++++ Library/Homebrew/upgrade.rb | 3 +- 6 files changed, 103 insertions(+), 11 deletions(-) diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 33b423560a..b7ba04d4ab 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -334,7 +334,8 @@ module Homebrew # Main block: if asking the user is enabled, show dry-run information. if ask - shared_download_queue&.fetch(heading: "Downloading bottle manifests") + shared_download_queue&.fetch(only: Resource::BottleManifest, + heading: "Downloading bottle manifests", allow_failures: true) Install.ask_formulae( formulae_installer, dependants, diff --git a/Library/Homebrew/download_queue.rb b/Library/Homebrew/download_queue.rb index af05e36505..c1f358c436 100644 --- a/Library/Homebrew/download_queue.rb +++ b/Library/Homebrew/download_queue.rb @@ -106,9 +106,17 @@ module Homebrew # on bottle manifests without reporting in-flight bottles before their # downloads heading has been printed. A `heading:` is printed only when # there is something to report, so every report gets a heading and empty - # fetches stay silent. - sig { params(only: T.nilable(T::Class[Downloadable]), heading: T.nilable(String)).void } - def fetch(only: nil, heading: nil) + # fetches stay silent. With `allow_failures:`, failures are still + # reported with a ✘ line but neither raise nor mark the fetch or run + # as failed, for metadata prefetches such as the bottle manifest of a + # version whose bottle has not been published yet, where dependency + # resolution just falls back to a full install; known-bad cached files + # from checksum mismatches are still removed. + sig { + params(only: T.nilable(T::Class[Downloadable]), heading: T.nilable(String), + allow_failures: T::Boolean).void + } + def fetch(only: nil, heading: nil, allow_failures: false) @fetch_failed = false @deferred_failure_messages = [] context_before_fetch = Context.current @@ -132,8 +140,19 @@ module Homebrew rescue CancelledDownloadError next rescue ChecksumMismatchError => e + if allow_failures + report_tolerated_failure(downloadable) + # Remove the known-bad download so it cannot be reused. + unlink_cached_download(downloadable) + next + end + @fetch_failed = true ofail "#{downloadable.download_queue_type} reports different checksum: #{e.expected}" + rescue + raise unless allow_failures + + report_tolerated_failure(downloadable) end else message_length_max = fetchable_downloads.keys.map do |download| @@ -162,7 +181,11 @@ module Homebrew $stderr.puts "#{status} #{message}" end - if future.rejected? + if future.rejected? && allow_failures + # Remove known-bad downloads so they cannot be reused, while + # staying non-fatal for tolerated metadata prefetches. + unlink_cached_download(downloadable) if exception.is_a?(ChecksumMismatchError) + elsif future.rejected? if exception.is_a?(ChecksumMismatchError) @fetch_failed = true actual = Digest::SHA256.file(downloadable.cached_download).hexdigest @@ -173,8 +196,7 @@ module Homebrew puts "#{expected_message} #{actual}" end elsif exception.is_a?(CannotInstallFormulaError) - cached_download = downloadable.cached_download - cached_download.unlink if cached_download&.exist? + unlink_cached_download(downloadable) raise exception elsif bottle_manifest_error?(downloadable, exception) # Fatal: unlike a missing blob (which then fails to stage), a @@ -393,6 +415,24 @@ module Homebrew tty && !@dumb_tty end + sig { params(downloadable: Downloadable).void } + def unlink_cached_download(downloadable) + cached_download = downloadable.cached_download + cached_download.unlink if cached_download.exist? + end + + # Matches the parallel-mode ✘ report for failures the serial path + # tolerates instead of raising. + sig { params(downloadable: Downloadable).void } + def report_tolerated_failure(downloadable) + status = if tty + "#{Tty.red}✘#{Tty.reset}" + else + "✘" + end + $stderr.puts "#{status} #{downloadable.download_queue_message}" + end + sig { params(future: Concurrent::Promises::Future).returns(T.nilable(String)) } def status_from_future(future) case future.state diff --git a/Library/Homebrew/install.rb b/Library/Homebrew/install.rb index 0c33b31283..c995f7697b 100644 --- a/Library/Homebrew/install.rb +++ b/Library/Homebrew/install.rb @@ -352,14 +352,16 @@ module Homebrew valid_formula_installers = prelude_fetch_formulae(formula_installers, download_queue:) # Wait on just the bottle manifests dependency resolution needs so # in-flight bottles are only reported under the downloads heading. - download_queue.fetch(only: Resource::BottleManifest, heading: "Downloading bottle manifests") + download_queue.fetch(only: Resource::BottleManifest, heading: "Downloading bottle manifests", + allow_failures: true) [:prelude, :enqueue_fetch].each do |step| valid_formula_installers = select_formula_installers(valid_formula_installers, step:) next if step == :enqueue_fetch && !fetch_after_enqueue if step == :prelude - download_queue.fetch(only: Resource::BottleManifest, heading: "Downloading bottle manifests") + download_queue.fetch(only: Resource::BottleManifest, heading: "Downloading bottle manifests", + allow_failures: true) else heading = if show_downloads_heading combined_fetch_downloads_heading(formula_names: valid_formula_installers.map { |fi| fi.formula.name }) diff --git a/Library/Homebrew/test/cmd/upgrade_spec.rb b/Library/Homebrew/test/cmd/upgrade_spec.rb index e1d62bc831..52fbf8a8bd 100644 --- a/Library/Homebrew/test/cmd/upgrade_spec.rb +++ b/Library/Homebrew/test/cmd/upgrade_spec.rb @@ -1017,7 +1017,8 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do allow(formula).to receive(:latest_formula).and_return(formula) allow(Migrator).to receive(:migrate_if_needed) allow(Homebrew::DownloadQueue).to receive(:new).and_return(download_queue) - expect(download_queue).to receive(:fetch).with(heading: "Downloading bottle manifests") + expect(download_queue).to receive(:fetch) + .with(only: Resource::BottleManifest, heading: "Downloading bottle manifests", allow_failures: true) Homebrew::Upgrade.formula_installers([formula], flags: []) end diff --git a/Library/Homebrew/test/download_queue_spec.rb b/Library/Homebrew/test/download_queue_spec.rb index 1e9400cd5c..c226296837 100644 --- a/Library/Homebrew/test/download_queue_spec.rb +++ b/Library/Homebrew/test/download_queue_spec.rb @@ -102,6 +102,53 @@ RSpec.describe Homebrew::DownloadQueue do expect(download_queue.downloads).to be_empty end + it "reports but tolerates failed downloads when failures are allowed" do + allow(retryable_download).to receive(:fetch).and_raise(Resource::BottleManifest::Error.new("manifest missing")) + + download_queue.enqueue(downloadable) + + expect { download_queue.fetch(allow_failures: true) }.to output(/✘/).to_stderr + expect(download_queue.fetch_failed).to be false + expect(Homebrew).not_to have_failed + end + + it "tolerates failed downloads in serial mode when failures are allowed" do + allow(Homebrew::EnvConfig).to receive(:download_concurrency).and_return(1) + allow(retryable_download).to receive(:fetch).and_raise(Resource::BottleManifest::Error.new("manifest missing")) + + download_queue.enqueue(downloadable) + + expect { download_queue.fetch(allow_failures: true) }.to output(/✘/).to_stderr + expect(Homebrew).not_to have_failed + end + + it "removes known-bad cached files for tolerated checksum mismatches" do + cached_download.dirname.mkpath + cached_download.write("corrupt") + allow(retryable_download).to receive(:fetch) + .and_raise(ChecksumMismatchError.new(cached_download, Checksum.new("aa" * 32), Checksum.new("bb" * 32))) + + download_queue.enqueue(downloadable) + + expect { download_queue.fetch(allow_failures: true) }.to output(/✘/).to_stderr + expect(cached_download).not_to exist + expect(Homebrew).not_to have_failed + end + + it "removes known-bad cached files for tolerated checksum mismatches in serial mode" do + allow(Homebrew::EnvConfig).to receive(:download_concurrency).and_return(1) + cached_download.dirname.mkpath + cached_download.write("corrupt") + allow(retryable_download).to receive(:fetch) + .and_raise(ChecksumMismatchError.new(cached_download, Checksum.new("aa" * 32), Checksum.new("bb" * 32))) + + download_queue.enqueue(downloadable) + + expect { download_queue.fetch(allow_failures: true) }.to output(/✘/).to_stderr + expect(cached_download).not_to exist + expect(Homebrew).not_to have_failed + end + it "cancels remaining downloads and raises on a bottle manifest failure in serial mode" do allow(Homebrew::EnvConfig).to receive(:download_concurrency).and_return(1) allow(retryable_download).to receive(:fetch).and_raise(Resource::BottleManifest::Error.new("manifest missing")) diff --git a/Library/Homebrew/upgrade.rb b/Library/Homebrew/upgrade.rb index 7d45901068..c289fb40ce 100644 --- a/Library/Homebrew/upgrade.rb +++ b/Library/Homebrew/upgrade.rb @@ -134,7 +134,8 @@ module Homebrew end end - download_queue.fetch(heading: "Downloading bottle manifests") + download_queue.fetch(only: Resource::BottleManifest, heading: "Downloading bottle manifests", + allow_failures: true) ensure download_queue.shutdown end