Skip to content

Commit 3a71c65

Browse files
committed
[CMake] Don't override find_package() for builtins
ROOT redefined the global find_package() command to no-op for its builtins, so that sub-projects added later (notably the bundled LLVM) and transitive find modules (e.g. FindPNG -> find_package(ZLIB)) would reuse ROOT's builtin imported targets instead of re-discovering a system copy. This relied on undocumented CMake behaviour and recursed infinitely under tools that override find_package() themselves, such as vcpkg (#8633). The override only ever did real work for ZLIB and zstd: every other name in ROOT_BUILTINS was unnecessary (no find_package() consumer downstream). Both are now handled directly, so the override and the ROOT_BUILTINS list can be removed: * ZLIB (used by core/zip's find_package(ZLIB REQUIRED) and the transitive FindPNG -> find_package(ZLIB) of the asimage build): builtins/zlib pre-seeds the singular cache variables ZLIB_LIBRARY and ZLIB_INCLUDE_DIR that FindZLIB.cmake checks, so find_package() short-circuits to the builtin and keeps the ZLIB::ZLIB target. This also avoids the "FORCE specified without CACHE" error from select_library_configurations() that a real FindPNG -> FindZLIB chain would otherwise hit. * zstd (used only by the bundled LLVM): interpreter/CMakeLists.txt sets LLVM_ENABLE_ZSTD=OFF (and LLVM_ENABLE_ZLIB=OFF) for builtin builds, so LLVM never searches for the system copy. We can't point LLVM at the builtins instead, as it probes them with a configure-time compile/link check that the not-yet-built ExternalProject can't pass anyway. This makes ROOT compatible with vcpkg and other dependency providers without changing behaviour: in builtin builds LLVM is still built without zlib/zstd, and a system build is unaffected since find_package() is no longer shadowed. Closes #8633. 🤖 Done with the help of [Claude Code](https://claude.com/claude-code) (Claude Opus 4.8)
1 parent 936b7b2 commit 3a71c65

3 files changed

Lines changed: 22 additions & 25 deletions

File tree

builtins/zlib/CMakeLists.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,13 @@ set(ZLIB_INCLUDE_DIRS ${ZLIB_INCLUDE_DIR} PARENT_SCOPE)
7474
set(ZLIB_LIBRARIES ${ROOT_ZLIB_LIBRARY} PARENT_SCOPE)
7575
set(ZLIB_FOUND TRUE PARENT_SCOPE)
7676
set(ZLIB_VERSION ${ROOT_ZLIB_VERSION} PARENT_SCOPE)
77+
78+
# Pre-seed the singular variables FindZLIB.cmake consumes (its REQUIRED_VARS:
79+
# ZLIB_INCLUDE_DIR via find_path(), ZLIB_LIBRARY via its if(NOT ZLIB_LIBRARY)
80+
# guard), so a real find_package(ZLIB) short-circuits the system search and
81+
# reuses the builtin ZLIB::ZLIB target above. Setting this variable in the
82+
# PARENT_SCOPE is sufficient, because that's SearchInstalledSoftware.cmake
83+
# where also the other find_package(ZLIB) calls happen (indirectly via
84+
# FindPNG). So a CACHE entry is not required.
85+
set(ZLIB_INCLUDE_DIR ${ZLIB_INCLUDE_DIR} PARENT_SCOPE)
86+
set(ZLIB_LIBRARY ${ROOT_ZLIB_LIBRARY} PARENT_SCOPE)

cmake/modules/SearchInstalledSoftware.cmake

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,6 @@ if(NOT "${MISSING_PACKAGES}" STREQUAL "")
137137
message(FATAL_ERROR "The following packages need to be installed or enabled to build ROOT: ${MISSING_PACKAGES}")
138138
endif()
139139

