Skip to content

Commit 0ca587f

Browse files
committed
[CMake] Deal with semicolons in ENV passed to RootTestDriver
The processing of the environment variables in the RootTestDriver used excessive CMake string-to-list parsing, and the semicolons would have to be escaped for each of these parsings. That means the RootTestDriver command needed to include a magic number amount of escape characters to propagate semicolons to the final environment variables. The fix is to escape semicolons once in the beginning, and then re-escape them after each step that involves list parsing. Like this, the command can contain just bare semicolons without having to escape anything. In a next step, one can consider giving the same treatment to the `ROOT_ADD_TEST` macro, which also does excessive list parsing. Then we don't need tricks like the `$<SEMICOLON>` generator expression anymore to propagate semicolons into the final command.
1 parent 2be4fbe commit 0ca587f

2 files changed

Lines changed: 34 additions & 12 deletions

File tree

cmake/modules/RootTestDriver.cmake

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,33 @@ endif()
7272

7373
#---Set environment --------------------------------------------------------------------------------
7474
if(ENV)
75-
string(REPLACE "#" ";" _env ${ENV})
75+
# CMake treats ';' as a list separator. Since ENV values may legitimately
76+
# contain semicolons (e.g. PATH-like variables), we must escape them here
77+
# so they survive the upcoming list expansion in `foreach()`.
78+
string(REPLACE ";" "\\;" _env "${ENV}")
79+
80+
# Deserialize the ENV string into a CMake list.
81+
# This is the inverse operation of ROOT_ADD_TEST, which serializes the list
82+
# by joining elements with '#'.
83+
string(REPLACE "#" ";" _env "${_env}")
84+
7685
foreach(pair ${_env})
77-
string(REGEX REPLACE "^([^=]+)=(.*)$" "\\1;\\2" pair ${pair})
86+
87+
# During list expansion, CMake has already consumed any previous '\;'
88+
# escaping. Re-escape semicolons so the key=value split below treats the
89+
# value as a single string.
90+
string(REPLACE ";" "\\;" pair "${pair}")
91+
92+
# Split KEY=VALUE into a 2-element CMake list: [KEY;VALUE]
93+
string(REGEX REPLACE "^([^=]+)=(.*)$" "\\1;\\2" pair "${pair}")
94+
7895
list(GET pair 0 var)
7996
list(GET pair 1 val)
80-
set(ENV{${var}} ${val})
97+
98+
# As before, quoting is important here so the semicolons are not
99+
# interpreted as list separators.
100+
set(ENV{${var}} "${val}")
101+
81102
if(DBG)
82103
message(STATUS "testdriver[ENV]:${var}==>${val}")
83104
endif()

tutorials/CMakeLists.txt

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,17 @@ set(root_includepaths
1717
${DEFAULT_ROOT_INCLUDE_PATH}
1818
)
1919
cmake_path(CONVERT "${root_includepaths}" TO_NATIVE_PATH_LIST root_includepaths_native)
20+
set(root_includepaths_native "${CMAKE_BINARY_DIR}/tutorials/io/tree;${CMAKE_BINARY_DIR}/runtutorials")
2021

21-
# The environment argument gets serialized in ROOT_ADD_TEST for the
22-
# RootTestDriver, and then deserialized in the latter with multiple steps of
23-
# string parsing. We have to escape the semicolon excessively, because in every
24-
# parsing step we lose one semicolon. Also we use the SEMICOLON generator
25-
# expression, so at least we don't have to worry about escaping for the initial
26-
# configuration stage.
27-
# TODO: consider improving ROOT_ADD_TEST and RootTestDriver.cmake so this is not necessary.
28-
string(REPLACE ";" "\\\\\\\\$<SEMICOLON>" pythonpaths_escaped "${pythonpaths_native}")
29-
string(REPLACE ";" "\\\\\\\\$<SEMICOLON>" root_includepaths_escaped "${root_includepaths_native}")
22+
# Use the $<SEMICOLON> generator expression to defer semicolon expansion until
23+
# generator-expression evaluation. This avoids CMake interpreting literal ';'
24+
# as list separators during argument handling.
25+
#
26+
# ROOT_ADD_TEST performs multiple string-to-list conversions when serializing
27+
# the test command, which makes it unsafe to pass literal semicolons in
28+
# arguments until this behavior is fixed.
29+
string(REPLACE ";" "$<SEMICOLON>" pythonpaths_escaped "${pythonpaths_native}")
30+
string(REPLACE ";" "$<SEMICOLON>" root_includepaths_escaped "${root_includepaths_native}")
3031

3132
# - Set the environment for the tutorials, which also contains some environment
3233
# variables related to limiting the number of threads used by NumPy and

0 commit comments

Comments
 (0)