Skip to content

Commit 301e078

Browse files
[CMake] Make non-system include paths accessible at runtime
The current implementation faces issues with accessing header files from external libraries stored in non-system paths when the C++ interpreter tries to resolve them at runtime. This access is necessary under certain conditions even if a corresponding module file exists. To address this issue, we need to ensure that non-system path includes, which are known at build time, remain accessible at runtime. The proposed solution leverages the existing `ROOT_INCLUDE_PATH` environment variable. This commit introduces a new CMake variable `DEFAULT_ROOT_INCLUDE_PATH` and a helper function `BUILD_ROOT_INCLUDE_PATH`. This function allows us to append include paths of external libraries to `BUILD_ROOT_INCLUDE_PATH`, for example: `DEFAULT_ROOT_INCLUDE_PATH("${Vc_INCLUDE_DIR}" DEFAULT_ROOT_INCLUDE_PATH)` The `DEFAULT_ROOT_INCLUDE_PATH` variable is then utilized to populate the `ROOT_INCLUDE_PATH` environment variable in the configuration files (`thisroot.{sh,csh,fish,bat}`). If the library's include path is dynamically modified between build and run time, the configuration can be updated by patching the `thisroot.{sh,csh,fish,bat}` files.
1 parent 1fe0669 commit 301e078

File tree

10 files changed

+89
-7
lines changed

10 files changed

+89
-7
lines changed

CMakeLists.txt

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -548,10 +548,6 @@ install(
548548
PATTERN "pkgconfig" EXCLUDE
549549
)
550550

