Skip to content

[CMake] Don't override find_package() for builtins #22604

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

Open
guitargeek wants to merge 2 commits into
base: master
Choose a base branch
from
+26 −25
Open

[CMake] Don't override find_package() for builtins #22604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
936b7b2
[CMake] Add missing build dependency for builtin zstd
guitargeek Jun 14, 2026
3a71c65
[CMake] Don't override find_package() for builtins
guitargeek Jun 14, 2026
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
10 changes: 10 additions & 0 deletions builtins/zlib/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,13 @@ set(ZLIB_INCLUDE_DIRS ${ZLIB_INCLUDE_DIR} PARENT_SCOPE)
set(ZLIB_LIBRARIES ${ROOT_ZLIB_LIBRARY} PARENT_SCOPE)
set(ZLIB_FOUND TRUE PARENT_SCOPE)
set(ZLIB_VERSION ${ROOT_ZLIB_VERSION} PARENT_SCOPE)

# Pre-seed the singular variables FindZLIB.cmake consumes (its REQUIRED_VARS:
# ZLIB_INCLUDE_DIR via find_path(), ZLIB_LIBRARY via its if(NOT ZLIB_LIBRARY)
# guard), so a real find_package(ZLIB) short-circuits the system search and
# reuses the builtin ZLIB::ZLIB target above. Setting this variable in the
# PARENT_SCOPE is sufficient, because that's SearchInstalledSoftware.cmake
# where also the other find_package(ZLIB) calls happen (indirectly via
# FindPNG). So a CACHE entry is not required.
Comment on lines +78 to +84

@hageboeck hageboeck Jun 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about:

# The following variables can be set by users to override the behaviour of FindZLIB.cmake,
# so packages executing their own find_package(ZLIB) will find the builtin library.
# We use this to direct the zlib search of FindPNG to the builtin.

@guitargeek guitargeek Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure! Since we all agree here that we don't want the CACHE entry, the comment can be less defensive and shortened.

set(ZLIB_INCLUDE_DIR ${ZLIB_INCLUDE_DIR} PARENT_SCOPE)
set(ZLIB_LIBRARY ${ROOT_ZLIB_LIBRARY} PARENT_SCOPE)
4 changes: 4 additions & 0 deletions builtins/zstd/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ set_target_properties(ZSTD::ZSTD PROPERTIES
IMPORTED_LOCATION ${ROOT_ZSTD_LIBRARY}
INTERFACE_INCLUDE_DIRECTORIES ${ZSTD_INCLUDE_DIR}
)
# Make consumers of ZSTD::ZSTD wait for the ExternalProject that actually builds
# the library, otherwise a parallel or targeted build can try to link the not-yet
# existing libzstd archive (consistent with builtins/zlib's add_dependencies).
add_dependencies(ZSTD::ZSTD BUILTIN_ZSTD)

# Set the canonical output of find_package according to
# https://cmake.org/cmake/help/latest/manual/cmake-developer.7.html#standard-variable-names
Expand Down
25 changes: 0 additions & 25 deletions cmake/modules/SearchInstalledSoftware.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,6 @@ if(NOT "${MISSING_PACKAGES}" STREQUAL "")
message(FATAL_ERROR "The following packages need to be installed or enabled to build ROOT: ${MISSING_PACKAGES}")
endif()

#--- Redefine find_package for LLVM to pick up ROOT's builtins ----------------------
# TODO: Make this only local to LLVM?
macro(find_package)
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
_find_package(${ARGV})
endif()
endmacro()

#---On MacOSX, try to find frameworks after standard libraries or headers------------
set(CMAKE_FIND_FRAMEWORK LAST)

Expand All @@ -160,7 +152,6 @@ endif()

#---Check for Zlib ------------------------------------------------------------------
if(builtin_zlib)
list(APPEND ROOT_BUILTINS ZLIB)
add_subdirectory(builtins/zlib)
else()
# If not built-in, check if this is zlib-ng
Expand Down Expand Up @@ -216,7 +207,6 @@ if(NOT builtin_nlohmannjson)
endif()

if(builtin_nlohmannjson)
list(APPEND ROOT_BUILTINS BUILTIN_NLOHMANN)
add_subdirectory(builtins/nlohmann)
endif()

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

if(builtin_pcre)
add_subdirectory(builtins/pcre)
list(APPEND ROOT_BUILTINS BUILTIN_PCRE)
endif()

#---Check for LZMA-------------------------------------------------------------------
if(builtin_lzma)
list(APPEND ROOT_BUILTINS LZMA)
add_subdirectory(builtins/lzma)
endif()

#---Check for xxHash-----------------------------------------------------------------
if(builtin_xxhash)
list(APPEND ROOT_BUILTINS xxHash)
add_subdirectory(builtins/xxhash)
endif()

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

