-
Notifications
You must be signed in to change notification settings - Fork 253
Test with ARM64 Windows and also build wheels #864
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
Conversation
|
I'm feeling very uneasy updating a binary blob in a PR, so ideally I shouldn't be trusted with this. I could download minisign there, verify its checksum, and then use it to verify the libsodium release. And in addition a maintainer should also verify. Or a maintainer could re-do this PR. Thoughts on this? |
|
Isn't changing the MSVC version quite a big change to be slipped in a PR adding Windows arm64 builds? It wouldn't be my instinct to expect a problem coming from here when reading release notes/commits. |
|
I'm happy to limit that MSVC change to arm64 if wanted. Or split the MSVC change out into its own PR. I had hoped that running the test would rule out any issues this could generate, but I'm lacking experience there. |
|
I think I probably need to go ahead and do a PR to update to the latest 1.0.20-stable and also add the arm64 static lib at the same time. I'm comfortable with the switch to later MSVC though (since this is all ucrt anyway). I'm swamped for a variety of reasons but this should be some pretty simple work so I'll try to get to it ASAP so you can rebase and add the actual win arm64 build (if you're interested). |
|
Sounds good. Just ping me if needed. Regarding MSVC version, I noticed that in the wheel build it is explicitly selected via VsDevCmd.bat, but during testing I don't see anything like that. Does that mean setuptools will just use the newest one right now? |
|
Is it? I think we just set x64/x86 using vsdevcmd? I believe that does default to x64 in the actions environment. |
|
I don't see anything here: pynacl/.github/workflows/ci.yml Lines 97 to 139 in 5848022
though I might be missing something |
|
In the wheel builder we set x86/x64 but we don't set MSVC version. In standard CI we don't set anything because setuptools actually handles this properly automatically these days -- we could almost certainly remove it on the wheel builder side. |
|
hm, I thought the compiler for the extension has to match the compiler used for building the static libsodium library, something setuptools can't know about. |
|
That was a hard requirement with MSVC and Python until the advent of the ucrt. These days you can have some amount of mismatch (see: https://wiki.python.org/moin/WindowsCompilers). Basically all 14.x are compatible, which started with Visual Studio 2015. |
|
I meant the compatibility of static linking in a library into an extension, not the compatibility between the resulting extension and CPython. But maybe that's also not an issue anymore. I remember having errors due to that in pycairo some years ago. |
|
Okay we've updated to latest 1.0.20-stable and updated the msvc zip, so this can be rebased without that task now. |
libsodium provides v143 builds (built with Visual Studio 2022) so we can use them and switch to building with VS2022 instead of VS2019. This will be helpful for future arm64 builds since libsodium only provides v143 arm64 binaries. While at it, make sure that "VsDevCmd.bat" is also called in the test job and not just in the wheel build job, to make sure that the same compiler setup is used in both places.
GitHub now provides native arm64 windows runners using the "windows-11-arm" image, so we can easily add it to the test matrix. Python ARM64 builds are only available since 3.11, so only test with Python 3.11 to 3.13. The arm64 libsodium binaries have been added with c5253ad
Use the new native "windows-11-arm" Windows arm64 image for building. Since arm64 Python is only available as of Python 3.11, use that for building instead of 3.9, which is used for the other arches.
|
Thanks! I split it up into three separate commits while at it. See the commit messages for details. |
|
Hmm, we should likely also move to abi3 builds but I can look at that in a different PR. Edit: Having a baby has melted my brain, we've done abi3 here for years. |
|
Thanks! |
|
Great work! When it will be available on PyPi? |
|
No specific timeline, but I will try to get a release out soon. |
Since GitHub Actions now has ARM64 Windows runners we can test and build natively.