Skip to content
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

maint: use msys2-ucrt instead of msys2-mingw64 #41

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

Conversation

oscarbenjamin
Copy link
Collaborator

I think that linking against ucrt is preferable rather than msvcrt although currently it doesn't seem to cause any problems.

There is some information about this here:
https://www.msys2.org/docs/environments/

My understanding is that python.org builds of CPython link against ucrt from Python 3.5 onwards:
https://matthew-brett.github.io/pydagogue/python_msvc.html

@oscarbenjamin
Copy link
Collaborator Author

@oscarbenjamin
Copy link
Collaborator Author

This hasn't worked. The delvewheel output in the log has

2022-12-21T18:08:39.3257576Z External dependencies to copy into the wheel are
2022-12-21T18:08:39.3257972Z {   'libarb-2.dll',
2022-12-21T18:08:39.3258340Z     'libflint-17.dll',
2022-12-21T18:08:39.3258670Z     'libgcc_s_seh-1.dll',
2022-12-21T18:08:39.3259030Z     'libgmp-10.dll',
2022-12-21T18:08:39.3259388Z     'libmpfr-6.dll',
2022-12-21T18:08:39.3259757Z     'libwinpthread-1.dll'}
2022-12-21T18:08:39.3260178Z External dependencies not to copy into the wheel are
2022-12-21T18:08:39.3260656Z {'msvcrt.dll', 'kernel32.dll', 'user32.dll', 'python311.dll'}

So it's still linked against msvcrt.dll.

@oscarbenjamin
Copy link
Collaborator Author

The problem is that which gcc in CI gives:

/c/ProgramData/Chocolatey/bin/gcc

So it's picking up the wrong gcc somehow.

@oscarbenjamin
Copy link
Collaborator Author

Okay, this now fails in CI in the same way that it fails locally:

Error: Command python -c "import flint; print(str(flint.fmpz(2)))"
failed with code 3221225477. None

So the wheel gets built but crashes when used.

Basically with current master everything in the wheel is linked to msvcrt.dll. Although the wheel uses msvcrt.dll it does so consistently and that seems to work okay when the _flint.pyd is loaded into CPython which links with ucrt DLLs instead.

The changes here make it so that the dependencies are built linking to ucrt but the _flint.pyd itself still links to msvcrt. Having this inconsistency between _flint.pyd and the other DLLs in the shipped wheel makes it crash somehow. On the other hand it seems to work okay if everything in the wheel consistently uses msvcrt.dll even though that's inconsistent with what CPython itself does.

Having looked at other wheels I find that the wheels for numpy, scipy, gmpy2 etc also link to msvcrt.dll or vcruntime140.dll. Since scipy 1.9.0 the wheels are now ucrt only but this is a new thing that only happened when moving from setuptools/distutils to meson.

Also python-flint needs to move on from distutils and numpy.distutils which are both to be removed in Python 3.12. I propose to leave this PR as it is for now. I think it's fine for now to link against msvcrt.dll. The problem does need to be solved but so also does migrating to a new build system and it might be easier to solve the problem when that happens.

So there is a problem but and it needs to be solved soonish but I don't think it's a blocker. This PR can help as a starting point when it comes time to solve this but that can wait for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant