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

Author: guitargeek

core/base/CMakeLists.txt

@@ -233,15 +233,19 @@ set(full_core_filename $<TARGET_FILE_NAME:Core>)
 # Absolue CMAKE_INSTALL_<dir> 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)
 

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().native().c_str();
+
    return rootincdir;
 }
 

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)
 

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);
+}