From ec98372935328f9dbf433a00ea415c22b5522514 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Fri, 1 Aug 2025 14:37:43 +0200 Subject: [PATCH 1/2] [CMake] Set `rpath` for individual targets and not globally When using the `rpath=ON` option, the global CMake variable `CMAKE_INSTALL_RPATH` is mutated, which is problematic in several corner cases. This commit suggests to append the rpaths at the target property level, and also set the rpaths for the build tree separately so they make sense. A new ROOT macro is introduced for this rpath setting, which are also used for the PyROOT libraries, which also need custom rpaths. Closes #19327 and #19134. --- bindings/pyroot/cppyy/CPyCppyy/CMakeLists.txt | 23 +++---------- bindings/pyroot/pythonizations/CMakeLists.txt | 17 +--------- cmake/modules/RootBuildOptions.cmake | 17 ---------- cmake/modules/RootMacros.cmake | 32 +++++++++++++++++++ 4 files changed, 37 insertions(+), 52 deletions(-) diff --git a/bindings/pyroot/cppyy/CPyCppyy/CMakeLists.txt b/bindings/pyroot/cppyy/CPyCppyy/CMakeLists.txt index d6a78bcac626b..aa87317464eb1 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/CMakeLists.txt +++ b/bindings/pyroot/cppyy/CPyCppyy/CMakeLists.txt @@ -112,29 +112,14 @@ target_include_directories(CPyCppyy $ ) +set_property(GLOBAL APPEND PROPERTY ROOT_EXPORTED_TARGETS CPyCppyy) +set_property(GLOBAL APPEND PROPERTY ROOT_EXPORTED_TARGETS cppyy) + if(NOT MSVC) # Make sure that relative RUNPATH to main ROOT libraries is always correct. - - file(RELATIVE_PATH pymoduledir_to_libdir_build ${localruntimedir} "${localruntimedir}") - file(RELATIVE_PATH pymoduledir_to_libdir_install ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_PYTHONDIR} "${CMAKE_INSTALL_FULL_LIBDIR}") - - if(APPLE) - set_target_properties(cppyy PROPERTIES - BUILD_RPATH "@loader_path/${pymoduledir_to_libdir_build}" - INSTALL_RPATH "@loader_path/${pymoduledir_to_libdir_install}" - ) - else() - set_target_properties(cppyy PROPERTIES - BUILD_RPATH "$ORIGIN/${pymoduledir_to_libdir_build}" - INSTALL_RPATH "$ORIGIN/${pymoduledir_to_libdir_install}" - ) - endif() - + ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH(cppyy ${CMAKE_INSTALL_PYTHONDIR}) endif() -set_property(GLOBAL APPEND PROPERTY ROOT_EXPORTED_TARGETS CPyCppyy) -set_property(GLOBAL APPEND PROPERTY ROOT_EXPORTED_TARGETS cppyy) - # Install library install(TARGETS CPyCppyy EXPORT ${CMAKE_PROJECT_NAME}Exports RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT libraries diff --git a/bindings/pyroot/pythonizations/CMakeLists.txt b/bindings/pyroot/pythonizations/CMakeLists.txt index be4ab57f0d5ca..797c09e682e6f 100644 --- a/bindings/pyroot/pythonizations/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/CMakeLists.txt @@ -227,22 +227,7 @@ endif() if(NOT MSVC) # Make sure that relative RUNPATH to main ROOT libraries is always correct. - - file(RELATIVE_PATH pymoduledir_to_libdir_build ${pymoduledir_build} "${localruntimedir}") - file(RELATIVE_PATH pymoduledir_to_libdir_install ${CMAKE_INSTALL_PREFIX}/${pymoduledir_install} "${CMAKE_INSTALL_FULL_LIBDIR}") - - if(APPLE) - set_target_properties(${libname} PROPERTIES - BUILD_RPATH "@loader_path/${pymoduledir_to_libdir_build}" - INSTALL_RPATH "@loader_path/${pymoduledir_to_libdir_install}" - ) - else() - set_target_properties(${libname} PROPERTIES - BUILD_RPATH "$ORIGIN/${pymoduledir_to_libdir_build}" - INSTALL_RPATH "$ORIGIN/${pymoduledir_to_libdir_install}" - ) - endif() - + ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH(${libname} ${pymoduledir_install}) endif() # Install library diff --git a/cmake/modules/RootBuildOptions.cmake b/cmake/modules/RootBuildOptions.cmake index 42f8fbaa4f731..9e5187c6d15be 100644 --- a/cmake/modules/RootBuildOptions.cmake +++ b/cmake/modules/RootBuildOptions.cmake @@ -423,23 +423,6 @@ include(RootInstallDirs) # add to RPATH any directories outside the project that are in the linker search path set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) -if(rpath) - file(RELATIVE_PATH BINDIR_TO_LIBDIR "${CMAKE_INSTALL_FULL_BINDIR}" "${CMAKE_INSTALL_FULL_LIBDIR}") - - if(APPLE) - set(CMAKE_MACOSX_RPATH TRUE) - set(CMAKE_INSTALL_NAME_DIR "@rpath") - - set(_rpath_values "@loader_path" "@loader_path/${BINDIR_TO_LIBDIR}") - else() - set(_rpath_values "$ORIGIN" "$ORIGIN/${BINDIR_TO_LIBDIR}") - endif() - - set(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_RPATH-CACHED};${_rpath_values}" CACHE STRING "Install RPATH" FORCE) - - unset(BINDIR_TO_LIBDIR) -endif() - #---deal with the DCMAKE_IGNORE_PATH------------------------------------------------------------ if(macos_native) if(APPLE) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 56d0c51d5997b..3195ef5e7dfd8 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -975,6 +975,9 @@ function(ROOT_LINKER_LIBRARY library) #----Installation details------------------------------------------------------- if(NOT ARG_TEST AND NOT ARG_NOINSTALL AND CMAKE_LIBRARY_OUTPUT_DIRECTORY) + if(rpath) + ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH(${library} ${CMAKE_INSTALL_LIBDIR}) + endif() if(ARG_CMAKENOEXPORT) install(TARGETS ${library} RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT libraries LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT libraries @@ -1437,6 +1440,9 @@ function(ROOT_EXECUTABLE executable) endif() #----Installation details------------------------------------------------------ if(NOT ARG_NOINSTALL AND CMAKE_RUNTIME_OUTPUT_DIRECTORY) + if(rpath) + ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH(${executable} ${CMAKE_INSTALL_BINDIR}) + endif() if(ARG_CMAKENOEXPORT) install(TARGETS ${executable} RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT applications) else() @@ -2073,6 +2079,32 @@ function(generateManual name input output) install(FILES ${output} DESTINATION ${CMAKE_INSTALL_MANDIR}/man1) endfunction() +#---------------------------------------------------------------------------- +# --- ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH(target install_dir) +# +# Sets the INSTALL_RPATH for a given target so that it can find the ROOT shared +# libraries at runtime. The RPATH is set relative to the target's own location +# using $ORIGIN (or @loader_path on macOS). +# +# Arguments: +# target - The CMake target (e.g., a shared library or executable) +# install_dir - The install subdirectory relative to CMAKE_INSTALL_PREFIX +#---------------------------------------------------------------------------- +function(ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH target install_dir) + file(RELATIVE_PATH to_libdir "${CMAKE_INSTALL_PREFIX}/${install_dir}" "${CMAKE_INSTALL_FULL_LIBDIR}") + + # New path + if(APPLE) + set(new_rpath "@loader_path/${to_libdir}") + else() + set(new_rpath "$ORIGIN/${to_libdir}") + endif() + + # Append to existing RPATH + set_property(TARGET ${target} APPEND PROPERTY INSTALL_RPATH "${new_rpath}") +endfunction() + + #------------------------------------------------------------------------------- # # Former RoottestMacros.cmake starts here From 7af2b1d9c3e74dd44625409558f466ebc93d685f Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Fri, 1 Aug 2025 17:37:56 +0200 Subject: [PATCH 2/2] [CMake] Deprecate the `rpath` option and always append relative rpath Since ROOT 6.36, the relative `RPATH` to the main ROOT libraries is only appended to the existing path, and not replacing the path that is e.g. passed by the user via `CMAKE_INSTALL_RPATH`. Therefore, there is no problematic side effect of `rpath=ON` anymore. We can drop this option and always append to the runpath. This makes the ROOT configuration less confusing, also because from the option name "rpath", it is not clear what it did. --- README/ReleaseNotes/v638/index.md | 3 +++ cmake/modules/RootBuildOptions.cmake | 10 ++++++++-- cmake/modules/RootMacros.cmake | 4 ++-- rootx/CMakeLists.txt | 2 +- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/README/ReleaseNotes/v638/index.md b/README/ReleaseNotes/v638/index.md index 51654332851b8..346660b4d1593 100644 --- a/README/ReleaseNotes/v638/index.md +++ b/README/ReleaseNotes/v638/index.md @@ -7,6 +7,9 @@ * The `ROOT::Math::TDataPointN` class that can be used with the `ROOT::Math::KDETree` was removed. Use the templated `TDataPoint` instead. * The Parallel ROOT Facility, `PROOF`, has been removed from the repository. * After being deprecated for a long period, the `-r` option of `rootcling` has been removed. +* The `rpath` build option is deprecated. It is now without effect. + Relative RPATHs to the main ROOT libraries are unconditionally appended to all ROOT executables and libraries if the operating system supports it. + If you want a ROOT build without RPATHs, use the canonical CMake variable `CMAKE_SKIP_INSTALL_RPATH=TRUE`. ## Core Libraries * Behavior change: when selecting a template instantiation for a dictionary, all the template arguments have to be fully defined - the forward declarations are not enough any more. The error prompted by the dictionary generator will be `Warning: Unused class rule: MyTemplate`. diff --git a/cmake/modules/RootBuildOptions.cmake b/cmake/modules/RootBuildOptions.cmake index 9e5187c6d15be..c6d2cb8788c0d 100644 --- a/cmake/modules/RootBuildOptions.cmake +++ b/cmake/modules/RootBuildOptions.cmake @@ -154,7 +154,6 @@ ROOT_BUILD_OPTION(r OFF "Enable support for R bindings (requires R, Rcpp, and RI ROOT_BUILD_OPTION(roofit ON "Build the advanced fitting package RooFit, and RooStats for statistical tests. If xml is available, also build HistFactory.") ROOT_BUILD_OPTION(roofit_multiprocess OFF "Build RooFit::MultiProcess and multi-process RooFit::TestStatistics classes (requires ZeroMQ >= 3.4.5 built with -DENABLE_DRAFTS and cppzmq).") ROOT_BUILD_OPTION(root7 ON "Build ROOT 7 components of ROOT") -ROOT_BUILD_OPTION(rpath ON "Link libraries with built-in RPATH (run-time search path)") ROOT_BUILD_OPTION(runtime_cxxmodules ON "Enable runtime support for C++ modules") ROOT_BUILD_OPTION(shadowpw OFF "Enable support for shadow passwords") ROOT_BUILD_OPTION(shared ON "Use shared 3rd party libraries if possible") @@ -307,7 +306,6 @@ if(WIN32) set(davix_defvalue OFF) set(roofit_multiprocess_defvalue OFF) set(roottest_defvalue OFF) - set(rpath_defvalue OFF) set(runtime_cxxmodules_defvalue OFF) set(testing_defvalue OFF) set(vdt_defvalue OFF) @@ -406,6 +404,14 @@ foreach(opt ) endif() endforeach() +if(DEFINED rpath) + message(DEPRECATION ">>> Option 'rpath' is deprecated and without effect." + " See https://root.cern/doc/v638/release-notes.html" + " Relative RPATHs to the main ROOT libraries are unconditionally appended to all ROOT" + " executables and libraries." + "") # empty line at the end to make the deprecation message more visible +endif() + foreach(opt minuit2_mpi) if(${opt}) message(WARNING "The option '${opt}' can only be used to minimise thread-safe functions in Minuit2. It cannot be used for Histogram/Graph fitting and for RooFit. If you want to use Minuit2 with MPI support, it is better to build Minuit2 as a standalone library.") diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 3195ef5e7dfd8..f15667413769a 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -975,7 +975,7 @@ function(ROOT_LINKER_LIBRARY library) #----Installation details------------------------------------------------------- if(NOT ARG_TEST AND NOT ARG_NOINSTALL AND CMAKE_LIBRARY_OUTPUT_DIRECTORY) - if(rpath) + if(NOT MSVC) ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH(${library} ${CMAKE_INSTALL_LIBDIR}) endif() if(ARG_CMAKENOEXPORT) @@ -1440,7 +1440,7 @@ function(ROOT_EXECUTABLE executable) endif() #----Installation details------------------------------------------------------ if(NOT ARG_NOINSTALL AND CMAKE_RUNTIME_OUTPUT_DIRECTORY) - if(rpath) + if(NOT MSVC) ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH(${executable} ${CMAKE_INSTALL_BINDIR}) endif() if(ARG_CMAKENOEXPORT) diff --git a/rootx/CMakeLists.txt b/rootx/CMakeLists.txt index 39d1bf1461d45..1ec4ad445d4c5 100644 --- a/rootx/CMakeLists.txt +++ b/rootx/CMakeLists.txt @@ -28,6 +28,6 @@ target_include_directories(root PRIVATE ${CMAKE_BINARY_DIR}/ginclude # for RConfigure.h and rootCommandLineOptionsHelp.h ) -if(rpath) +if(NOT MSVC AND NOT CMAKE_SKIP_INSTALL_RPATH) target_compile_definitions(root PRIVATE IS_RPATH_BUILD) endif()