maint: add CI jobs to build and test a Windows wheel #26
Conversation
oscarbenjamin
force-pushed
the
pr_windows_ci
branch
2 times, most recently
from
fb9bea7 to
1441eb5
Compare
oscarbenjamin
force-pushed
the
pr_windows_ci
branch
from
1441eb5 to
dfa430a
Compare
|
I've rebased this on top of #28 |
bin/build_mingw64_wheel.sh
Outdated
| # Make the wheel relocatable | ||
| delvewheel repair dist/python_flint-0.3.0-cp310-cp310-win_amd64.whl \ | ||
| --add-path .local/bin:.local/lib/ \ | ||
| '--no-mangle=libflint-17.dll;libarb-2.dll;libgmp-10.dll;libmpfr-6.dll;libgcc_s_seh-1.dll' |
There was a problem hiding this comment.
Not mangling common DLLs like libgcc_s_seh-1.dll is going to cause conflicts with other libraries using the same DLL.
There was a problem hiding this comment.
Yes, I'm not entirely sure how to make it happen though. The suggestion is to use strip but I don't know exactly which file should be stripped. If it's the generated .pyd that needs to be stripped then that's awkward to arrange for because it gets generated inside distutils and comes out the other end buried inside a wheel.
There was a problem hiding this comment.
Looks like scipy just unpacks the wheel and runs strip on all DLLs:
https://github.com/scipy/scipy/blob/502df0ece7eb9525de8a7f243044b3fe18ad5cca/tools/wheels/repair_windows.sh#L14-L20
There was a problem hiding this comment.
I've used strip and removed the --no-mangle argument here and it seems to work.
setup.py
Outdated
| @@ -9,7 +9,7 @@ | |||
| from distutils.sysconfig import get_config_vars | |||
|
|
|||
| if sys.platform == 'win32': | |||
| libraries = ["arb", "flint", "mpir", "mpfr", "pthreads"] | |||
| libraries = ["arb", "flint", "mpfr", "gmp"] | |||
There was a problem hiding this comment.
Can this be configurable? It depends on if we use MinGW or MSVC toolchains
There was a problem hiding this comment.
I think I'll peg this to an environment variable for now. Longer term I think we want a build system that doesn't use setup.py or distutils at all and that probably means that exactly how it would be configured would be quite different.
|
So the Windows job runs fine, builds a wheel and then passes all tests. The failing tests are 32 bit Linux and I think are some other problem with the CI arrangement maybe. |
oscarbenjamin
force-pushed
the
pr_windows_ci
branch
2 times, most recently
from
d2bd567 to
70b3fde
Compare
oscarbenjamin
force-pushed
the
pr_windows_ci
branch
from
fca4a5e to
8000775
Compare
|
Okay, that was a long slog! With this PR we now have Windows wheels built and tested in CI along with Linux and OSX wheels. I knew it would be hard to get Windows working which is why I've put this off for almost two years. It's working now in CI and I've also tested downloading the wheels and using them locally. Hopefully that means that no one needs to actually use Windows directly any more! The most significant remaining item is OSX on ARM but that's much less of an issue than Windows 64-bit. I'm going to merge this now and then do two things:
After those are done I think it's time for a new release of python-flint with binary wheels on PyPI. |
|
That's awesome! Thanks! |
This adds a script to build a wheel on Windows amd64 using mingw-w64 under msys2 based on the recipe discussed in #1. Another CI job will test the installed wheel.
The resulting wheel is broken because of at least #10 if not other problems as well. With this though it should at least in principle be possible to see in CI whether or not the module works on Windows.
I haven't integrated this with the other build scripts yet although mostly this leverages the existing
bin/build_dependencies_unix.shscript. Currently this changessetup.pywhich it probably shouldn't so I should come up with a better solution for that before this is merged (presumably someone is depending on that Windows specific code in the setup.py). I don't think it's worth trying to integrate building Windows wheels with the other cibuildwheel setup until at least it is possible to build a working wheel on Windows.Both setup.py, distutils and numpy.distutils are deprecated and numpy.distutils will apparently be removed in Python 3.12. A better fix for the build system would be to use
buildas a frontend:https://pypa-build.readthedocs.io/en/latest/
Then the backend would be something like meson-python or cmake I guess:
https://labs.quansight.org/blog/2021/07/moving-scipy-to-meson
If eventually a future build system will be used it's probably better to leave setup.py alone so that it can continue to work for anyone who is still using it.
Once I've reverted the changes to setup.py I suggest: