Make python@3.9 the main formula

[fxcoudert]

Taking over #8977 (maybe)

[Rylan12]

The nicotine-plus issue should be fixed in Homebrew/homebrew-core#63397

The gnuradio and gr-osmosdr issues should be solved by Homebrew/homebrew-core#63345

[fxcoudert]

• This will not be fixed, because it depends on Make Python 3.9 our main Python version #8977 which depends on this one

python@3.8:
  * Versioned formulae in homebrew/core should use `keg_only :versioned_formula`

• The others have pull requests, but the gnuradio one will not be merged easily, more debugging is required
• Linux test was cancelled, because macOS failed (I suppose, it does not say)

Yet, I believe merging this is necessary to move forward and having python@3.9 as the main formula as soon as possible is good because it minimises version mismatch (see for example Homebrew/homebrew-core#63359)

@MikeMcQuaid what is your opinion? I know you don't like merging things that aren't green, but in this case there is a chicken-and-egg problem

[MikeMcQuaid]

I know you don't like merging things that aren't green

It's not that I don't like to but it's that we can't do that for Homebrew/brew CI because it will literally break every future PR. There's nothing in the CI that's checking Python specifically here. If this is merged: every Homebrew/brew (and, as it's brew audit --skip-style, Homebrew/homebrew-core) PR will fail.

• Linux test was cancelled, because macOS failed (I suppose, it does not say)

Yes.

• This will not be fixed, because it depends on Make Python 3.9 our main Python version #8977 which depends on this one

That PR was an exact dupe of this one? Do you mean another one?

[fxcoudert]

Sorry, I meant: this PR blocks Homebrew/homebrew-core#63346, and Homebrew/homebrew-core#63346 blocks this PR

[fxcoudert]

The issue comes from the fact that we're storing information about the formula in two different places (the formula itself, and the allow-list in brew), on two different repositories — and they have to be kept in sync.

If the audit was not blocking, and was "informative" only, then we could proceed directly: the maintainer reads the audit warning, makes an informed decision that it's OK to proceed. Instead, we lose maintainer time to keep things in sync. And during this, we can't even run CI on the main PR (Homebrew/homebrew-core#63346) to actually test the content of the PR, because the tap_syntax prevents the other tests from running. We're shooting ourselves in the foot.

[fxcoudert]

• gnuradio is fixed
• making this pass by making both 3.8 and 3.9 allowed

[Rylan12]

I think this makes sense. The plan is to:

1. Merge this allowing 3.9 to be non-keg-only
2. Merge Make python@3.9 the main python homebrew-core#63346 making 3.9 non-keg-only and 3.8 keg-only
3. Follow up PR here removing 3.8 from this list

Right?

[fxcoudert]

@Rylan12 yes. As described above, I wish it were simpler, and did not require three successive PRs to do one thing.

[Rylan12]

I wonder, can the Point Of Truth for which Python formula is the main one come from Homebrew/core?

I'm thinking something like this:

• Have brew audit mandate that only one python formula can be non-keg-only
• Have brew audit mandate the the python and python@3 aliases point only to the non-keg-only python formula
• Modify VERSIONED_KEG_ONLY_ALLOWLIST to include the formula that the python alias points to rather than a hardcoded value
• Any other lists that need to be updated with each new python version instead use the python aliases to determine the formula name

That way, all that needs to be done to finish the migration is a Homebrew/core PR that switches the keg-only formula and updated the aliases (like Homebrew/homebrew-core#63346).

[MikeMcQuaid]

If the audit was not blocking, and was "informative" only, then we could proceed directly: the maintainer reads the audit warning, makes an informed decision that it's OK to proceed. Instead, we lose maintainer time to keep things in sync. And during this, we can't even run CI on the main PR (Homebrew/homebrew-core#63346) to actually test the content of the PR, because the tap_syntax prevents the other tests from running. We're shooting ourselves in the foot.

I sympathise but: what happened when we had "non-blocking" audits is maintainers regularly (despite being asked not to) merged PRs with these audits unfixed. The status quo is annoying in terms of the numbers of PRs required but it's nicer to both our CI and contributors to not have brew audit failing for known reasons.

That's fine for --strict and --new-formula but not for brew audit itself.

I'm thinking something like this:

• Have brew audit mandate that only one python formula can be non-keg-only
• Have brew audit mandate the the python and python@3 aliases point only to the non-keg-only python formula
• Modify VERSIONED_KEG_ONLY_ALLOWLIST to include the formula that the python alias points to rather than a hardcoded value
• Any other lists that need to be updated with each new python version instead use the python aliases to determine the formula name

That way, all that needs to be done to finish the migration is a Homebrew/core PR that switches the keg-only formula and updated the aliases (like Homebrew/homebrew-core#63346).

This seems sensible to me. In general I'm 👍🏻 trying to move logic like this to be inferred from Homebrew/core to avoid the issues like those @fxcoudert references above. If we can do more of these for Homebrew/core-only audits while avoiding people deciding to "temporarily opt-out" of the audit in Homebrew/core: that feels like the best of both worlds.

[fxcoudert]

I'd say we should move all the data in homebrew-core, where it belongs, and have only the logic in brew itself. Same as I said there: #8985 (comment)

The allow-list ends up being to synced with formulas because it's in a different repo. Wouldn't it be better to have a way maintain this information in the same place? I.e., in a given formula, tag it with “this is a prerelease but it's okay”? That way, when we update, we can check if it's still relevant — and it gets removed when the formula is deprecated or removed.

Allow-lists in brew give us more work, for no real benefit. Having the allow-list in the formula means all information is kept in the same place, less work, fewer chances of it becoming out-of-sync. not_keg :versioned_formula, or some other way to mark the formula.

[MikeMcQuaid]

Allow-lists in brew give us more work, for no real benefit. Having the allow-list in the formula means all information is kept in the same place, less work, fewer chances of it becoming out-of-sync. not_keg :versioned_formula, or some other way to mark the formula.

I disagree having allow lists in formulae has no benefit. It is a benefit that exceptions to brew audit require a PR to this repository. If that information is stored in Homebrew/homebrew-core: people will start adding exceptions all over the place rather than fixing them.

[MikeMcQuaid]

Something that is relevant to both here and Homebrew/homebrew-core#63345 (so I'll repeat myself in both): the goal is to have a Homebrew/homebrew-core that (eventually) requires green CI for all PRs to be merged. This is the case on this repository (and, frankly, most repositories) and would be better for both a contributor understanding and security perspective.

[fxcoudert]

people will start adding exceptions all over the place

Maintainers will review PRs in homebrew-core as they review PRs here: based on the merits of adding the exception. The fact that it's one repo or the other shouldn't affect the conclusion.

It is a benefit that exceptions to brew audit require a PR to this repository

So we're voluntarily adding burden on the maintainers. Not a great direction to go, I believe.

[MikeMcQuaid]

So we're voluntarily adding burden on the maintainers. Not a great direction to go, I believe.

Almost every "safety" check we have voluntarily adds burden to maintainers. CI, tests, audits, style checks: these all add a burden. The question is if the burden is either unnecessary or outweighs the benefits. In this case I do not consider either to be the case.

[Rylan12]

In general, the maintainers who approve/merge PRs that change the allowlists are the Homebrew/core maintainers. In my experience, it follows the typical Homebrew/core workflow (opening a PR, waiting for an approval, merging after approved) but in a different repo. For that reason, it seems like it would make more sense to have those changes be made in the Homebrew/core PR. No need to bother non-core maintainers with PRs that they won't bother looking at.

Also, shouldn't the decisions about these lists be made by the Homebrew/core mainatiners? If someone's a core maintainer but not a brew maintainer, they should still be able to approve/merge these changes as it's directly relevant to their work. Similarly, it doesn't make sense for a cask-maintainer with write access to Homebrew/brew to be able to approve/merge allowlist PRs unless they're also a core-maintainer (this is just an example and is not me pointing fingers at anyone).

It seems like having these lists in Homebrew/brew is just adding unnecessary steps/complexity.

[MikeMcQuaid]

I think in the "happy path" case where e.g. an item is added to a list, approved and merged the steps may be unnecessary.

The difference is: this is not what always happens. Things I've seen already happen with allow lists in Homebrew/brew that make me want to keep this process:

• a formula is added to a list by a contributor when it should not be
• a formula is added to the wrong list (provided by macOS vs. shadows macOS)
• multiple formulae of the same version are added to a list
• an exception is added to an allowlist when it could be more easily fixed in the formula
• an exception is added to an allowlist when we'd previously agreed it wouldn't be and the formula should instead be fixed/deprecated/disabled

[Rylan12]

I understand those concerns, but I don't see how keeping these in Homebrew/brew solves any of those problems.

[MikeMcQuaid]

@Rylan12 If these are in separate files in Homebrew/core i.e. not in formula/DSLs and we have a tighter group of people requested for review and wait for those reviews before those changes are merged (similar to/part of #8989): it seems like it would have the same result and I'd be open to it. It doesn't seem like that reduces the overhead much to me but, if others would rather it, I'd be 👍🏻 on trying this out for at least some allowlists as an experiment and then we can consider moving more/all of them across if it's successful.

[Rylan12]

Okay, that's fair. I'll give it some more thought and will open a PR for further discussion if I make any progress. Thanks for the discussion!