Skip to content

Run delvewheel with path to required msvcp140.dll #28679

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Run delvewheel with path to required msvcp140.dll #28679

wants to merge 2 commits into from

Conversation

ianthomas23
Copy link
Member

Following recommendation at adang1345/delvewheel#52 (comment), here telling delvewheel to use the msvc140.dll from the correct directory when building wheels on Windows. The default behaviour without this is to search the PATH and as there are so many packages pre-installed on github runners that it finds the following

C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.422-5\x64\bin\msvcp140.dll
C:\Program Files\ImageMagick-7.1.1-Q16-HDRI\msvcp140.dll
C:\Windows\system32\msvcp140.dll
C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code\msvcp140.dll
C:\Program Files\LLVM\bin\msvcp140.dll

and we do not want it to use the first one.

I am hoping that this will fix issue #28551.

@ianthomas23 ianthomas23 added the CI: Run cibuildwheel Run wheel building tests on a PR label Aug 8, 2024
@ianthomas23
Copy link
Member Author

This is indeed packaging the correct msvcp140.dll, here is the relevant section of the build logs:

Repairing wheel...
  + delvewheel repair -vv --add-path C:\\Windows\\system32 -w C:\Users\runneradmin\AppData\Local\Temp\cibw-run-_hq65n1k\cp312-win_amd64\repaired_wheel C:\Users\runneradmin\AppData\Local\Temp\cibw-run-_hq65n1k\cp312-win_amd64\built_wheel\matplotlib-3.10.0.dev536+gdb47ec9b5e-cp312-cp312-win_amd64.whl
  C:\Users\runneradmin\AppData\Local\Temp\cibw-run-_hq65n1k\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(
  extracting matplotlib-3.10.0.dev536+gdb47ec9b5e-cp312-cp312-win_amd64.whl to C:\Users\RUNNER~1\AppData\Local\Temp\tmp1sg5zeul
  repairing C:\Users\runneradmin\AppData\Local\Temp\cibw-run-_hq65n1k\cp312-win_amd64\built_wheel\matplotlib-3.10.0.dev536+gdb47ec9b5e-cp312-cp312-win_amd64.whl
  finding DLL dependencies
  analyzing package-level extension module matplotlib\ft2font.cp312-win_amd64.pyd
  analyzing package-level extension module matplotlib\_c_internal_utils.cp312-win_amd64.pyd
  analyzing package-level extension module matplotlib\_image.cp312-win_amd64.pyd
  analyzing package-level extension module matplotlib\_path.cp312-win_amd64.pyd
  analyzing package-level extension module matplotlib\_qhull.cp312-win_amd64.pyd
  analyzing package-level extension module matplotlib\_tri.cp312-win_amd64.pyd
  analyzing package-level extension module matplotlib\_ttconv.cp312-win_amd64.pyd
  analyzing package-level extension module matplotlib\backends\_backend_agg.cp312-win_amd64.pyd
  analyzing package-level extension module matplotlib\backends\_tkagg.cp312-win_amd64.pyd
  External dependencies to copy into the wheel are
  {'msvcp140.dll'}
  External dependencies not to copy into the wheel are
  {   'api-ms-win-crt-convert-l1-1-0.dll',
      'api-ms-win-crt-environment-l1-1-0.dll',
      'api-ms-win-crt-filesystem-l1-1-0.dll',
      'api-ms-win-crt-heap-l1-1-0.dll',
      'api-ms-win-crt-locale-l1-1-0.dll',
      'api-ms-win-crt-math-l1-1-0.dll',
      'api-ms-win-crt-runtime-l1-1-0.dll',
      'api-ms-win-crt-stdio-l1-1-0.dll',
      'api-ms-win-crt-string-l1-1-0.dll',
      'api-ms-win-crt-time-l1-1-0.dll',
      'api-ms-win-crt-utility-l1-1-0.dll',
      'comctl32.dll',
      'kernel32.dll',
      'ole32.dll',
      'psapi.dll',
      'python312.dll',
      'shell32.dll',
      'user32.dll',
      'vcruntime140.dll',
      'vcruntime140_1.dll'}
  copying DLLs into matplotlib.libs
  copying C:\Windows\system32\msvcp140.dll -> C:\Users\RUNNER~1\AppData\Local\Temp\tmp1sg5zeul\matplotlib.libs\msvcp140.dll
  mangling DLL names
  repairing ft2font.cp312-win_amd64.pyd -> ft2font.cp312-win_amd64.pyd
  repairing _c_internal_utils.cp312-win_amd64.pyd -> _c_internal_utils.cp312-win_amd64.pyd
  repairing _image.cp312-win_amd64.pyd -> _image.cp312-win_amd64.pyd
  repairing _path.cp312-win_amd64.pyd -> _path.cp312-win_amd64.pyd
  repairing _qhull.cp312-win_amd64.pyd -> _qhull.cp312-win_amd64.pyd
  repairing _tri.cp312-win_amd64.pyd -> _tri.cp312-win_amd64.pyd
  repairing _ttconv.cp312-win_amd64.pyd -> _ttconv.cp312-win_amd64.pyd
  repairing _backend_agg.cp312-win_amd64.pyd -> _backend_agg.cp312-win_amd64.pyd
  repairing _tkagg.cp312-win_amd64.pyd -> _tkagg.cp312-win_amd64.pyd
  repairing msvcp140.dll -> msvcp140-09080ac70[276](https://github.com/matplotlib/matplotlib/actions/runs/10297789361/job/28501611415?pr=28679#step:5:287)21632c0b28e3c03ff757.dll
  patching matplotlib\__init__.py
  patching mpl_toolkits\__init__.py
  updating matplotlib-3.10.0.dev536+gdb47ec9b5e.dist-info\RECORD

Looking at the DLL properties indicates that the DLL version is 14.40.33810.0, again confirmation that this is recent.

Using this locally with a source build of ContourPy show the full test suite passing. The same tests using Matplotlib 3.9.1.post0 fail. So it is looking promising so far.

I will reenable building the complete set of wheels for further testing.

@ianthomas23 ianthomas23 mentioned this pull request Aug 8, 2024
Closed
@ianthomas23
Copy link
Member Author

@ksunden I see that you did a similar thing to kiwisolver recently. Is the path you used there better than this one?

@tacaswell tacaswell added this to the v3.9.2 milestone Aug 8, 2024
@ksunden
Copy link
Member

ksunden commented Aug 8, 2024

That path was provided only for the cross compilation for ARM, so I do not think it applies for us.

That said, may wish to go back and specify for the x64 builds there to be safe

@QuLogic QuLogic mentioned this pull request Aug 8, 2024
Merged
5 tasks
@ianthomas23
Copy link
Member Author

Closing this as #28687 is a better solution.

@ianthomas23 ianthomas23 deleted the delvewheel-add-path branch August 12, 2024 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CI: Run cibuildwheel Run 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
@ianthomas23 @ksunden @tacaswell