Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ksunden merged 1 commit intomatplotlib:v3.9.xfromQuLogic:static-msvc
Aug 12, 2024

Conversation

QuLogic
Copy link
Member

PR summary

This may prevent conflicts with other wheels that use the runtime at a different version, andmay be an alternative to#28679.

PR checklist

@QuLogicQuLogic added this to thev3.9.2 milestoneAug 8, 2024
@QuLogicQuLogic added the CI: Run cibuildwheelRun wheel building tests on a PR labelAug 8, 2024
Copy link
Member

@ianthomas23ianthomas23 left a 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 fromhttps://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 athttps://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.

@ksunden
Copy link
Member

For the record, this appears to makedelvewheel entirely unnecessary: It does not appear to do anything, either bundling dlls nor editing__init__.py files.

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 ata different version.
@QuLogic
Copy link
MemberAuthor

For the record, this appears to makedelvewheel entirely unnecessary: It does not appear to do anything, either bundling dlls nor editing__init__.py files.

This is only true for CPython; on PyPy, it bundlesVCRUNTIME140_1.dll.

@QuLogicQuLogic marked this pull request as ready for reviewAugust 9, 2024 23:19
@QuLogic
Copy link
MemberAuthor

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

@QuLogic
Copy link
MemberAuthor

On the latestv3.9.x build, we have:

$ objdump -x _c_internal_utils.cp313-win_amd64.pyd | rg 'DLL Name' | sortDLL Name: api-ms-win-crt-heap-l1-1-0.dllDLL Name: api-ms-win-crt-math-l1-1-0.dllDLL Name: api-ms-win-crt-runtime-l1-1-0.dllDLL Name: api-ms-win-crt-string-l1-1-0.dllDLL Name: KERNEL32.dllDLL Name: msvcp140-cb1364a8f14ec1d3687d6faef0fd327e.dllDLL Name: ole32.dllDLL Name: python313.dllDLL Name: SHELL32.dllDLL Name: USER32.dllDLL Name: VCRUNTIME140.dll

On this PR, we have:

$ objdump -x _c_internal_utils.cp313-win_amd64.pyd | rg 'DLL Name' | sortDLL Name: api-ms-win-crt-heap-l1-1-0.dllDLL Name: api-ms-win-crt-math-l1-1-0.dllDLL Name: api-ms-win-crt-runtime-l1-1-0.dllDLL Name: api-ms-win-crt-string-l1-1-0.dllDLL Name: KERNEL32.dllDLL Name: ole32.dllDLL Name: python313.dllDLL Name: SHELL32.dllDLL Name: USER32.dllDLL Name: VCRUNTIME140_1.dllDLL Name: VCRUNTIME140.dll

That is, we have droppedmsvcp140-cb1364a8f14ec1d3687d6faef0fd327e.dll, and addedVCRUNTIME140_1.dll. According to@zooba (who suggested this method in the first place), we can't avoid that using this method, but it should be fine, as Python 3.8+ included that DLL itself, and we require 3.9.

PyPy3.9doesn't includeVCRUNTIME140_1.dll, but it also doesn't includeVCRUNTIME140.dll (as CPython does), and says to install the vcredist on its download page.delvewheel does currently bundle the former, but I'm not sure if we should disable that and leave PyPy to the vcredist install.

@QuLogic
Copy link
MemberAuthor

We also saved a bit on the wheel sizes:

$ du -bc *.whl7959294matplotlib-3.9.1.post2.dev46+g3ed3d7b19a-cp310-cp310-win_amd64.whl7969068matplotlib-3.9.1.post2.dev46+g3ed3d7b19a-cp311-cp311-win_amd64.whl7972949matplotlib-3.9.1.post2.dev46+g3ed3d7b19a-cp312-cp312-win_amd64.whl7973076matplotlib-3.9.1.post2.dev46+g3ed3d7b19a-cp313-cp313-win_amd64.whl7954828matplotlib-3.9.1.post2.dev46+g3ed3d7b19a-cp39-cp39-win_amd64.whl7980386matplotlib-3.9.1.post2.dev46+g3ed3d7b19a-pp39-pypy39_pp73-win_amd64.whl47809601total

vs:

