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

[Python][Release] Add delvewheel for our Windows wheels #33981

Closed
raulcd opened this issue Feb 1, 2023 · 4 comments
Closed

[Python][Release] Add delvewheel for our Windows wheels #33981

raulcd opened this issue Feb 1, 2023 · 4 comments

Comments

@raulcd
Copy link
Member

raulcd commented Feb 1, 2023

Describe the enhancement requested

For Python wheels on Linux/macOS we are using the corresponding auditwheel and delocate.

We should add delvewheel to our Windows wheels.

From the delvewheel GitHub Readme:

delvewheel is a command-line tool for creating Python wheel packages for Windows that have DLL dependencies that may not be present on the target system. It is functionally similar to [auditwheel](https://github.com/pypa/auditwheel) (for Linux) and [delocate](https://github.com/matthew-brett/delocate) (for macOS).

Suppose that you have built a Python wheel for Windows containing an extension module, but the wheel depends on DLLs that are present in the build environment but may not be present on the end user's machine. This tool determines which DLLs a wheel depends on (aside from system libraries) and copies those DLLs into the wheel.

We are currently copying manually the following which should be taken care of with delvewheel:

@REM bundle the msvc runtime
cp "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Redist\MSVC\14.16.27012\x64\Microsoft.VC141.CRT\msvcp140.dll" pyarrow\

https://github.com/apache/arrow/blob/master/ci/scripts/python_wheel_windows_build.bat#L122-L124

Component(s)

Python, Release

@raulcd
Copy link
Member Author

raulcd commented Dec 18, 2024

It seems this is not the encouraged, see mentioned issue above.

@raulcd raulcd closed this as not planned Won't fix, can't repro, duplicate, stale Dec 18, 2024
@jorisvandenbossche
Copy link
Member

@raulcd I think this is unrelated to the discussion about whether or not including the C++ runtime dll. We still have plently of DLLs that we have to include in the wheels (i.e. to start, the various arrow libraries). And as far as I understand, delvewheel can be used to properly include those in the wheels (instead of doing that manually).

@raulcd
Copy link
Member Author

raulcd commented Dec 18, 2024

oh! I misinterpreted the issue as it was only referencing msvcp140.dll

raulcd added a commit to raulcd/arrow that referenced this issue Dec 19, 2024
pitrou pushed a commit to raulcd/arrow that referenced this issue Jan 2, 2025
pitrou added a commit that referenced this issue Jan 4, 2025
…#35323)

### Rationale for this change

We need to ship the C++ standard library with our Windows wheels, as it is not guaranteed that a recent enough version is present on the system. However, some other Python libraries may require an even more recent version than the one we ship. This may incur crashes when PyArrow is imported before such other Python library, as the older version of the C++ standard library would be used by both.

### What changes are included in this PR?

Use a [fixed-up version](adang1345/delvewheel#59) of delvewheel that allows us to name-mangle an individual DLL, and name-mangle `msvcp140.dll` to ensure that other Python libraries do not reuse the version we ship.

### Are these changes tested?

By regular wheel build tests.

* Closes: #44855
* GitHub Issue: #33981
* GitHub Issue: #44855

Lead-authored-by: Antoine Pitrou <[email protected]>
Co-authored-by: Raúl Cumplido <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
@raulcd
Copy link
Member Author

raulcd commented Jan 16, 2025

@raulcd raulcd closed this as completed Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants