-
-
Notifications
You must be signed in to change notification settings - Fork 10k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
cmd/info: only display keg info if tap matches #19295
Conversation
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.
Seems fine though do we want to handle the case where a tab tap is nil
any differently?
Thank you, this resolves the core bug. I am curious on the overall expected use of homebrew with taps, however. The recent trajectory of homebrew has been to promote more third party taps and less in core. (Removing build options and whatnot such that homebrew core is the 80% happy path, and pushing customization into taps.) However, the overall use of homebrew does not make taps a "friendly" experience. To wit:
Will not give desired results. While this PR fixes the core bug (brew-info providing false information), it doesn't improve the overall cohesive use of brew with the ever-growing-presence of 3rd party taps. |
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.
I think this should probably be reverted. We don't have many(/any?) other commands where we require fully scoping formulae from taps like this. Instead it feels like we should use the actual formula from the tap if the opt-linked keg is from that tap.
Thoughts?
@MikeMcQuaid I think agree. And that would also improve the cohesiveness issue I commented above. If I have (tap) formula X installed, and I say The downside, then, is that |
FYI We're seeing this change break some of our internal tooling that uses We should be able to work around it, but I suspect this is a breaking change that may affect many others. |
@jrobotham-square I'm curious how your scripts have been detecting whether the tool that's installed is from core or your tap? (That was the bug this originally attempted to address.) |
For scripts you should be using A possible solution here is to:
I think that covers all cases? |
Short answer - I don't think our internal tooling was actually doing this properly and we've probably just been lucky it hasn't bitten us earlier. (it's some tooling our team has inherited)
Yeah - makes sense - being coupled to the FWIW Our script currently has a mix of fully qualified references and short names. I think we've been relying on this bug for it to work up until now. |
This seems ideal for now. Reverting this until then in #19324 until then.
I disagree with this statement.
The only real "fix" here is something we've recommended for a long time but should consider adding more warnings and/or documentation for: using the same names as homebrew/core formulae in taps is simply a bad idea with a lot of rough edges, some of which we can improve, some we cannot. |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?Fixes #19294.