diff --git a/Library/Homebrew/bundle/extensions/cargo.rb b/Library/Homebrew/bundle/extensions/cargo.rb index 202341d5c1796..5ae02bf221976 100644 --- a/Library/Homebrew/bundle/extensions/cargo.rb +++ b/Library/Homebrew/bundle/extensions/cargo.rb @@ -160,7 +160,7 @@ def installed_packages @installed_packages = packages.dup end - sig { override.params(name: String, executable: Pathname).void } + sig { override.params(name: String, executable: Pathname).returns(T::Boolean) } def uninstall_package!(name, executable: Pathname.new("")) Bundle.system(executable.to_s, "uninstall", name, verbose: false) end diff --git a/Library/Homebrew/bundle/extensions/extension.rb b/Library/Homebrew/bundle/extensions/extension.rb index 03246ae181f81..2cd1d75b20e5f 100644 --- a/Library/Homebrew/bundle/extensions/extension.rb +++ b/Library/Homebrew/bundle/extensions/extension.rb @@ -9,6 +9,7 @@ module Bundle class Extension < Homebrew::Bundle::PackageType extend T::Helpers + extend Utils::Output::Mixin abstract! @@ -63,9 +64,9 @@ def self.package_manager_executable! package_manager_executable || raise("#{package_manager_name} is not installed") end - sig { params(executable: Pathname).returns(T::Hash[String, String]) } - def self.package_manager_env(executable) - { "PATH" => "#{executable.dirname}:#{ORIGINAL_PATHS.join(":")}" } + sig { params(_executable: Pathname).returns(T::Hash[String, String]) } + def self.package_manager_env(_executable) + {} end sig { @@ -239,17 +240,22 @@ def self.legacy_check_step sig { params(items: T::Array[String]).void } def self.cleanup!(items) executable = package_manager_executable - return if executable.nil? + if executable.nil? + ofail "Failed to uninstall #{banner_name}: executable not found" unless items.empty? + return + end - with_env(package_manager_env(executable)) do - items.each do |name| - uninstall_package!(name, executable:) + successes = with_env(package_manager_env(executable)) do + items.count do |item| + success = uninstall_package!(item, executable:) + ofail "Failed to uninstall #{banner_name}: #{cleanup_item_name(item)}" unless success + success end end - puts "Uninstalled #{items.size} #{banner_name}#{"s" if items.size != 1}" + puts "Uninstalled #{Utils.pluralize(package_description, successes, include_count: true)}" end - sig { params(name: String, executable: Pathname).void } + sig { params(name: String, executable: Pathname).returns(T::Boolean) } def self.uninstall_package!(name, executable: Pathname.new("")) raise NotImplementedError, "#{self} must override `uninstall_package!` or `cleanup!`." end diff --git a/Library/Homebrew/bundle/extensions/flatpak.rb b/Library/Homebrew/bundle/extensions/flatpak.rb index 974acc9307578..c24315abafdd9 100644 --- a/Library/Homebrew/bundle/extensions/flatpak.rb +++ b/Library/Homebrew/bundle/extensions/flatpak.rb @@ -408,12 +408,9 @@ def cleanup_items(entries) packages - kept_flatpaks end - sig { params(flatpaks: T::Array[String]).void } - def cleanup!(flatpaks) - flatpaks.each do |flatpak_name| - Kernel.system("flatpak", "uninstall", "-y", "--system", flatpak_name) - end - puts "Uninstalled #{flatpaks.size} flatpak#{"s" if flatpaks.size != 1}" + sig { override.params(name: String, executable: Pathname).returns(T::Boolean) } + def uninstall_package!(name, executable: Pathname.new("")) + Kernel.system(executable.to_s, "uninstall", "-y", "--system", name) == true end end diff --git a/Library/Homebrew/bundle/extensions/go.rb b/Library/Homebrew/bundle/extensions/go.rb index 2fc9e6b318a2b..18debdfdd51b4 100644 --- a/Library/Homebrew/bundle/extensions/go.rb +++ b/Library/Homebrew/bundle/extensions/go.rb @@ -98,7 +98,10 @@ def installed_packages sig { override.params(items: T::Array[String]).void } def cleanup!(items) go = package_manager_executable - return if go.nil? + if go.nil? + ofail "Failed to remove Go executables: go executable not found" unless items.empty? + return + end gobin = Utils.popen_read(go, "env", "GOBIN", err: :err).chomp gopath = Utils.popen_read(go, "env", "GOPATH", err: :err).chomp @@ -118,10 +121,17 @@ def cleanup!(items) module_path = path_line.split("\t")[2]&.strip next unless items.include?(module_path) - FileUtils.rm_f(binary) + begin + File.unlink(binary) + rescue Errno::ENOENT + next + rescue SystemCallError => e + ofail "Failed to remove Go executable #{binary}: #{e}" + next + end removed += 1 end - puts "Uninstalled #{removed} #{banner_name}#{"s" if removed != 1}" + puts "Removed #{Utils.pluralize("Go executable", removed, include_count: true)}" end end end diff --git a/Library/Homebrew/bundle/extensions/krew.rb b/Library/Homebrew/bundle/extensions/krew.rb index f4029a839169d..fc143621fa116 100644 --- a/Library/Homebrew/bundle/extensions/krew.rb +++ b/Library/Homebrew/bundle/extensions/krew.rb @@ -33,6 +33,11 @@ def package_manager_executable @package_manager_executable ||= T.let(which("kubectl-krew", ORIGINAL_PATHS), T.nilable(Pathname)) end + sig { override.params(executable: Pathname).returns(T::Hash[String, String]) } + def package_manager_env(executable) + { "PATH" => "#{executable.dirname}:#{ORIGINAL_PATHS.join(":")}" } + end + sig { override.returns(T::Array[String]) } def packages packages = @packages @@ -82,7 +87,7 @@ def parse_plugin_list(output) end private :parse_plugin_list - sig { override.params(name: String, executable: Pathname).void } + sig { override.params(name: String, executable: Pathname).returns(T::Boolean) } def uninstall_package!(name, executable: Pathname.new("")) Bundle.system(executable.to_s, "uninstall", name, verbose: false) end diff --git a/Library/Homebrew/bundle/extensions/mac_app_store.rb b/Library/Homebrew/bundle/extensions/mac_app_store.rb index 30308431c56fc..7113f1b41d070 100644 --- a/Library/Homebrew/bundle/extensions/mac_app_store.rb +++ b/Library/Homebrew/bundle/extensions/mac_app_store.rb @@ -154,15 +154,9 @@ def cleanup_items(entries) .map { |app| cleanup_item(app) } end - sig { override.params(items: T::Array[String]).void } - def cleanup!(items) - mas = package_manager_executable - return if mas.nil? - - items.each do |item| - Bundle.system(mas, "uninstall", parse_cleanup_item(item).id, verbose: false) - end - puts "Uninstalled #{items.size} Mac App Store app#{"s" if items.size != 1}" + sig { override.params(name: String, executable: Pathname).returns(T::Boolean) } + def uninstall_package!(name, executable: Pathname.new("")) + Bundle.system(executable, "uninstall", parse_cleanup_item(name).id, verbose: false) end sig { params(id: Integer).returns(T::Boolean) } diff --git a/Library/Homebrew/bundle/extensions/npm.rb b/Library/Homebrew/bundle/extensions/npm.rb index 6eecb3d2c57ad..8009d1816f821 100644 --- a/Library/Homebrew/bundle/extensions/npm.rb +++ b/Library/Homebrew/bundle/extensions/npm.rb @@ -39,6 +39,11 @@ def package_manager_executable which("npm", ORIGINAL_PATHS) end + sig { override.params(executable: Pathname).returns(T::Hash[String, String]) } + def package_manager_env(executable) + { "PATH" => "#{executable.dirname}:#{ORIGINAL_PATHS.join(":")}" } + end + sig { override.params(name: String, _options: Homebrew::Bundle::EntryOptions).returns(String) } def install_verb(name = "", _options = {}) package_installed?(name) ? "Upgrading" : "Installing" @@ -141,7 +146,7 @@ def outdated_packages @outdated_packages = [] end - sig { override.params(name: String, executable: Pathname).void } + sig { override.params(name: String, executable: Pathname).returns(T::Boolean) } def uninstall_package!(name, executable: Pathname.new("")) Bundle.system(executable.to_s, "uninstall", "-g", name, verbose: false) end diff --git a/Library/Homebrew/bundle/extensions/uv.rb b/Library/Homebrew/bundle/extensions/uv.rb index 58b98844c5b7e..e49e96e884214 100644 --- a/Library/Homebrew/bundle/extensions/uv.rb +++ b/Library/Homebrew/bundle/extensions/uv.rb @@ -363,7 +363,7 @@ def package_installed?(name, with: nil, source: nil) installed_packages.include?(package_record(name, with:, source:)) end - sig { override.params(name: String, executable: Pathname).void } + sig { override.params(name: String, executable: Pathname).returns(T::Boolean) } def uninstall_package!(name, executable: Pathname.new("")) Bundle.system(executable.to_s, "tool", "uninstall", name, verbose: false) end diff --git a/Library/Homebrew/bundle/extensions/vscode_extension.rb b/Library/Homebrew/bundle/extensions/vscode_extension.rb index 922260e22f638..f4b7c286bac7d 100644 --- a/Library/Homebrew/bundle/extensions/vscode_extension.rb +++ b/Library/Homebrew/bundle/extensions/vscode_extension.rb @@ -184,14 +184,9 @@ def cleanup_items(entries) packages - kept_extensions end - sig { params(extensions: T::Array[String]).void } - def cleanup!(extensions) - vscode = package_manager_executable - return if vscode.nil? - - extensions.each do |extension| - Kernel.system(vscode.to_s, "--uninstall-extension", extension) - end + sig { override.params(name: String, executable: Pathname).returns(T::Boolean) } + def uninstall_package!(name, executable: Pathname.new("")) + Kernel.system(executable.to_s, "--uninstall-extension", name) == true end end end diff --git a/Library/Homebrew/bundle/extensions/winget.rb b/Library/Homebrew/bundle/extensions/winget.rb index d369f1e5ac8dd..0a0b1ffd197a4 100644 --- a/Library/Homebrew/bundle/extensions/winget.rb +++ b/Library/Homebrew/bundle/extensions/winget.rb @@ -322,17 +322,11 @@ def cleanup_items(entries) packages_to_cleanup.map { |app| cleanup_item(app) } end - sig { override.params(items: T::Array[String]).void } - def cleanup!(items) - winget = package_manager_executable - return if winget.nil? - - items.each do |item| - app = parse_cleanup_item(item) - Bundle.system(winget, "uninstall", "--id", app.id, "--exact", "--source", app.source, - "--accept-source-agreements", "--disable-interactivity", verbose: false) - end - puts "Uninstalled #{items.size} WinGet package#{"s" if items.size != 1}" + sig { override.params(name: String, executable: Pathname).returns(T::Boolean) } + def uninstall_package!(name, executable: Pathname.new("")) + app = parse_cleanup_item(name) + Bundle.system(executable, "uninstall", "--id", app.id, "--exact", "--source", app.source, + "--accept-source-agreements", "--disable-interactivity", verbose: false) end sig { params(id: String, source: String).returns(T::Boolean) } diff --git a/Library/Homebrew/bundle/subcommand/cleanup.rb b/Library/Homebrew/bundle/subcommand/cleanup.rb index 38b93eb871f3d..c7c308499f538 100644 --- a/Library/Homebrew/bundle/subcommand/cleanup.rb +++ b/Library/Homebrew/bundle/subcommand/cleanup.rb @@ -16,6 +16,8 @@ module Homebrew module Cmd class Bundle < Homebrew::AbstractCommand class CleanupSubcommand < Homebrew::AbstractSubcommand + extend Utils::Output::Mixin + subcommand_args do usage_banner <<~EOS `brew bundle cleanup`: @@ -164,16 +166,24 @@ def self.cleanup(global: false, file: nil, force: false, zap: false, dsl: nil, else [] end - Kernel.system HOMEBREW_BREW_FILE, "uninstall", "--cask", *args, "--force", *casks - puts "Uninstalled #{casks.size} cask#{"s" if casks.size != 1}" + if Kernel.system HOMEBREW_BREW_FILE, "uninstall", "--cask", *args, "--force", *casks + puts "Uninstalled #{Utils.pluralize("cask", casks.size, include_count: true)}" + else + ofail "Failed to uninstall casks. Some may have been removed." + end end if formulae.any? - Kernel.system HOMEBREW_BREW_FILE, "uninstall", "--formula", "--force", *formulae - puts "Uninstalled #{formulae.size} formula#{"e" if formulae.size != 1}" + if Kernel.system HOMEBREW_BREW_FILE, "uninstall", "--formula", "--force", *formulae + puts "Uninstalled #{Utils.pluralize("formula", formulae.size, include_count: true)}" + else + ofail "Failed to uninstall formulae. Some may have been removed." + end end - Kernel.system HOMEBREW_BREW_FILE, "untap", *taps if taps.any? + if taps.any? && !Kernel.system(HOMEBREW_BREW_FILE, "untap", *taps) + ofail "Failed to untap. Some taps may have been removed." + end cleanup_extensions.each do |extension, items| next if items.empty? diff --git a/Library/Homebrew/test/bundle/cargo_spec.rb b/Library/Homebrew/test/bundle/cargo_spec.rb index 1135dff832d4c..902d03f7f050d 100644 --- a/Library/Homebrew/test/bundle/cargo_spec.rb +++ b/Library/Homebrew/test/bundle/cargo_spec.rb @@ -395,6 +395,15 @@ expect(described_class.cleanup_items(entries)).to eql(%w[fd-find bat]) end + it "returns the uninstall command result" do + allow(Homebrew::Bundle).to receive(:system) + .with("/tmp/rust/bin/cargo", "uninstall", "example", verbose: false).and_return(false) + + expect( + described_class.uninstall_package!("example", executable: Pathname("/tmp/rust/bin/cargo")), + ).to be(false) + end + it "returns frozen empty array when cargo is not installed" do allow(described_class).to receive(:package_manager_installed?).and_return(false) entries = [Homebrew::Bundle::Dsl::Entry.new(:cargo, "ripgrep")] diff --git a/Library/Homebrew/test/bundle/flatpak_spec.rb b/Library/Homebrew/test/bundle/flatpak_spec.rb index 78201e20ff7af..b431fc751b82b 100644 --- a/Library/Homebrew/test/bundle/flatpak_spec.rb +++ b/Library/Homebrew/test/bundle/flatpak_spec.rb @@ -6,6 +6,35 @@ require "bundle/extensions/flatpak" RSpec.describe Homebrew::Bundle::Flatpak do + describe "cleanup" do + test_each([false, nil]) do |result| + it "continues after #{result.inspect} with the resolved executable and unchanged environment" do + Homebrew.failed = false + ENV["PATH"] = "/usr/bin:/bin" + allow(described_class).to receive(:package_manager_executable) + .and_return(Pathname("/example/bin/flatpak")) + expect(Kernel).to receive(:system) + .with("/example/bin/flatpak", "uninstall", "-y", "--system", "org.example.App").ordered do + expect(ENV.fetch("PATH")).to eq("/usr/bin:/bin") + result + end + expect(Kernel).to receive(:system) + .with("/example/bin/flatpak", "uninstall", "-y", "--system", "org.example.Second") + .ordered.and_return(true) + + expect do + described_class.cleanup!(%w[org.example.App org.example.Second]) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output( + a_string_including("Failed to uninstall Flatpak packages: org.example.App"), + ).to_stderr) + .and(output("Uninstalled 1 flatpak\n").to_stdout), + ) + end + end + end + describe "checking" do subject(:checker) { described_class.new } diff --git a/Library/Homebrew/test/bundle/go_spec.rb b/Library/Homebrew/test/bundle/go_spec.rb index 3c0f37998b90e..c0828dbb1db6d 100644 --- a/Library/Homebrew/test/bundle/go_spec.rb +++ b/Library/Homebrew/test/bundle/go_spec.rb @@ -134,6 +134,26 @@ .to eql(%w[github.com/golangci/golangci-lint/v2/cmd/golangci-lint]) end + it "reports a missing executable when removals are queued" do + Homebrew.failed = false + allow(described_class).to receive(:package_manager_executable).and_return(nil) + + expect do + described_class.cleanup!(["example.com/tools/example"]) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output(/Failed to remove Go executables: go executable not found/).to_stderr), + ) + end + + it "does not report a missing executable when no removals are queued" do + allow(described_class).to receive(:package_manager_executable).and_return(nil) + + expect do + described_class.cleanup!([]) + end.to(output("").to_stderr.and(output("").to_stdout)) + end + it "removes binaries by their literal names" do binary = "/Users/test/go/bin/cr;ush $(touch pwned)" allow(described_class).to receive(:`).and_raise("Shell execution is not permitted") @@ -148,11 +168,72 @@ allow(Utils).to receive(:popen_read).with(Pathname("go"), "version", "-m", binary, err: File::NULL) .and_return("\tpath\tgithub.com/charmbracelet/crush\n") - expect(FileUtils).to receive(:rm_f).with(binary) + expect(File).to receive(:unlink).with(binary) described_class.cleanup!(["github.com/charmbracelet/crush"]) end + context "with selected executables" do + let(:executables) { %w[/tmp/go/bin/first /tmp/go/bin/second] } + + before do + Homebrew.failed = false + allow(Utils).to receive(:popen_read) + .with(Pathname("go"), "env", "GOBIN", err: :err).and_return("/tmp/go/bin") + allow(Utils).to receive(:popen_read) + .with(Pathname("go"), "env", "GOPATH", err: :err).and_return("/tmp/go") + allow(File).to receive(:directory?).with("/tmp/go/bin").and_return(true) + allow(Dir).to receive(:glob).with("/tmp/go/bin/*").and_return(executables) + executables.each do |executable| + allow(File).to receive(:executable?).with(executable).and_return(true) + allow(File).to receive(:directory?).with(executable).and_return(false) + allow(File).to receive(:symlink?).with(executable).and_return(false) + allow(Utils).to receive(:popen_read) + .with(Pathname("go"), "version", "-m", executable, err: File::NULL) + .and_return("\tpath\texample.com/tools/example\n") + allow(File).to receive(:unlink).with(executable).and_return(1) + end + end + + it "counts removed executables rather than packages" do + expect do + described_class.cleanup!(["example.com/tools/example"]) + end.to output("Removed 2 Go executables\n").to_stdout.and output("").to_stderr + end + + it "does not mark successful cleanup as failed" do + allow(described_class).to receive(:puts) + + expect do + described_class.cleanup!(["example.com/tools/example"]) + end.not_to change(Homebrew, :failed?) + end + + it "reports deletion errors and continues with the next executable" do + expect(File).to receive(:unlink).with(executables.first).ordered.and_raise(Errno::EACCES) + expect(File).to receive(:unlink).with(executables.last).ordered.and_return(1) + + expect do + described_class.cleanup!(["example.com/tools/example"]) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output("Removed 1 Go executable\n").to_stdout) + .and(output( + a_string_including("Failed to remove Go executable /tmp/go/bin/first"), + ).to_stderr), + ) + end + + it "does not count an already absent executable as removed" do + allow(File).to receive(:unlink).with(executables.first).and_raise(Errno::ENOENT) + + expect do + described_class.cleanup!(["example.com/tools/example"]) + end.to output("Removed 1 Go executable\n").to_stdout + .and output("").to_stderr + end + end + it "returns frozen empty array when go is not installed" do allow(described_class).to receive(:package_manager_installed?).and_return(false) entries = [Homebrew::Bundle::Dsl::Entry.new(:go, "github.com/charmbracelet/crush")] diff --git a/Library/Homebrew/test/bundle/krew_spec.rb b/Library/Homebrew/test/bundle/krew_spec.rb index 57bec5b6e228b..766b4638db36e 100644 --- a/Library/Homebrew/test/bundle/krew_spec.rb +++ b/Library/Homebrew/test/bundle/krew_spec.rb @@ -146,6 +146,16 @@ expect(described_class.cleanup_items(entries)).to eql(%w[ns neat]) end + it "returns the uninstall command result" do + allow(Homebrew::Bundle).to receive(:system).with("/usr/local/bin/kubectl-krew", "uninstall", "example", + verbose: false) + .and_return(false) + + expect( + described_class.uninstall_package!("example", executable: Pathname("/usr/local/bin/kubectl-krew")), + ).to be(false) + end + it "uninstalls plugins" do expect(Homebrew::Bundle).to receive(:system) do |*args, verbose:| expect(ENV.fetch("PATH", "")).to start_with("/usr/local/bin:") @@ -154,7 +164,7 @@ true end - expect { described_class.cleanup!(["ns"]) }.to output(/Uninstalled 1 Krew plugin/).to_stdout + expect { described_class.cleanup!(["ns"]) }.to output("Uninstalled 1 krew plugin\n").to_stdout end end end diff --git a/Library/Homebrew/test/bundle/mac_app_store_spec.rb b/Library/Homebrew/test/bundle/mac_app_store_spec.rb index 2823d76965af0..8fc17d327c704 100644 --- a/Library/Homebrew/test/bundle/mac_app_store_spec.rb +++ b/Library/Homebrew/test/bundle/mac_app_store_spec.rb @@ -350,7 +350,28 @@ expect(Homebrew::Bundle).to receive(:system).with(Pathname("mas"), "uninstall", "456", verbose: false) .and_return(true) - expect { described_class.cleanup!(items) }.to output(/Uninstalled 1 Mac App Store app/).to_stdout + expect { described_class.cleanup!(items) }.to output("Uninstalled 1 app\n").to_stdout + end + + it "reports failed removals by readable app name without changing PATH" do + Homebrew.failed = false + ENV["PATH"] = "/usr/bin:/bin" + items = described_class.cleanup_items([Homebrew::Bundle::Dsl::Entry.new(:mas, "foo", id: 123)]) + allow(Homebrew::Bundle).to receive(:system) + .with(Pathname("mas"), "uninstall", "456", verbose: false) do + expect(ENV.fetch("PATH")).to eq("/usr/bin:/bin") + false + end + + expect do + described_class.cleanup!(items) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output( + a_string_including("Failed to uninstall Mac App Store dependencies: bar (456)"), + ).to_stderr) + .and(output("Uninstalled 0 apps\n").to_stdout), + ) end end end diff --git a/Library/Homebrew/test/bundle/npm_spec.rb b/Library/Homebrew/test/bundle/npm_spec.rb index 0acdf3a2e0099..30c9c5d995200 100644 --- a/Library/Homebrew/test/bundle/npm_spec.rb +++ b/Library/Homebrew/test/bundle/npm_spec.rb @@ -218,6 +218,81 @@ end end + describe ".cleanup! inherited from Extension" do + before do + Homebrew.failed = false + allow(described_class).to receive(:package_manager_executable) + .and_return(Pathname("/usr/bin/false")) + end + + it "reports a missing executable when removals are queued" do + allow(described_class).to receive(:package_manager_executable).and_return(nil) + + expect do + described_class.cleanup!(["example"]) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output(/Failed to uninstall npm packages: executable not found/).to_stderr), + ) + end + + it "does not report a missing executable when no removals are queued" do + allow(described_class).to receive(:package_manager_executable).and_return(nil) + + expect do + described_class.cleanup!([]) + end.to(output("").to_stderr.and(output("").to_stdout)) + end + + it "reports a failed uninstall command" do + expect do + described_class.cleanup!(["example"]) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output(/^Uninstalled 0 npm packages$/).to_stdout) + .and(output(/Failed to uninstall npm packages: example/).to_stderr), + ) + end + + it "reports successful cleanup" do + allow(Homebrew::Bundle).to receive(:system).and_return(true) + + expect do + described_class.cleanup!(["example"]) + end.to output("Uninstalled 1 npm package\n").to_stdout.and output("").to_stderr + end + + it "does not mark successful cleanup as failed" do + allow(Homebrew::Bundle).to receive(:system).and_return(true) + allow(described_class).to receive(:puts) + + expect { described_class.cleanup!(["example"]) }.not_to change(Homebrew, :failed?) + end + + it "counts successful commands and continues after failure" do + { "first" => true, "second" => false, "third" => true }.each do |name, success| + expect(Homebrew::Bundle).to receive(:system) + .with("/usr/bin/false", "uninstall", "-g", name, verbose: false).ordered.and_return(success) + end + + expect do + described_class.cleanup!(%w[first second third]) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output("Uninstalled 2 npm packages\n").to_stdout) + .and(output(/Failed to uninstall npm packages: second/).to_stderr), + ) + end + + it "does not clear an earlier failure after successful uninstalls" do + Homebrew.failed = true + allow(Homebrew::Bundle).to receive(:system).and_return(true) + allow(described_class).to receive(:puts) + + expect { described_class.cleanup!(["example"]) }.not_to change(Homebrew, :failed?) + end + end + describe "installing" do context "when npm is not installed" do before do diff --git a/Library/Homebrew/test/bundle/uv_spec.rb b/Library/Homebrew/test/bundle/uv_spec.rb index 121ee4f3c42e4..f0e54eaf842c2 100644 --- a/Library/Homebrew/test/bundle/uv_spec.rb +++ b/Library/Homebrew/test/bundle/uv_spec.rb @@ -593,6 +593,30 @@ expect(described_class.cleanup_items(entries)).to eql(%w[mkdocs black]) end + it "returns the uninstall command result" do + allow(Homebrew::Bundle).to receive(:system) + .with("/tmp/uv/bin/uv", "tool", "uninstall", "example", verbose: false).and_return(false) + + expect( + described_class.uninstall_package!("example", executable: Pathname("/tmp/uv/bin/uv")), + ).to be(false) + end + + it "preserves PATH when uninstalling tools" do + ENV["PATH"] = "/existing/bin" + uninstall_path = "" + allow(Homebrew::Bundle).to receive(:system) + .with("/tmp/uv/bin/uv", "tool", "uninstall", "example", verbose: false) do + uninstall_path = ENV.fetch("PATH") + true + end + allow(described_class).to receive(:puts) + + described_class.cleanup!(["example"]) + + expect(uninstall_path).to eq("/existing/bin") + end + it "returns frozen empty array when uv is not installed" do allow(described_class).to receive(:package_manager_installed?).and_return(false) entries = [Homebrew::Bundle::Dsl::Entry.new(:uv, "ruff")] diff --git a/Library/Homebrew/test/bundle/vscode_extension_spec.rb b/Library/Homebrew/test/bundle/vscode_extension_spec.rb index 34975497a57de..e3ee45468d8a0 100644 --- a/Library/Homebrew/test/bundle/vscode_extension_spec.rb +++ b/Library/Homebrew/test/bundle/vscode_extension_spec.rb @@ -7,6 +7,32 @@ require "extend/kernel" RSpec.describe Homebrew::Bundle::VscodeExtension do + describe "cleanup" do + test_each([false, nil]) do |result| + it "reports #{result.inspect} and continues cleanup" do + Homebrew.failed = false + ENV["PATH"] = "/usr/bin:/bin" + allow(described_class).to receive(:package_manager_executable).and_return(Pathname("code")) + expect(Kernel).to receive(:system) + .with("code", "--uninstall-extension", "publisher.first").ordered do + expect(ENV.fetch("PATH")).to eq("/usr/bin:/bin") + result + end + expect(Kernel).to receive(:system) + .with("code", "--uninstall-extension", "publisher.second") + .ordered.and_return(true) + + expect do + described_class.cleanup!(%w[publisher.first publisher.second]) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output(a_string_including("publisher.first")).to_stderr) + .and(output("Uninstalled 1 vscode extension\n").to_stdout), + ) + end + end + end + describe "dumping" do subject(:dumper) { described_class } diff --git a/Library/Homebrew/test/bundle/winget_spec.rb b/Library/Homebrew/test/bundle/winget_spec.rb index 776cdabe043a2..545a123352ee7 100644 --- a/Library/Homebrew/test/bundle/winget_spec.rb +++ b/Library/Homebrew/test/bundle/winget_spec.rb @@ -368,7 +368,30 @@ "--accept-source-agreements", "--disable-interactivity", verbose: false) .and_return(true) - expect { described_class.cleanup!(items) }.to output(/Uninstalled 1 WinGet package/).to_stdout + expect { described_class.cleanup!(items) }.to output("Uninstalled 1 winget package\n").to_stdout + end + + it "reports failed removals by app name and source without changing PATH" do + Homebrew.failed = false + ENV["PATH"] = "/usr/bin:/bin" + items = described_class.cleanup_items([ + Homebrew::Bundle::Dsl::Entry.new(:winget, "Steam", id: "Valve.Steam"), + ]) + allow(Homebrew::Bundle).to receive(:system) + .with(Pathname("winget.exe"), "uninstall", "--id", "XPDC2RH70K22MN", + "--exact", "--source", "msstore", + "--accept-source-agreements", "--disable-interactivity", verbose: false) do + expect(ENV.fetch("PATH")).to eq("/usr/bin:/bin") + false + end + + expect do + described_class.cleanup!(items) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output(a_string_including("Discord (XPDC2RH70K22MN, msstore)")).to_stderr) + .and(output("Uninstalled 0 winget packages\n").to_stdout), + ) end end end diff --git a/Library/Homebrew/test/cmd/bundle/cleanup_subcommand_spec.rb b/Library/Homebrew/test/cmd/bundle/cleanup_subcommand_spec.rb index 2520d96323f55..8a51d8f7f9312 100644 --- a/Library/Homebrew/test/cmd/bundle/cleanup_subcommand_spec.rb +++ b/Library/Homebrew/test/cmd/bundle/cleanup_subcommand_spec.rb @@ -403,7 +403,8 @@ end it "uninstalls casks" do - expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--cask", "--force", "a", "b") + expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--cask", "--force", "a", + "b").and_return(true) expect(described_class).to receive(:system_output_no_stderr).and_return("") expect do described_class.cleanup(force: true) @@ -428,7 +429,8 @@ end it "uninstalls casks" do - expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--cask", "--zap", "--force", "a", "b") + expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--cask", "--zap", "--force", "a", + "b").and_return(true) expect(described_class).to receive(:system_output_no_stderr).and_return("") expect do described_class.cleanup(force: true, zap: true) @@ -456,7 +458,8 @@ end it "uninstalls formulae" do - expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--formula", "--force", "a", "b") + expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--formula", "--force", "a", + "b").and_return(true) expect(described_class).to receive(:system_output_no_stderr).and_return("") expect do described_class.cleanup(force: true) @@ -483,7 +486,7 @@ end it "untaps taps" do - expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "untap", "a", "b") + expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "untap", "a", "b").and_return(true) expect(described_class).to receive(:system_output_no_stderr).and_return("") described_class.cleanup(force: true) end @@ -507,7 +510,7 @@ end it "uninstalls extensions" do - expect(Kernel).to receive(:system).with("code", "--uninstall-extension", "GitHub.codespaces") + expect(Kernel).to receive(:system).with("code", "--uninstall-extension", "GitHub.codespaces").and_return(true) expect(described_class).to receive(:system_output_no_stderr).and_return("") described_class.cleanup(force: true) end @@ -526,11 +529,13 @@ allow(described_class).to receive_messages(casks_to_uninstall: [], formulae_to_uninstall: [], taps_to_untap: []) allow(Homebrew::Bundle::VscodeExtension).to receive(:cleanup_items).and_return([]) - allow(Homebrew::Bundle::Flatpak).to receive(:cleanup_items).and_return(%w[org.gnome.Calculator]) + allow(Homebrew::Bundle::Flatpak).to receive_messages(cleanup_items: %w[org.gnome.Calculator], + package_manager_executable: Pathname("flatpak")) end it "uninstalls flatpaks" do - expect(Kernel).to receive(:system).with("flatpak", "uninstall", "-y", "--system", "org.gnome.Calculator") + expect(Kernel).to receive(:system).with("flatpak", "uninstall", "-y", "--system", + "org.gnome.Calculator").and_return(true) expect(described_class).to receive(:system_output_no_stderr).and_return("") expect do described_class.cleanup(force: true) @@ -577,12 +582,14 @@ it "cleans up without suggesting --force when it prompts" do allow(Homebrew::Ask).to receive(:confirm?).with(action: "cleanup").and_return(true) allow(Homebrew::Bundle).to receive(:mark_as_installed_on_request!) - allow(Kernel).to receive(:system) + allow(Kernel).to receive(:system).and_return(true) allow(described_class).to receive(:system_output_no_stderr).and_return("") allow(Formatter).to receive(:columns).with(%w[a b]).and_return("a b") - expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--cask", "--force", "a", "b") - expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--formula", "--force", "a", "b") - expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "untap", "a", "b") + expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--cask", "--force", "a", + "b").and_return(true) + expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "uninstall", "--formula", "--force", "a", + "b").and_return(true) + expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "untap", "a", "b").and_return(true) expect(Homebrew::Cleanup).to receive(:dry_run_output).with(quiet: true).and_return("") expect { described_class.cleanup(ask: true) }.not_to output(/Run .brew bundle cleanup --force/).to_stdout @@ -623,6 +630,89 @@ end end + describe "cleanup failure reporting" do + before do + described_class.reset! + Homebrew::Bundle.extensions.each do |extension| + allow(extension).to receive(:cleanup_items).and_return([]) + end + Homebrew.failed = false + allow(Homebrew::Trust).to receive(:replace!) + allow(Homebrew::Bundle).to receive(:mark_as_installed_on_request!) + allow(Homebrew::Bundle::Npm).to receive(:cleanup_items).and_return(["remaining"]) + allow(Homebrew::Bundle::Npm).to receive(:cleanup!) + allow(described_class).to receive_messages(casks_to_uninstall: %w[first second], + formulae_to_uninstall: %w[first second], + taps_to_untap: %w[first/tap second/tap], + system_output_no_stderr: "") + allow(Kernel).to receive(:system).and_return(true) + end + + let(:dsl) { Homebrew::Bundle::Dsl.new(StringIO.new("")) } + + test_each([ + [["uninstall", "--cask", "--force", "first", "second"], "Failed to uninstall casks"], + [["uninstall", "--cask", "--zap", "--force", "first", "second"], "Failed to uninstall casks"], + [["uninstall", "--formula", "--force", "first", "second"], "Failed to uninstall formulae"], + [["untap", "first/tap", "second/tap"], "Failed to untap"], + ].product([false, nil])) do |(arguments, failure_message), result| + it "retains failure when #{arguments.join(" ")} returns #{result.inspect}" do + allow(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, *arguments).and_return(result) + expect(Kernel).to receive(:system) + .with(HOMEBREW_BREW_FILE, "uninstall", "--formula", "--force", "first", "second") + expect(Kernel).to receive(:system).with(HOMEBREW_BREW_FILE, "untap", "first/tap", "second/tap") + allow(described_class).to receive(:puts) + expect(Homebrew::Bundle::Npm).to receive(:cleanup!).with(["remaining"]) + expect(described_class).to receive(:system_output_no_stderr).with(HOMEBREW_BREW_FILE, + "cleanup").and_return("") + + expect do + described_class.cleanup(force: true, zap: arguments.include?("--zap"), dsl:) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output(a_string_including(failure_message, "may have been removed")).to_stderr), + ) + end + end + + it "does not report successful removals when cask and formula commands fail" do + %w[cask formula].each do |type| + allow(Kernel).to receive(:system) + .with(HOMEBREW_BREW_FILE, "uninstall", "--#{type}", "--force", "first", "second") + .and_return(false) + end + + expect do + described_class.cleanup(force: true, dsl:, taps: false) + end.not_to output(/Uninstalled/).to_stdout + end + + it "preserves failure set before cleanup" do + Homebrew.failed = true + allow(described_class).to receive(:puts) + + expect do + described_class.cleanup(force: true, dsl:) + end.not_to change(Homebrew, :failed?) + end + + it "retains an extension failure after successful cache cleanup" do + expect(Homebrew::Bundle::Npm).to receive(:cleanup!).ordered.and_call_original + allow(Homebrew::Bundle::Npm).to receive(:package_manager_executable) + .and_return(Pathname("/usr/bin/false")) + allow(described_class).to receive(:puts) + expect(described_class).to receive(:system_output_no_stderr) + .with(HOMEBREW_BREW_FILE, "cleanup").ordered.and_return("") + + expect do + described_class.cleanup(force: true, dsl:, casks: false, formulae: false, taps: false) + end.to( + change(Homebrew, :failed?).from(false).to(true) + .and(output(/Failed to uninstall npm packages: remaining/).to_stderr), + ) + end + end + describe "#system_output_no_stderr" do it "discards stderr without closing it" do stdout = nil @@ -654,7 +744,7 @@ ) allow(Homebrew::Bundle::VscodeExtension).to receive(:cleanup_items).and_return([]) allow(Homebrew::Bundle::Flatpak).to receive(:cleanup_items).and_return([]) - allow(Kernel).to receive(:system) + allow(Kernel).to receive(:system).and_return(true) allow(described_class).to receive(:system_output_no_stderr).and_return("") allow_any_instance_of(Pathname).to receive(:read).and_return("") end