Build and test windows wheels

[skirpichev]

Closes #468

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.52%. Comparing base (38bf13e) to head (bbc5586).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #469   +/-   ##
=======================================
  Coverage   85.52%   85.52%           
=======================================
  Files          50       50           
  Lines       11656    11656           
  Branches     2202     2202           
=======================================
  Hits         9969     9969           
  Misses       1687     1687           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[casevh]

Why aren't the patches related to mp_bitcnt_t (sp?) carried over to the Windows compilation of GMP?

Question: Assuming we use delve-wheel, what GMP/MPFR/MPC libraries will a cython/C-API extension use when gmpy2 v2.2.0 is upgraded to v2.2.1? (The problem is (possible) shared internal state in the GMP library that is inherited by MPFR and MPC.)

[skirpichev]

Why aren't the patches related to mp_bitcnt_t (sp?) carried over to the Windows compilation of GMP?

That does make sense for me, but shouldn't you submit these patches to upstream? BTW, was this applied to previous stable release?

Assuming we use delve-wheel, what GMP/MPFR/MPC libraries will a cython/C-API extension use when gmpy2 v2.2.0 is upgraded to v2.2.1?

If we will bundle same versions of libraries across minor releases gmpy2 - I doubt there will be problems.

I think we could adopt scipy-like approach (for openblas), i.e. with separate wheels for libraries. Probably, python-flint could reuse these libraries. But this issue might be more complex to address just in the next release. So, in a meantime I'll adopt same approach as for previous gmpy2 releases: no mangling (unless it can't be disabled as with auditwheel), libraries are in the gmpy2.libs directory.

[skirpichev]

@casevh, I think it's ready for review. But I would appreciate if you take look on #470 first.

open issues:

• mp_bitcnt_t patch on M$ Windows
• enable --with-pic for all platforms?
• use windows-2022 image?

PS: As I have not access to M$ Windows - I would appreciate if someone else could test generated wheels.

[casevh]

I have done a quick test of the Python 3.11 Windows wheel and it worked just fine.

I don't think the version of Windows will matter but I would probably move to 2022 just to avoid having to move at some time in the future.

[skirpichev]

I would probably move to 2022 just to avoid having to move at some time in the future.

Done.

[eendebakpt]

I have done a quick test of the Python 3.11 Windows wheel and it worked just fine.

I don't think the version of Windows will matter but I would probably move to 2022 just to avoid having to move at some time in the future.

@casevh Where I can I find the windows wheels from this PR?

[casevh]

Most recents wheels should be at https://github.com/aleaxit/gmpy/actions/runs/8229996172/artifacts/1314269722

This is a later build than I've tested so let us know if it doesn't work for you.

[eendebakpt]

Most recents wheels should be at https://github.com/aleaxit/gmpy/actions/runs/8229996172/artifacts/1314269722

This is a later build than I've tested so let us know if it doesn't work for you.

Thanks! Installation went fine (python 3.12) and my code runs fine so far as I have tested.

[skirpichev]

Build wheels should be available with every commit, that has green CI check. Go to any "Build wheels" job and then to the "Summary" page.

Edit: last commit add documentation for building release wheels on M$. Let me know if this complicates code review: we can factor out this change for later prs.

[casevh]

Is this PR ready to merge?

Has the mpbitcnt_t patch been included?

Are there any ordering dependancies between the three PRs?

[skirpichev]

Is this PR ready to merge?

It's ready for review:)

Has the mpbitcnt_t patch been included?

No, for reasons I mentioned above.

Now I did this (last commit). This seems to be a bad idea, however.

Are there any ordering dependancies between the three PRs?

There is only one other ready for review pr: #471. I think it shouldn't conflict with the current one.

[casevh]

That does make sense for me, but shouldn't you submit these patches to upstream? BTW, was this applied to previous stable release?

The initial motivation to patch mp_bitcnt_t was based on this comment in the GMP manual:

"Counts of bits of a multi-precision number are represented in the C type mp_bitcnt_t. Currently this is always an unsigned long, but on some systems it will be an unsigned long long in the future."

Patched versions of GMP 6.2 were used for gmpy2 2.1.x.

The second patch is coding error in a function introduced in GMP 6.3. It has been reported.

The patch fixes the following problem. mpz_sizeinbase(mpz, 2) returns size_t which is 64 bits on 64-bit versions of Windows. But all the bit manipulation functions use mp_bitnct_t. Without the patch, mp_bitcnt_t is only 32 bits. Functions such as mpz_popcount(mpz) will fail for large nunbers with many bits set.

Assuming we use delve-wheel, what GMP/MPFR/MPC libraries will a cython/C-API extension use when gmpy2 v2.2.0 is upgraded to v2.2.1?

If we will bundle same versions of libraries across minor releases gmpy2 - I doubt there will be problems.