551-
if(Vc_INCLUDE_DIR)
552-
set(MODULES_ROOT_INCPATH "ROOT_INCLUDE_PATH=${Vc_INCLUDE_DIR}:${ROOT_INCLUDE_PATH}")
553-
endif()
554-
555551
# modules.idx
556552
if(runtime_cxxmodules)
557553
ROOT_GET_LIBRARY_OUTPUT_DIR(library_output_dir)
@@ -561,6 +557,7 @@ if(runtime_cxxmodules)
561557
ROOTIGNOREPREFIX=1 ROOT_HIST=0 $<TARGET_FILE:root.exe> -l -q -b)
562558
else()
563559
set(modules_idx_cmd COMMAND ${ld_library_path}=${library_output_dir}:$ENV{${ld_library_path}}
560+
ROOT_INCLUDE_PATH=${DEFAULT_ROOT_INCLUDE_PATH}
564561
ROOTIGNOREPREFIX=1 ROOT_HIST=0 $<TARGET_FILE:root.exe> -l -q -b)
565562
endif()
566563
add_custom_command(OUTPUT ${library_output_dir}/modules.idx
@@ -582,9 +579,10 @@ add_custom_target(hsimple ALL DEPENDS tutorials/hsimple.root)
582579
add_dependencies(hsimple onepcm)
583580
if(WIN32)
584581
set(hsimple_cmd COMMAND ${CMAKE_COMMAND} -E env PATH="${CMAKE_RUNTIME_OUTPUT_DIRECTORY}\\\;%PATH%"
582+
ROOT_INCLUDE_PATH="${DEFAULT_ROOT_INCLUDE_PATH}"
585583
ROOTIGNOREPREFIX=1 ROOT_HIST=0 $<TARGET_FILE:root.exe> -l -q -b -n -x ${CMAKE_SOURCE_DIR}/tutorials/hsimple.C -e return)
586584
else()
587-
set(hsimple_cmd COMMAND ${MODULES_ROOT_INCPATH} ${ld_library_path}=${CMAKE_LIBRARY_OUTPUT_DIRECTORY}:$ENV{${ld_library_path}}
585+
set(hsimple_cmd COMMAND ROOT_INCLUDE_PATH="${DEFAULT_ROOT_INCLUDE_PATH}" ${ld_library_path}=${CMAKE_LIBRARY_OUTPUT_DIRECTORY}:$ENV{${ld_library_path}}
588586
ROOTIGNOREPREFIX=1 ROOT_HIST=0 $<TARGET_FILE:root.exe> -l -q -b -n -x ${CMAKE_SOURCE_DIR}/tutorials/hsimple.C -e return)
589587
endif()
590588
add_custom_command(OUTPUT tutorials/hsimple.root

cmake/modules/RootMacros.cmake

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,6 +2114,49 @@ function(ROOT_APPEND_LIBDIR_TO_INSTALL_RPATH target install_dir)
21142114
set_property(TARGET ${target} APPEND PROPERTY INSTALL_RPATH "${new_rpath}")
21152115
endfunction()
21162116

2117+
#----------------------------------------------------------------------------
2118+
# If path is a system include path, set the return variable
2119+
# is_system_include_path to true.
2120+
# The 1st argument is the path that should be checked
2121+
# The 2nd argument is the return value
2122+
#----------------------------------------------------------------------------
2123+
function (IS_SYSTEM_INCLUDE_PATH path is_system_include_path)
2124+
foreach (dir ${CMAKE_SYSTEM_INCLUDE_PATH})
2125+
if ("${path}" STREQUAL "${dir}")
2126+
set(${is_system_include_path} TRUE PARENT_SCOPE)
2127+
return()
2128+
endif()
2129+
endforeach()
2130+
set(${is_system_include_path} FALSE PARENT_SCOPE)
2131+
endfunction()
2132+
2133+
#----------------------------------------------------------------------------
2134+
# Include paths of third-party libraries stored outside of system directories
2135+
# must be added to the DEFAULT_ROOT_INCLUDE_PATH variable to ensure that the
2136+
# C++ interpreter 'cling' can locate these header files at runtime.
2137+
# Use this function to add these paths to DEFAULT_ROOT_INCLUDE_PATH.
2138+
#----------------------------------------------------------------------------
2139+
function (BUILD_ROOT_INCLUDE_PATH path)
2140+
# filter out paths into the ROOT src, build, and install directories
2141+
if((${path} MATCHES "${CMAKE_SOURCE_DIR}(/.*)?") OR
2142+
(${path} MATCHES "${CMAKE_BINARY_DIR}(/.*)?") OR
2143+
(${path} MATCHES "${CMAKE_INSTALL_PREFIX}(/.*)?"))
2144+
return()
2145+
endif()
2146+
IS_SYSTEM_INCLUDE_PATH("${path}" is_system_include_path)
2147+
if (NOT is_system_include_path)
2148+
if ("${DEFAULT_ROOT_INCLUDE_PATH}" STREQUAL "")
2149+
set(DEFAULT_ROOT_INCLUDE_PATH "${path}" PARENT_SCOPE)
2150+
else()
2151+
if(WIN32)
2152+
set(ROOT_PATH_SEPARATOR ";")
2153+
elseif(UNIX)
2154+
set(ROOT_PATH_SEPARATOR ":")
2155+
endif()
2156+
set(DEFAULT_ROOT_INCLUDE_PATH "${DEFAULT_ROOT_INCLUDE_PATH}${ROOT_PATH_SEPARATOR}${path}" PARENT_SCOPE)
2157+
endif()
2158+
endif()
2159+
endfunction()
21172160

21182161
#-------------------------------------------------------------------------------
21192162
#

cmake/modules/SearchInstalledSoftware.cmake

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1383,6 +1383,7 @@ elseif(vc)
13831383
endif()
13841384
if(Vc_FOUND)
13851385
set_property(DIRECTORY APPEND PROPERTY INCLUDE_DIRECTORIES ${Vc_INCLUDE_DIR})
1386+
BUILD_ROOT_INCLUDE_PATH("${Vc_INCLUDE_DIR}")
13861387
endif()
13871388
endif()
13881389

@@ -1616,6 +1617,8 @@ if(vdt OR builtin_vdt)
16161617
install(DIRECTORY ${CMAKE_BINARY_DIR}/include/vdt
16171618
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} COMPONENT extra-headers)
16181619
set_property(GLOBAL APPEND PROPERTY ROOT_BUILTIN_TARGETS VDT::VDT)
1620+
else()
1621+
BUILD_ROOT_INCLUDE_PATH("${VDT_INCLUDE_DIR}")
16191622
endif()
16201623
endif()
16211624

config/thisroot.bat

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,6 @@ cd /D %OLDPATH%
1212
set PATH=%ROOTSYS%\bin;%PATH%
1313
set CMAKE_PREFIX_PATH=%ROOTSYS%;%CMAKE_PREFIX_PATH%
1414
set PYTHONPATH=%ROOTSYS%\bin;%PYTHONPATH%
15+
set ROOT_INCLUDE_PATH=@DEFAULT_ROOT_INLUCDE_PATH@;%ROOT_INCLUDE_PATH%
1516
set OLDPATH=
1617
set THIS=

config/thisroot.csh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,14 @@ if ($?old_rootsys) then
169169
-e "s;^$old_rootsys/etc/notebook${DOLLAR};;g"`
170170
endif
171171

172+
if ($?ROOT_INCLUDE_PATH) then
173+
setenv ROOT_INCLUDE_PATH `set DOLLAR='$'; echo $ROOT_INCLUDE_PATH | \
174+
sed -e "s;:@DEFAULT_ROOT_INCLUDE_PATH@:;:;g" \
175+
-e "s;:@DEFAULT_ROOT_INCLUDE_PATH@${DOLLAR};;g" \
176+
-e "s;^@DEFAULT_ROOT_INCLUDE_PATH@:;;g" \
177+
-e "s;^@DEFAULT_ROOT_INCLUDE_PATH@${DOLLAR};;g"`
178+
endif
179+
172180
endif
173181

174182

@@ -239,6 +247,12 @@ else
239247
setenv JUPYTER_CONFIG_PATH ${ROOTSYS}/etc/notebook
240248
endif
241249

250+
if ($?ROOT_INCLUDE_PATH) then
251+
setenv ROOT_INCLUDE_PATH @DEFAULT_ROOT_INLUCDE_PATH@:$ROOT_INCLUDE_PATH
252+
else
253+
setenv ROOT_INCLUDE_PATH @DEFAULT_ROOT_INLUCDE_PATH@
254+
endif
255+
242256
endif # if ("$thisrootdir" != "")
243257

244258
set thisrootdir=

config/thisroot.fish

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ update_path MANPATH "$old_rootsys" "/man" @mandir@
5555
update_path CMAKE_PREFIX_PATH "$old_rootsys" "" $ROOTSYS
5656
update_path JUPYTER_PATH "$old_rootsys" "/etc/notebook" $ROOTSYS/etc/notebook
5757
update_path JUPYTER_CONFIG_PATH "$old_rootsys" "/etc/notebook" $ROOTSYS/etc/notebook
58+
update_path ROOT_INCLUDE_PATH "@DEFAULT_ROOT_INCLUDE_PATH@" "" "@DEFAULT_ROOT_INCLUDE_PATH@"
5859

5960
functions -e update_path
6061
set -e old_rootsys

config/thisroot.ps1

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ $ROOTSYS = split-path -parent (get-item $scriptPath)
77
$env:PATH = $ROOTSYS + '\bin;' + $env:PATH
88
$env:CMAKE_PREFIX_PATH = $ROOTSYS + ';' + $env:CMAKE_PREFIX_PATH
99
$env:PYTHONPATH = $ROOTSYS + '\bin;' + $env:PYTHONPATH
10+
$env:ROOT_INCLUDE_PATH = "@DEFAULT_ROOT_INCLUDE_PATH@" + ';' + $env:ROOT_INCLUDE_PATH

config/thisroot.sh

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ clean_environment()
8989
default_manpath=""
9090
fi
9191
fi
92+
if [ -n "${ROOT_INCLUDE_PATH-}" ]; then
93+
drop_from_path "$ROOT_INCLUDE_PATH" "@DEFAULT_ROOT_INCLUDE_PATH@"
94+
ROOT_INCLUDE_PATH=$newpath
95+
fi
9296
}
9397

9498
set_environment()
@@ -162,6 +166,14 @@ set_environment()
162166
else
163167
JUPYTER_CONFIG_PATH=$ROOTSYS/etc/notebook:$JUPYTER_CONFIG_PATH; export JUPYTER_CONFIG_PATH
164168
fi
169+
170+
if [ -z "${ROOT_INCLUDE_PATH-}" ]; then
171+
ROOT_INCLUDE_PATH=@DEFAULT_ROOT_INCLUDE_PATH@
172+
export ROOT_INCLUDE_PATH
173+
else
174+
ROOT_INCLUDE_PATH=@DEFAULT_ROOT_INCLUDE_PATH@:$ROOT_INCLUDE_PATH
175+
export ROOT_INCLUDE_PATH
176+
fi
165177
}
166178

167179
getTrueShellExeName() { # mklement0 https://stackoverflow.com/a/23011530/7471760

roottest/root/io/transient/base/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ ROOTTEST_ADD_TEST(hadd_autoload
2323
COMMAND hadd -f data_merge.root data1.root data2.root
2424
OUTREF hadd_autoload${outref_suffix}.ref
2525
FIXTURES_REQUIRED root-io-transient-base-WriteFile-fixture
26-
ENVIRONMENT ROOT_INCLUDE_PATH=${CMAKE_CURRENT_SOURCE_DIR})
26+
ENVIRONMENT ROOT_INCLUDE_PATH=${CMAKE_CURRENT_SOURCE_DIR}:${DEFAULT_ROOT_INCLUDE_PATH})

roottest/root/meta/countIncludePaths.C

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,18 @@ int countIncludePaths()
1717
// Exclude from the test the builds with R as external package
1818
if (std::string::npos != includePath.find("RInside/include")) return 0;
1919

20+
// count paths coming from the ROOT_INCLUDE_PATH environment variable
21+
// and exclude them
22+
int nEnvVarPaths = 0;
23+
auto *envVarCStr = std::getenv("ROOT_INCLUDE_PATH");
24+
if (envVarCStr) {
25+
std::string envVar(envVarCStr);
26+
nEnvVarPaths = countSubstring(envVar, ":") + 1 - (envVar.back() == ':') - (envVar.front() == ':');
27+
}
28+
2029
// At most 10
2130
auto nPaths = countSubstring(includePath, "-I");
22-
if (nPaths > 10){
31+
if ((nPaths - nEnvVarPaths) > 10) {
2332
std::cerr << "The number of include paths is too high (>9) " << nPaths
2433
<< ". The number of \"-I\"s has been counted in the include path of ROOT (gSystem->GetIncludePath()=" << includePath << ")." << std::endl;
2534
return nPaths;

0 commit comments

Comments
 (0)