-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Add compatibility_version
DSL to Formula class
#20804
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
Open
Copilot
wants to merge
4
commits into
main
Choose a base branch
from
copilot/fix-e80a41bf-7101-4535-a922-d62d3d8e7d9a
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+93
−18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add compatibility_version DSL method to Formula class (similar to revision and version_scheme) - Initialize @compatibility_version instance variable in Formula#initialize - Include compatibility_version in Formula#to_hash for serialization - Store compatibility_version in Tab runtime dependencies and source versions - Update Dependency#installed? to check compatibility_version for dependency satisfaction - Add tests for compatibility_version DSL in formula and tab specs Co-authored-by: MikeMcQuaid <[email protected]>
compatibility_version
/api_version
/abi_version
DSLcompatibility_version
DSL to Formula class
Copilot finished work on behalf of
MikeMcQuaid
October 2, 2025 16:23
MikeMcQuaid
reviewed
Oct 7, 2025
Add attr_reader for compatibility_version to allow accessing it via formula.compatibility_version instead of formula.class.compatibility_version. Updated all usages to use the cleaner instance method consistently. Co-authored-by: MikeMcQuaid <[email protected]>
@copilot fix this CI failure:
|
Add missing minimum_compatibility_version keyword argument to UsesFromMacOSDependency#installed? to match the base Dependency class signature. Co-authored-by: MikeMcQuaid <[email protected]>
... Fixed in commit d4a8c2e. Updated |
Copilot stopped work on behalf of
MikeMcQuaid due to an error
October 8, 2025 08:06
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
compatibility_version
DSL method to Formula class (similar torevision
andversion_scheme
)@compatibility_version
instance variable in Formula#initializeformula_to_dep_hash
in Tab class to includecompatibility_version
in runtime dependenciescompatibility_version
in source versionsDependency#installed?
to checkcompatibility_version
for dependency satisfactioncompatibility_version
DSL in formula specscompatibility_version
in Tab specscompatibility_version
in serializationformula.compatibility_version
helper method for cleaner access (instead offormula.class.compatibility_version
)UsesFromMacOSDependency#installed?
signature to includeminimum_compatibility_version
parameterOriginal prompt
This section details on the original issue you should resolve
<issue_title>Implement a
compatibility_version
/api_version
/abi_version
DSL</issue_title><issue_description>### Verification
brew install wget
. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.Provide a detailed description of the proposed feature
Formulae should have an (optional?)
compatibility_version
field. Alternative suggested names for this field areapi_version
orabi_version
.The
compatibility_version
field should capture suitability for use as a dependency. In particular, if a formula has acompatibility_version
of1
, then it need not be upgraded when installing or upgrading dependencies as long as dependencies are known to work with versions of the formula that havecompatibility_version 1
.A crucial part of this feature involves including CI testing so that:
compatibility_version
is bumped whenever neededcompatibility_version
is not bumped when a bump is not neededTo do the above, we would likely need to run the following dependent tests (or something similar to them):
If
compatibility_version
is bumped, we need to check that each dependent is broken when using the previouscompatibility_version
, but is not broken under the newercompatibility_version
.If
compatibility_version
is not bumped, then we need to check that each dependent still works when using the oldest bottles that have the samecompatibility_version
.What is the motivation for the feature?
This should help minimise the number of formulae that
brew {install,upgrade} foo
needs to upgrade to ensure that user installs are not broken.How will the feature be relevant to at least 90% of Homebrew users?
This should help address a pain point that many users have expressed many times over the years.
What alternatives to the feature have been considered?
Doing nothing.</issue_description>
Comments on the Issue (you are @copilot in this section)
@MikeMcQuaid Thanks for write-up @carlocab, this is great! Thoughts:Suggest "optional while we're rolling it out" and then "mandatory for anything that has a dependency on anything else" (i.e. if something has no dependancies: not required, if a dependency is added later: CI fails on the dependent add until it's added to the dependency).
For an initial value for this field: we could set it to the
.dylib
/.so
"major" version for libraries and/or the major version for things that seem at least vaguely semver.I think this logic is essentially the same way we detect "
revision
bumps" today: we download the bottle, we attemptbrew linkage
andbrew test
on that bottle and, if they both pass, no bump is needed.The tricky bit is deciding if some dependencies need a bump and others don't (and also the cases where the linkage or test has bitrot into failing). Perhaps it should be something like >=50% of total dependencies needed bumped?
This will be trickier. Might be a bit of a manual/human process here or
brew test-bot
outputs something at an earlier stage that ensures that e.g. a label is set in the PR and a bump is allowed?Will want to make sure we socialise (very) hard that
compatibility_version
should NEVER be bumped except whenbrew test-bot
says that it's needed.Note: this should be stored in the
INSTALL_RECEIPT.json
.</comment_new><comment_new>@carlocab
Somewhat. The tricky part is the following. Suppose formula
foo
has versions1.0
,1.1
, and1.2
, and all those versions havecompatibility_version 1
. In our current CI setup, when we bumpfoo
to version1.3
but keepcompatibility_version 1
, we test the dependents offoo
against version1.3
. However, I think we need to test dependents at least against version1.0
as well.The ...
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.