build(cmake): guard libtiff's missing Deflate target - #5313
Conversation
Static libtiff package exports can reference Deflate::Deflate without importing the target. Quietly load libdeflate's config and supply the alias before TIFF discovery when needed. Fixes AcademySoftwareFoundation#4439 Assisted-by: Codex / GPT-5 Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
Reference the upstream libtiff report from the defensive target guard. Assisted-by: Codex / GPT-5 Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a CMake-side compatibility guard in OIIO’s dependency discovery to prevent configuration failures when a previously installed/static libtiff config exports TIFF::tiff with an unresolved link interface dependency on Deflate::Deflate (libtiff config-mode defect).
Changes:
- Before
checked_find_package(TIFF), conditionallyfind_package(libdeflate CONFIG QUIET)ifDeflate::Deflatedoes not already exist. - Alias
Deflate::Deflatetolibdeflate::libdeflate_staticorlibdeflate::libdeflate_sharedwhen available.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Define libtiff's missing Deflate target before the installed static OpenImageIO package resolves its TIFF dependency. Assisted-by: Codex / GPT-5 Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
| # Static libtiff configs may reference this target without importing it. | ||
| # https://gitlab.com/libtiff/libtiff/-/work_items/871 | ||
| if (NOT TARGET Deflate::Deflate) | ||
| find_package (libdeflate CONFIG QUIET) |
There was a problem hiding this comment.
Internally, we use checked_find_package, which has lots of bells and whistles, including that it will be included in the build log report of which dependencies were found at which version.
There was a problem hiding this comment.
There's something about when + how checked_find_package does what it does that makes it difficult for the config that libtiff sets up that fails to survive a rebuild, when the locally-built libtiff is rediscovered... I don't really understand enough about what's happening behind the scenes to grok exactly what's going on. But the only thing that seems to work is forcing tiff to always build locally (when rebuilding using an existing build dir persisting previously built deps).
There was a problem hiding this comment.
so... what do we do here? I really don't want to set a precedent of having some packages use a bare find_package, which bypasses the nice version report, as well as preclude using the auto-builders.
There was a problem hiding this comment.
No, I understand -- I'll sic a robot on this to better understand why this is happening and how to fix in checked_find_package -- in a worst case scenario, we might have to do some contrived patching of libtiff's find module for deflate, or patch deflate itself. I went through discovering all of this a long time ago, and now it's all coming back to me... this was very confusing, and I still don't have the language to describe what I'm experiencing concisely. Hopefully robots can help with that, too.
In the meantime, no pressure -- let's not merge until we have something that works with checked_find_package. I'll dig into this as I have cycles, and/or maybe libtiff folks will address this upstream. I'll mark this PR as a draft for now.
There was a problem hiding this comment.
I have no doubt that there's room for improvement of checked_find_package. Among other things, the auto-build works well when the dependency is not found at all, but has some sharp edges if another copy (particular one that was rejected for being older than our required minimum) is elsewhere on the system, sometimes it can leak into the components we are trying to build and use locally. I suspect there are various ways my cobbled-together cmake code in that part of the code has some weaknesses.
There was a problem hiding this comment.
Got to the bottom of it. Two things compose:
- libtiff's installed config exports
TIFF::tiffreferencingDeflate::Deflatebut never imports it (their template still says# TODO: import dependencies). - Since CMake 3.29,
FindTIFFprobesfind_package(Tiff CONFIG QUIET)first, so even our module-modechecked_find_package(TIFF)loads that config when one is on the prefix path — and the local deps dist dir always is.
On a fresh run, build_TIFF.cmake defines Deflate::Deflate before the export loads. On reconfigure, the cached TIFF is found, the recipe is skipped, nothing defines the target, generate fails. The prerequisite lived inside the recipe that gets skipped exactly when its output is rediscovered. (Hence BUILD_LOCAL_DEPS=TIFF "working" — it re-runs the recipe.)
Per your point, the guard is now checked_find_package (libdeflate VERSION_MIN 1.18) before TIFF discovery — dependency report and auto-builders both work through it (with system packages hidden, it auto-built libdeflate itself). The consumer-side guard in the installed OIIO config keeps plain find_package, since checked_find_package doesn't exist there. Verified the fresh-build → reconfigure cycle fails on main and passes here, on CMake 4.3.1 and 3.29.6; healthy system-libtiff configures unaffected.
Proof it unblocks real work: I have a working branch adding a local-build recipe for libjxl (plus submodule support in the dep builder). With this fix it builds and links; on main it can't even complete an incremental reconfigure. That's the loop that kept blocking me from adding recipes for the complex dependencies.
Filing separately: the installed static-OIIO config also can't stand alone for CMath::CMath/GIF::GIF; and your "sharp edges" comment confirmed — dep child builds can resolve system packages the parent is ignoring (CMAKE_IGNORE_PATH doesn't cover config searches; a Homebrew Imath 3.2 got into a local OpenEXR build and broke the link against our Imath 3.1.10). One-line fix via CMAKE_IGNORE_PREFIX_PATH, small PR coming.
CI is green on the updated branch, so I've marked this ready for review.
Route the pre-TIFF libdeflate guard through checked_find_package so it appears in the dependency report and can use the local auto-builders, rather than a bare find_package. Also document why the guard must precede TIFF discovery: since CMake 3.29, FindTIFF probes the config package first, so a previously auto-built static libtiff rediscovered from the local deps cache is loaded after build_TIFF.cmake (which formerly supplied the Deflate::Deflate alias) has been skipped. Assisted-by: Claude Code (Fable 5) Signed-off-by: Zach Lewis <zachcanbereached@gmail.com>
lgritz
left a comment
There was a problem hiding this comment.
LGTM.
Thanks for the fixes, I am happier with this being checked_find_package so that it shows up properly in the build report.
Description
Fixes configuration failures when OIIO finds a previously autobuilt static libtiff package whose exported
TIFF::tifftarget referencesDeflate::Deflatewithout importing or defining that target.Before TIFF discovery, this adds a narrow compatibility guard that quietly loads libdeflate's config package and aliases its static or shared target as
Deflate::Deflatewhen necessary.Fixes #4439.
Root cause
libtiff links static TIFF builds against
Deflate::Deflate, causing the installedTIFF::tifftarget to export$<LINK_ONLY:Deflate::Deflate>.However, libtiff's installed
tiff-config.cmakeloads that exported target without importing its dependencies. The corresponding upstream template still contains# TODO: import dependencies.Upstream report: https://gitlab.com/libtiff/libtiff/-/work_items/871
OIIO encounters this during
checked_find_package(TIFF). CMake'sFindTIFFmodule first attempts libtiff's config package, which exposes the malformed export. This is not specific to CMake 4.3: I reproduced it with CMake 3.29.6 and 4.3.1. Issue #4439 likewise originally reported CMake 3.29+ on Linux and macOS rather than a Homebrew-specific failure.Why fix this in OIIO?
The underlying package-export defect belongs in libtiff, but an upstream fix will not repair already-installed libtiff releases or OIIO dependency caches. OIIO also builds the affected static libtiff configuration itself.
This also breaks OIIO's ability to self-build (some) dependencies that [in]directly depend on libtiff or deflate. (Not for any dependencies for which we currently have self-building recipes so far; but this has blocked me in the past from submitting PRs for some of the more complex (sub)dependencies (libheif comes to mind; maybe jxl...? can't remember, it was more than a year ago when I last looked into this)
The guard is intentionally defensive:
Deflate::Deflatedoes not already exist;Testing
Tested on:
Verification performed:
Steps taken to verify:
SKBUILD_BUILD_DIR,IGNORE_HOMEBREWED_DEPS=1, and without forcing a local TIFF rebuild. The first build autobuilt static TIFF 4.7.1 and libdeflate 1.23; the second found the cached TIFF package and completed without the previousDeflate::Deflateerror.No robots worth speaking of were harmed in the making of this PR
Assisted-by: Codex / GPT-5
Codex was used to trace the CMake package-discovery chain, inspect the libtiff exports, reproduce the failure, prepare the compatibility guard, and run the verification described above.