diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index 73da5afdf6..0bffe3a4e1 100644 --- a/Library/Homebrew/api.rb +++ b/Library/Homebrew/api.rb @@ -230,7 +230,7 @@ module Homebrew ENV["HOMEBREW_API_UPDATED"] = "1" begin - download_queue.fetch + download_queue.fetch(heading: "Downloading Homebrew API data") ensure download_queue.shutdown end diff --git a/Library/Homebrew/cask/reinstall.rb b/Library/Homebrew/cask/reinstall.rb index 8766abf192..958789d4d0 100644 --- a/Library/Homebrew/cask/reinstall.rb +++ b/Library/Homebrew/cask/reinstall.rb @@ -57,9 +57,9 @@ module Cask unless skip_prefetch Homebrew::Install.enqueue_cask_installers(cask_installers, download_queue:) - oh1 "Fetching downloads for: #{casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence}", - truncate: false - download_queue.fetch + download_queue.fetch( + heading: Homebrew::Install.combined_fetch_downloads_heading(cask_names: casks.map(&:full_name)), + ) end ensure download_queue.shutdown if created_download_queue diff --git a/Library/Homebrew/cask/upgrade.rb b/Library/Homebrew/cask/upgrade.rb index aa653734c3..15037d92c3 100644 --- a/Library/Homebrew/cask/upgrade.rb +++ b/Library/Homebrew/cask/upgrade.rb @@ -241,13 +241,13 @@ module Cask end fetchable_casks = upgradable_casks.map(&:last) - fetchable_casks_sentence = fetchable_casks.map { |cask| Formatter.identifier(cask.full_name) }.to_sentence Homebrew::Install.enqueue_cask_installers(fetchable_cask_installers, download_queue: prefetch_download_queue) - if fetchable_casks.any? - oh1 "Fetching downloads for: #{fetchable_casks_sentence}", truncate: false - prefetch_download_queue.fetch - end + prefetch_download_queue.fetch( + heading: Homebrew::Install.combined_fetch_downloads_heading( + cask_names: fetchable_casks.map(&:full_name), + ), + ) ensure prefetch_download_queue.shutdown if created_download_queue end diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 89b7df8590..33b423560a 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -276,9 +276,6 @@ module Homebrew return if formulae.any? && installed_formulae.empty? && casks.empty? - Install.perform_preinstall_checks_once - Install.check_cc_argv(args.cc) - formulae_installer = Install.formula_installers( installed_formulae, installed_on_request: !args.as_dependency?, @@ -305,14 +302,20 @@ module Homebrew ) shared_download_queue = T.let(nil, T.nilable(Homebrew::DownloadQueue)) - if !ask && !args.dry_run? && formulae_installer.any? + if !args.dry_run? && formulae_installer.any? shared_download_queue = Homebrew::DownloadQueue.new(pour: true) + # Start bottle manifest (and, once downloads are confirmed, bottle) + # transfers before the local-only work below. formulae_installer = Install.prelude_fetch_formulae(formulae_installer, - download_queue: shared_download_queue) + download_queue: shared_download_queue, + metadata_only: ask) end - dependants = begin - Upgrade.dependants( + begin + Install.perform_preinstall_checks_once + Install.check_cc_argv(args.cc) + + dependants = Upgrade.dependants( installed_formulae, flags: args.flags_only, ask: ask, @@ -328,30 +331,31 @@ module Homebrew verbose: args.verbose?, dry_run: args.dry_run?, ) - # Ensure the early download queue is shut down on interrupts. + + # Main block: if asking the user is enabled, show dry-run information. + if ask + shared_download_queue&.fetch(heading: "Downloading bottle manifests") + Install.ask_formulae( + formulae_installer, + dependants, + flags: args.flags_only, + force_bottle: args.force_bottle?, + build_from_source_formulae: args.build_from_source_formulae, + interactive: args.interactive?, + keep_tmp: args.keep_tmp?, + debug_symbols: args.debug_symbols?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + ) + end + # Ensure the early download queue is shut down on interrupts and declined prompts. rescue Exception # rubocop:disable Lint/RescueException shared_download_queue&.shutdown raise end - # Main block: if asking the user is enabled, show dry-run information. - if ask - Install.ask_formulae( - formulae_installer, - dependants, - flags: args.flags_only, - force_bottle: args.force_bottle?, - build_from_source_formulae: args.build_from_source_formulae, - interactive: args.interactive?, - keep_tmp: args.keep_tmp?, - debug_symbols: args.debug_symbols?, - force: args.force?, - debug: args.debug?, - quiet: args.quiet?, - verbose: args.verbose?, - ) - end - if !args.dry_run? && (formulae_installer.any? || fetch_casks.any?) download_queue = T.let(shared_download_queue || Homebrew::DownloadQueue.new(pour: true), Homebrew::DownloadQueue) @@ -360,10 +364,6 @@ module Homebrew Cask::Upgrade.show_upgrade_summary( upgrade_casks.map { |cask| "#{cask.full_name} #{cask.installed_version} -> #{cask.version}" }, ) - Install.show_combined_fetch_downloads_heading( - formula_names: formulae_installer.map { |fi| fi.formula.name }, - cask_names: fetch_casks.map(&:full_name), - ) formulae_installer = Install.enqueue_formulae(formulae_installer, download_queue:) @@ -386,7 +386,10 @@ module Homebrew Install.enqueue_cask_installers(fetch_cask_installers, download_queue:) end - download_queue.fetch + download_queue.fetch(heading: Install.combined_fetch_downloads_heading( + formula_names: formulae_installer.map { |fi| fi.formula.name }, + cask_names: fetch_casks.map(&:full_name), + )) ensure download_queue.shutdown end diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb index ebb6694d8f..a5700a8a3d 100644 --- a/Library/Homebrew/cmd/reinstall.rb +++ b/Library/Homebrew/cmd/reinstall.rb @@ -244,11 +244,6 @@ module Homebrew shared_download_queue ||= Homebrew::DownloadQueue.new(pour: true) download_queue = shared_download_queue begin - Install.show_combined_fetch_downloads_heading( - formula_names: formulae_installers.map { |fi| fi.formula.name }, - cask_names: casks.map(&:full_name), - ) - valid_formula_installers = Install.enqueue_formulae(formulae_installers, download_queue:) @@ -268,7 +263,10 @@ module Homebrew ) end Install.enqueue_cask_installers(fetch_cask_installers, download_queue:) - download_queue.fetch + download_queue.fetch(heading: Install.combined_fetch_downloads_heading( + formula_names: valid_formula_installers.map { |fi| fi.formula.name }, + cask_names: casks.map(&:full_name), + )) casks_prefetched = true valid_formula_installers ensure diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index 4162be29d8..41261c0f78 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -263,21 +263,19 @@ module Homebrew begin formulae_prefetched = upgrade_outdated_formulae!( formulae, - prefetch_only: true, - download_queue: shared_download_queue, - prefetch_names: prefetched_formulae_names, - prefetch_upgrades: prefetched_formulae_upgrades, - show_upgrade_summary: false, - show_downloads_heading: false, + prefetch_only: true, + download_queue: shared_download_queue, + prefetch_names: prefetched_formulae_names, + prefetch_upgrades: prefetched_formulae_upgrades, + show_upgrade_summary: false, ) prefetched_casks = prefetch_outdated_casks!( casks, - download_queue: shared_download_queue, - prefetch_names: prefetched_cask_names, - prefetch_upgrades: prefetched_cask_upgrades, - prefetch_casks: prefetched_cask_upgrade_casks, - prefetch_errors: prefetched_cask_errors, - show_downloads_heading: false, + download_queue: shared_download_queue, + prefetch_names: prefetched_cask_names, + prefetch_upgrades: prefetched_cask_upgrades, + prefetch_casks: prefetched_cask_upgrade_casks, + prefetch_errors: prefetched_cask_errors, ) unless ask Cask::Upgrade.show_upgrade_summary( @@ -285,11 +283,10 @@ module Homebrew dry_run: args.dry_run?, ) end - Install.show_combined_fetch_downloads_heading( + shared_download_queue.fetch(heading: Install.combined_fetch_downloads_heading( formula_names: prefetched_formulae_names, cask_names: prefetched_cask_names, - ) - shared_download_queue.fetch + )) if shared_download_queue.fetch_failed formulae_prefetched = false prefetched_casks = false @@ -414,12 +411,6 @@ module Homebrew Install.perform_preinstall_checks_once - if formulae_to_install.any? do |formula| - formula.bottle&.github_packages_manifest_resource&.downloaded_and_valid? == false - end - oh1 "Downloading bottle manifests" - end - formulae_installer = Upgrade.formula_installers( formulae_to_install, flags: args.flags_only, @@ -602,15 +593,14 @@ module Homebrew sig { params( - formulae: T::Array[Formula], - prefetch_only: T::Boolean, - use_prefetched: T::Boolean, - dry_run: T::Boolean, - download_queue: T.nilable(Homebrew::DownloadQueue), - prefetch_names: T.nilable(T::Array[String]), - prefetch_upgrades: T.nilable(T::Array[String]), - show_upgrade_summary: T::Boolean, - show_downloads_heading: T::Boolean, + formulae: T::Array[Formula], + prefetch_only: T::Boolean, + use_prefetched: T::Boolean, + dry_run: T::Boolean, + download_queue: T.nilable(Homebrew::DownloadQueue), + prefetch_names: T.nilable(T::Array[String]), + prefetch_upgrades: T.nilable(T::Array[String]), + show_upgrade_summary: T::Boolean, ).returns(T::Boolean) } def upgrade_outdated_formulae!(formulae, prefetch_only: false, use_prefetched: false, @@ -618,8 +608,7 @@ module Homebrew download_queue: nil, prefetch_names: nil, prefetch_upgrades: nil, - show_upgrade_summary: true, - show_downloads_heading: true) + show_upgrade_summary: true) return false if args.cask? use_prefetched_context = use_prefetched && @prefetched_formulae_upgrade_context @@ -634,11 +623,6 @@ module Homebrew prefetch_download_queue = download_queue || Homebrew.default_download_queue valid_formula_installers = Install.enqueue_formulae(context.formulae_installer, download_queue: prefetch_download_queue) - if show_downloads_heading - Install.show_combined_fetch_downloads_heading( - formula_names: valid_formula_installers.map { |fi| fi.formula.name }, - ) - end prefetch_names&.replace(valid_formula_installers.map { |fi| fi.formula.name }) prefetch_upgrades&.replace(formula_upgrade_descriptions(valid_formula_installers.map(&:formula))) @prefetched_formulae_upgrade_context = FormulaeUpgradeContext.new( @@ -719,13 +703,11 @@ module Homebrew prefetch_names: T.nilable(T::Array[String]), prefetch_upgrades: T.nilable(T::Array[String]), prefetch_casks: T.nilable(T::Array[Cask::Cask]), - prefetch_errors: T.nilable(T::Array[StandardError]), - show_downloads_heading: T::Boolean) + prefetch_errors: T.nilable(T::Array[StandardError])) .returns(T::Boolean) } def prefetch_outdated_casks!(casks, download_queue:, prefetch_names: nil, - prefetch_upgrades: nil, prefetch_casks: nil, prefetch_errors: nil, - show_downloads_heading: true) + prefetch_upgrades: nil, prefetch_casks: nil, prefetch_errors: nil) return false if args.formula? casks = minimum_version_casks(casks, quiet: true) @@ -783,8 +765,6 @@ module Homebrew prefetch_upgrades&.replace( outdated_casks.map { |cask| "#{cask.full_name} #{cask.installed_version} -> #{cask.version}" }, ) - Install.show_combined_fetch_downloads_heading(cask_names:) if show_downloads_heading - true rescue => e ofail e diff --git a/Library/Homebrew/download_queue.rb b/Library/Homebrew/download_queue.rb index cd0ab83ec9..af05e36505 100644 --- a/Library/Homebrew/download_queue.rb +++ b/Library/Homebrew/download_queue.rb @@ -100,15 +100,34 @@ module Homebrew end end - sig { void } - def fetch + # Waits for and reports queued downloads. With `only:`, limits that to + # downloadables of the given class, leaving the rest enqueued and + # unreported for a later fetch, e.g. so dependency resolution can wait + # 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) @fetch_failed = false @deferred_failure_messages = [] context_before_fetch = Context.current - return if downloads.empty? + fetchable_downloads = if only + downloads.select { |downloadable, _| downloadable.is_a?(only) } + else + downloads + end + return if fetchable_downloads.empty? + + if heading + oh1 heading, truncate: false + # Reach the pipe before any unbuffered stderr report lines when + # stdout is block-buffered. + $stdout.flush + end if concurrency == 1 - downloads.each do |downloadable, promise| + fetchable_downloads.each do |downloadable, promise| promise.wait! rescue CancelledDownloadError next @@ -117,13 +136,15 @@ module Homebrew ofail "#{downloadable.download_queue_type} reports different checksum: #{e.expected}" end else - message_length_max = downloads.keys.map { |download| download.download_queue_message.length }.max || 0 - remaining_downloads = downloads.dup.to_a + message_length_max = fetchable_downloads.keys.map do |download| + download.download_queue_message.length + end.max || 0 + remaining_downloads = fetchable_downloads.dup.to_a previous_pending_line_count = 0 max_lines = [concurrency, Tty.height].min resolution = Concurrent::Event.new - downloads.each_value { |future| future.on_resolution! { resolution.set } } + fetchable_downloads.each_value { |future| future.on_resolution! { resolution.set } } begin stdout_print_and_flush_if_tty Tty.hide_cursor @@ -250,9 +271,15 @@ module Homebrew # aborts the fetch above. Context.current = context_before_fetch if context_before_fetch - downloads.clear - @downloads_by_location.clear - @symlink_targets.clear + if only + # Keep unfetched downloads (and their location dedup entries) queued + # for the next fetch. + fetchable_downloads.each_key { |downloadable| downloads.delete(downloadable) } + else + downloads.clear + @downloads_by_location.clear + @symlink_targets.clear + end end sig { returns(T::Boolean) } diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 09de5b61c7..d401453206 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -146,10 +146,12 @@ class FormulaInstaller @download_queue = T.let(Homebrew.default_download_queue, Homebrew::DownloadQueue) @api_bottle = T.let(nil, T.nilable(Bottle)) @api_bottle_loaded = T.let(false, T::Boolean) + @enqueued_bottle_download = T.let(nil, T.nilable(Downloadable)) # Take the original formula instance, which might have been swapped from an API instance to a source instance @formula = T.let(T.must(previously_fetched_formula), Formula) if previously_fetched_formula + @ran_prelude_fetch_metadata = T.let(false, T::Boolean) @ran_prelude_fetch = T.let(false, T::Boolean) @ran_prelude = T.let(false, T::Boolean) end @@ -298,41 +300,47 @@ class FormulaInstaller ) || false end - sig { void } - def prelude_fetch - return if @ran_prelude_fetch + sig { params(metadata_only: T::Boolean).void } + def prelude_fetch(metadata_only: false) + unless @ran_prelude_fetch_metadata + deprecate_disable_type = DeprecateDisable.type(formula) + if deprecate_disable_type.present? + message = "#{formula.full_name} has been #{DeprecateDisable.message(formula)}" - deprecate_disable_type = DeprecateDisable.type(formula) - if deprecate_disable_type.present? - message = "#{formula.full_name} has been #{DeprecateDisable.message(formula)}" - - case deprecate_disable_type - when :deprecated - opoo message - when :disabled - if force? + case deprecate_disable_type + when :deprecated opoo message - else - GitHub::Actions.puts_annotation_if_env_set!(:error, message) - raise CannotInstallFormulaError, message + when :disabled + if force? + opoo message + else + GitHub::Actions.puts_annotation_if_env_set!(:error, message) + raise CannotInstallFormulaError, message + end end end + + # Run the formula-self forbidden checks before any source or bottle + # download is enqueued so a forbidden formula never triggers a fetch. + forbidden_tap_check(formula_only: true) + forbidden_formula_check(formula_only: true) + + # Needs to be done before expand_dependencies for compute_dependencies + fetch_bottle_tab(enqueue: true) if pour_bottle? + + fetch_fetch_deps unless ignore_deps? + + @ran_prelude_fetch_metadata = true end - # Run the formula-self forbidden checks before any source or bottle - # download is enqueued so a forbidden formula never triggers a fetch. - forbidden_tap_check(formula_only: true) - forbidden_formula_check(formula_only: true) + return if metadata_only || @ran_prelude_fetch if pour_bottle? - # Needs to be done before expand_dependencies for compute_dependencies - fetch_bottle_tab(enqueue: true) + @enqueued_bottle_download = enqueue_bottle_download(stage: true) elsif formula.loaded_from_api? Homebrew::API::Formula.source_download(formula, download_queue:, enqueue: true) end - fetch_fetch_deps unless ignore_deps? - @ran_prelude_fetch = true end @@ -1490,7 +1498,7 @@ on_request: installed_on_request?, options:) sig { void } def fetch enqueue_fetch - download_queue.fetch + download_queue.fetch(heading: "Fetching downloads for: #{Formatter.identifier(formula.full_name)}") end sig { void } @@ -1498,23 +1506,14 @@ on_request: installed_on_request?, options:) return if previously_fetched_formula downloadable_object = T.let(nil, T.nilable(Downloadable)) - bottle_download = T.let(nil, T.nilable(Downloadable)) check_attestation = T.let(false, T::Boolean) local_bottle_path = formula.local_bottle_path bottle_install = !only_deps? && local_bottle_path.nil? && pour_bottle?(output_warning: true) - # We skip `gh` to avoid a bootstrapping cycle, in the off-chance a user attempts - # to explicitly `brew install gh` without already having a version for bootstrapping. - # We also skip bottle installs from local bottle paths, as these are done in CI + # We skip bottle installs from local bottle paths, as these are done in CI # as part of the build lifecycle before attestations are produced. - verify_attestation = bottle_install && - Homebrew::EnvConfig.verify_attestations? && - (formula.tap&.core_tap? || false) && - formula.name != "gh" - if bottle_install && @ran_prelude - bottle_download = downloadable - check_attestation = verify_attestation && !bottle_download.cached_download.exist? - download_queue.enqueue(bottle_download, check_attestation:, stage: false) - end + verify_attestation = bottle_install && verify_bottle_attestation? + bottle_download = @enqueued_bottle_download + bottle_download = enqueue_bottle_download(stage: false) if bottle_download.nil? && bottle_install && @ran_prelude fetch_dependencies @@ -1535,8 +1534,10 @@ on_request: installed_on_request?, options:) downloadable_object = downloadable end - # Check attestation after download completes. - download_queue.enqueue(downloadable_object, check_attestation:) + # Check attestation after download completes. Skip downloads already + # enqueued (with staging) by `prelude_fetch` so a completed early fetch is + # not requeued and reported a second time. + download_queue.enqueue(downloadable_object, check_attestation:) if @enqueued_bottle_download.nil? self.class.fetched << formula rescue CannotInstallFormulaError @@ -1547,6 +1548,28 @@ on_request: installed_on_request?, options:) raise end + # Start the formula's own bottle download without waiting for its bottle + # manifest or dependency resolution; both call sites have already checked + # `pour_bottle?`. + sig { params(stage: T::Boolean).returns(T.nilable(Downloadable)) } + def enqueue_bottle_download(stage:) + return if only_deps? || formula.local_bottle_path + + bottle_download = downloadable + check_attestation = verify_bottle_attestation? && !bottle_download.cached_download.exist? + download_queue.enqueue(bottle_download, check_attestation:, stage:) + bottle_download + end + + sig { returns(T::Boolean) } + def verify_bottle_attestation? + # We skip `gh` to avoid a bootstrapping cycle, in the off-chance a user attempts + # to explicitly `brew install gh` without already having a version for bootstrapping. + Homebrew::EnvConfig.verify_attestations? && + (formula.tap&.core_tap? || false) && + formula.name != "gh" + end + sig { returns(Downloadable) } def downloadable if (bottle_path = formula.local_bottle_path) diff --git a/Library/Homebrew/install.rb b/Library/Homebrew/install.rb index 1ea35a925c..0c33b31283 100644 --- a/Library/Homebrew/install.rb +++ b/Library/Homebrew/install.rb @@ -344,25 +344,28 @@ module Homebrew shutdown_download_queue: true, show_downloads_heading: true ) - formulae_names_to_install = formula_installers.map { |fi| fi.formula.name } - return formula_installers if formulae_names_to_install.empty? + return formula_installers if formula_installers.empty? download_queue = T.let(download_queue || Homebrew::DownloadQueue.new(pour: true), Homebrew::DownloadQueue) - if show_downloads_heading - formula_sentence = formulae_names_to_install.map { |name| Formatter.identifier(name) }.to_sentence - oh1 "Fetching downloads for: #{formula_sentence}", truncate: false - end - begin valid_formula_installers = prelude_fetch_formulae(formula_installers, download_queue:) - download_queue.fetch + # 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") [:prelude, :enqueue_fetch].each do |step| valid_formula_installers = select_formula_installers(valid_formula_installers, step:) next if step == :enqueue_fetch && !fetch_after_enqueue - download_queue.fetch + if step == :prelude + download_queue.fetch(only: Resource::BottleManifest, heading: "Downloading bottle manifests") + else + heading = if show_downloads_heading + combined_fetch_downloads_heading(formula_names: valid_formula_installers.map { |fi| fi.formula.name }) + end + download_queue.fetch(heading:) + end end ensure download_queue.shutdown if shutdown_download_queue @@ -375,22 +378,34 @@ module Homebrew params( formula_installers: T::Array[FormulaInstaller], download_queue: Homebrew::DownloadQueue, + metadata_only: T::Boolean, ).returns(T::Array[FormulaInstaller]) } - def prelude_fetch_formulae(formula_installers, download_queue:) + def prelude_fetch_formulae(formula_installers, download_queue:, metadata_only: false) formula_installers.each do |fi| fi.download_queue = download_queue end - select_formula_installers(formula_installers, step: :prelude_fetch) + # Only pass the keyword when limiting the fetch so mocks and + # overrides expecting the historical no-argument call keep working. + action = ->(fi) { metadata_only ? fi.prelude_fetch(metadata_only: true) : fi.prelude_fetch } + select_formula_installers(formula_installers, action:) end sig { - params(formula_installers: T::Array[FormulaInstaller], step: Symbol).returns(T::Array[FormulaInstaller]) + params( + formula_installers: T::Array[FormulaInstaller], + step: T.nilable(Symbol), + action: T.nilable(T.proc.params(formula_installer: FormulaInstaller).void), + ).returns(T::Array[FormulaInstaller]) } - def select_formula_installers(formula_installers, step:) + def select_formula_installers(formula_installers, step: nil, action: nil) formula_installers.select do |fi| - fi.public_send(step) + if action + action.call(fi) + elsif step + fi.public_send(step) + end true rescue CannotInstallFormulaError => e ofail e.message @@ -412,13 +427,13 @@ module Homebrew ) end - sig { params(formula_names: T::Array[String], cask_names: T::Array[String]).void } - def show_combined_fetch_downloads_heading(formula_names: [], cask_names: []) + sig { params(formula_names: T::Array[String], cask_names: T::Array[String]).returns(T.nilable(String)) } + def combined_fetch_downloads_heading(formula_names: [], cask_names: []) combined_fetch_targets = formula_names.map { |name| Formatter.identifier(name) } + cask_names.map { |name| Formatter.identifier(name) } return if combined_fetch_targets.empty? - oh1 "Fetching downloads for: #{combined_fetch_targets.to_sentence}", truncate: false + "Fetching downloads for: #{combined_fetch_targets.to_sentence}" end sig { params(cask_installers: T::Array[T.untyped], download_queue: Homebrew::DownloadQueue).void } @@ -436,9 +451,8 @@ module Homebrew end if source_downloads.any? - oh1 "Downloading Cask files" source_downloads.each { |source_download| download_queue.enqueue(source_download) } - download_queue.fetch + download_queue.fetch(only: Cask::Download, heading: "Downloading Cask files") end valid_cask_installers.each do |cask_installer| diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index c770d60309..f39994e88f 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -351,7 +351,7 @@ RSpec.describe Homebrew::Cmd::InstallCmd do allow(Homebrew.messages).to receive(:display_messages) expect(Homebrew::DownloadQueue).to receive(:new).ordered.and_return(download_queue) expect(formula_installer).to receive(:download_queue=).with(download_queue).ordered - expect(formula_installer).to receive(:prelude_fetch).ordered + expect(formula_installer).to receive(:prelude_fetch).with(no_args).ordered expect(Homebrew::Upgrade).to receive(:dependants).ordered.and_return(dependants) expect(Homebrew::Install).to receive(:enqueue_formulae) .with([formula_installer], download_queue:) @@ -363,6 +363,43 @@ RSpec.describe Homebrew::Cmd::InstallCmd do cmd.run end + it "drains metadata-only prelude fetches before the dry-run plan when asking" do + cmd = described_class.new(["testball"]) + download_queue = instance_double(Homebrew::DownloadQueue, shutdown: nil, + downloads: { instance_double(Downloadable) => nil }) + formula = formula("testball") do + T.bind(self, T.class_of(Formula)) + url "https://brew.sh/testball-0.1.tar.gz" + end + formula_installer = instance_double(FormulaInstaller, formula:) + dependants = Homebrew::Upgrade::Dependents.new(upgradeable: [], pinned: [], skipped: []) + + allow(Tap).to receive_messages(with_formula_name: nil, with_cask_token: nil) + allow(Homebrew::Trust).to receive(:trust_fully_qualified_items!) + allow(cmd.args.named).to receive(:to_formulae_and_casks).with(warn: false).and_return([formula]) + allow(Homebrew::Install).to receive(:perform_preinstall_checks_once) + allow(Homebrew::Install).to receive(:check_cc_argv) + allow(Homebrew::Install).to receive_messages(install_formula?: true, formula_installers: [formula_installer]) + allow(Homebrew::Install).to receive(:install_formulae) + allow(Homebrew::Upgrade).to receive(:upgrade_dependents) + allow(Homebrew::Cleanup).to receive(:periodic_clean!) + allow(Homebrew.messages).to receive(:display_messages) + expect(Homebrew::DownloadQueue).to receive(:new).ordered.and_return(download_queue) + expect(formula_installer).to receive(:download_queue=).with(download_queue).ordered + expect(formula_installer).to receive(:prelude_fetch).with(metadata_only: true).ordered + expect(Homebrew::Upgrade).to receive(:dependants).ordered.and_return(dependants) + expect(download_queue).to receive(:fetch).ordered + expect(Homebrew::Install).to receive(:ask_formulae).ordered + expect(Homebrew::Install).to receive(:enqueue_formulae) + .with([formula_installer], download_queue:) + .ordered + .and_return([formula_installer]) + expect(download_queue).to receive(:fetch).ordered + expect(download_queue).to receive(:shutdown).ordered + + cmd.run + end + it "does not install `homebrew/cask` when a cask remains unavailable" do cmd = described_class.new(["foo"]) cask_tap = CoreCaskTap.instance @@ -505,11 +542,12 @@ RSpec.describe Homebrew::Cmd::InstallCmd do true end + expect(download_queue).to receive(:fetch) + .with(heading: "Fetching downloads for: testball_bottle and codex") expect { cmd.run }.to output(<<~EOS).to_stdout ==> Upgrading 1 outdated package: codex 0.117.0 -> 0.118.0 - ==> Fetching downloads for: testball_bottle and codex EOS end end diff --git a/Library/Homebrew/test/cmd/reinstall_spec.rb b/Library/Homebrew/test/cmd/reinstall_spec.rb index 82ad3c51c9..83b0b55b81 100644 --- a/Library/Homebrew/test/cmd/reinstall_spec.rb +++ b/Library/Homebrew/test/cmd/reinstall_spec.rb @@ -68,7 +68,6 @@ RSpec.describe Homebrew::Cmd::Reinstall do allow(Homebrew::Reinstall).to receive(:build_install_context).and_return(reinstall_context) allow(Homebrew::Upgrade).to receive(:dependants).and_return(dependants) allow(Homebrew::Install).to receive(:ask_formulae) - allow(Homebrew::Install).to receive(:show_combined_fetch_downloads_heading) allow(Homebrew::Install).to receive(:enqueue_formulae).and_return([formula_installer]) allow(Homebrew::Install).to receive(:enqueue_cask_installers) allow(Cask::Installer).to receive(:new).and_return(instance_double(Cask::Installer)) diff --git a/Library/Homebrew/test/cmd/upgrade_spec.rb b/Library/Homebrew/test/cmd/upgrade_spec.rb index 24c3b84407..e1d62bc831 100644 --- a/Library/Homebrew/test/cmd/upgrade_spec.rb +++ b/Library/Homebrew/test/cmd/upgrade_spec.rb @@ -427,12 +427,11 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do expect(cmd).to receive(:upgrade_outdated_formulae!) .with( [], - prefetch_only: true, + prefetch_only: true, download_queue:, - prefetch_names: [], - prefetch_upgrades: [], - show_upgrade_summary: false, - show_downloads_heading: false, + prefetch_names: [], + prefetch_upgrades: [], + show_upgrade_summary: false, ) .ordered .and_return(true) @@ -440,11 +439,10 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do .with( [], download_queue:, - prefetch_names: [], - prefetch_upgrades: [], - prefetch_casks: [], - prefetch_errors: [], - show_downloads_heading: false, + prefetch_names: [], + prefetch_upgrades: [], + prefetch_casks: [], + prefetch_errors: [], ) .ordered .and_return(true) @@ -758,12 +756,13 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do allow(Homebrew::Cleanup).to receive(:periodic_clean!) allow(Homebrew::Reinstall).to receive(:reinstall_pkgconf_if_needed!) allow(Homebrew.messages).to receive(:display_messages) + expect(download_queue).to receive(:fetch) + .with(heading: "Fetching downloads for: deno and codex") expect { cmd.run }.to output(<<~EOS).to_stdout ==> Upgrading 2 outdated packages: deno 2.7.10 -> 2.7.11 codex 0.117.0 -> 0.118.0 - ==> Fetching downloads for: deno and codex EOS end @@ -885,19 +884,21 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do allow(Cask::Installer).to receive(:new).and_return(installer) expect(installer).to receive(:prelude_fetch_download).and_return(source_download) expect(download_queue).to receive(:enqueue).with(source_download).ordered - expect(download_queue).to receive(:fetch).ordered - expect(download_queue).to receive(:fetch).ordered + expect(download_queue).to receive(:fetch) + .with(only: Cask::Download, heading: "Downloading Cask files") + .ordered + expect(download_queue).to receive(:fetch) + .with(heading: "Fetching downloads for: deno and codex") + .ordered allow(Cask::Upgrade).to receive_messages(outdated_casks: [cask], upgrade_casks!: true) allow(Homebrew::Cleanup).to receive(:periodic_clean!) allow(Homebrew::Reinstall).to receive(:reinstall_pkgconf_if_needed!) allow(Homebrew.messages).to receive(:display_messages) expect { cmd.run }.to output(<<~EOS).to_stdout - ==> Downloading Cask files ==> Upgrading 2 outdated packages: deno 2.7.10 -> 2.7.11 codex 0.117.0 -> 0.118.0 - ==> Fetching downloads for: deno and codex EOS end @@ -944,7 +945,6 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do prefetch_upgrades:, prefetch_casks:, prefetch_errors:, - show_downloads_heading: false, ), ).to be(true) expect(prefetch_names).to eq(["codex"]) @@ -986,7 +986,9 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do end allow(Cask::Installer).to receive(:new).and_return(installer) expect(installer).to receive(:prelude_fetch_download).and_return(nil) - expect(download_queue).to receive(:fetch).once + expect(download_queue).to receive(:fetch) + .with(heading: "Fetching downloads for: deno and codex") + .once allow(Cask::Upgrade).to receive_messages(outdated_casks: [cask], upgrade_casks!: true) allow(Homebrew::Cleanup).to receive(:periodic_clean!) allow(Homebrew::Reinstall).to receive(:reinstall_pkgconf_if_needed!) @@ -996,12 +998,10 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do ==> Upgrading 2 outdated packages: deno 2.7.10 -> 2.7.11 codex 0.117.0 -> 0.118.0 - ==> Fetching downloads for: deno and codex EOS end - it "prints a bottle manifest heading before formula prefetches" do - cmd = described_class.new([]) + it "passes a bottle manifest heading to the tab prefetch queue" do formula = formula("deno") do T.bind(self, T.class_of(Formula)) url "https://brew.sh/deno-2.7.11.tar.gz" @@ -1012,37 +1012,14 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do Utils::Bottles.tag.to_sym => "d7b9f4e8bf83608b71fe958a99f19f2e5e68bb2582965d32e41759c24f1aef97" end end + download_queue = instance_double(Homebrew::DownloadQueue, enqueue: nil, shutdown: nil) - allow(formula).to receive_messages(outdated?: true, latest_formula: formula, latest_version_installed?: false) - allow(Homebrew::Install).to receive(:perform_preinstall_checks_once) - allow(Homebrew::Upgrade).to receive(:formula_installers).and_return([]) + 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 do - cmd.formulae_upgrade_context([formula], show_upgrade_summary: false) - end.to output("==> Downloading bottle manifests\n").to_stdout - end - - it "omits the bottle manifest heading for cached formula manifests" do - cmd = described_class.new([]) - formula = formula("deno") do - T.bind(self, T.class_of(Formula)) - url "https://brew.sh/deno-2.7.11.tar.gz" - - bottle do - root_url HOMEBREW_BOTTLE_DEFAULT_DOMAIN - sha256 cellar: :any_skip_relocation, - Utils::Bottles.tag.to_sym => "d7b9f4e8bf83608b71fe958a99f19f2e5e68bb2582965d32e41759c24f1aef97" - end - end - - allow(formula).to receive_messages(outdated?: true, latest_formula: formula, latest_version_installed?: false) - allow(formula.bottle&.github_packages_manifest_resource).to receive(:downloaded_and_valid?).and_return(true) - allow(Homebrew::Install).to receive(:perform_preinstall_checks_once) - allow(Homebrew::Upgrade).to receive(:formula_installers).and_return([]) - - expect do - cmd.formulae_upgrade_context([formula], show_upgrade_summary: false) - end.not_to output(/Downloading bottle manifests/).to_stdout + Homebrew::Upgrade.formula_installers([formula], flags: []) end it "does not trust failed shared prefetches" do diff --git a/Library/Homebrew/test/download_queue_spec.rb b/Library/Homebrew/test/download_queue_spec.rb index 271066a71a..1e9400cd5c 100644 --- a/Library/Homebrew/test/download_queue_spec.rb +++ b/Library/Homebrew/test/download_queue_spec.rb @@ -71,6 +71,28 @@ RSpec.describe Homebrew::DownloadQueue do expect(failure_index).to be > show_cursor_index end + it "fetches only downloads of the given class and keeps others queued unreported" do + manifest = instance_double( + Resource::BottleManifest, + cached_download: HOMEBREW_CACHE/"downloads/testball_manifest.json", + checksum: nil, + downloaded_and_valid?: true, + downloader: nil, + download_queue_message: "Bottle Manifest testball", + download_queue_name: "testball", + download_queue_type: "Bottle Manifest", + ) + allow(manifest).to receive(:is_a?) { |klass| klass == Resource::BottleManifest } + + download_queue.enqueue(manifest) + download_queue.enqueue(downloadable) + + expect do + download_queue.fetch(only: Resource::BottleManifest) + end.to output(/Bottle Manifest testball/).to_stderr + expect(download_queue.downloads.keys).to eq [downloadable] + end + it "raises and clears queue state on a bottle manifest failure in parallel mode" do allow(retryable_download).to receive(:fetch).and_raise(Resource::BottleManifest::Error.new("manifest missing")) diff --git a/Library/Homebrew/test/formula_installer_spec.rb b/Library/Homebrew/test/formula_installer_spec.rb index 29e3c5ebb3..3c16d624dd 100644 --- a/Library/Homebrew/test/formula_installer_spec.rb +++ b/Library/Homebrew/test/formula_installer_spec.rb @@ -380,6 +380,17 @@ RSpec.describe FormulaInstaller do installer.enqueue_fetch end + + it "does not requeue a bottle already enqueued by the prelude fetch" do + allow(installer).to receive(:pour_bottle?).and_return(true) + expect(download_queue).to receive(:enqueue) + .with(bottle, check_attestation: false, stage: true).once + installer.prelude_fetch + + expect(installer).to receive(:fetch_dependencies) + + installer.enqueue_fetch + end end describe "linking defaults" do @@ -1254,41 +1265,72 @@ RSpec.describe FormulaInstaller do end describe "#prelude_fetch" do - it "uses API bottle metadata for API-loaded formula manifests" do - formula = formula("deno") do - T.bind(self, T.class_of(Formula)) - url "https://brew.sh/deno-2.7.11.tar.gz" + context "with an API-loaded bottled formula" do + let(:deno_formula) do + formula("deno") do + T.bind(self, T.class_of(Formula)) + url "https://brew.sh/deno-2.7.11.tar.gz" + end + end + let(:formula_struct) do + Homebrew::API::FormulaStruct.new( + bottle_checksums: [ + { + cellar: :any_skip_relocation, + Utils::Bottles.tag.to_sym => "d7b9f4e8bf83608b71fe958a99f19f2e5e68bb2582965d32e41759c24f1aef97", + }, + ], + bottle_present: true, + desc: "deno", + homepage: "https://brew.sh", + license: "MIT", + ruby_source_checksum: "abc123", + stable_present: true, + stable_version: "2.7.11", + ) + end + let(:installer) do + installer = described_class.new(deno_formula, ignore_deps: true) + installer.download_queue = instance_double(Homebrew::DownloadQueue) + installer end - formula_struct = Homebrew::API::FormulaStruct.new( - bottle_checksums: [ - { - cellar: :any_skip_relocation, - Utils::Bottles.tag.to_sym => "d7b9f4e8bf83608b71fe958a99f19f2e5e68bb2582965d32e41759c24f1aef97", - }, - ], - bottle_present: true, - desc: "deno", - homepage: "https://brew.sh", - license: "MIT", - ruby_source_checksum: "abc123", - stable_present: true, - stable_version: "2.7.11", - ) - installer = described_class.new(formula, ignore_deps: true) - installer.download_queue = instance_double(Homebrew::DownloadQueue) - allow(formula).to receive_messages( - bottle_tag?: true, - core_formula?: true, - loaded_from_internal_api?: true, - pour_bottle?: true, - ) - allow(Homebrew::API::Internal).to receive(:formula_struct).with("deno").and_return(formula_struct) - expect(formula).not_to receive(:bottle_for_tag) - expect(formula).not_to receive(:bottle) - expect(installer.download_queue).to receive(:enqueue).with(an_instance_of(Resource::BottleManifest)) + before do + allow(deno_formula).to receive_messages( + bottle_tag?: true, + core_formula?: true, + loaded_from_internal_api?: true, + pour_bottle?: true, + ) + allow(Homebrew::API::Internal).to receive(:formula_struct).with("deno").and_return(formula_struct) + end - installer.prelude_fetch + it "uses API bottle metadata to enqueue the manifest and bottle" do + expect(deno_formula).not_to receive(:bottle_for_tag) + expect(deno_formula).not_to receive(:bottle) + expect(installer.download_queue).to receive(:enqueue).with(an_instance_of(Resource::BottleManifest)) + expect(installer.download_queue).to receive(:enqueue) + .with(an_instance_of(Bottle), check_attestation: false, stage: true) + + installer.prelude_fetch + end + + it "enqueues only the bottle manifest when fetching metadata" do + expect(installer.download_queue).to receive(:enqueue).with(an_instance_of(Resource::BottleManifest)) + + installer.prelude_fetch(metadata_only: true) + end + + it "enqueues the bottle without repeating metadata work after a metadata-only run" do + expect(installer.download_queue).to receive(:enqueue).with(an_instance_of(Resource::BottleManifest)).once + + installer.prelude_fetch(metadata_only: true) + + expect(installer.download_queue).to receive(:enqueue) + .with(an_instance_of(Bottle), check_attestation: false, stage: true) + + installer.prelude_fetch + end end it "does not repeat source download prelude work" do diff --git a/Library/Homebrew/upgrade.rb b/Library/Homebrew/upgrade.rb index c8e55922ba..7d45901068 100644 --- a/Library/Homebrew/upgrade.rb +++ b/Library/Homebrew/upgrade.rb @@ -134,7 +134,7 @@ module Homebrew end end - download_queue.fetch + download_queue.fetch(heading: "Downloading bottle manifests") ensure download_queue.shutdown end diff --git a/Library/Homebrew/utils/phase_timings.rb b/Library/Homebrew/utils/phase_timings.rb index f24a95556a..d9c79e06bc 100644 --- a/Library/Homebrew/utils/phase_timings.rb +++ b/Library/Homebrew/utils/phase_timings.rb @@ -54,7 +54,10 @@ module Homebrew if defined?(Homebrew::API::Internal) instrument(Homebrew::API::Internal.singleton_class, :formula_struct, "api_metadata_load") end - instrument(Homebrew::Install.singleton_class, :formula_installers, "planning") if defined?(Homebrew::Install) + if defined?(Homebrew::Install) + instrument(Homebrew::Install.singleton_class, :formula_installers, "planning") + instrument(Homebrew::Install.singleton_class, :perform_preinstall_checks_once, "preinstall_checks") + end if defined?(FormulaInstaller) instrument(FormulaInstaller, :prelude, "planning") instrument(FormulaInstaller, :compute_dependencies, "dependency_resolution")