Skip to content

Add Windows 11 arm to ci #7517

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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Add Windows 11 arm to ci #7517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
033ee95
Add Windows 11 arm to ci
mcbarton Apr 17, 2025
c8a166a
Try to fix release workflow
mcbarton Apr 19, 2025
5d515e6
Try to fix release workflow
mcbarton Apr 19, 2025
a0bfb70
Try to fix release workflow
mcbarton Apr 19, 2025
89c5bb3
Update create_release.yml
mcbarton Apr 19, 2025
a34728d
Update create_release.yml
mcbarton Apr 19, 2025
e5e72fa
Update create_release.yml
mcbarton Apr 19, 2025
6658bcc
Update .github/workflows/ci.yml
mcbarton Apr 22, 2025
4ae8668
Apply PR suggestions
mcbarton Apr 23, 2025
861c270
Revert change
mcbarton Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
os: [ubuntu-latest, macos-latest, windows-latest, windows-11-arm]
steps:
- uses: actions/setup-python@v5
with:
Expand All @@ -77,7 +77,7 @@ jobs:

- name: install ninja (win)
run: choco install ninja
if: matrix.os == 'windows-latest'
if: startsWith(matrix.os, 'windows')

- name: install v8
run: |
Expand All @@ -98,7 +98,7 @@ jobs:
- name: cmake (win)
# -G "Visual Studio 15 2017"
run: cmake -S . -B out -DCMAKE_INSTALL_PREFIX=out/install
if: matrix.os == 'windows-latest'
if: startsWith(matrix.os, 'windows')

- name: build
run: cmake --build out --config Release -v
Expand All @@ -108,7 +108,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') }}

- name: Upload artifacts
uses: actions/upload-artifact@v4
Expand Down Expand Up @@ -368,8 +368,11 @@ jobs:

# Windows + gcc needs work before the tests will run, so just test the compile
build-mingw:
name: mingw
runs-on: windows-latest
name: mingw-${{ matrix.os }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't know whether windows arm was needed for this part of ci. Added for now, and will remove if requested.

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine to keep.

runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [windows-latest, windows-11-arm]
steps:
- uses: actions/setup-python@v5
with:
Expand Down
16 changes: 11 additions & 5 deletions .github/workflows/create_release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [macos-latest, windows-latest]
os: [macos-latest, windows-latest, windows-11-arm]
defaults:
run:
shell: bash
Expand All @@ -31,7 +31,7 @@ jobs:

- name: install ninja (win)
run: choco install ninja
if: matrix.os == 'windows-latest'
if: startsWith(matrix.os, 'windows')

- name: mkdir
run: mkdir -p out
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need a different installation directory here? If we don't, can we just reuse the previous step for both windows builders?

Copy link
Contributor Author

@mcbarton mcbarton Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks 👍

if: matrix.os == 'windows-11-arm'

- name: build
run: cmake --build out -v --config Release --target install

Expand All @@ -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') }}
Copy link
Member

Choose a reason for hiding this comment

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

Why is ${{ .. }} needed here but not elsewhere where startsWith i used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to revert this change back to what it was previously, as the workflow broke without the brackets


- name: archive
id: archive
Expand All @@ -73,11 +78,12 @@ jobs:
cmake -E sha256sum $TARBALL > $SHASUM
echo "TARBALL=$TARBALL" >> $GITHUB_OUTPUT
echo "SHASUM=$SHASUM" >> $GITHUB_OUTPUT
if: matrix.os != 'windows-11-arm'

- name: archive-arm64
id: archive-arm64
run: |
OSNAME=$(echo ${{ matrix.os }} | sed 's/-latest//')
OSNAME=$(echo ${{ matrix.os }} | sed 's/-latest//' | sed 's/-11-arm//')
VERSION=$GITHUB_REF_NAME
PKGNAME="binaryen-$VERSION-arm64-$OSNAME"
TARBALL=$PKGNAME.tar.gz
Expand All @@ -89,7 +95,7 @@ jobs:
cmake -E sha256sum $TARBALL > $SHASUM
echo "TARBALL=$TARBALL" >> $GITHUB_OUTPUT
echo "SHASUM=$SHASUM" >> $GITHUB_OUTPUT
if: matrix.os == 'macos-latest'
if: ${{ matrix.os == 'macos-latest' || matrix.os == 'windows-11-arm' }}

- name: upload tarball
uses: softprops/action-gh-release@v1
Expand Down
Loading