Add Windows 11 arm to ci #7517
Add Windows 11 arm to ci #7517
Conversation
|
As I modify the create_release workflow as part of this PR I did a test release on my fork here https://github.com/mcbarton/binaryen/releases/tag/test-windows-arm . Once this workflow run has finished running https://github.com/mcbarton/binaryen/actions/runs/14522766598 the artifacts should appear there. |
|
Something went wrong with the test release. The Windows arm binary ended up with x86_64 in the name, and it shouldn't have 11 in the name. I will fix tomorrow. |
|
@kripken @tlively This PR is ready for review, as my last test release (https://github.com/mcbarton/binaryen/releases/tag/test_release_8) appears to have worked. There is now a Windows arm release binary, as well as a Windows x86_64 one. |
There was a problem hiding this comment.
This seems potentially useful, thanks, but I have to admit I don't know how important ARM is for the Windows platform (mainly since I don't know much about Windows these days). Do you have a need for it yourself, or have you seen someone asking for these builds?
I don't have any need for it myself, but someone was asking for Windows arm support for wasi-sdk here WebAssembly/wasi-sdk#522 , and I thought it shouldn't be too difficult to add once Githubs Windows arm runners became available. I'm trying to add Windows arm support to all the WebAssembly repos. I made a PR for Windows arm support in wasi-sdk here WebAssembly/wasi-sdk#524 , and for the wasi-testsuite here WebAssembly/wasi-testsuite#105 (just waiting for someone to activate the workflow on this one) . I also plan to move onto adding it to the wasi-libc repo next assuming its something the organisation wants/sees a need for. |
Co-authored-by: Alon Zakai <alonzakai@gmail.com>
| @@ -47,6 +47,11 @@ jobs: | |||
| run: cmake -S . -B out -DCMAKE_INSTALL_PREFIX=out/install | |||
| if: matrix.os == 'windows-latest' | |||
|
|
|||
| - name: cmake (win arm64) | |||
| # -G "Visual Studio 15 2017" | |||
| run: cmake -S . -B out -DCMAKE_INSTALL_PREFIX=out-arm64/install | |||
There was a problem hiding this comment.
Why do we need a different installation directory here? If we don't, can we just reuse the previous step for both windows builders?
There was a problem hiding this comment.
You need a different installation directory due that being the assumption of the folder name in the archive-arm64 stage of the ci already in place (see https://github.com/mcbarton/binaryen/blob/6658bcc05d2c695a9fc9b5dae8ed623a9dfae9ef/.github/workflows/create_release.yml#L92 ) . The reason its done in this way currently is because you create osx x86 and osx arm64 binaries both from the osx arm64 runners as far as I can tell https://github.com/mcbarton/binaryen/blob/6658bcc05d2c695a9fc9b5dae8ed623a9dfae9ef/.github/workflows/create_release.yml#L41
.github/workflows/ci.yml
Outdated
| runs-on: ${{ matrix.os }} | ||
| strategy: | ||
| matrix: | ||
| os: [windows-latest,windows-11-arm] |
There was a problem hiding this comment.
Space after comma here and below.
| @@ -56,7 +61,7 @@ jobs: | |||
|
|
|||
| - name: strip | |||
| run: find out*/install/ -type f -perm -u=x -exec strip -x {} + | |||
| if: matrix.os != 'windows-latest' | |||
| if: ${{ !startsWith(matrix.os, 'windows') }} | |||
There was a problem hiding this comment.
Why is ${{ .. }} needed here but not elsewhere where startsWith i used?
There was a problem hiding this comment.
This was due to me not using startswith much. I didn't know whether I could do the negation of startswith without the ${{ .. }} . Will change to just !startsWith(matrix.os, 'windows')
There was a problem hiding this comment.
Had to revert this change back to what it was previously, as the workflow broke without the brackets
|
@sbc100, any final comments? |
|
@tlively @kripken can this PR go in now that its been approved? Also, do either of you have the necessary permissions to activate the workflows on this PR WebAssembly/wasi-testsuite#105 in wasi-testsuite? |
That's pretty slow, much slower than all other existing builds... is this particular build really worth the overhead?
I don't have permissions there, sorry. |
If you see here #7517 (comment) I added this job not knowing whether it was needed. Happy to remove this job if you'd like me to. |
|
Yes, please remove it - to use up so much time for a mode we don't strongly need feels excessive. |
Done. |
|
@kripken @tlively @sbc100 Something is causing the Windows 11 arm job to hang while running a test (this didn't happen in previous ci runs). You will want to cancel the job manually, and rerun. You won't want it to hang that long due to wasted Github runner minutes, but If you don't see this message in time, the job will automatically get killed after 6 hours. |
mcbarton
force-pushed
the
Add-Windows-11-ci
branch
from
dcca0f2 to
bd12c83
Compare
According to https://github.blog/changelog/2025-04-14-windows-arm64-hosted-runners-now-available-in-public-preview/ Windows arm Github runners are now available for free for public repositories. Therefore I have added them to binaryen ci.