sync installed_as_dependency when changing installed_on_request#21755
sync installed_as_dependency when changing installed_on_request#21755ooye-sanket wants to merge 2 commits intoHomebrew:mainfrom
Conversation
When marking a formula as not installed on request, also set installed_as_dependency to true, to ensure brew bundle dump correctly excludes it. Previously, brew tab --no-installed-on-request had no effect on bundle dump because the dump logic checks both Fixes Homebrew#21751
|
@ooye-sanket thanks for the PR! I actually think the bigger change we should make is get rid of |
|
Thanks @MikeMcQuaid! I'd love to take on the bigger refactor. Before I start, could you clarify the approach?
|
Yep!
No, think we can skip/drop that too.
Can update this PR! |
…ce of truth Replace all uses of installed_as_dependency with !installed_on_request. Stop storing installed_as_dependency in new Tabs for both formulae and casks. installed_on_request is now the single source of truth. Fixes Homebrew#21751
|
Hi @MikeMcQuaid, I've updated the PR with the full refactor as you suggested! Here's what was done:
Could you confirm if this approach looks correct, or if there's anything I may have missed or handled incorrectly? Happy to make any adjustments! |
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Looks great, nice work @ooye-sanket! One more tiny suggestion.
| statuses = [] | ||
| statuses << "installed on request" if args.installed_on_request? && tab.installed_on_request | ||
| statuses << "installed as dependency" if args.installed_as_dependency? && tab.installed_as_dependency | ||
| statuses << "installed as dependency" if args.installed_as_dependency? && !tab.installed_on_request |
There was a problem hiding this comment.
Could add a commented-out odeprecated for the args.installed_as_dependency? so we can deprecate it in a future release.
When marking a formula as not installed on request, also set
installed_as_dependencytotrue, to ensurebrew bundle dumpcorrectly excludes it.
Previously,
brew tab --no-installed-on-requesthad no effect onbrew bundle dumpbecause the dump logic inbrew.rbchecks both fields:Setting
installed_on_requesttofalsealone was not enough ifinstalled_as_dependencywas alsofalse— the second condition wouldstill evaluate to
true, causing the formula to be included in the dump.Fixes #21751
@MikeMcQuaid can you please review it!!
brew lgtm(style, typechecking and tests) with your changes locally?