Compile LibTIFF with CMake on Windows #5359
Conversation
nulano
changed the title
|
Great - I reckon 8.2 is a good time to make the switch and I/O API change as any, cmake has been supported by libtiff for a while already... |
| # FIXME the following define should be detected automatically | ||
| # based on system libtiff, see #4237 | ||
| if PLATFORM_MINGW: | ||
| if sys.platform == "win32": | ||
| defs.append(("USE_WIN32_FILEIO", None)) |
There was a problem hiding this comment.
So if this PR "switches winbuild to compile libtiff using CMake which enables USE_WIN32_FILEIO on all win32 platforms by default"... why do we need this def?
There was a problem hiding this comment.
Have to keep Pillow's src/libImaging/TiffDecode.c in sync w/ the way libtiff was built, i.e. because of the whole mess behind #4237 (this fix was intoduced in #4890); maybe better leave the comment for posterity?
Unfortunately libtiff doesn't expose this build time defined macro in a public header like it does for some other (similar) stuff, so one has to set it externally...
There was a problem hiding this comment.
Alternatively we could do away with this case and define completely, and just use _WIN32 in TiffDecode.c directly if we take a leap of faith and assume all Windows libtiff builds will have Win32 I/O enabled from now on, like e.g. Poppler does since 2014.
There was a problem hiding this comment.
LibTIFF also checks this define in tiffio.h when typedefing thandle_t. While all alternatives have the same binary size, it is probably safest to define the macro to make sure. It is also probably simpler to set it in setup.py instead of just TiffDecode.c.
I added a longer explanatory comment to this line:
This define needs to be defined if-and-only-if it was defined
when compiling LibTIFF. LibTIFF doesn't expose it intiffconf.h,
so we have to guess; by default it is defined in all Windows builds.
See #4237, #5243, #5359 for more information.
The downside of waiting is that when it is released (which could happen at any point in time), we will be failing to support the latest libtiff until our next scheduled release. |
|
Btw, I found it interesting that the vcpkg build of libitiff turns off Win32 I/O for UWP only... Maybe it really is not available on the phone, don't know that platform well at all... All other Windows builds I know (libtiff cmake & autotools default, mingw, vcpkg, anaconda...) have it enabled, with the exception of conda-forge that patched it off because of #4237. If we merge this for all Windows (not just mingw), they will have to remove the patch, or keep patching Pillow as well... |
|
Thanks all! |
Alternative to #5243, see the discussion there for more information.
Switches winbuild to compile libtiff using CMake which enables
USE_WIN32_FILEIOon all win32 platforms by default.Compiling with NMake will not be supported when the next version of libtiff is released, see https://gitlab.com/libtiff/libtiff/-/merge_requests/223.