if(builtin_zstd)
list(APPEND ROOT_BUILTINS zstd)
list(APPEND ROOT_BUILTINS ZSTD)
add_subdirectory(builtins/zstd)
endif()

#---Check for LZ4--------------------------------------------------------------------
if(builtin_lz4)
list(APPEND ROOT_BUILTINS LZ4)
add_subdirectory(builtins/lz4)
endif()

Expand Down Expand Up @@ -482,7 +465,6 @@ if(opengl)
ROOT_FIND_REQUIRED_DEP(gl2ps builtin_gl2ps)
if (builtin_gl2ps)
add_subdirectory(builtins/gl2ps)
list(APPEND ROOT_BUILTINS BUILTIN_GL2PS)
endif()
elseif(builtin_gl2ps)
message(SEND_ERROR "gl2ps features enabled with \"builtin_gl2ps=ON\" require \"opengl=ON\"")
Expand Down Expand Up @@ -550,7 +532,6 @@ if(builtin_openssl)
set(builtin_openssl OFF CACHE BOOL "Disabled because there is no internet connection" FORCE)
set(ssl OFF CACHE BOOL "Disabled because there is no internet connection" FORCE)
else()
list(APPEND ROOT_BUILTINS OpenSSL)
add_subdirectory(builtins/openssl)
endif()
endif()
Expand Down Expand Up @@ -619,7 +600,6 @@ if(fftw3)
endif()
endif()
if(builtin_fftw3)
list(APPEND ROOT_BUILTINS BUILTIN_FFTW3)
add_subdirectory(builtins/fftw3)
set(fftw3 ON CACHE BOOL "Enabled because builtin_fftw3 requested (${fftw3_description})" FORCE)
endif()
Expand Down Expand Up @@ -713,7 +693,6 @@ if(builtin_xrootd)
if(NOT ssl AND NOT builtin_openssl)
message(FATAL_ERROR "Building XRootD ('builtin_xrootd'=On) requires ssl support ('ssl' or 'builtin_openssl').")
endif()
list(APPEND ROOT_BUILTINS BUILTIN_XROOTD)
add_subdirectory(builtins/xrootd)
set(xrootd ON CACHE BOOL "Enabled because builtin_xrootd requested (${xrootd_description})" FORCE)
endif()
Expand Down Expand Up @@ -780,7 +759,6 @@ if(opengl)
ROOT_FIND_REQUIRED_DEP(FTGL builtin_ftgl)
if (builtin_ftgl)
add_subdirectory(builtins/ftgl)
list(APPEND ROOT_BUILTINS BUILTIN_FTGL)
endif()
elseif(builtin_ftgl)
message(SEND_ERROR "FTGL features enabled with \"builtin_ftgl=ON\" require \"opengl=ON\"")
Expand Down Expand Up @@ -940,7 +918,6 @@ if(builtin_tbb)
endif()

if(builtin_tbb)
list(APPEND ROOT_BUILTINS BUILTIN_TBB)
add_subdirectory(builtins/tbb)
endif()

Expand Down Expand Up @@ -969,7 +946,6 @@ if(vdt OR builtin_vdt)
endif()
endif()
if(builtin_vdt)
list(APPEND ROOT_BUILTINS VDT)
add_subdirectory(builtins/vdt)
endif()
endif()
Expand Down Expand Up @@ -1129,7 +1105,6 @@ if(mathmore OR builtin_gsl OR (tmva-cpu AND use_gsl_cblas))
endif()
endif()
else()
list(APPEND ROOT_BUILTINS BUILTIN_GSL)
add_subdirectory(builtins/gsl)
set(mathmore ON CACHE BOOL "Enabled because builtin_gsl requested (${mathmore_description})" FORCE)
endif()
Expand Down
12 changes: 12 additions & 0 deletions interpreter/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ set(LLVM_ENABLE_FFI OFF CACHE BOOL "")
set(LLVM_ENABLE_OCAMLDOC OFF CACHE BOOL "")
set(LLVM_ENABLE_Z3_SOLVER OFF CACHE BOOL "")
set(LLVM_ENABLE_WARNINGS OFF CACHE BOOL "")

# When ROOT provides zlib/zstd as builtins, disable them in the bundled LLVM so it
# cannot discover and link the *system* copy (mixing two versions in one process).
# We cannot point LLVM at the builtins instead: it probes them with a configure-time
# compile/link check, but the builtin libraries are only produced later by their
# ExternalProject, so the check would always fail.
Comment on lines +25 to +29

@hageboeck hageboeck Jun 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, but does it still manage to use those libraries if they are disabled?

if(builtin_zlib)
set(LLVM_ENABLE_ZLIB OFF CACHE STRING "Disabled because ROOT uses builtin zlib" FORCE)
endif()
if(builtin_zstd)
set(LLVM_ENABLE_ZSTD OFF CACHE STRING "Disabled because ROOT uses builtin zstd" FORCE)
endif()
set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
Expand Down
Loading