I think we could adopt scipy-like approach (for openblas), i.e. with separate wheels for libraries. Probably, python-flint could reuse these libraries. But this issue might be more complex to address just in the next release. So, in a meantime I'll adopt same approach as for previous gmpy2 releases: no mangling (unless it can't be disabled as with auditwheel), libraries are in the gmpy2.libs directory.

[skirpichev]

The initial motivation to patch mp_bitcnt_t was based on this comment in the GMP manual:

Perhaps, we should prepare a real patch and convince gmp people to accept it.

Patched versions of GMP 6.2 were used for gmpy2 2.1.x.

Ok, I see.

Should I adapt instructions in https://gmpy2.readthedocs.io/en/latest/install.html#installation to mention that patch? Sadly, we can't just instruct people to use gmp package from the msys2.

The second patch is coding error in a function introduced in GMP 6.3.

Sorry, I miss that. Is it was committed before? mingw64/msys2_64_shared.sh includes only the mp_bitcnt_t patch.

[skirpichev]

Last commit include the mp_bitcnt_t patch as a plain diff.

[casevh]

I tested a couple of the wheels and they look fine. I will merge this PR and will merge #474 later. Then I'll do some extensive testing on Windows.

[skirpichev]

Then I'll do some extensive testing on Windows.

Let me know if you find any problems.

I would appreciate, if we could make new alpha release, with support for the CPython 3.13.

BTW, as now we built wheels for all supported platforms in CI - you may consider using https://github.com/pypa/gh-action-pypi-publish to automate PyPI uploads. See mpmath workflow: https://github.com/mpmath/mpmath/blob/3512b74baa53122269a7e47a35f61e9d96b50725/.github/workflows/test.yml#L83-L88

[casevh]

I agree on a new alpha release with support for Python 3.13. I'd like to target a beta release shortly after Python 3.13.0b1 and a final release to coincide with the official release of 3.13.0.

[casevh]

FYI. Error in the building of the Windows wheels. Here is the error message:
Repairing wheel...

• scripts\cibw_repair_wheel_command_windows.bat C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\repaired_wheel C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\built_wheel\gmpy2-2.2.0a2-cp37-cp37m-win_amd64.whl

D:\a\gmpy\gmpy>set WHEELHOUSE=C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\repaired_wheel

D:\a\gmpy\gmpy>set WHEELNAME=C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\built_wheel\gmpy2-2.2.0a2-cp37-cp37m-win_amd64.whl

D:\a\gmpy\gmpy>msys2 -c scripts/cibw_repair_wheel_command_windows.sh

• wheel='C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\built_wheel\gmpy2-2.2.0a2-cp37-cp37m-win_amd64.whl'

• dest_dir='C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\repaired_wheel'

• delvewheel repair 'C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\built_wheel\gmpy2-2.2.0a2-cp37-cp37m-win_amd64.whl' -w 'C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\repaired_wheel' --add-path .local/bin --no-mangle-all
  repairing C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\built_wheel\gmpy2-2.2.0a2-cp37-cp37m-win_amd64.whl
  finding DLL dependencies
  copying DLLs into gmpy2-2.2.0a2.data\platlib
  skip mangling DLL names
  repackaging wheel
  fixed wheel written to C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\repaired_wheel\gmpy2-2.2.0a2-cp37-cp37m-win_amd64.whl

• cp .local/bin/gmp.lib .local/bin/mpfr.lib .local/bin/mpc.lib 'C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\repaired_wheel'

• cd 'C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\repaired_wheel'

• wheel unpack --dest . gmpy2-2.2.0a2-cp37-cp37m-win_amd64.whl
  Unpacking to: gmpy2-2.2.0a2...OK

• mv gmp.lib mpc.lib mpfr.lib 'gmpy2-/gmpy2.libs'
  mv: target 'gmpy2-/gmpy2.libs' is not a directory
  Error: Command scripts\cibw_repair_wheel_command_windows.bat C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\repaired_wheel C:\Users\runneradmin\AppData\Local\Temp\cibw-run-df5edqr9\cp37-win_amd64\built_wheel\gmpy2-2.2.0a2-cp37-cp37m-win_amd64.whl
  failed with code 1. None

                                                        ✕ 1.18s
  

Error: Process completed with exit code 1.

[skirpichev]

This is from #474? I'm working on this. It seems, the delvewheel, like the delocate-wheel, uses special directory in this case.

[casevh]

Just let me know if I need to push #474. Case

…

On Sun, Mar 24, 2024 at 10:06 PM Sergey B Kirpichev < ***@***.***> wrote: This is from #474 <#474>? I'm working on this. It seems, the delvewheel <https://github.com/adang1345/delvewheel/tree/bc55faf1ba382b7906b127ec9df15e35c44a941d/delvewheel>, like the delocate-wheel, uses special directory in this case. — Reply to this email directly, view it on GitHub <#469 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMR23YISLDW4L2KTBSUPDDYZ6WDZAVCNFSM6AAAAABEKNSXRCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJXGIZDCMJUGU> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>