Skip to content
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

workflows/triage: add linux-only label #64294

Merged
merged 1 commit into from
Nov 10, 2020

Conversation

SeekingMeaning
Copy link
Contributor

@SeekingMeaning SeekingMeaning commented Nov 7, 2020

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

This allows us to quickly identify bump PRs as updates to Linux formulae, which for the time being, should not be updated in homebrew-core; see #64175 (comment)

I was not really expecting people to upgrade linux-only formulae here before completing the merging of both repositories. Especially as no bottles are built; and not reverse dependencies are tested. So we may introduce breakages in linuxbrew-core, which is far from ideal.

@BrewTestBot BrewTestBot added the automerge-skip `brew pr-automerge` will skip this pull request label Nov 7, 2020
@SeekingMeaning SeekingMeaning changed the title workflows/triage: add linux label workflows/triage: add linux-only label Nov 7, 2020
@SMillerDev
Copy link
Member

This should also behave as a do not merge-label then for testbot.

@SMillerDev
Copy link
Member

I don't think it's a good idea to add do not merge directly, I'd say it should be a new label that also blocks automerge. But ultimately dependencies not being tested in linuxbrew is the real problem that should be fixed.

@SeekingMeaning SeekingMeaning marked this pull request as draft November 7, 2020 15:06
MikeMcQuaid
MikeMcQuaid previously approved these changes Nov 9, 2020
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 to linux-only label, agreed about not having a do not merge yet.

@iMichka
Copy link
Member

iMichka commented Nov 9, 2020

I am ok for a linux-only label too.

But ultimately dependencies not being tested in linuxbrew is the real problem that should be fixed.

They are tested, but in linuxbrew-core. Ideally one would first open a PR in linuxbrew-core, check if everything is fine, and then backport the change here.
Or not make any version bumps here for the time being, we are not in a hurry.

This situation will last until we have merged both repositories: we made huge progress but there are still a lot of things to finish.

@SMillerDev
Copy link
Member

They are tested, but in linuxbrew-core. Ideally one would first open a PR in linuxbrew-core, check if everything is fine, and then backport the change here.

IMHO that's an unworkable situation. I'd say we need to test it here, or not have it here at all. Otherwise it's a ton of work for everyone.

@MikeMcQuaid
Copy link
Member

This situation will last until we have merged both repositories: we made huge progress but there are still a lot of things to finish.

Do you have a (very) rough time estimate?

@iMichka
Copy link
Member

iMichka commented Nov 9, 2020

The other strategy was to migrate all the linux changes to this repo in one huge PR, when we are ready. But this also means reviewing a PR that will contain more than 2000 formula changes, and this PR will be an endless moving target.

I prefer to add things here slowly, with the risk that some formulae are tested on Linux only with a delay. That does not change much to the current situation where homebrew-core formulae are only tested on Linux when we merge it to linuxbrew-core.

The idea is that the changes we make to the homebrew-core repo:

  • are tested on mac and not break anything for mac users
  • are tested on linux with delay, after the merge, as it has always been
  • some formulae are partially tested for linux in homebrew-core, on a best-effort basis

@iMichka
Copy link
Member

iMichka commented Nov 9, 2020

Do you have a (very) rough time estimate?

6 months? 1 year? It depends on how much help I get to migrate everything. I'll do a status update on Homebrew/brew#7028 soon.

@SMillerDev
Copy link
Member

I prefer to add things here slowly, with the risk that some formulae are tested on Linux only with a delay. That does not change much to the current situation where homebrew-core formulae are only tested on Linux when we merge it to linuxbrew-core.

Is there really no way to properly test this in core on both platforms? Maybe a migration "per family" as we would do with things like Python is the way to go here? I'd love to help, but it's hard to keep track of the status for me.

@MikeMcQuaid
Copy link
Member

6 months? 1 year? It depends on how much help I get to migrate everything. I'll do a status update on Homebrew/brew#7028 soon.

Thanks, that's helpful. Given that:

  • some formulae are partially tested for linux in homebrew-core, on a best-effort basis

I think the level of testing done here should be ramped up. It may not be feasible to do reverse dependency checking (although it's probably a good idea, IMO) but we could at least be creating and testing bottles without upload.

@iMichka
Copy link
Member

iMichka commented Nov 10, 2020

I think the level of testing done here should be ramped up. It may not be feasible to do reverse dependency checking (although it's probably a good idea, IMO) but we could at least be creating and testing bottles without upload.

Agreed.

I think reverse dependency checking only happens for formulae with a bottle on Linux, as we did not bottle everything until now.
The issue is that some formulae have different revisions on linux and on mac, so we can not just "fetch" bottles as-is from linux and use them for testing in the homebrew-core.

What I would like is to slowly introduce linux CI for a few "main" formuale in homebrew-core in the next months, to make sure these do not get broken by updates. I am thinking about things like gcc, pkg-config, openssl, unzip, perl, python ...
Of course these will not be able to use bottles for their builds, so we can not go too deep into the dependency tree.

👍🏻 to linux-only label, agreed about not having a do not merge yet.

Can this PR be changed to add just the first label.

After some thoughts: I am totally fine that things get bumped here, I was just surprised because I did not think it would happen. But it is just the logical thing to do. We will monitor breakages in linuxbrew-core after merges, like we do for all the other packages. Nothing really changed in the process.

@SMillerDev
Copy link
Member

Just wondering, what's stopping us from already bottling the formula we have migrated here? That seems like the easiest thing. That way you "actually" migrate them rather than upstreaming the changes.

@iMichka
Copy link
Member

iMichka commented Nov 10, 2020

We could, if it is acceptable to build bottles in this repo, step-by-step. I'll write some stuff about this in Homebrew/brew#7028 then, as there are a few things to be discussed before we do that, and I do not want to hijack this PR for this discussion.

@SMillerDev
Copy link
Member

I'd say we should make bottles here if we can. That's the end goal anyway.

@iMichka
Copy link
Member

iMichka commented Nov 10, 2020

Cool. Let's discuss this in Homebrew/brew#7028 for the practical details

For this PR: we would like to have only the linux-only label.

@SeekingMeaning SeekingMeaning marked this pull request as ready for review November 10, 2020 17:14
@iMichka iMichka merged commit 6b9abd6 into Homebrew:master Nov 10, 2020
@iMichka
Copy link
Member

iMichka commented Nov 10, 2020

Thanks all for the input!

@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 11, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge-skip `brew pr-automerge` will skip this pull request outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants