autoremove: ignore build deps when built from src #13839
Conversation
Now the build dependencies of formula that were installed from source will not be removed by `brew autoremove`. This is especially helpful for those who've installed brew in an alternative prefix.
|
Review period will end on 2022-09-13 at 00:00:00 UTC. |
BrewTestBot
added
the
waiting for feedback
Library/Homebrew/formula.rb
Outdated
| # An array of all installed {Formula} without {Formula} dependents | ||
| # An array of all installed {Formula} without runtime {Formula} | ||
| # dependents for bottles and without build {Formula} dependents | ||
| # for those built from source. | ||
| # @private | ||
| def self.formulae_with_no_formula_dependents(formulae) |
There was a problem hiding this comment.
Is this method still useful/used after being removed from leave?
There was a problem hiding this comment.
Nope, it might be worth creating another file for all of these methods. They are tangentially related to Formula but now they're getting very specific.
There was a problem hiding this comment.
I don't think it makes much sense to merge them into Cleanup though.
There was a problem hiding this comment.
I ended up moving that logic into Utils::Autoremove.
There was a problem hiding this comment.
For my attempt at this I got very confused about these functions. They generally consider runtime & cask dependencies, but are named like no_dependents. So making an alternate version that also considered build dependencies would end up looking like no_dependents_including_build_dependents. Then you could think of recommended etc. dependencies and need 6 versions including no_dependents_at_all_i_mean_it. 😵
Anyway, I'm glad you figured out the logic and a saner organization of it to boot. 🍻
Library/Homebrew/formula.rb
Outdated
| # @private | ||
| def self.formulae_with_no_formula_dependents(formulae) | ||
| return [] if formulae.blank? | ||
|
|
||
| formulae - formulae.flat_map(&:runtime_formula_dependencies) | ||
| formulae - formulae.each_with_object([]) do |formula, dependents| | ||
| dependents.concat(formula.runtime_formula_dependencies) |
There was a problem hiding this comment.
| dependents.concat(formula.runtime_formula_dependencies) | |
| dependents += formula.runtime_formula_dependencies |
There was a problem hiding this comment.
Is this just a style thing? From what I understand Array#concat is in-place while Array#+= always creates a new array. It probably doesn't make a big difference either way but I'm just interested in why. Happy to change it though.
Library/Homebrew/formula.rb
Outdated
|
|
||
| # Include build dependencies when the formula is not a bottle | ||
| unless Tab.for_keg(formula.any_installed_keg).poured_from_bottle | ||
| dependents.concat(formula.deps.select(&:build?).map(&:to_formula)) |
There was a problem hiding this comment.
| dependents.concat(formula.deps.select(&:build?).map(&:to_formula)) | |
| dependents += formula.deps.select(&:build?).map(&:to_formula) |
BrewTestBot
removed
the
waiting for feedback
|
Review period ended. |
|
Great work, thanks again @apainintheneck! |
apainintheneck
deleted the
no-autoremove-build-deps-when-built-from-src
branch
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?Alternate approach to #13831.
Now the build dependencies of formula that were installed from source will not be removed by
brew autoremove. This is especially helpful for those who've installed brew in an alternative prefix.The problem that could come up is that if the formula is out of date the original build dependencies will be different from those in the current formula file but this should work well in most cases. With casks we store the original cask file but we don't seem to do the same thing with formulae.