From ae110cab717565b3fa4e5697401bc8c0b4dd5e66 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Mon, 2 Feb 2026 18:48:40 -0800 Subject: [PATCH] Don't rescue Errno::ENOTEMPTY in recursive_rmdir --- .../cask/artifact/abstract_uninstall.rb | 2 +- Library/Homebrew/cask/utils.rb | 2 +- Library/Homebrew/cleanup.rb | 2 +- Library/Homebrew/test/cleanup_spec.rb | 53 +++++++++++++++---- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb index d870effb55..7082c657a1 100644 --- a/Library/Homebrew/cask/artifact/abstract_uninstall.rb +++ b/Library/Homebrew/cask/artifact/abstract_uninstall.rb @@ -582,7 +582,7 @@ module Cask begin Utils.gain_permissions_rmdir(resolved_path, command:) - rescue Errno::ENOTEMPTY + rescue Errno::ENOTEMPTY, ErrorDuringExecution next false end diff --git a/Library/Homebrew/cask/utils.rb b/Library/Homebrew/cask/utils.rb index 0f09dec08b..52f4f3064a 100644 --- a/Library/Homebrew/cask/utils.rb +++ b/Library/Homebrew/cask/utils.rb @@ -104,7 +104,7 @@ module Cask # before using `sudo` and `chown`. ohai "Using sudo to gain ownership of path '#{path}'" command.run("chown", - args: command_args + ["--", User.current, path], + args: command_args + ["--", User.current.to_s, path], sudo: true) tried_ownership = true # retry chflags/chmod after chown diff --git a/Library/Homebrew/cleanup.rb b/Library/Homebrew/cleanup.rb index 4c66a32ffe..ef762795d3 100644 --- a/Library/Homebrew/cleanup.rb +++ b/Library/Homebrew/cleanup.rb @@ -169,7 +169,7 @@ module Homebrew resource_version = bottle.resource.version return false unless resource_version - return resource_version != GitHubPackages.version_rebuild(resource_version, bottle.rebuild) + return version != GitHubPackages.version_rebuild(resource_version, bottle.rebuild) end return false if formula.blank? diff --git a/Library/Homebrew/test/cleanup_spec.rb b/Library/Homebrew/test/cleanup_spec.rb index 5e36b1dbba..573f67a1ec 100644 --- a/Library/Homebrew/test/cleanup_spec.rb +++ b/Library/Homebrew/test/cleanup_spec.rb @@ -445,20 +445,51 @@ RSpec.describe Homebrew::Cleanup do end end - it "does not remove bottle manifests for the latest installed version when using GitHub Packages" do - manifest = HOMEBREW_CACHE/"foo_bottle_manifest--1.0.json" - FileUtils.touch manifest + context "when the cache path is a bottle manifest file" do + let(:bottle_manifest_path) { (HOMEBREW_CACHE/"testball_bottle_manifest--1.0.bottle_manifest.json") } - resource = instance_double(Resource, version: Version.new("1.0.arm64_sonoma")) - bottle = instance_double(Bottle, resource:, rebuild: 0) - formula = instance_double(Formula, latest_version_installed?: true, bottle:) + before do + HOMEBREW_CACHE.mkpath + FileUtils.touch bottle_manifest_path + (HOMEBREW_CELLAR/"testball"/"0.1/bin").mkpath + FileUtils.touch(CoreTap.instance.new_formula_path("testball")) + end - allow(Formulary).to receive(:from_rack).and_return(nil, formula) - allow(described_class).to receive(:excluded_versions_from_cleanup).with(formula).and_return([]) + it "does not remove the file when bottle resource version is nil" do + allow(Formulary).to receive(:from_rack).with(HOMEBREW_CELLAR/"testball_bottle_manifest").and_return(nil) + allow(Formulary).to receive(:from_rack).and_call_original + allow(Formulary).to receive(:from_rack).with(HOMEBREW_CELLAR/"testball").and_wrap_original do |m, *args| + formula = m.call(*args) + if formula + bottle_nil_version = instance_double(Bottle, + resource: instance_double(Resource, version: nil), + rebuild: 0) + allow(formula).to receive(:bottle).and_return(bottle_nil_version) + end + formula + end + cleanup.cleanup_cache([{ path: bottle_manifest_path, type: nil }]) + expect(bottle_manifest_path).to exist + end - cleanup.cleanup_cache - - expect(manifest).to exist + it "removes the file when path version differs from bottle version_rebuild" do + pathname_mismatch = (HOMEBREW_CACHE/"testball_bottle_manifest--2.0.bottle_manifest.json") + FileUtils.touch pathname_mismatch + allow(Formulary).to receive(:from_rack).with(HOMEBREW_CELLAR/"testball_bottle_manifest").and_return(nil) + allow(Formulary).to receive(:from_rack).and_call_original + allow(Formulary).to receive(:from_rack).with(HOMEBREW_CELLAR/"testball").and_wrap_original do |m, *args| + formula = m.call(*args) + if formula + bottle_double = instance_double(Bottle, + resource: instance_double(Resource, version: Version.new("1.0")), + rebuild: 0) + allow(formula).to receive(:bottle).and_return(bottle_double) + end + formula + end + cleanup.cleanup_cache([{ path: pathname_mismatch, type: nil }]) + expect(pathname_mismatch).not_to exist + end end end