-
-
Notifications
You must be signed in to change notification settings - Fork 458
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
node: Add Windows ARM64 prebuilds to CI #440
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the ping! Yes, we can take this, but I'll note this workflow is just for the manually-submitted job to publish to NPM; we're still not compiling for aarch64-apple-darwin or aarch64-pc-windows-msvc on every PR. With macOS I didn't worry about it because we do compile the non-bridge crates for aarch64-apple-ios; do you feel confident that aarch64-pc-windows-msvc is sufficiently exercised by the other AArch64 platforms + AMD64 Windows?
.github/workflows/npm.yml
Outdated
@@ -56,7 +58,7 @@ jobs: | |||
|
|||
- run: npx prebuildify --napi -t ${{ steps.get-nvm-version.outputs.node-version }} --arch arm64 | |||
working-directory: node | |||
if: matrix.os == 'macos-11' | |||
if: matrix.os != 'ubuntu-latest' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this a little more semantic: we can rename additional-rust-target
to arm64-rust-target
and then this can check whether arm64-rust-target
is present at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense! Updated in a2b1ae1 - seems to work correctly: https://github.com/dennisameling/libsignal-client/runs/4893965003?check_suite_focus=true
a2b1ae1
to
a16837f
Compare
I would say yes. I've been building this in my fork for a few months from a x64 host and all is good.
Would you like me to add |
Let's leave the per-PR CI as it is for now, then. Setting up a custom GitHub Actions runner is one more thing we'd have to keep running, and without that we'd just be testing compilation, which is pretty unlikely to break just for ARM64 Windows. Thank you for doing this! |
Just published v0.12.1 with an ARM64 Windows prebuild. :-) |
This PR adds prebuilds for Windows ARM64 to CI. I've been building these in my fork for a few months and haven't encountered issues with it.
As discussed in #426 (comment), "discussion with the Desktop folks is leaning positive about it", so I took the liberty to create this PR 😊