Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cleanup: allow users to specify formulae to skip cleaning #11931

Merged
merged 1 commit into from
Aug 30, 2021
Merged

cleanup: allow users to specify formulae to skip cleaning #11931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 18 additions & 3 deletions Library/Homebrew/cleanup.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,18 @@ def self.install_formula_clean!(f)
cleanup = Cleanup.new
if cleanup.periodic_clean_due?
cleanup.periodic_clean!
elsif f.latest_version_installed?
elsif f.latest_version_installed? && !cleanup.skip_clean_formula?(f)
cleanup.cleanup_formula(f)
end
end

def skip_clean_formula?(f)
return false if Homebrew::EnvConfig.no_cleanup_formulae.blank?

skip_clean_formulae = Homebrew::EnvConfig.no_cleanup_formulae.split(",")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's worth adding this behaviour into EnvConfig itself under the type? We also seem to alternate between comma and space separated on our configuration; might be worth doubling down on one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually also have an option that takes a path to a file that specifies the list. We should probably only pick one approach, but this will require a deprecation cycle.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or: we support both space-separated and comma-separated in the cases where we can use both (e.g. environment variables), document consistently and only use comma-separated in the cases where only it works (e.g. arguments)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, fine with me. I'll try to look into it.

skip_clean_formulae.include?(f.name) || (skip_clean_formulae & f.aliases).present?
end

def periodic_clean_due?
return false if Homebrew::EnvConfig.no_install_cleanup?

Expand All @@ -186,7 +193,10 @@ def periodic_clean!

def clean!(quiet: false, periodic: false)
if args.empty?
Formula.installed.sort_by(&:name).each do |formula|
Formula.installed
.sort_by(&:name)
.reject { |f| skip_clean_formula?(f) }
.each do |formula|
cleanup_formula(formula, quiet: quiet, ds_store: false, cache_db: false)
end
cleanup_cache
Expand Down Expand Up @@ -223,7 +233,12 @@ def clean!(quiet: false, periodic: false)
nil
end

cleanup_formula(formula) if formula
if formula && skip_clean_formula?(formula)
onoe "Refusing to clean #{formula} because it is listed in " \
"#{Tty.bold}HOMEBREW_NO_CLEANUP_FORMULAE#{Tty.reset}!"
elsif formula
cleanup_formula(formula)
end
cleanup_cask(cask) if cask
end
end
Expand Down
4 changes: 4 additions & 0 deletions Library/Homebrew/env_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ module EnvConfig
"formulae.",
boolean: true,
},
HOMEBREW_NO_CLEANUP_FORMULAE: {
description: "A comma-separated list of formulae. Homebrew will refuse to clean up a " \
"formula if it appears on this list.",
},
HOMEBREW_NO_COLOR: {
description: "If set, do not print text with colour added.",
default_text: "`$NO_COLOR`.",
Expand Down