-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
formula_auditor: skip non-core compatibility bumps check #21184
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
base: main
Are you sure you want to change the base?
Conversation
MikeMcQuaid
left a comment
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'm not sure this is the right thing.
The intention of compatibility_version is that we can rely on that rather than versions/revisions to detect whether we need to upgrade a dependency. We used to assume "we need the newest version of every recursive dependency always". We then went "we need at least the version declared in the bottle manifest". Compatability version allows us to say "we need at least this compatibility version" which means we can avoid upgrading dependencies unnecessarily when their ABI or API doesn't change.
This check should instead do something like check if things are bottled. It doesn't make sense when building from source but does make sense if you're installing bottles.
I kind of get it conceptually, but I don't see Should I be changing the |
|
@scpeters Apologies for delay and confusion. Does
No. This shouldn't be needed unless one of these require a rebuild in the other. |
|
MikeMcQuaid
left a comment
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.
The behaviour you describe is expected but I agree it doesn't make sense for your use-case, at least yet. Let's just merge this and can reconsider when we're further along with this in homebrew/core. Thanks for explaining @scpeters!
|
@scpeters Can you fix up the failing test and then will merge? Thanks! |
brew lgtm(style, typechecking and tests) with your changes locally?The audit added in #20936 doesn't always make sense for formulae in 3rd-party taps, which may need to be revision bumped in order to rebuild bottles after a core formula changes. When minor or major upgrades are made in homebrew-core, we typically recursively remove bottles of dependents in our 3rd-party tap; for example see the tracking issue osrf/homebrew-simulation#3270 and dependent bottle removal PR osrf/homebrew-simulation#3271 opened in response to the libwebsockets update to 4.5.0 in Homebrew/homebrew-core#256449. After quickly removing the broken bottles, we open a separate pull request with revision bumps in order to rebuild bottles, such as osrf/homebrew-simulation#3272, which currently fails the compatibility bumps check.
I read the feature request for
compatibility_versionin #19202, but I'm not sure why it should be added to a pull request like osrf/homebrew-simulation#3272. I opened this PR to disable the check for non-core formulae, which would resolve my issue, but I am open to other suggestions.