Merge pull request #23403 from Homebrew/tolerate-missing-bottle-manifests

Tolerate missing bottle manifests in prefetches

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,

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

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 })

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

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"))

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