Skip to content

Centralize CI config and make it more uniform (Adds macOS arm64 pypy CI) #2945

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

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

ankith26
Copy link
Member

@ankith26 ankith26 commented Jun 20, 2024

The goal of this PR is to centralize all common cibuildwheel config, and make our CI more uniform.

We currently take different strategies on different workflows. manylinux takes the "build for all pythons in one workflow per arch" strategy. Mac takes an (ever so slightly harder to maintain) strategy of handwritten "split jobs evenly" while windows takes the "every run on its own" thing.
I figured that the strategy manylinux takes is actually the most efficient, even if it's slower than the rest. Why?

  • We can re-use more stuff across runs - cibuildwheel dependencies, buildtime dependencies, SDL prebuilt downloads (in windows) and doc building.
  • By using less concurrent workflows per PR we are actually making room for more across-PR CI concurrency.
  • Simpler config to maintain. Want to support a new python version on CI? It's now a one line change in pyproject.toml
  • There is no real advantage from a PR maker POV if all windows builds can finish in 8 minutes, and the linux one is going to take like 15 minutes anyways which the PR maker has to wait for.
  • It is also wasteful if there is say, a windows specific CI fail that just causes multiple separate runs to fail when one fail would be enough to convey the message.

Also switches build backend to build[uv] which should again save some CI time.

I may also explore more in the caching area in future PRs and see how that improves things.

@ankith26 ankith26 requested a review from a team as a code owner June 20, 2024 17:37
@ankith26 ankith26 force-pushed the ankith26-ci-upgrade branch 3 times, most recently from 297d723 to a91e0db Compare June 22, 2024 14:24
@ankith26
Copy link
Member Author

ankith26 commented Jun 22, 2024

While doing this PR I also happened to

  • Add CI for pypy arm64 on macOS which we completely missed somehow
  • Add numpy-based tests on linux

Here is a table of cumulated-time comparisons, along with per config notes

Platform Before (main) After (this PR) Extra Notes
Windows x86_64 37m 33s (3m 37s, 3m 10s, 3m 52s, 4m 23s, 4m 13s, 4m 30s, 4m 50s, 8m 58s) 14m 53s Numpy tests removed on pypy 3.10
Windows x86 19m 22s (4m 16s, 3m 5s, 4m 6s, 4m 45s, 3m 10s) 7m 10s -
macOS arm64 15m 0s (10m 23s, 4m 37s) 9m 51s Added 3 new pypy runs
macOS x86_64 27m 7s (7m 49s, 8m 30s, 10m 48s) 15m 55s Numpy tests removed on pypy 3.10
Linux arm64 13m 43s 11m 48s Added numpy for tests where possible
Linux x86_64 11m 43s 10m 44s Added numpy for tests where possible
Linux x86 12m 58s 11m 1s -

@ankith26 ankith26 changed the title Centralize CI config and make it more uniform Centralize CI config and make it more uniform (Adds macOS arm64 pypy CI) Jun 22, 2024
@yunline yunline added the CI Issue with the Continuous Integration (CI), the actions/bots that test things label Jun 24, 2024
@ankith26 ankith26 force-pushed the ankith26-ci-upgrade branch from a91e0db to bb792ec Compare July 8, 2024 08:13
Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this Ankith.

Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

LGTM!

@ankith26 ankith26 added this to the 2.5.1 milestone Jul 22, 2024
@ankith26 ankith26 merged commit bf4d301 into main Jul 22, 2024
22 checks passed
@ankith26 ankith26 deleted the ankith26-ci-upgrade branch July 22, 2024 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Issue with the Continuous Integration (CI), the actions/bots that test things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants