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.

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

Enable HOMEBREW_AUTOREMOVE by default #17261

Merged
merged 1 commit into from
May 9, 2024
Merged

Enable HOMEBREW_AUTOREMOVE by default #17261

merged 1 commit into from
May 9, 2024

Conversation

MikeMcQuaid
Copy link
Member

Enabled HOMEBREW_AUTOREMOVE by default, and added HOMEBREW_NO_AUTOREMOVE to disable it.

I've been running this for years and it's been rock-solid. An incoming minor release feels like a good time to change this default and document it.

@MikeMcQuaid MikeMcQuaid requested a review from a team May 9, 2024 10:23
Library/Homebrew/env_config.rb Outdated Show resolved Hide resolved
Enabled `HOMEBREW_AUTOREMOVE` by default, and added
`HOMEBREW_NO_AUTOREMOVE` to disable it.

Co-authored-by: Ruoyu Zhong <[email protected]>
@MikeMcQuaid MikeMcQuaid changed the title Enable HOMEBREW_AUTOREMOVE by autoremove_default Enable HOMEBREW_AUTOREMOVE by default May 9, 2024
@p-linnane p-linnane merged commit 1497a73 into master May 9, 2024
26 checks passed
@p-linnane p-linnane deleted the autoremove_default branch May 9, 2024 16:57
@apainintheneck
Copy link
Contributor

I forgot about this one though I've had it set for years now too. I'm happy that it's finally the default.

@jacktose
Copy link
Contributor

jacktose commented May 29, 2024

Whoa! That removed all my build dependencies! I don't want to have to re-build all of those every time I upgrade a built formula. Is there an option for the autoremove logic to leave formulae that were installed as build dependencies?

E.g.:

$ brew autoremove -n
==> Would autoremove 1 unneeded formula:
sqlite
$ brew uses --installed sqlite
$ brew uses --installed sqlite --include-build
util-linux
$ brew info util-linux
==> util-linux: stable 2.40.1
...
Installed
/data/shared/lusers/enneking/homebrew/Cellar/util-linux/2.39.3 (412 files, 24.4MB)
  Built from source on 2023-12-18 at 16:36:02

@Bo98
Copy link
Member

Bo98 commented May 29, 2024

Hmm yeah that's probably a fair point for the non-default-prefix and/or unsupported OS case. Maybe worth disabling by default on those environments.

@carlocab
Copy link
Member

Yea, we already skip attempting to reinstall outdated dependents (as long as linkage isn't broken) if it will result in a source build.

@carlocab
Copy link
Member

Whoa! That removed all my build dependencies! I don't want to have to re-build all of those every time I upgrade a built formula. Is there an option for the autoremove logic to leave formulae that were installed as build dependencies?

Not yet, but you can export HOMEBREW_NO_AUTOREMOVE=1 for now.

@ZhongRuoyu
Copy link
Member

Is there an option for the autoremove logic to leave formulae that were installed as build dependencies?

I have an in-progress work that allows you to explicitly mark/unmark formulae as installed on request, as discussed in #17125 (comment). When a formula is installed on request, it does not get removed by brew autoremove.

My Mac is with Genius Bar right now. I'll make a PR when I get it back.

@apainintheneck
Copy link
Contributor

Whoa! That removed all my build dependencies! I don't want to have to re-build all of those every time I upgrade a built formula. Is there an option for the autoremove logic to leave formulae that were installed as build dependencies?

Maybe we could just exclude all build dependencies used by formula that were built-from-source?

@MikeMcQuaid
Copy link
Member Author

Maybe we could just exclude all build dependencies used by formula that were built-from-source?

This seems like the right thing to do 👍🏻

@jacktose
Copy link
Contributor

Maybe we could just exclude all build dependencies used by formula that were built-from-source?

That's exactly what I'd hope would happen.

@apainintheneck
Copy link
Contributor

Ah, I'm an idiot. I should have checked the existing logic before commenting. It looks like it already accounts for build dependencies when checking what's autoremovable. Maybe there's a bug in the current logic.

# An array of all installed {Formula} without runtime {Formula}
# dependents for bottles and without build {Formula} dependents
# for those built from source.
# @private
sig { params(formulae: T::Array[Formula]).returns(T::Array[Formula]) }
def formulae_with_no_formula_dependents(formulae)
dependents = T.let([], T::Array[Formula])
formulae.each do |formula|
dependents += formula.runtime_formula_dependencies
# Ignore build dependencies when the formula is a bottle
next if formula.any_installed_keg&.tab&.poured_from_bottle
formula.deps.select(&:build?).each do |dep|
dependents << dep.to_formula
rescue FormulaUnavailableError
# do nothing
end
end
formulae - dependents
end

@jacktose
Copy link
Contributor

jacktose commented Jun 1, 2024

I wish I'd saved the output when it unexpectedly removed like 15 deps. But my sqlite example above shows there's still something amiss.

@apainintheneck
Copy link
Contributor

Yeah, I agree. It might be worth making a new issue about it while we investigate.

@MikeMcQuaid
Copy link
Member Author

Yes, please open issue(s) here rather than talking on a merged PR, thanks.

@jacktose
Copy link
Contributor

jacktose commented Jun 5, 2024

Opened #17433.

@github-actions github-actions bot added the outdated PR was locked due to age label Jul 6, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants