Skip to content

Enable native AArch64 Ubuntu CI jobs #127584

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 2 commits into from
Dec 4, 2024

Conversation

diegorusso
Copy link
Contributor

The Arm runners are now enabled for the JIT builds.
We exclude the Arm runners on forks for both Tests and JIT jobs.

The Arm runners are now enabled for the JIT builds.
We exclude the Arm runners for forks fro both Tests and JIT builds.
@diegorusso
Copy link
Contributor Author

This is the continuation of #125786

Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

JIT changes look good, thanks for tackling this! CI-driven development is no fun, I know.

@hugovk
Copy link
Member

hugovk commented Dec 4, 2024

Hmm, I do find it somewhat misleading that the jobs just "passes" immediately (well, not quite immediately, it takes about 11s to spin up and checkout), and would prefer to skip it entirely.

image

And when we do run, we're skipping three irrelevant jobs, and there's nothing much special we're sharing in the setup config to benefit from it (just checkout and setup-python):

image

And I did like the refactored one had more readable job names and grouped the three operating systems in the summary:

image

Compared to this:

image

But from a practical point of view it's not such a big deal, and the important thing is to start testing the new runners, so I've approved!

Thanks all for your work!

@brandtbucher
Copy link
Member

Hmm, I do find it somewhat misleading that the jobs just "passes" immediately (well, not quite immediately, it takes about 11s to spin up and checkout), and would prefer to skip it entirely.

But the alternative is just not running the job at all, right? Maybe when this happens we can just echo one of GitHub's magic warning messages to stdout so at least a yellow box appears?

I'm not strongly opposed to refactoring if many others feel that way, I just didn't think that this particular change warranted rewriting the whole file and adding a bunch of new logic. I personally find the current structure a easier to follow:

  • All of the names and configurations of the jobs fit in one screenful.
  • All of the commands that the jobs actually run, across all OSes, fit in one screenful.
  • It's pretty easy to do things like support more than one LLVM version (we've done this in the past), etc.

If rewriting it so every OS is its own separate set of jobs would help in some real way, I'm probably okay with it in its own issue/PR. But simpler things like changing the names are easy enough to do in the current file. And it seems like we'll need "clever" tricks for handling the paid runners anyways.

Copy link
Member

@savannahostrowski savannahostrowski left a comment

Choose a reason for hiding this comment

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

Much simpler! Thanks Diego!

@savannahostrowski savannahostrowski merged commit 7c5a6f6 into python:main Dec 4, 2024
63 checks passed
@diegorusso diegorusso deleted the arm-runners2 branch December 4, 2024 22:22
@diegorusso
Copy link
Contributor Author

PR #125786 was back ported to 3.13. I don't think we need this for 3.13 as we don't build the JIT in this version. Thoughts?

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra CI, GitHub Actions, buildbots, Dependabot, etc. skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants