-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
BLD: Include MSVCP140 runtime statically #28687
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the Windows wheels from https://github.com/matplotlib/matplotlib/actions/runs/10314253959 locally against both a ContourPy clean install and nightly wheels (without any use of delvewheel) and they pass the full test suite and the simple import-to-exception reproducer. I've also tested them in github runners at https://github.com/ianthomas23/mpl-test/actions/runs/10318743303; it is the last 3 tests that use the new statically-linked mpl wheels.
On that basis I would happy to see this merged and included in a new release and nightly wheels whenever that is possible.
For the record, this appears to make I'm wondering if we go this route do we wish to remove that step entirely as it is a no-op, or do we wish to keep it as a "if we ever need it again, we want it to just work and not have to remember we need it"? Personally I lean towards removing the thing we are not using, but willing to be convinced otherwise. |
This should prevent conflicts with other wheels that use the runtime at a different version.
This is only true for CPython; on PyPy, it bundles |
I modified @BertJorissen's example to test all Python versions and then take the wheels from this PR, and they all passed: https://github.com/QuLogic/mplpb11/actions/runs/10326876201 |
On the latest
On this PR, we have:
That is, we have dropped PyPy3.9 doesn't include |
We also saved a bit on the wheel sizes:
vs:
or just under 140K/2% less each. |
I would be inclined to remove use of delvewheel. |
I'll go ahead and merge, we can revisit possibly removing delvewheel entirely as a followup, particularly if we have problems on PyPy |
As of matplotlib#28687, our extensions depend on `VCRUNTIME140_1.dll`, and this was allowed because Python 3.8+ started shipping that file. The original report in matplotlib#18292 was for Python 3.7, which didn't ship the DLL, but we require Python 3.10 now, so it's safe again. Since we can use that dependency, there's no need to disable the option that started requiring it in the first place. As noted in the original blog post [1], this will make our extensions smaller, and slightly faster. [1] https://devblogs.microsoft.com/cppblog/making-cpp-exception-handling-smaller-x64/
Can someone confirm if and/or how these changes propagate through to the nightly wheels at https://anaconda.org/scientific-python-nightly-wheels/matplotlib? Certainly the changes are in the main branch: matplotlib/.github/workflows/cibuildwheel.yml Lines 108 to 111 in 86f04cb
The latest nightly wheel at time of writing for 3.12 is
and |
The Windows wheel is correctly built for the ones that made it: https://github.com/matplotlib/matplotlib/actions/runs/10415168089/job/28845271052#step:4:305 but the full wheel builds have not yet passed since the merge up due to issues on macOS: https://github.com/matplotlib/matplotlib/actions/runs/10415168089/job/28845271328 |
Thanks. I am, understandably I hope, getting rather sensitive about this! |
So it's not specifically macOS, but that's just the fastest builder to hit the problem, which is building Since I opened a PR on kiwisolver to enable PyPy 3.10 wheels: nucleic/kiwi#182 And a PR here to avoid the buggy |
Published Matplotlib nightly wheels do now include the fix, and ContourPy CI is all passing. |
Convert to wheel.mk. Wrappers 1.4.7 | Solver 1.4.2 | 03/09/2024 no library changes only fixes to the build infrastructure Wrappers 1.4.6 | Solver 1.4.2 | 03/09/2024 drop support for Python 3.7 PR #183 add support for Python 3.13 PR #183 update linking strategy on Windows when building wheels PR #183 This is in line with Matplotlib strategy matplotlib/matplotlib#28687
PR summary
This may prevent conflicts with other wheels that use the runtime at a different version, and may be an alternative to #28679.
PR checklist