Setup `brew determine-test-runners` for macOS 15 x86_64

Bo98 commented 1 month ago

Verification

[X] This issue's title and/or description do not reference a single formula e.g. 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

We're ready to enable macOS 15 x86_64 for PRs but the setup is a little more complex.

We need brew determine-test-runners to add macOS 15 x86_64 to the matrix if any of the following are true:

The package is pkg-config or pkgconf
formula.class.pour_bottle_only_if == :clt_installed
Any recursive dependent of the tested formula satisfies dependent_formula.class.pour_bottle_only_if == :clt_installed
--all-supported is passed

The last condition may require making --eval-all mandatory.

What is the motivation for the feature?

In order to have proper testing of macOS 15 x86_64 and not require manual post-merge building.

It is also blocking https://github.com/Homebrew/homebrew-core/pull/190981.

How will the feature be relevant to at least 90% of Homebrew users?

They will get upgraded to compatible bottles rather than temporarily get incompatible bottles that break their workflow.

What alternatives to the feature have been considered?

None

carlocab commented 1 month ago

Any recursive dependent of the tested formula satisfies dependent_formula.class.pour_bottle_only_if == :clt_installed

This currently includes all Python dependents, no? Though I suppose we want to and should fix that.

Bo98 commented 1 month ago

Any recursive dependent of the tested formula satisfies dependent_formula.class.pour_bottle_only_if == :clt_installed

This currently includes all Python dependents, no? Though I suppose we want to and should fix that.

All Python dependencies.

python@3.12 ended up being a dependency of LLVM anyway so we have already built that depedency tree.

carlocab commented 1 month ago

Any recursive dependent of the tested formula satisfies dependent_formula.class.pour_bottle_only_if == :clt_installed

This currently includes all Python dependents, no? Though I suppose we want to and should fix that.

All Python dependencies.

So, should the original post instead say

Any recursive dependency of the tested formula satisfies dependent_formula.class.pour_bottle_only_if == :clt_installed

? (Or am I confused?)

Bo98 commented 1 month ago

If you are building e.g. mpdecimal, a Python dependency, you need to check dependents to find Python.

(btw I've also added a note about --all-supported to the original issue as I forgot about that case)

Bo98 commented 1 month ago

Alternatively: you could find all formula matching pour_bottle_only_if, calculate their dependency trees, union into one and then check for intersections between that list and the tested formulae.

Might be computationally cheaper given you don't need to check every dependency tree though haven't tried it out.

MikeMcQuaid commented 1 month ago

--all-supported is passed

What is this for/to do?

Any recursive dependent of the tested formula satisfies dependent_formula.class.pour_bottle_only_if == :clt_installed

Could this be non-recursive?

Feels like adding a new DSL for this might be nicer and a bit less involved?

carlocab commented 1 month ago

If you are building e.g. mpdecimal, a Python dependency, you need to check dependents to find Python.

Ah, gotcha, thanks for the explanation.

Bo98 commented 1 month ago

What is this for/to do?

Cache workflow + brew doctor workflow here.

Could this be non-recursive?

No, we build full dependency trees. For the affected formulae this isn't that big if we maybe flip it and check the dependency trees rather than constructing dependent trees.

Feels like adding a new DSL for this might be nicer and a bit less involved?

The DSL is effectively there for most cases except the pkg-config exception. We just need it to cover the full dep tree.

carlocab commented 1 month ago

Any recursive dependent of the tested formula satisfies dependent_formula.class.pour_bottle_only_if == :clt_installed

This is pretty straightforward to do now with TestRunnerFormula#dependents.

Though the implementation of that is really inefficient -- it walks Formula.all for each runner and tested formula. This is part of why it's really slow, but the slowness didn't matter as much previously (since it could run concurrently with formula builds).

Ideally it would construct the dependency tree once for each runner and then query that for each tested formula instead.

Bo98 commented 1 month ago

We can probably walk through it once to fetch the pour_bottle formulae (which we can likely assume isn't OS-specific) and then only construct the dependency trees for those formulae.

MikeMcQuaid commented 1 month ago

What is this for/to do?

Cache workflow + brew doctor workflow here.

Ok, gotcha.

For the affected formulae this isn't that big if we maybe flip it and check the dependency trees rather than constructing dependent trees.

This seems nicer.

The DSL is effectively there for most cases except the pkg-config exception. We just need it to cover the full dep tree.

I think it's kind of there but very unintuitive to folks without a deep understanding of Homebrew and/or macOS SDKs.

I think a new DSL with audits or cops to enforce them based on the cases we already know would be nice rather than an implied DSL like we have today.

Homebrew / brew