ENH: support shared libraries on Windows when explicitly enabled #551
Conversation
dnicolodi
force-pushed
the
shared-libs-win32
branch
2 times, most recently
from
950e49d
to
d56441c
Compare
Assume all platforms except Windows and macOS use ELF binaries.
dnicolodi
force-pushed
the
shared-libs-win32
branch
2 times, most recently
from
eaa7d4f
to
d8e872a
Compare
dnicolodi
force-pushed
the
shared-libs-win32
branch
from
d8e872a
to
1778ab6
Compare
| and uses RPATH or equivalent mechanisms to have Python modules and native | ||
| executables load them form there. Windows does not have an equivalent | ||
| mechanism to set the DLL load path. Supporting shared libraries on Windows | ||
| requires collaboration from the package. To make sure that package authors |
There was a problem hiding this comment.
This doesn't seem to explain (or link to something that explains) what sort of "collaboration" is needed.
| @@ -430,6 +432,9 @@ def _install_path(self, wheel_file: mesonpy._wheelfile.WheelFile, origin: Path, | |||
|
|
|||
| if self._has_internal_libs: | |||
| if _is_native(origin): | |||
| if sys.platform == 'win32' and not self._shared_libs_win32: | |||
| raise NotImplementedError(f'Bundling libraries in wheel is not supported on {sys.platform}') | |||
There was a problem hiding this comment.
This should mention the configuration option?
| @@ -757,6 +763,10 @@ def __init__( | |||
| if not value: | |||
| self._limited_api = False | |||
|
|
|||
| # Shared library support on Windows requires collaboration | |||
| # from the package, make sure the developpers aknowledge this. | |||
There was a problem hiding this comment.
| # from the package, make sure the developpers aknowledge this. | |
| # from the package; make sure the developers acknowledge this. |
There was a problem hiding this comment.
This looks like a good approach, I like it. A couple of thoughts:
-
Internal shared libraries can be made to work today already in a package using
meson-python, when explicitly installing the shared library without the package and usingdelvewheel(in an elaborate way) andinstall_rpathon other platforms, as demonstrated by BUG: Fix for scipy.special seterr, geterr, errstate scipy/scipy#20321. I think this PR is fine as is; I did have a concern that not having the option set shouldn't start raising an exception, so let's make sure that that remains the case. -
As @QuLogic commented, it'd be good to elaborate on the best ways for a package to do this. I think there are two:
- Using
os.add_dll_directory, which should be reliable on Python >= 3.10 - Preloading the DLL with
ctypes.WinDLL- we've done this for a long time in NumPy and SciPy already for external shared libraries: https://github.com/scipy/scipy/blob/bb92c8014e21052e7dde67a76b28214dd1dcb94a/tools/openblas_support.py#L239-L274 (it should require fewer lines of code for an internal shared library that will always be present)
- Using
No description provided.