-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cleanup: remove older versions when reasonable #401
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -466,10 +466,10 @@ def installed_prefixes | |
rack.directory? ? rack.subdirs : [] | ||
end | ||
|
||
# All of current installed kegs. | ||
# All of current installed kegs, sorted by version. | ||
# @private | ||
def installed_kegs | ||
installed_prefixes.map { |dir| Keg.new(dir) } | ||
installed_prefixes.map { |dir| Keg.new(dir) }.sort_by(&:version) | ||
end | ||
|
||
# The directory where the formula's binaries should be installed. | ||
|
@@ -1318,8 +1318,6 @@ def to_hash | |
} | ||
end | ||
|
||
hsh["installed"] = hsh["installed"].sort_by { |i| Version.create(i["version"]) } | ||
|
||
hsh | ||
end | ||
|
||
|
@@ -1526,7 +1524,20 @@ def eligible_kegs_for_cleanup | |
end | ||
end | ||
end | ||
elsif installed_prefixes.any? && !pinned? | ||
elsif installed_prefixes.length > 1 | ||
if linked_keg.exist? | ||
# If the most recent available version is not installed, cleanup | ||
# only those older than the currently-linked one. | ||
linked_version = PkgVersion.parse(linked_keg.resolved_path.basename.to_s) | ||
eligible_for_cleanup = installed_kegs.select { |k| linked_version > k.version } | ||
opoo "Skipping (old) #{linked_keg.resolved_path} due to it being linked" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it OK to not additionally mention any newer (similarly-outdated, unlinked) kegs that are also being skipped here? (I think it's reasonable.) |
||
else | ||
# If no version is linked, keep only the newest one. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this reasonable logic? Should completely unlinked formulas just get cleaned up no matter what? (It would be a LOT more aggressive than the current behavior, though, which feels almost mean to me.) |
||
eligible_for_cleanup = installed_kegs | ||
last_keg = eligible_for_cleanup.pop | ||
opoo "Skipping (old, unlinked) #{last_keg} due to it being the latest version installed" | ||
end | ||
elsif !pinned? | ||
# If the cellar only has one version installed, don't complain | ||
# that we can't tell which one to keep. Don't complain at all if the | ||
# only installed version is a pinned formula. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I just use
Keg.new(...)
(or something) here for maybe-slightly-clearer semantics? Or something else? Or is this fine?