$ du -bc *.whl7819858matplotlib-3.10.0.dev46+gb9ddba6d6c-cp310-cp310-win_amd64.whl7829606matplotlib-3.10.0.dev46+gb9ddba6d6c-cp311-cp311-win_amd64.whl7833037matplotlib-3.10.0.dev46+gb9ddba6d6c-cp312-cp312-win_amd64.whl7833265matplotlib-3.10.0.dev46+gb9ddba6d6c-cp313-cp313-win_amd64.whl7814134matplotlib-3.10.0.dev46+gb9ddba6d6c-cp39-cp39-win_amd64.whl7845889matplotlib-3.10.0.dev46+gb9ddba6d6c-pp39-pypy39_pp73-win_amd64.whl46975789total

or just under 140K/2% less each.

@ianthomas23
Copy link
Member

PyPy3.9 doesn't include VCRUNTIME140_1.dll, but it also doesn't include VCRUNTIME140.dll (as CPython does), and says to install the vcredist on its download page. delvewheel does currently bundle the former, but I'm not sure if we should disable that and leave PyPy to the vcredist install.

I would be inclined to remove use of delvewheel.

@ksunden
Copy link
Member

I'll go ahead and merge, we can revisit possibly removing delvewheel entirely as a followup, particularly if we have problems on PyPy

@ksundenksunden merged commit7be8675 intomatplotlib:v3.9.xAug 12, 2024
41 checks passed
@QuLogicQuLogic deleted the static-msvc branchAugust 12, 2024 21:27
QuLogic added a commit to QuLogic/matplotlib that referenced this pull requestAug 15, 2024
As ofmatplotlib#28687, our extensions depend on `VCRUNTIME140_1.dll`, and thiswas allowed because Python 3.8+ started shipping that file. The originalreport inmatplotlib#18292 was for Python 3.7, which didn't ship the DLL, but werequire Python 3.10 now, so it's safe again.Since we can use that dependency, there's no need to disable the optionthat started requiring it in the first place. As noted in the originalblog post [1], this will make our extensions smaller, and slightlyfaster.[1]https://devblogs.microsoft.com/cppblog/making-cpp-exception-handling-smaller-x64/
@ianthomas23
Copy link
Member

Can someone confirm if and/or how these changes propagate through to the nightly wheels athttps://anaconda.org/scientific-python-nightly-wheels/matplotlib?

Certainly the changes are in the main branch:

CIBW_CONFIG_SETTINGS_WINDOWS:>-
setup-args="--vsenv"
setup-args="-Db_vscrt=mt"
setup-args="-Dcpp_link_args=['ucrt.lib','vcruntime.lib','/nodefaultlib:libucrt.lib','/nodefaultlib:libvcruntime.lib']"

The latest nightly wheel at time of writing for 3.12 ismatplotlib-3.10.0.dev548+g9f49b07ba9-cp312-cp312-win_amd64.whl and I can see that it was uploaded in ghahttps://github.com/matplotlib/matplotlib/actions/runs/10359513569/job/28676108425. But the new compilation flags do not appear to be used (see thepython -m pip wheel line):

