Allow linking to zlib import library on Windows by cubanpit · Pull Request #8519

Allow linking to zlib import library on Windows by cubanpit · Pull Request #8519 · python-pillow/Pillow

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.

If compiled as shared library zlib produces `zlib1.dll` and `zdll.lib`.

cubanpit changed the title ~~Allow linking to shared zlib~~ Allow linking to shared zlib on Windows

Nov 1, 2024

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?

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?

		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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So then the different name is not because it is shared?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Perhaps

	feature.set("zlib", "zdll") # different name if shared
	feature.set("zlib", "zdll") # dll import library

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co-authored-by: Ondrej Baranovič <ondreko.tiba@gmail.com>

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 ~~Allow linking to shared zlib on Windows~~ Allow linking to zlib import library on Windows

Nov 8, 2024