From 6b8877ebffe85c891f538ebe386f43b41fdbf0d5 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Fri, 16 Jan 2026 16:42:21 +0100 Subject: [PATCH] [core] Use shared library dir as anchor in `GetIncludeDir()` Using the shared library directory as the anchor to resolve the resource directories like `GetIncludeDir()` is more direct and robust than using `GetRootSys()`, which loses its meaning in some installations (e.g. what should it be when ROOT is installed by the package manager? Just `/`?). Most importantly, this change lifts some more constraint from how the install tree can be reorganized after the fact. One can for example move the include and library directory around as wanted, as long as their relative paths don't change. This fixes the Python wheels, which were broken by commit a5b1ed9. As of commit a5b1ed9, ROOT became smarter about finding the include path in the install tree, inferring the correct relative path from $ROOTSYS and the CMAKE_INSTALL_INCLUDEDIR variable at build time. Before, that only happened for gnuinstall=ON, but now it always happens. However, for the Python wheel, we are breaking the assumptions that ROOT makes by moving around directories in the install tree. This commit fixes that situation. --- core/base/CMakeLists.txt | 14 +++++++++----- core/base/src/TROOT.cxx | 26 ++++++++++++++++---------- core/base/test/CMakeLists.txt | 1 + core/base/test/TROOTTests.cxx | 10 ++++++++++ 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/core/base/CMakeLists.txt b/core/base/CMakeLists.txt index 35d1d3a42d3bb..e3b6159943813 100644 --- a/core/base/CMakeLists.txt +++ b/core/base/CMakeLists.txt @@ -233,15 +233,19 @@ set(full_core_filename $) # Absolue CMAKE_INSTALL_ paths are discouraged in CMake, but some # packagers use them anyway. So we support it. if(IS_ABSOLUTE ${CMAKE_INSTALL_INCLUDEDIR}) - set(install_includedir_is_absolute 1) + set(install_lib_to_include "${CMAKE_INSTALL_INCLUDEDIR}") else() - set(install_includedir_is_absolute 0) + if(IS_ABSOLUTE ${CMAKE_INSTALL_LIBDIR}) + set(libdir "${CMAKE_INSTALL_LIBDIR}") + else() + set(libdir "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}") + endif() + file(RELATIVE_PATH install_lib_to_include "${libdir}" "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}") + unset(libdir) endif() -file(TO_NATIVE_PATH "${CMAKE_INSTALL_INCLUDEDIR}" install_includedir_native) target_compile_options(Core PRIVATE -DLIB_CORE_NAME=${full_core_filename} - -DINSTALL_INCLUDEDIR=${install_includedir_native} - -DINSTALL_INCLUDEDIR_IS_ABSOLUTE=${install_includedir_is_absolute} + -DINSTALL_LIB_TO_INCLUDE="${install_lib_to_include}" ) add_dependencies(Core ensure_build_tree_marker) diff --git a/core/base/src/TROOT.cxx b/core/base/src/TROOT.cxx index 9a58378ebea58..a9d9bb5cdc189 100644 --- a/core/base/src/TROOT.cxx +++ b/core/base/src/TROOT.cxx @@ -3182,21 +3182,27 @@ const TString &TROOT::GetIncludeDir() if (!rootincdir.IsNull()) return rootincdir; - const std::string &sep = ROOT::FoundationUtils::GetPathSeparator(); + namespace fs = std::filesystem; + + // The shared library directory can be found automatically, because the + // libCore is loaded by definition when using TROOT. It's used as the anchor + // to resolve the ROOT include directory, using the correct relative path + // for either the build or install tree. + fs::path libPath = GetSharedLibDir().Data(); // Check if we are in the build tree using the build tree marker file - const bool isBuildTree = std::filesystem::exists((GetSharedLibDir() + sep + "root-build-tree-marker").Data()); + const bool isBuildTree = fs::exists(libPath / "root-build-tree-marker"); - if (isBuildTree) { - rootincdir = GetRootSys() + sep + "include"; - } else { -#if INSTALL_INCLUDEDIR_IS_ABSOLUTE - rootincdir = _R_QUOTEVAL_(INSTALL_INCLUDEDIR); -#else - rootincdir = GetRootSys() + sep + _R_QUOTEVAL_(INSTALL_INCLUDEDIR); -#endif + fs::path includePath = isBuildTree ? "../include" : INSTALL_LIB_TO_INCLUDE; + + // The INSTALL_LIB_TO_INCLUDE might already be absolute + if (!includePath.is_absolute()) { + includePath = libPath / includePath; } + // Normalize to get rid of the "../" in relative paths + rootincdir = includePath.lexically_normal().string(); + return rootincdir; } diff --git a/core/base/test/CMakeLists.txt b/core/base/test/CMakeLists.txt index e839d78255c9f..291d4399cbf87 100644 --- a/core/base/test/CMakeLists.txt +++ b/core/base/test/CMakeLists.txt @@ -20,6 +20,7 @@ if(ROOT_NEED_STDCXXFS) target_link_libraries(CoreBaseTests PRIVATE stdc++fs) endif() target_compile_options(CoreBaseTests PRIVATE -DEXPECTED_SHARED_LIBRARY_DIR="${localruntimedir}") +target_compile_options(CoreBaseTests PRIVATE -DEXPECTED_INCLUDE_DIR="${CMAKE_BINARY_DIR}/include") ROOT_ADD_GTEST(CoreErrorTests TErrorTests.cxx LIBRARIES Core) diff --git a/core/base/test/TROOTTests.cxx b/core/base/test/TROOTTests.cxx index 5bc454b9e164a..0e3c097ef20c0 100644 --- a/core/base/test/TROOTTests.cxx +++ b/core/base/test/TROOTTests.cxx @@ -41,3 +41,13 @@ TEST(TROOT, GetSharedLibDir) EXPECT_EQ(libDir, libDirRef); } + +TEST(TROOT, GetIncludeDir) +{ + namespace fs = std::filesystem; + + fs::path includeDir = gROOT->GetIncludeDir().Data(); + fs::path includeDirRef = EXPECTED_INCLUDE_DIR; + + EXPECT_EQ(includeDir, includeDirRef); +}