140-
#--- Redefine find_package for LLVM to pick up ROOT's builtins ----------------------
141-
# TODO: Make this only local to LLVM?
142-
macro(find_package)
143-
if(NOT "${ARGV0}" IN_LIST ROOT_BUILTINS) # ROOT_BUILTINS are the variable names, not the same as ROOT_BUILTIN_TARGETS used for move-header dependency
144-
_find_package(${ARGV})
145-
endif()
146-
endmacro()
147-
148140
#---On MacOSX, try to find frameworks after standard libraries or headers------------
149141
set(CMAKE_FIND_FRAMEWORK LAST)
150142

@@ -160,7 +152,6 @@ endif()
160152

161153
#---Check for Zlib ------------------------------------------------------------------
162154
if(builtin_zlib)
163-
list(APPEND ROOT_BUILTINS ZLIB)
164155
add_subdirectory(builtins/zlib)
165156
else()
166157
# If not built-in, check if this is zlib-ng
@@ -216,7 +207,6 @@ if(NOT builtin_nlohmannjson)
216207
endif()
217208

218209
if(builtin_nlohmannjson)
219-
list(APPEND ROOT_BUILTINS BUILTIN_NLOHMANN)
220210
add_subdirectory(builtins/nlohmann)
221211
endif()
222212

@@ -259,7 +249,6 @@ endif()
259249
#---Check for Freetype---------------------------------------------------------------
260250
ROOT_FIND_REQUIRED_DEP(Freetype builtin_freetype) # needed for asimage, but also outside of it (for "graf" target)
261251
if(builtin_freetype)
262-
list(APPEND ROOT_BUILTINS BUILTIN_FREETYPE)
263252
add_subdirectory(builtins/freetype)
264253
elseif(NOT Freetype_VERSION AND FREETYPE_VERSION_STRING)
265254
# on mac brew installed freetype version_string is returned
@@ -299,18 +288,15 @@ endif()
299288

300289
if(builtin_pcre)
301290
add_subdirectory(builtins/pcre)
302-
list(APPEND ROOT_BUILTINS BUILTIN_PCRE)
303291
endif()
304292

305293
#---Check for LZMA-------------------------------------------------------------------
306294
if(builtin_lzma)
307-
list(APPEND ROOT_BUILTINS LZMA)
308295
add_subdirectory(builtins/lzma)
309296
endif()
310297

311298
#---Check for xxHash-----------------------------------------------------------------
312299
if(builtin_xxhash)
313-
list(APPEND ROOT_BUILTINS xxHash)
314300
add_subdirectory(builtins/xxhash)
315301
endif()
316302

@@ -320,14 +306,11 @@ if(ZSTD_FOUND AND ZSTD_VERSION VERSION_LESS 1.0.0)
320306
endif()
321307

322308
if(builtin_zstd)
323-
list(APPEND ROOT_BUILTINS zstd)
324-
list(APPEND ROOT_BUILTINS ZSTD)
325309
add_subdirectory(builtins/zstd)
326310
endif()
327311

328312
#---Check for LZ4--------------------------------------------------------------------
329313
if(builtin_lz4)
330-
list(APPEND ROOT_BUILTINS LZ4)
331314
add_subdirectory(builtins/lz4)
332315
endif()
333316

