search

python-pillow / Pillow

Python Imaging Library (Fork)
https://python-pillow.org
Other
12.32k stars 2.23k forks source link

Allow linking to zlib import library on Windows #8519

Closed cubanpit closed 1 week ago

cubanpit commented 2 weeks ago

This allows to link to zlib when used as shared library, in my environment I do not activate all of the external libraries so that part might influence the result, but I think it can still be valuable for others to try the same.

radarhere commented 2 weeks ago

I do not activate all of the external libraries

Could you provide some more information about why your environment might be different to other environments that have used Pillow previously?

cubanpit commented 2 weeks ago

Sorry, I might have used a confusing phrasing. I compile Pillow with only a few external libraries, I do not include TIFF support for example, this is what I meant. I use a standard environment to compile Pillow, a Windows 10 container with MSVC 2022 build tools.

radarhere commented 2 weeks ago

Thanks. Perhaps I needed to phrase my question better - Pillow has been around for a while, and no one else has suggested supporting 'zdll' as far as I can see. Do you have any thoughts about what you are doing differently that means that you're the first? I'm not trying to say that this change isn't necessary, I'd just like to get a better understanding of the need.

Just to confirm - you've tested this locally and it works for you?

nulano commented 2 weeks ago

It seems to be the default name for the dll import library when building zlib from source: https://github.com/madler/zlib/blob/develop/win32/Makefile.msc#L18C14-L18C21

radarhere commented 2 weeks ago

Just to confirm - you've tested this locally and it works for you?

cubanpit commented 2 weeks ago

Yes, I tested this change (with necessary adaptation) on versions 9.4.0, 10.3.0 and 11.0.0.

kmilos commented 1 week ago

So if a user has both a static zlib.lib and the zdll.lib implib the static one will be preferred anyway and there is no way to link to the shared one?

A better "fix" would be to ask/patch upstream to align their win32/Makefile.msc w/ CMakeLists.txt to produce zlib.lib (implib, as already expected and supported) and explicit zlibstatic.lib instead...

Cf. https://gitlab.kitware.com/cmake/cmake/-/blob/894ac85d6ba202093601d2aca4aceaeefbe6c98e/Modules/FindZLIB.cmake#L115-L121 (search order differs depending on the intent; however, as the search order here matches the shared branch, one could leave this change as is I suppose...)