Skip to content

Commit 5f2fa12

Browse files
committed
Improve cleanup ask output
- Avoid noisy command previews during `brew upgrade --ask`. - Share one dry-run cleanup heading across formulae. - Get upgrade cleanup output from one formula-only dry run. - Reuse bundle cleanup output formatting for consistency. - Avoid shelling out for bundle cleanup dry runs. - Reuse `Cleanup#cleanup_formula` so dry runs show removals. - Skip empty cleanup headings when there is no work.
1 parent 10a163a commit 5f2fa12

5 files changed

Lines changed: 91 additions & 21 deletions

File tree

Library/Homebrew/bundle/subcommand/cleanup.rb

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
require "abstract_subcommand"
55
require "bundle/extensions/extension"
6+
require "cleanup"
67

78
require "utils/formatter"
89
require "utils"
@@ -172,13 +173,9 @@ def self.cleanup(global: false, file: nil, force: false, zap: false, dsl: nil,
172173
would_uninstall = true
173174
end
174175

175-
cleanup = system_output_no_stderr(HOMEBREW_BREW_FILE, "cleanup", "--dry-run")
176-
unless cleanup.empty?
177-
puts "Would `brew cleanup`:"
178-
puts cleanup
179-
end
176+
would_cleanup = Cleanup.printed_dry_run_output?(Cleanup.dry_run_output)
180177

181-
puts "Run `brew bundle cleanup --force` to make these changes." if would_uninstall || !cleanup.empty?
178+
puts "Run `brew bundle cleanup --force` to make these changes." if would_uninstall || would_cleanup
182179
exit 1 if would_uninstall
183180
end
184181
end

Library/Homebrew/cleanup.rb

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
require "utils/bottles"
55
require "utils/output"
66
require "installed_dependents"
7+
require "stringio"
78

89
require "formula"
910
require "cask/cask_loader"
@@ -272,21 +273,53 @@ def prune? = @prune
272273
sig { returns(T::Boolean) }
273274
def scrub? = @scrub
274275

275-
sig { params(formula: Formula, dry_run: T::Boolean).void }
276-
def self.install_formula_clean!(formula, dry_run: false)
277-
return if Homebrew::EnvConfig.no_install_cleanup?
278-
return unless formula.latest_version_installed?
279-
return if skip_clean_formula?(formula)
276+
sig { params(output: String, ohai: T::Boolean).returns(T::Boolean) }
277+
def self.printed_dry_run_output?(output, ohai: false)
278+
return false if output.blank?
280279

281-
if dry_run
282-
ohai "Would run `brew cleanup #{formula}`"
280+
if ohai
281+
ohai "Would `brew cleanup`"
283282
else
284-
ohai "Running `brew cleanup #{formula}`..."
283+
puts "Would `brew cleanup`:"
285284
end
285+
print output
286+
puts unless output.end_with?("\n")
287+
true
288+
end
286289

287-
puts_no_install_cleanup_disable_message_if_not_already!
288-
return if dry_run
290+
sig { params(args: String, formulae: T::Array[Formula]).returns(String) }
291+
def self.dry_run_output(*args, formulae: [])
292+
output = StringIO.new
293+
old_stdout = $stdout
294+
begin
295+
$stdout = output
296+
cleanup = Cleanup.new(*args, dry_run: true)
297+
if formulae.empty?
298+
cleanup.clean!
299+
else
300+
formulae.each { |formula| cleanup.cleanup_formula(formula) }
301+
end
302+
ensure
303+
$stdout = old_stdout
304+
end
305+
output.string
306+
end
307+
308+
sig { params(formulae: T::Array[Formula]).returns(T::Array[Formula]) }
309+
def self.install_cleanup_formulae(formulae)
310+
return [] if Homebrew::EnvConfig.no_install_cleanup?
289311

312+
formulae.select do |formula|
313+
formula.latest_version_installed? && !skip_clean_formula?(formula)
314+
end
315+
end
316+
317+
sig { params(formula: Formula).void }
318+
def self.install_formula_clean!(formula)
319+
return if install_cleanup_formulae([formula]).blank?
320+
321+
ohai "Running `brew cleanup #{formula}`..."
322+
puts_no_install_cleanup_disable_message_if_not_already!
290323
Cleanup.new.cleanup_formula(formula)
291324
end
292325

Library/Homebrew/test/cmd/bundle/cleanup_subcommand_spec.rb

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@
382382
it "lists casks, formulae and taps" do
383383
expect(Formatter).to receive(:columns).with(%w[a b]).exactly(5).times.and_return("a b")
384384
expect(Kernel).not_to receive(:system)
385-
expect(klass).to receive(:system_output_no_stderr).and_return("")
385+
expect(Homebrew::Cleanup).to receive(:dry_run_output).and_return("")
386386
output_pattern = Regexp.new(
387387
"Would uninstall casks:.*Would uninstall formulae:.*Would untap:.*" \
388388
"Would uninstall VSCode extensions:.*Would uninstall flatpaks:",
@@ -405,20 +405,25 @@
405405
end
406406

407407
define_method(:sane?) do
408-
expect(klass).to receive(:system_output_no_stderr).and_return("cleaned")
408+
expect(klass).not_to receive(:system_output_no_stderr)
409+
expect(Homebrew::Cleanup).to receive(:dry_run_output).and_return("cleaned")
409410
end
410411

411412
context "with --force" do
412413
it "prints output" do
413-
sane?
414+
expect(klass).to receive(:system_output_no_stderr).and_return("cleaned")
414415
expect { klass.cleanup(force: true) }.to output(/cleaned/).to_stdout
415416
end
416417
end
417418

418419
context "without --force" do
419420
it "prints output" do
420421
sane?
421-
expect { klass.cleanup }.to output(/cleaned/).to_stdout
422+
expect { klass.cleanup }.to output(<<~EOS).to_stdout
423+
Would `brew cleanup`:
424+
cleaned
425+
Run `brew bundle cleanup --force` to make these changes.
426+
EOS
422427
end
423428
end
424429
end

Library/Homebrew/test/cmd/upgrade_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,34 @@ class #{Formulary.class_s(name)} < Formula
334334
.to eq(["testball 0.1 -> 0.2 (500B)"])
335335
end
336336

337+
it "prints dry-run cleanup output from one formula cleanup run" do
338+
formula = formula("testball") do
339+
url "https://brew.sh/testball-0.2"
340+
end
341+
other_formula = formula("otherball") do
342+
url "https://brew.sh/otherball-0.2"
343+
end
344+
345+
allow(Homebrew::Install).to receive(:print_dry_run_dependencies)
346+
allow(formula).to receive(:latest_version_installed?).and_return(true)
347+
allow(other_formula).to receive(:latest_version_installed?).and_return(true)
348+
expect(Homebrew::Cleanup).to receive(:dry_run_output)
349+
.with(formulae: [formula, other_formula])
350+
.and_return("Would remove: #{HOMEBREW_CELLAR}/testball/0.1 (1KB)\n")
351+
352+
with_env(HOMEBREW_NO_ENV_HINTS: "1") do
353+
expect do
354+
Homebrew::Upgrade.upgrade_formulae(
355+
[FormulaInstaller.new(formula), FormulaInstaller.new(other_formula)],
356+
dry_run: true,
357+
)
358+
end.to output(<<~EOS).to_stdout
359+
==> Would `brew cleanup`
360+
Would remove: #{HOMEBREW_CELLAR}/testball/0.1 (1KB)
361+
EOS
362+
end
363+
end
364+
337365
it "omits dry-run dependencies already listed in the final summary" do
338366
formula = formula("yt-dlp") do
339367
url "https://brew.sh/yt-dlp-2026.3.17_2.tar.gz"

Library/Homebrew/upgrade.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,14 @@ def upgrade_formulae(formula_installers, dry_run: false, verbose: false, fetch:
142142

143143
valid_formula_installers.each do |fi|
144144
upgrade_formula(fi, dry_run:, verbose:, skip_formula_names:)
145-
Cleanup.install_formula_clean!(fi.formula, dry_run:)
145+
Cleanup.install_formula_clean!(fi.formula) unless dry_run
146+
end
147+
return unless dry_run
148+
149+
formulae_to_clean = Cleanup.install_cleanup_formulae(valid_formula_installers.map(&:formula))
150+
if formulae_to_clean.present? &&
151+
Cleanup.printed_dry_run_output?(Cleanup.dry_run_output(formulae: formulae_to_clean), ohai: true)
152+
Cleanup.puts_no_install_cleanup_disable_message_if_not_already!
146153
end
147154
end
148155

0 commit comments

Comments
 (0)