@@ -482,7 +465,6 @@ if(opengl)
482465
ROOT_FIND_REQUIRED_DEP(gl2ps builtin_gl2ps)
483466
if (builtin_gl2ps)
484467
add_subdirectory(builtins/gl2ps)
485-
list(APPEND ROOT_BUILTINS BUILTIN_GL2PS)
486468
endif()
487469
elseif(builtin_gl2ps)
488470
message(SEND_ERROR "gl2ps features enabled with \"builtin_gl2ps=ON\" require \"opengl=ON\"")
@@ -550,7 +532,6 @@ if(builtin_openssl)
550532
set(builtin_openssl OFF CACHE BOOL "Disabled because there is no internet connection" FORCE)
551533
set(ssl OFF CACHE BOOL "Disabled because there is no internet connection" FORCE)
552534
else()
553-
list(APPEND ROOT_BUILTINS OpenSSL)
554535
add_subdirectory(builtins/openssl)
555536
endif()
556537
endif()
@@ -619,7 +600,6 @@ if(fftw3)
619600
endif()
620601
endif()
621602
if(builtin_fftw3)
622-
list(APPEND ROOT_BUILTINS BUILTIN_FFTW3)
623603
add_subdirectory(builtins/fftw3)
624604
set(fftw3 ON CACHE BOOL "Enabled because builtin_fftw3 requested (${fftw3_description})" FORCE)
625605
endif()
@@ -713,7 +693,6 @@ if(builtin_xrootd)
713693
if(NOT ssl AND NOT builtin_openssl)
714694
message(FATAL_ERROR "Building XRootD ('builtin_xrootd'=On) requires ssl support ('ssl' or 'builtin_openssl').")
715695
endif()
716-
list(APPEND ROOT_BUILTINS BUILTIN_XROOTD)
717696
add_subdirectory(builtins/xrootd)
718697
set(xrootd ON CACHE BOOL "Enabled because builtin_xrootd requested (${xrootd_description})" FORCE)
719698
endif()
@@ -780,7 +759,6 @@ if(opengl)
780759
ROOT_FIND_REQUIRED_DEP(FTGL builtin_ftgl)
781760
if (builtin_ftgl)
782761
add_subdirectory(builtins/ftgl)
783-
list(APPEND ROOT_BUILTINS BUILTIN_FTGL)
784762
endif()
785763
elseif(builtin_ftgl)
786764
message(SEND_ERROR "FTGL features enabled with \"builtin_ftgl=ON\" require \"opengl=ON\"")
@@ -940,7 +918,6 @@ if(builtin_tbb)
940918
endif()
941919

942920
if(builtin_tbb)
943-
list(APPEND ROOT_BUILTINS BUILTIN_TBB)
944921
add_subdirectory(builtins/tbb)
945922
endif()
946923

@@ -969,7 +946,6 @@ if(vdt OR builtin_vdt)
969946
endif()
970947
endif()
971948
if(builtin_vdt)
972-
list(APPEND ROOT_BUILTINS VDT)
973949
add_subdirectory(builtins/vdt)
974950
endif()
975951
endif()
@@ -1129,7 +1105,6 @@ if(mathmore OR builtin_gsl OR (tmva-cpu AND use_gsl_cblas))
11291105
endif()
11301106
endif()
11311107
else()
1132-
list(APPEND ROOT_BUILTINS BUILTIN_GSL)
11331108
add_subdirectory(builtins/gsl)
11341109
set(mathmore ON CACHE BOOL "Enabled because builtin_gsl requested (${mathmore_description})" FORCE)
11351110
endif()

interpreter/CMakeLists.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@ set(LLVM_ENABLE_FFI OFF CACHE BOOL "")
2121
set(LLVM_ENABLE_OCAMLDOC OFF CACHE BOOL "")
2222
set(LLVM_ENABLE_Z3_SOLVER OFF CACHE BOOL "")
2323
set(LLVM_ENABLE_WARNINGS OFF CACHE BOOL "")
24+
25+
# When ROOT provides zlib/zstd as builtins, disable them in the bundled LLVM so it
26+
# cannot discover and link the *system* copy (mixing two versions in one process).
27+
# We cannot point LLVM at the builtins instead: it probes them with a configure-time
28+
# compile/link check, but the builtin libraries are only produced later by their
29+
# ExternalProject, so the check would always fail.
30+
if(builtin_zlib)
31+
set(LLVM_ENABLE_ZLIB OFF CACHE STRING "Disabled because ROOT uses builtin zlib" FORCE)
32+
endif()
33+
if(builtin_zstd)
34+
set(LLVM_ENABLE_ZSTD OFF CACHE STRING "Disabled because ROOT uses builtin zstd" FORCE)
35+
endif()
2436
set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
2537
set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
2638
set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")

0 commit comments

Comments
 (0)