Skip to content

Commit

Permalink
cleanup: improve cleanup version detection.
Browse files Browse the repository at this point in the history
We're using `formula.eligible_kegs_for_cleanup` to figure out which
formula should be kept or removed based on e.g. `brew pin` status but
we didn't use this sufficiently in `brew cleanup` to avoid cleaning up
all cached files related to a pinned keg.

Instead, let's use a (cached) call to
`formula.eligible_kegs_for_cleanup` to ensure that we check all
related resources, manifests, etc. for pinned bottles rather than just
the latest version.
  • Loading branch information
MikeMcQuaid committed Jan 23, 2025
1 parent a3b0f9e commit fa8ada3
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
16 changes: 16 additions & 0 deletions Library/Homebrew/cleanup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ def stale_api_source?(pathname, scrub)
package.tap_git_head != git_head
end

sig { params(formula: Formula).returns(T::Set[String]) }
def excluded_versions_from_cleanup(formula)
@excluded_versions_from_cleanup ||= {}
@excluded_versions_from_cleanup[formula.name] ||= begin
eligible_kegs_for_cleanup = formula.eligible_kegs_for_cleanup(quiet: true)
Set.new((formula.installed_kegs - eligible_kegs_for_cleanup).map { |keg| keg.version.to_s })
end
end

sig { params(pathname: Pathname, scrub: T::Boolean).returns(T::Boolean) }
def stale_formula?(pathname, scrub)
return false unless HOMEBREW_CELLAR.directory?
Expand Down Expand Up @@ -131,6 +140,7 @@ def stale_formula?(pathname, scrub)
nil
end

formula_excluded_versions_from_cleanup = nil
if formula.blank? && formula_name.delete_suffix!("_bottle_manifest")
formula = begin
Formulary.from_rack(HOMEBREW_CELLAR/formula_name)
Expand All @@ -140,6 +150,9 @@ def stale_formula?(pathname, scrub)

return false if formula.blank?

formula_excluded_versions_from_cleanup = excluded_versions_from_cleanup(formula)

Check warning on line 153 in Library/Homebrew/cleanup.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cleanup.rb#L153

Added line #L153 was not covered by tests
return false if formula_excluded_versions_from_cleanup.include?(version.to_s)

# We can't determine an installed rebuild and parsing manifest version cannot be reliably done.
return false unless formula.latest_version_installed?

Expand All @@ -158,6 +171,9 @@ def stale_formula?(pathname, scrub)
return true unless patch_hashes&.include?(Checksum.new(version.to_s))
elsif resource_name && stable && (resource_version = stable.resources[resource_name]&.version)
return true if resource_version != version
elsif (formula_excluded_versions_from_cleanup ||= excluded_versions_from_cleanup(formula).presence) &&
formula_excluded_versions_from_cleanup.include?(version.to_s)
return false
elsif (formula.latest_version_installed? && formula.pkg_version.to_s != version) ||
formula.pkg_version.to_s > version
return true
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/test/cleanup_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,8 @@
FileUtils.touch testball
FileUtils.touch testball_resource
(HOMEBREW_CELLAR/"testball"/"0.0.1").mkpath
# Create the latest version of testball so the older version is eligible for cleanup.
(HOMEBREW_CELLAR/"testball"/"0.1/bin").mkpath
FileUtils.touch(CoreTap.instance.new_formula_path("testball"))
end

Expand Down

0 comments on commit fa8ada3

Please # to comment.