build: Fix autobuild stale-cache bugs, unify build_*.cmake scripts - #5410
Conversation
We've had a longstanding issue where the autobuild works great when a dependency is not found on the system, but not when the dependency is already on the system, but not an adequately new version to satisfy our requirements. We have been plagued with the re-find sometimes picking up components from the wrong, old version. The first find_package() call is rejected after finding the too-old dependency. But it had already cached `<pkg>_INCLUDE_DIR` and similar items. find_path() and find_library() skip searching again once those cache variables are set. So the second find_package call silently reuses parts of the rejected library. We'd accumulated some cruft in the build_DEP.cmake files to try to work around this, but not in a uniform way, and still with some problems. The fix is in dependency_utils.cmake. It lives in the shared `<pkg>_REFIND` logic. Right before the refind's find_package() call, it now clears those stale cache entries. This clears both the MODULE-mode variables (`<PKG>_LIBRARY`, `<PKG>_INCLUDE_DIR`, etc.) and the CONFIG-mode `<Pkg>_DIR`. After fixing this for TIFF initially, an audit of the other `build_*.cmake` scripts found the same raw find_package() pattern in two more places: build_GIF.cmake and build_libuhdr.cmake. Converted both to REFIND too. While in there, unified all 21 build_*.cmake scripts on one minimal REFIND footer. This removed clutter that had built up per script over time. Some scripts had a Pkg_ROOT assignment that just duplicated what build_dependency_with_cmake() already sets after install. Some had a hand-forced Pkg_VERSION assignment that papered over whatever the refind actually found. OpenColorIO and PNG each had a manual unset() block too. It duplicated what the shared REFIND cache-clearing now handles centrally. Also fixed pystring's REFIND flag, which was FALSE. So pystring never re-found itself after a local build. OpenColorIO's script had grown its own workaround for this: a manual find_package(pystring REQUIRED) call. That workaround is no longer needed. Removed expat's Windows-only block: a find_package() call plus set_cache(expat_LIBRARY ...). That variable is never read anywhere else in the tree. expat now forces CONFIG mode on every platform. It no longer relies on case-insensitive filesystems to match CMake's stock FindEXPAT.cmake by accident. Assisted-by: Claude Code / claude-sonnet-5 Signed-off-by: Larry Gritz <lg@larrygritz.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens OpenImageIO’s CMake “local dependency build + refind” workflow by preventing stale cache variables from a rejected system dependency from contaminating the subsequent find_package() after a local build. It also standardizes the “REFIND footer” patterns across many build_*.cmake scripts to reduce per-dependency drift and inconsistent workarounds.
Changes:
- Clear common cached
<PKG>_INCLUDE_DIR*/<PKG>_LIBRARY*(and<Pkg>_DIR) variables before the REFINDfind_package()inchecked_find_package(). - Convert remaining raw
find_package()usages in local-build scripts to the shared*_REFINDmechanism and remove redundant per-script ROOT/DIR/VERSION hacks. - Normalize per-dependency REFIND arguments (CONFIG vs MODULE) and simplify platform-specific special cases (notably expat).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cmake/dependency_utils.cmake | Clears stale cached Find-module and Config-hint variables before the REFIND find_package() call. |
| src/cmake/build_ZLIB.cmake | Removes redundant ROOT/VERSION overrides; relies on standardized REFIND behavior. |
| src/cmake/build_yaml-cpp.cmake | Removes redundant DIR/VERSION hints; uses REFIND + CONFIG consistently. |
| src/cmake/build_WebP.cmake | Removes redundant ROOT hint; uses standardized REFIND flags. |
| src/cmake/build_TIFF.cmake | Replaces direct find_package(TIFF ...) with REFIND mechanism and explicit REFIND version. |
| src/cmake/build_pystring.cmake | Fixes pystring to actually REFIND after local build; removes bespoke unsets. |
| src/cmake/build_PNG.cmake | Removes large manual unset block and routes re-find through the unified REFIND flow (with HINTS/NO_DEFAULT_PATH). |
| src/cmake/build_openjph.cmake | Removes redundant ROOT/VERSION hints; uses standardized REFIND flags. |
| src/cmake/build_OpenJPEG.cmake | Removes redundant ROOT/VERSION hints; uses REFIND + CONFIG consistently. |
| src/cmake/build_OpenColorIO.cmake | Removes manual cache-clearing/workarounds and uses the shared REFIND mechanism (EXACT CONFIG). |
| src/cmake/build_minizip-ng.cmake | Switches REFIND behavior toward CONFIG-based re-find. |
| src/cmake/build_libuhdr.cmake | Converts to REFIND flow while keeping MODULE mode (no upstream config package). |
| src/cmake/build_libjpeg-turbo.cmake | Removes redundant ROOT hints; uses REFIND + CONFIG consistently. |
| src/cmake/build_libdeflate.cmake | Removes redundant ROOT hints; keeps REFIND + CONFIG flow. |
| src/cmake/build_GIF.cmake | Replaces direct find_package(GIF ...) call with REFIND flags (EXACT CONFIG). |
| src/cmake/build_Freetype.cmake | Removes redundant ROOT hints; uses REFIND + CONFIG consistently. |
| src/cmake/build_fmt.cmake | Adds CONFIG refind behavior for fmt’s CMake package discovery. |
| src/cmake/build_expat.cmake | Simplifies expat refind to a consistent CONFIG-based approach and removes Windows-only special casing. |
Suppressed comments (1)
src/cmake/build_fmt.cmake:32
- In the REFIND path, fmt_REFIND_VERSION is not set. That means the second find_package(fmt ...) call will not enforce a minimum/expected version, and can re-select the same too-old fmt that was just rejected (especially if it’s earlier on CMAKE_PREFIX_PATH), defeating the local-build fallback.
# Signal to caller that we need to find again at the installed location
set (fmt_REFIND TRUE)
set (fmt_REFIND_ARGS CONFIG)
set (fmt_VERSION ${fmt_BUILD_VERSION})
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Larry Gritz <lg@larrygritz.com>
|
Any comments on this PR? I'm considering it a priority to merge this as soon as I can, because I have some other adjustments to auto-builders and required dependency versions, but since they exposed the flaws that led to this logic fix, they can only be submitted after this fix is applied. |
|
This looks terrific...! This must have been at the root of what was giving me so much difficulty with incremental builds trying to reuse self-built dependencies still hanging out in the build dir from a previous build (the Deflate::Deflate shenanigans) |
|
Is that an approval? |
|
That's an approval. |
421d522
into
AcademySoftwareFoundation:main
We've had a longstanding issue where the autobuild works great when a dependency is not found on the system, but not when the dependency is already on the system, but not an adequately new version to satisfy our requirements. We have been plagued with the re-find sometimes picking up components from the wrong, old version.
The first find_package() call is rejected after finding the too-old dependency. But it had already cached
<pkg>_INCLUDE_DIRand similar items. find_path() and find_library() skip searching again once those cache variables are set. So the second find_package call silently reuses parts of the rejected library. We'd accumulated some cruft in the build_DEP.cmake files to try to work around this, but not in a uniform way, and still with some problems.The fix is in dependency_utils.cmake. It lives in the shared
<pkg>_REFINDlogic. Right before the refind's find_package() call, it now clears those stale cache entries. This clears both the MODULE-mode variables (<PKG>_LIBRARY,<PKG>_INCLUDE_DIR, etc.) and the CONFIG-mode<Pkg>_DIR.After fixing this for TIFF initially, an audit of the other
build_*.cmakescripts found the same raw find_package() pattern in two more places: build_GIF.cmake and build_libuhdr.cmake. Converted both to REFIND too.While in there, unified all 21 build_*.cmake scripts on one minimal REFIND footer. This removed clutter that had built up per script over time. Some scripts had a Pkg_ROOT assignment that just duplicated what build_dependency_with_cmake() already sets after install. Some had a hand-forced Pkg_VERSION assignment that papered over whatever the refind actually found. OpenColorIO and PNG each had a manual unset() block too. It duplicated what the shared REFIND cache-clearing now handles centrally.
Also fixed pystring's REFIND flag, which was FALSE. So pystring never re-found itself after a local build. OpenColorIO's script had grown its own workaround for this: a manual find_package(pystring REQUIRED) call. That workaround is no longer needed.
Removed expat's Windows-only block: a find_package() call plus set_cache(expat_LIBRARY ...). That variable is never read anywhere else in the tree. expat now forces CONFIG mode on every platform. It no longer relies on case-insensitive filesystems to match CMake's stock FindEXPAT.cmake by accident.
Assisted-by: Claude Code / claude-sonnet-5