nucleic / kiwi

Efficient C++ implementation of the Cassowary constraint solving algorithm
https://kiwisolver.readthedocs.io/en/latest/
Other
692 stars 88 forks source link

Enable delvewheel in CIBW on Windows #179

Closed ksunden closed 4 months ago

ksunden commented 4 months ago

This will package the MSVC runtime DLL in a way that works reliably and does not require external C dependencies to be installed when installing the wheel.

This is what we do in Matplotlib, and we have had a handful of reports of failure to import caused by _cext within kiwisolver, this should resolve those.

Closes #178

I haven't tried with PyInstaller, but possibly related to #168 as well.

See also #173

MatthieuDartiailh commented 4 months ago

Thanks for taking the time to do this. I will merge and plan to make a new release ASAP.

MatthieuDartiailh commented 4 months ago

I did a test run https://github.com/nucleic/kiwi/actions/runs/9789785072/job/27030113241 which failed to find the right dll. I assume I could remove the option older redistributable version but I am curious if you have seen such issue for matplotlib.

ksunden commented 4 months ago

Hmmm... most likely related to Windows ARM builds specifically, which we don't do in Matplotlib, so have not had any issues. Nothing obvious on initial look, but can look into it more later.

If that is the case, worst case is to take it out of the ARM build (which is separate section here, so not hard to do)

MatthieuDartiailh commented 4 months ago

To maximize compatibility I currently disable linking against VC2014_1. The runner may be missing the old dll no ?

ksunden commented 4 months ago

https://github.com/adang1345/delvewheel/issues/44#issuecomment-1966515052

Looks like I was right about the arm being the problem (since it's cross compiling the PATH is picking up x64 dlls)

ksunden commented 3 months ago

Well, as it turns out, shortly after I made this PR, we discovered some problems with msvcp*.dll... After a fair bit of searching for solutions, and consultation with Microsoft Engineers, we decided to actually pass the proper flags to statically link msvcp, removing both the need to have it installed separately or problems with mixed versions.

Here is what we ended up doing:

https://github.com/matplotlib/matplotlib/pull/28687

We have not removed delvewheel yet, though it only affects pypy now, since CPython bundles the vcruntime*.dll, which we still dynamically link to, but does not have to be in wheels.