Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile LibTIFF with CMake on Windows #5359

Merged
merged 3 commits into from
Apr 1, 2021
Merged

Compile LibTIFF with CMake on Windows #5359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f4deb4d
compile libtiff with cmake
nulano Aug 30, 2020
1105c5d
Updated nasm to 2.15.05
radarhere Mar 24, 2021
7c48f12
add explanatory comment for USE_WIN32_FILEIO
nulano Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/test-windows.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ jobs:
- name: Install dependencies
id: install
run: |
7z x winbuild\depends\nasm-2.14.02-win64.zip "-o$env:RUNNER_WORKSPACE\"
echo "$env:RUNNER_WORKSPACE\nasm-2.14.02" >> $env:GITHUB_PATH
7z x winbuild\depends\nasm-2.15.05-win64.zip "-o$env:RUNNER_WORKSPACE\"
echo "$env:RUNNER_WORKSPACE\nasm-2.15.05" >> $env:GITHUB_PATH

winbuild\depends\gs9533w32.exe /S
echo "C:\Program Files (x86)\gs\gs9.53.3\bin" >> $env:GITHUB_PATH
Expand Down
8 changes: 5 additions & 3 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,11 @@ def build_extensions(self):
if feature.tiff:
libs.append(feature.tiff)
defs.append(("HAVE_LIBTIFF", None))
# FIXME the following define should be detected automatically
# based on system libtiff, see #4237
if PLATFORM_MINGW:
if sys.platform == "win32":
# This define needs to be defined if-and-only-if it was defined
# when compiling LibTIFF. LibTIFF doesn't expose it in `tiffconf.h`,
# so we have to guess; by default it is defined in all Windows builds.
# See #4237, #5243, #5359 for more information.
defs.append(("USE_WIN32_FILEIO", None))
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

@kmilos kmilos Mar 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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...

Copy link
Contributor

@kmilos kmilos Mar 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 in tiffconf.h,
so we have to guess; by default it is defined in all Windows builds.
See #4237, #5243, #5359 for more information.

if feature.xcb:
libs.append(feature.xcb)
Expand Down
6 changes: 3 additions & 3 deletions winbuild/build_prepare.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ def cmd_msbuild(
"filename": "tiff-4.2.0.tar.gz",
"dir": "tiff-4.2.0",
"build": [
cmd_copy(r"{winbuild_dir}\tiff.opt", "nmake.opt"),
cmd_nmake("makefile.vc", "clean"),
cmd_nmake("makefile.vc", "lib"),
cmd_cmake("-DBUILD_SHARED_LIBS:BOOL=OFF"),
cmd_nmake(target="clean"),
cmd_nmake(target="tiff"),
],
"headers": [r"libtiff\tiff*.h"],
"libs": [r"libtiff\*.lib"],
Expand Down
220 changes: 0 additions & 220 deletions winbuild/tiff.opt
View file
Open in desktop

This file was deleted.