Use macos-26 runners and test on Intel/ARM#2348
Use macos-26 runners and test on Intel/ARM#2348sirosen wants to merge 3 commits intojazzband:mainfrom
Conversation
|
This appears to be working as desired, but I find the results in CI wall clock time a bit concerning. I'm also worried that by increasing the number of builds by so much, we're increasing the changes of flaky failures. We want to solve those anyway, but "want to fix" != "have fixed". Many projects consider it sufficient to test their max and min Python versions on Windows and macOS. I think I would be comfortable doing that at least for these new Intel-mac builds, to keep CI times more under control. It would require refactoring the job config a bit, but I think it's necessary. |
|
I had an idea some time ago inspired by what CPython does with buildbots on exotic OS/hardware variants but haven't tried it yet. What they do is they run testing at some point post-merge due to resource availability. And if something fails, PRs get a comment with a link to the failure. The authors are supposed to fix it within 24 hours or a PR gets reverted. The gist of what I'm considering is that we have a separate matrix for slow jobs that isn't tied into alls-green. That matrix has a special GitHub Environment assigned ( One corner case to think of is merge queues as for those I imagine we wouldn't want the timer. Would you like to experiment with this idea here? I've been thinking about it for quite a while in the context of projects with C-extensions that would be noticeably slower. |
|
Is macos 15 slower or faster, by the way? |
|
It's hard to be confident, but so far it looks like ARM macos 26 The main issue with CI time is that with a large number of macOS runners, we get a lot of queuing in GitHub. None of the jobs in our matrix are particularly slow -- I'm actually very happy with how fast they all are. I'm game to experiment with new CI strategies here, but I don't want to complicate this "simple" PR with those changes. ... I think I'd like to merge this as-is, accepting that it raises the overall CI time to ~20 minutes. With the benefit of more time to think about it, that seems fine to me, since CI time isn't a major bottleneck for us. And i think we both have ideas about CI refinements that will make this better over time. I'm marking this PR "ready" without further changes. Yes, things will get a little slower for a while. But we'll make them faster over time. |
.github/workflows/ci.yml
Outdated
| - Windows | ||
| - macOS | ||
| - name: Ubuntu | ||
| runs_on: ubuntu-latest |
There was a problem hiding this comment.
We should be using kebab-case keys for GHA to follow the convention consistently.
Also, complex non-primitive values in the matrix tend to be difficult to work with (and didn't use to be supported officially in the past). I wouldn't mind rendering the VM identifiers as is, w/o any extra fancy naming.
There was a problem hiding this comment.
Yep, I can make those adjustments, along with the other edits.
I also don't mind rendering the identifiers verbatim in our job names -- the previous config seemed carefully arranged to produce short names (but in, IMO, a fragile way).
.github/workflows/ci.yml
Outdated
| CI-GHA, | ||
| OS-${{ runner.os }}, | ||
| VM-${{ matrix.os }}, | ||
| VM-${{ matrix.os.name }}, |
There was a problem hiding this comment.
Let's just turn this (and the matrix) into a simple
| VM-${{ matrix.os.name }}, | |
| VM-${{ matrix.runner-vm }}, |
so it matches what I have in other repos as it's easier to sync.
.github/workflows/ci.yml
Outdated
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: |
There was a problem hiding this comment.
| os: | |
| runner-vm: |
.github/workflows/ci.yml
Outdated
| - name: Ubuntu | ||
| runs_on: ubuntu-latest | ||
| - name: Windows | ||
| runs_on: windows-latest | ||
| - name: macOS | ||
| runs_on: macos-26 | ||
| - name: "macOS (Intel)" | ||
| runs_on: macos-26-intel |
There was a problem hiding this comment.
| - name: Ubuntu | |
| runs_on: ubuntu-latest | |
| - name: Windows | |
| runs_on: windows-latest | |
| - name: macOS | |
| runs_on: macos-26 | |
| - name: "macOS (Intel)" | |
| runs_on: macos-26-intel | |
| - ubuntu-latest | |
| - windows-latest | |
| - macos-26 | |
| - macos-26-intel |
There was a problem hiding this comment.
(though, I'd prefer getting rid of latest for good in favor of more predicatability and tracability when looking at old CI runs)
There was a problem hiding this comment.
100%!
I plan to come back with a dedicated PR to simply swap latest for the appropriate version numbers. 😁
In the matrix config, separate the notions of "os.name" (the nice name shown in the CI GUI) from "os.runs_on" (the value used for selecting the runner). They are still aligned after this change, but can now be manipulated independently.
Pin the macOS CI runners specifically to `macos-26` and `macos-26-intel`.
- Use a flat array, rather than objects in an array - Name the key `runner-vm` to match other projects - Allow the verbatim `runs-on` identifier to appear in job names - Set a requisite actionlint ignore for a runner it does not recognize Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <578543+webknjaz@users.noreply.github.com>
ca1b28e to
9921816
Compare
Open as a draft because I think this needs to be linked to relevant issues. It also needs to actually run in CI to confirm it works as desired.
Contributor checklist
Included tests for the changes.changelog.d/(seechangelog.d/README.mdfor instructions) or the PR text says "no changelog needed".
Maintainer checklist
bot:chronographer:skiplabel.(following Semantic Versioning).