Building cp312-win_amd64 wheelCPython 3.12 Windows 64bitInstalling Python cp312...                                                              ✓ 9.33sSetting up build environment...                                                              ✓ 6.35sInstalling build tools...                                                              ✓ 0.36sRunning before_build...                                                              ✓ 1.24sBuilding wheel...    + python -m pip wheel 'C:\Users\runneradmin\AppData\Local\Temp\cibw-sdist-1qoik8g_\matplotlib-3.10.0.dev548+g9f49b07ba9' '--wheel-dir=C:\Users\runneradmin\AppData\Local\Temp\cibw-run-taxp7jhy\cp312-win_amd64\built_wheel' --no-deps --config-settings=setup-args=--vsenv  Processing c:\users\runneradmin\appdata\local\temp\cibw-sdist-1qoik8g_\matplotlib-3.10.0.dev548+g9f49b07ba9Installing build dependencies: started    Installing build dependencies: finished with status 'done'    Getting requirements to build wheel: started    Getting requirements to build wheel: finished with status 'done'    Installing backend dependencies: started    Installing backend dependencies: finished with status 'done'    Preparing metadata (pyproject.toml): started    Preparing metadata (pyproject.toml): finished with status 'done'  Building wheels for collected packages: matplotlib    Building wheel for matplotlib (pyproject.toml): started    Building wheel for matplotlib (pyproject.toml): finished with status 'done'    Created wheel for matplotlib: filename=matplotlib-3.10.0.dev548+g9f49b07ba9-cp312-cp312-win_amd64.whl size=7795952 sha[256](https://github.com/matplotlib/matplotlib/actions/runs/10359513569/job/28676108425#step:5:266)=c9b1db3dd57b9ce2c6c1e920d496192fbbbd3f3f849df8b663124c0e66d1c09e    Stored in directory: c:\users\runneradmin\appdata\local\pip\cache\wheels\1d\8f\a3\78586ff923c54b655f8b92bad2fea456736530302a2803c5e9  Successfully built matplotlib                                                             ✓ 63.18sRepairing wheel...    + delvewheel repair -w C:\Users\runneradmin\AppData\Local\Temp\cibw-run-taxp7jhy\cp312-win_amd64\repaired_wheel C:\Users\runneradmin\AppData\Local\Temp\cibw-run-taxp7jhy\cp312-win_amd64\built_wheel\matplotlib-3.10.0.dev548+g9f49b07ba9-cp312-cp312-win_amd64.whl  C:\Users\runneradmin\AppData\Local\Temp\cibw-run-taxp7jhy\cp312-win_amd64\build\venv\Lib\site-packages\delvewheel\_wheel_repair.py:344: UserWarning: mpl_toolkits does not contain __init__.py. If it is a namespace package, use the --namespace-pkg option. Otherwise, create an empty __init__.py file to silence this warning.    warnings.warn(  repairing C:\Users\runneradmin\AppData\Local\Temp\cibw-run-taxp7jhy\cp312-win_amd64\built_wheel\matplotlib-3.10.0.dev548+g9f49b07ba9-cp312-cp312-win_amd64.whl  finding DLL dependencies  copying DLLs into matplotlib.libs  mangling DLL names  patching matplotlib\__init__.py  patching mpl_toolkits\__init__.py  repackaging wheel  fixed wheel written to C:\Users\runneradmin\AppData\Local\Temp\cibw-run-taxp7jhy\cp312-win_amd64\repaired_wheel\matplotlib-3.10.0.dev548+g9f49b07ba9-cp312-cp312-win_amd64.whl

anddelvewheel repairs the wheel which is what it used to do but should now be a no-op here. I am looking at this because this wheel is still failing my reproducer testshttps://github.com/ianthomas23/mpl-test/actions/runs/10417044468 and I was expecting the forward-ported changes to have made their way through to the nightly wheels by now.

@QuLogic
Copy link
MemberAuthor

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

@ianthomas23
Copy link
Member

Thanks. I am, understandably I hope, getting rather sensitive about this!

@QuLogic
Copy link
MemberAuthor

QuLogic commentedAug 17, 2024•
edited
Loading

So it's not specifically macOS, but that's just the fastest builder to hit the problem, which is buildingkiwisolver on PyPy (which we don't do on Windows for nightlies yet due to missing dependencies, so that's why it passed.)

Sincekiwisolver only has PyPy 3.9 wheels, and we are building nightlies on PyPy 3.10, it triggers a source build, and we are hitting a bug insetuptools:pypa/distutils#283 I debugged that and suggested a fix there, but not sure when that might be out.

I opened a PR on kiwisolver to enable PyPy 3.10 wheels:nucleic/kiwi#182

And a PR here to avoid the buggysetuptools:#28735

ianthomas23 reacted with thumbs up emoji

@ianthomas23
Copy link
Member

Published Matplotlib nightly wheels do now include the fix, and ContourPy CI is all passing.

story645 reacted with hooray emoji

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull requestSep 13, 2024
Convert to wheel.mk.Wrappers 1.4.7 | Solver 1.4.2 | 03/09/2024    no library changes only fixes to the build infrastructureWrappers 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 strategymatplotlib/matplotlib#28687
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ianthomas23ianthomas23ianthomas23 approved these changes

@ksundenksundenksunden approved these changes

Assignees
No one assigned
Labels
CI: Run cibuildwheelRun wheel building tests on a PR
Projects
None yet
Milestone
v3.9.2
Development

Successfully merging this pull request may close these issues.

3 participants
@QuLogic@ksunden@ianthomas23
[8]ページ先頭
©2009-2025 Movatter.jp