brew icon indicating copy to clipboard operation
brew copied to clipboard

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

Open Bo98 opened this issue 1 year ago • 11 comments

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

Bo98 avatar Sep 19 '24 15:09 Bo98

  • 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.

carlocab avatar Sep 19 '24 15:09 carlocab

  • 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.

[email protected] ended up being a dependency of LLVM anyway so we have already built that depedency tree.

Bo98 avatar Sep 19 '24 15:09 Bo98

  • 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?)

carlocab avatar Sep 19 '24 16:09 carlocab

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 avatar Sep 19 '24 16:09 Bo98

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.

Bo98 avatar Sep 19 '24 16:09 Bo98

  • --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?

MikeMcQuaid avatar Sep 19 '24 16:09 MikeMcQuaid

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

Ah, gotcha, thanks for the explanation.

carlocab avatar Sep 19 '24 16:09 carlocab

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.

Bo98 avatar Sep 19 '24 16:09 Bo98

  • 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.

carlocab avatar Sep 19 '24 17:09 carlocab

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.

Bo98 avatar Sep 19 '24 17:09 Bo98

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.

MikeMcQuaid avatar Sep 20 '24 07:09 MikeMcQuaid

Pretty sure this was done?

MikeMcQuaid avatar Jun 06 '25 07:06 MikeMcQuaid