Allow linking to zlib import library on Windows #8519
Conversation
If compiled as shared library zlib produces `zlib1.dll` and `zdll.lib`.
cubanpit
changed the title
Could you provide some more information about why your environment might be different to other environments that have used Pillow previously? |
|
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. |
|
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? |
|
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 |
setup.py
Outdated
| @@ -690,6 +690,8 @@ def build_extensions(self) -> None: | |||
| feature.set("zlib", "z") | |||
| elif sys.platform == "win32" and _find_library_file(self, "zlib"): | |||
| feature.set("zlib", "zlib") # alternative name | |||
| elif sys.platform == "win32" and _find_library_file(self, "zdll"): | |||
| feature.set("zlib", "zdll") # different name if shared | |||
There was a problem hiding this comment.
So then the different name is not because it is shared?
There was a problem hiding this comment.
As far as I understand it is, the .dll library is the shared library and zdll.lib helps linking to it, if zlib is built as static library I think only zlib.lib is generated on Windows.
The reason nobody tried this is that zlib as shared library is not a common use case, most people use static libraries on Windows, in particular for smaller ones.
There was a problem hiding this comment.
Good point. Perhaps
| feature.set("zlib", "zdll") # different name if shared | |
| feature.set("zlib", "zdll") # dll import library |
There was a problem hiding this comment.
As far as I understand it is, the
.dlllibrary is the shared library andzdll.libhelps linking to it, if zlib is built as static library I think onlyzlib.libis generated on Windows.
Correct, see Microsoft Docs : https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-creation#creating-an-import-library
Co-authored-by: Ondrej Baranovič <[email protected]>
|
Just to confirm - you've tested this locally and it works for you? |
|
Yes, I tested this change (with necessary adaptation) on versions 9.4.0, 10.3.0 and 11.0.0. |
radarhere
changed the title
|
So if a user has both a static A better "fix" would be to ask/patch upstream to align their 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...) |
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.