adang1345 / delvewheel

Self-contained Python wheels for Windows
MIT License
117 stars 12 forks source link

shows bundled dlls as 'needed' #5

Closed minrk closed 3 years ago

minrk commented 3 years ago

I'm so pleased to see a Windows wheel repair tool!

I'm trying this out because I've just made a pyzmq release with missing DLL dependencies and I'd like to use your tool to help not make that mistake in the future. However, my existing bundling is resulting in some unexpected behavior. Running on pyzmq 21 wheel for Python 3.8, I get:

Analyzing pyzmq-21.0.0-cp38-cp38-win_amd64.whl

The following dependent DLLs will be copied into the wheel.
    vcruntime140.dll (C:\Program Files\Python38\vcruntime140.dll)
    vcruntime140_1.dll (C:\Program Files\Python38\vcruntime140_1.dll)
    libzmq.cp38-win_amd64.pyd (Error: Not Found)
    msvcp140.dll (Error: Not Found)

The following dependent DLLs will not be copied into the wheel.
    advapi32.dll
    api-ms-win-crt-convert-l1-1-0.dll
    api-ms-win-crt-heap-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-utility-l1-1-0.dll
    iphlpapi.dll
    kernel32.dll
    python38.dll
    ws2_32.dll

The problem is that msvcp140.dll and libzmq.cp38-win_amd64.pyd are already present in the top-level of the wheel due to my own incomplete steps to bundle dependencies, so should be found and not added for bundling. I would also expect msvcp140.dll to be found, since it's in the compiler's runtime-redist directory. Is this not searched by default?

It can be found with:

from setuptools import msvc
from distutils.util import get_platform

vcvars = msvc.msvc14_get_vc_env(get_platform())
vcruntime = vcvars["py_vcruntime_redist"]
redist_dir, _ = os.path.split(vcruntime)

where all the vc++ redist dlls such as msvcp140.dll, vcruntime, etc. are in redist_dir. It seems like it would make sense to add this to the default search path if it exists.

adang1345 commented 3 years ago

At the moment, ctypes.util.find_library() is used to search for dependent shared libraries. As far as I can tell, this just looks through the PATH environment variable. I agree that it makes sense to add the compiler's runtime-redist directory as well to the search path by default.

We also don't check whether a needed DLL is already in the wheel. I agree that to work with other bundling tools or techniques that already place some DLLs into the wheel, we should better handle this situation.

minrk commented 3 years ago

Makes sense, thanks! I also think copying the vcruntime files from the Python root directory isn't the right thing to do, as they don't need to be shipped as a duplicate if they come with Python itself. That indicates the following changes should be made, if I understand correctly: