From dc0ed6dc3563fe4fb36cacd7b6de3fd8f704e31a Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Mon, 17 Aug 2026 09:14:42 +0100 Subject: [PATCH] Defer package summaries until cleanup - collect formula and cask caveats without inline output - share cleanup and message finalisation across commands - print one caveat summary before the final upgrade result --- Library/Homebrew/cask/installer.rb | 23 ++++++++++++------- Library/Homebrew/cask/upgrade.rb | 4 +--- Library/Homebrew/cmd/install.rb | 14 +++++------ Library/Homebrew/cmd/reinstall.rb | 11 ++++----- Library/Homebrew/cmd/upgrade.rb | 10 ++++---- Library/Homebrew/formula_installer.rb | 2 -- Library/Homebrew/install.rb | 16 +++++++++++++ Library/Homebrew/test/cask/installer_spec.rb | 8 +++++-- Library/Homebrew/test/cmd/install_spec.rb | 4 ++-- Library/Homebrew/test/cmd/reinstall_spec.rb | 4 ++-- Library/Homebrew/test/cmd/upgrade_spec.rb | 2 +- .../Homebrew/test/formula_installer_spec.rb | 16 +++++++++++++ Library/Homebrew/test/install_spec.rb | 17 ++++++++++++++ 13 files changed, 93 insertions(+), 38 deletions(-) diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index e774043a4ccce..0e3bcb8d4f32d 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -115,10 +115,8 @@ def zap? = @zap def self.caveats(cask) odebug "Printing caveats" - caveats = cask.caveats - return if caveats.empty? - - Homebrew.messages.record_caveats(cask.token, caveats) + caveats = record_caveats(cask) + return unless caveats <<~EOS #{ohai_title "Caveats"} @@ -126,6 +124,15 @@ def self.caveats(cask) EOS end + sig { params(cask: ::Cask::Cask).returns(T.nilable(String)) } + def self.record_caveats(cask) + caveats = cask.caveats + return if caveats.empty? + + Homebrew.messages.record_caveats(cask.token, caveats) + caveats + end + sig { params(quiet: T.nilable(T::Boolean), timeout: T.nilable(T.any(Integer, Float))).void } def fetch(quiet: nil, timeout: nil) odebug "Cask::Installer#fetch" @@ -175,7 +182,7 @@ def install prelude - print caveats + record_caveats fetch uninstall_existing_cask if reinstall? @@ -553,9 +560,9 @@ def dependency_installers(defer_fetch: false) [cask_installers, formula_installers] end - sig { returns(T.nilable(String)) } - def caveats - self.class.caveats(@cask) + sig { void } + def record_caveats + self.class.record_caveats(@cask) end sig { returns(Pathname) } diff --git a/Library/Homebrew/cask/upgrade.rb b/Library/Homebrew/cask/upgrade.rb index cc0fead4f3c40..4c83fa5976dc7 100644 --- a/Library/Homebrew/cask/upgrade.rb +++ b/Library/Homebrew/cask/upgrade.rb @@ -423,9 +423,7 @@ def self.upgrade_cask( # Start new cask's installation steps new_cask_installer.prelude - if (caveats = new_cask_installer.caveats) - puts caveats - end + new_cask_installer.record_caveats new_cask_installer.fetch diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index fe18499f5f1c2..2d3fc54ce7c94 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -11,7 +11,6 @@ require "formula_installer" require "development_tools" require "install" -require "cleanup" require "upgrade" require "trust" @@ -496,13 +495,12 @@ def run end end - unless args.dry_run? - Cleanup.install_clean!(formulae: installed_or_upgraded_formulae, - casks: installed_or_upgraded_casks) - end - Cleanup.periodic_clean!(dry_run: args.dry_run?) - - Homebrew.messages.display_messages(display_times: args.display_times?) + Install.finish_installation( + formulae: installed_or_upgraded_formulae, + casks: installed_or_upgraded_casks, + dry_run: args.dry_run?, + display_times: args.display_times?, + ) rescue FormulaUnreadableError, FormulaClassUnavailableError, TapFormulaUnreadableError, TapFormulaClassUnavailableError => e require "utils/backtrace" diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb index ed64a705f5cc3..51b50677570eb 100644 --- a/Library/Homebrew/cmd/reinstall.rb +++ b/Library/Homebrew/cmd/reinstall.rb @@ -4,10 +4,8 @@ require "abstract_command" require "formula_installer" require "development_tools" -require "messages" require "install" require "reinstall" -require "cleanup" require "cask/utils" require "cask/installer" require "cask/reinstall" @@ -351,10 +349,11 @@ def run unavailable_errors.each { |e| ofail e } - Cleanup.install_clean!(formulae: reinstalled_formulae, casks: reinstalled_casks) - Cleanup.periodic_clean! - - Homebrew.messages.display_messages(display_times: args.display_times?) + Install.finish_installation( + formulae: reinstalled_formulae, + casks: reinstalled_casks, + display_times: args.display_times?, + ) end end end diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index 1ff1e99cdcb80..b999b7453d498 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -341,10 +341,12 @@ def run Homebrew::Reinstall.reinstall_pkgconf_if_needed!(dry_run: args.dry_run?) - Cleanup.install_clean!(formulae: @upgraded_formulae, casks: @upgraded_casks) unless args.dry_run? - Cleanup.periodic_clean!(dry_run: args.dry_run?) - - Homebrew.messages.display_messages(display_times: args.display_times?) + Install.finish_installation( + formulae: @upgraded_formulae, + casks: @upgraded_casks, + dry_run: args.dry_run?, + display_times: args.display_times?, + ) show_final_upgrade_summary end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 659885b3bb951..718827338696f 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -961,8 +961,6 @@ def caveats Homebrew.messages.record_completions_and_elisp(caveats.completions_and_elisp) return if caveats.caveats.empty? - @show_summary_heading = true - ohai "Caveats", caveats.to_s Homebrew.messages.record_caveats(formula.name, caveats) end diff --git a/Library/Homebrew/install.rb b/Library/Homebrew/install.rb index dd4b375edfdfa..be34c4a14bf0f 100644 --- a/Library/Homebrew/install.rb +++ b/Library/Homebrew/install.rb @@ -9,6 +9,8 @@ require "upgrade" require "download_queue" require "ask" +require "cleanup" +require "messages" require "utils/output" require "utils/topological_hash" @@ -508,6 +510,20 @@ def enqueue_cask_installers(cask_installers, download_queue:) downloads_succeeded end + sig { + params( + formulae: T::Array[Formula], + casks: T::Array[Cask::Cask], + dry_run: T::Boolean, + display_times: T::Boolean, + ).void + } + def finish_installation(formulae:, casks:, dry_run: false, display_times: false) + Cleanup.install_clean!(formulae:, casks:) unless dry_run + Cleanup.periodic_clean!(dry_run:) + Homebrew.messages.display_messages(force_caveats: true, display_times:) + end + sig { params(formula_installers: T::Array[FormulaInstaller], installed_on_request: T::Boolean, build_bottle: T::Boolean, force_bottle: T::Boolean, diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index 8c517ccd8627f..4fee9e5f50644 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -274,12 +274,16 @@ def stub_dmg_extraction expect(no_checksum).to be_installed end - it "prints caveats if they're present" do + it "records caveats without printing them inline" do with_caveats = Cask::CaskLoader.load(cask_path("with-caveats")) + expect(Homebrew.messages).to receive(:record_caveats) + .with(with_caveats.token, with_caveats.caveats) + expect(described_class).not_to receive(:caveats) + expect do described_class.new(with_caveats).install - end.to output(/Here are some things you might want to know/).to_stdout + end.not_to output(/Here are some things you might want to know/).to_stdout expect(with_caveats).to be_installed end diff --git a/Library/Homebrew/test/cmd/install_spec.rb b/Library/Homebrew/test/cmd/install_spec.rb index bc0668332c7cc..98e0fdf36bb92 100644 --- a/Library/Homebrew/test/cmd/install_spec.rb +++ b/Library/Homebrew/test/cmd/install_spec.rb @@ -380,7 +380,7 @@ .ordered expect(Homebrew::Cleanup).to receive(:periodic_clean!).with(dry_run: false).ordered expect(Homebrew.messages).to receive(:display_messages) - .with(display_times: false) + .with(force_caveats: true, display_times: false) .ordered cmd.run @@ -482,7 +482,7 @@ .ordered expect(Homebrew::Cleanup).to receive(:periodic_clean!).with(dry_run: false).ordered expect(Homebrew.messages).to receive(:display_messages) - .with(display_times: false) + .with(force_caveats: true, display_times: false) .ordered cmd.run diff --git a/Library/Homebrew/test/cmd/reinstall_spec.rb b/Library/Homebrew/test/cmd/reinstall_spec.rb index 60af2cba335fc..20715b5d79dcd 100644 --- a/Library/Homebrew/test/cmd/reinstall_spec.rb +++ b/Library/Homebrew/test/cmd/reinstall_spec.rb @@ -71,7 +71,7 @@ .ordered expect(Homebrew::Cleanup).to receive(:periodic_clean!).ordered expect(Homebrew.messages).to receive(:display_messages) - .with(display_times: false) + .with(force_caveats: true, display_times: false) .ordered cmd.run @@ -182,7 +182,7 @@ .ordered expect(Homebrew::Cleanup).to receive(:periodic_clean!).ordered expect(Homebrew.messages).to receive(:display_messages) - .with(display_times: false) + .with(force_caveats: true, display_times: false) .ordered cmd.run diff --git a/Library/Homebrew/test/cmd/upgrade_spec.rb b/Library/Homebrew/test/cmd/upgrade_spec.rb index ee3dd27dbaa0e..f46177c839f1b 100644 --- a/Library/Homebrew/test/cmd/upgrade_spec.rb +++ b/Library/Homebrew/test/cmd/upgrade_spec.rb @@ -471,7 +471,7 @@ def stub_formula_upgrade_installers expect(Homebrew::Cleanup).to receive(:install_clean!).with(formulae: [], casks: []).ordered expect(Homebrew::Cleanup).to receive(:periodic_clean!).with(dry_run: false).ordered expect(Homebrew.messages).to receive(:display_messages) - .with(display_times: false) + .with(force_caveats: true, display_times: false) .ordered expect(cmd).to receive(:show_final_upgrade_summary).with(no_args).ordered diff --git a/Library/Homebrew/test/formula_installer_spec.rb b/Library/Homebrew/test/formula_installer_spec.rb index a52177ad95a74..716ad380e8eca 100644 --- a/Library/Homebrew/test/formula_installer_spec.rb +++ b/Library/Homebrew/test/formula_installer_spec.rb @@ -1426,6 +1426,22 @@ class #{Formulary.class_s(f_name)} < Formula describe "#caveats" do subject(:formula_installer) { described_class.new(Testball.new) } + it "records caveats without printing them inline" do + formula = Testball.new + installer = described_class.new(formula, installed_on_request: true) + caveats = instance_double( + Caveats, + caveats: "Add testball to your PATH", + completions_and_elisp: [], + empty?: false, + ) + + allow(Caveats).to receive(:new).with(formula).and_return(caveats) + expect(Homebrew.messages).to receive(:record_caveats).with(formula.name, caveats) + + expect { installer.caveats }.not_to output.to_stdout + end + it "shows audit problems if HOMEBREW_DEVELOPER is set" do ENV["HOMEBREW_DEVELOPER"] = "1" with_env(HOMEBREW_NO_INSTALL_FROM_API: "1") do diff --git a/Library/Homebrew/test/install_spec.rb b/Library/Homebrew/test/install_spec.rb index 718dc1fb1f137..d66756cc41cc4 100644 --- a/Library/Homebrew/test/install_spec.rb +++ b/Library/Homebrew/test/install_spec.rb @@ -101,6 +101,23 @@ end end + describe "::finish_installation" do + it "cleans packages before reporting caveats" do + formula = instance_double(Formula) + cask = instance_double(Cask::Cask) + + expect(Homebrew::Cleanup).to receive(:install_clean!) + .with(formulae: [formula], casks: [cask]) + .ordered + expect(Homebrew::Cleanup).to receive(:periodic_clean!).with(dry_run: false).ordered + expect(Homebrew.messages).to receive(:display_messages) + .with(force_caveats: true, display_times: true) + .ordered + + described_class.finish_installation(formulae: [formula], casks: [cask], display_times: true) + end + end + describe "::enqueue_cask_installers" do it "fetches source API downloads before enqueueing cask downloads" do source_download = instance_double(Homebrew::API::SourceDownload)