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

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 AI, which suggested an initial solution that I
refined.

Author: guitargeek

builtins/zlib/CMakeLists.txt

@@ -74,3 +74,9 @@ 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)
+
+# 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.
+set(ZLIB_INCLUDE_DIR ${ZLIB_INCLUDE_DIR} PARENT_SCOPE)
+set(ZLIB_LIBRARY ${ROOT_ZLIB_LIBRARY} PARENT_SCOPE)

cmake/modules/SearchInstalledSoftware.cmake

@@ -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)
 
@@ -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
@@ -216,7 +207,6 @@ if(NOT builtin_nlohmannjson)
 endif()
 
 if(builtin_nlohmannjson)
-  list(APPEND ROOT_BUILTINS BUILTIN_NLOHMANN)
   add_subdirectory(builtins/nlohmann)
 endif()
 
@@ -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
@@ -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()
 
@@ -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()
 
@@ -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\"")
@@ -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()
@@ -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()
@@ -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()
@@ -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\"")
@@ -940,7 +918,6 @@ if(builtin_tbb)
 endif()
 
 if(builtin_tbb)
-  list(APPEND ROOT_BUILTINS BUILTIN_TBB)
   add_subdirectory(builtins/tbb)
 endif()
 
@@ -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()
@@ -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()

interpreter/CMakeLists.txt

@@ -21,6 +21,27 @@ 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 as a builtin, disable it 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 zlib builtin instead: it probes it with
+# a configure-time compile check, but the builtin library is only produced
+# later at build time, so the check would always fail. For symmetry and to
+# preserve historic behavior, also disable ZSTD if we use the ROOT builtin,
+# even though LLVM doesn't do the configure-time compile check there. If no
+# builtins are used, we remove unnecessary degrees of freedom from the build by
+# setting the zlib/zstd flags to FORCE_ON, because zlib and zstd are hard
+# requirements of ROOT that are available anyway.
+if(builtin_zlib)
+  set(LLVM_ENABLE_ZLIB OFF CACHE STRING "Disabled because ROOT uses builtin zlib" FORCE)
+else()
+  set(LLVM_ENABLE_ZLIB FORCE_ON CACHE STRING "Always enabled because it's already a dependency of ROOT" FORCE)
+endif()
+if(builtin_zstd)
+  set(LLVM_ENABLE_ZSTD OFF CACHE STRING "Disabled because ROOT uses builtin zstd" FORCE)
+else()
+  set(LLVM_ENABLE_ZSTD FORCE_ON CACHE STRING "Always enabled because it's already a dependency of ROOT" FORCE)
+endif()
 set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
 set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
 set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")