Add an Arm job #22
Add an Arm job #22
Conversation
This is required to build wheels for Arm.
|
Update README to add Arm in addition to x86 on description. |
rolfmorel
left a comment
rolfmorel
left a comment
There was a problem hiding this comment.
LGTM -- it being a near carbon-copy of the x86 workflow makes sense to me (while suppressing a deep-seated need to deduplicate all the things) 👍
| # Ensure that only a single job or workflow using the same | ||
| # concurrency group will run at a time. This would cancel | ||
| # any in-progress jobs in the same github workflow and github | ||
| # ref (e.g. refs/heads/main or refs/pull/<pr_number>/merge). | ||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
Betraying my inexperience with Github workflows: this section and the scheduled time being the same for the x86 workflow won't conflict? I guess it depends on whether the "concurrency group" is shared or not.
Just thought to double check.
There was a problem hiding this comment.
IIUC, "concurrency" refers to ... concurrency with respect to one workflow config. Separate workflow configs are not affected by this. That's my understanding.
Tl;Dr This should not be a problem.
There was a problem hiding this comment.
Makes sense -- thanks for explaining 👍
There was a problem hiding this comment.
Do we want to rename the file as well?
There was a problem hiding this comment.
I've merged two files.
| jobs: | ||
| build_linux: | ||
| name: Linux Arm Build | ||
| runs-on: ubuntu-24.04-arm |
There was a problem hiding this comment.
Thanks for the change.
The steps here look identical to linux x86 (which makes sense) -- can we use strategy.matrix to run both architecture in a single CI to avoid duplication?
strategy:
matrix:
os: [ubuntu-24.04, ubuntu-24.04-arm]
runs-on: ${{ matrix.os }}
A similar strategy is used to run the CI for different py versions on line 29.
There was a problem hiding this comment.
For whatever reason I assumed that the runner cannot be specified via a matrix, but I think that I was wrong :) Please see the latest commit and thanks for nudging me in this direction.
I was under the impression that the runner is fixed within a workflow file, but I think that I was wrong. In the latest revision I merged the two Workflow files. |
|
ATM Ci is failing, but it doesn't look like an issue with this PR? Should we merge? Alternatively, could somebody run CI using this branch so that we test it? I don't have the write access :( Thanks |
marbre
left a comment
marbre
left a comment
There was a problem hiding this comment.
LGTM, I guess there was no test run triggered yet?
| @@ -1,4 +1,4 @@ | |||
| name: Build Release and Publish | |||
| name: Build Release and Publish (x86_64 + AArch64) | |||
There was a problem hiding this comment.
Names get rather long, let's maybe go with what we had before.
No. I see no option in my fork to trigger it. |
|
You need to merge to |
Another option is to temporarily add a |
|
I think this is fine to merge and fix forward if needed. Let me know if you want to me to click the button |
I have no write access for this repo, so please merge this for me, thanks! |
5 participants
This is required to build wheels for Arm.