Enable native AArch64 Ubuntu CI jobs for the JIT

[diegorusso]

Enable native AArch64 Ubuntu CI jobs for the JIT. In order to achieve that I had to refactor the jit.yml workflow:

• Split jobs by OS (Windows, Mac, Linux)
• LLVM version is now a workflow variable accessible by all jobs
• Ubuntu updated from 22.04 to 24.04
• Run the pipeline only when the branch is main or 3.* like we have in build.yaml

The refactoring was needed because these jobs used different fields from the earlier unified matrix. By splitting it, we can better tailor what fields to use for every OS.
Also this allows to exclude AArch64 Ubuntu jobs on forks. Earlier this wasn't possible because the workflow was using in the matrix definition an include that is always executed after an exclude. There was no way to exclude AArch64 Linux runners for forks.
As a consequence, the job names are not re
porting anymore the target/compiler but more generically: OS (arch or runner, Debug/Release) like the image below:

[diegorusso]

This is a continuation of #125786

[hugovk]

Exciting!

Could we add something like is-fork from https://github.com/python/cpython/pull/121809/files to only include these custom runners for upstream CPython runs and not for any runs on forks?

[diegorusso]

Exciting!

Could we add something like is-fork from https://github.com/python/cpython/pull/121809/files to only include these custom runners for upstream CPython runs and not for any runs on forks?

Thanks @hugovk for the help on Discord DM :) I think we have now what you were asking.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[brandtbucher]

This is probably going to conflict with GH-127212. I think maybe we should merge that one first (resolving the conflict shouldn't be too bad, since you're just removing the conflicting lines anyways).

[brandtbucher]

Happy with the JIT changes, thanks!

[savannahostrowski]

Copying this from a conversation on Discord for posterity.

Thinking about this more, I think there may be a missing security boundary here. While the logic to check if the PR is on a fork may limit runs, I'm pretty sure that this is still a security risk because someone could open a PR to their fork, which removes the logic that checks if it's a fork, and then the custom runners would be used.

Unless I'm missing something, I think we may need to think through this a bit more? GitHub also recommends that self-hosted runners only be used on private repos.

https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#self-hosted-runner-security

Update: the ownership of the hardware here was ambiguous. Looks like this is a non-concern as we are using GH managed runners.

[diegorusso]

Hello, I reworked the PR "completely". @savannahostrowski @brandtbucher please have a look. I've dismissed your approval because now the PR is very different from before.

Please review again. The PR has changed a lot from the approval

[hugovk]

Thank you!

[savannahostrowski]

One nit but, otherwise, this LGTM! Thanks for fighting with this, Diego!

[savannahostrowski]

Thanks Diego! This looks great to me!

[savannahostrowski]

@brandtbucher If you're okay with these changes, I'd be ready to merge this.

[brandtbucher]

So, it looks like it works correctly (thanks for figuring it out). I'm just bummed because I find the new structure quite a bit harder to follow and work with than before. This seems like a lot of churn just to turn off two jobs on forks.

My understanding is that the restructuring is necessary because we can't exclude jobs from a matrix that's built up of includes, since excludes runs before includes.

I'd like to explore the options a bit further before landing this, since GHA has lots of different, uh, "features" for this sort of logic. For instance: could we turn this line into runner.os == 'Linux' && (matrix.architecture == 'x86_64' || !is-fork) (or whatever the correct syntax is)? Or is that too late in the process?

[brandtbucher]

See GH-127581 for an alternative approach. The two AArch64 Linux jobs run as expected on CPython org CI, and complete immediately on the fork's CI.

When the runners (presumably) become free for all, we can just remove the github.repository_owner == 'python' guards.

[diegorusso]

We tried at some point to put the if in the job but it doesn't work. The reason is very simple: in order to run the job, it requires a runner, but on forks the aarch64 runners will never be available because they are not free (that's the same reason we exclude Cirrus macOS from forks)
So the line you suggested is executed only on runners which are assigned to the job. To show you what it is happening, I've removed the exclude and added that if (that will be never executed anyway) on a branch on my fork: diegorusso@9a1130b

As you can see from the screenshot below, the aarch64 jobs will be pending forever because the matrix includes them but they will never be allocated on forks. The only way possible is to exclude them from the matrix altogether with exclude.

The only way possible is to exclude them when building the matrix. Unfortunately as the include is run after the exclude, if we have both, it will schedule just the release aarch64 job on forks (thanks to @hugovk to figure this out) because by default debug is not added hence it is false.

When reading the fields in the include, I realized that:

• target is used just in names of the job
• architecture is used just for windows builds. The architecture can be extracted in another way
• the runner can be stripped if we split these jobs

Hence I decided to do the refactoring. As further improvement, this could lead to an additional refactoring by moving the build logic in the reusable-*.yml definitions. So we have one place where the build logic is contained.

I'm open to other solutions (if there are)

[diegorusso]

After messaging with @brandtbucher on Discord, I decided to close this PR as leave it for history.
I'll open a new PR that implements what Brandt has suggested here: #127581

Moreover:

• we don't need the whole refactoring
• LLVM version can stay in the matrix
• the exclude in the build.yml is OK
• we don't need to filter JIT workflow for specific branches. We actually want the JIT workflow to run on every branch
• It's OK to update the ubuntu version to 24.04

[diegorusso]

This is the new PR: #127584