Of course there is a lot of overlap effectively down to perhaps 3-4 cases as you have.
I think making a CMAKE_PATHSEP variable would be straightforward, if the CMake maintainers are open to such a contribution. It would possibly help deduplicate internal CMake code as well.
I could at best submit an imperfect patch; I don’t know what the values should be for Cygwin and MSYS, and for cross-builds I imagine we need a second variable for the target system. And all that aside, I tried poking into the CMake sources and I don’t see where such things are declared. So, I could give it a shot if I had lots of hints…
When I read them, the docs for those things didn’t seem to indicate support for adding elements to list-like environment variables (e.g. PATH on Windows, where ; is the separator). Did I misunderstand?
Ah, well no, not that, but I only looked at the cmake man page where -E env is documented, which doesn’t mention these operators. There’s no obvious direction that I should go look up the ENVIRONMENT_MODIFICATION test property (not everyone is reading the HTML docs).
--modify ENVIRONMENT_MODIFICATION
New in version 3.25.
Apply a single ENVIRONMENT_MODIFICATION operation to the
modified environment.
The NAME=VALUE and --unset=NAME options are equivalent to
--modify NAME=set:VALUE and --modify NAME=unset:,
respectively. Note that --modify NAME=reset: resets NAME
to the value it had when cmake launched (or unsets it),
not to the most recent NAME=VALUE option.
-- New in version 3.24.
Added support for the double dash argument --. Use -- to
stop interpreting options/environment variables and treat
the next argument as the command, even if it start with -
or contains a =.
So would that get me out of having to use an escaped semicolon in this example?
Ah, the link is dropped from the --help docs . Can you file an issue to improve wording to at least encourage looking there?
Yes, semicolon quoting test properties through a generic API was the straw that finally made me say “bah, I just need to implement this in cmake -E env”. .
works as you expect (path2:path1:$PATH) so that you can build these up piecemeal instead of having to collate them ahead of time (sorting may be required though).
So, I tried this, and I’m not convinced it’s an improvement:
modified cmake/modules/FindSwiftXCTest.cmake
@@ -125,9 +125,9 @@ function(add_swift_xctest test_target testee)
OUTPUT ${test_main}
# If the executable target depends on DLLs their directories need to be injected into the PATH
# or they won't be found and the target will fail to run, so invoke it through cmake. Because
- COMMAND
- ${CMAKE_COMMAND} -E env
- "PATH=$<JOIN:$<TARGET_RUNTIME_DLL_DIRS:GenerateXCTestMain>;$ENV{PATH},;>"
+ COMMAND ${CMAKE_COMMAND} -E env --modify
+ "$<LIST:TRANSFORM,$<TARGET_RUNTIME_DLL_DIRS:GenerateXCTestMain>,PREPEND,PATH=path_list_prepend:>"
--
$<TARGET_FILE:GenerateXCTestMain> -o ${test_main} ${sources}
DEPENDS ${sources} GenerateXCTestMain
@@ -141,16 +141,10 @@ function(add_swift_xctest test_target testee)
add_test(NAME ${test_target}
COMMAND ${test_target})
- # When $ENV{PATH} is interpreted as a list on Windows, trailing backslashes in elements
- # (e.g. "C:\path\to\foo\;C:\another\path\...") will end up causing semicolons to be escaped.
- string(REPLACE "\\" "/" path "$ENV{PATH}")
-
- # Escape the semicolons when forming the environment setting. As explained in
- # https://stackoverflow.com/a/59866840/125349, “this is not the last place the list will be used
- # (and interpreted by CMake). [It] is then used to populate CTestTestfile.cmake, which is later
- # read by CTest to setup your test environment.”
set_tests_properties(${test_target}
- PROPERTIES ENVIRONMENT "PATH=$<JOIN:$<TARGET_RUNTIME_DLL_DIRS:${test_target}>;${path},\\;>")
+ PROPERTIES ENVIRONMENT_MODIFICATION
+ # For each DLL dependency X, make the PATH=path_list_prepend:X environment modification.
+ "$<LIST:TRANSFORM,$<TARGET_RUNTIME_DLL_DIRS:${test_target}>,PREPEND,PATH=path_list_prepend:>")
In the 2nd case it’s clearly a huge win, although the repetition in PREPEND,PATH=path_list_prepend: is a little baffling until you figure it out. I fear the first case is still not actually correct: to make it robust I’d need to generate surrounding quotes around each of the arguments to -E env --modify. Generating one set of quotes surrounding the wholesale setting of PATH was easy, but making this correct would require two additional levels of generator expression AFAICT. The problem is that the simple version will often pass testing because of local conditions (no spaces in path names). I only point this out because real-world usage seems to indicate the new feature might need some adjustment.
Using the SHELL_PATH expression in the first case, as suggested by @starball, might be a slight improvement in explicitness. Since there are no DLLs on any platform but Windows, though, the existing code worked everywhere.
It’s been a while, so I don’t remember why I did that. I’d guess that when the list is empty if you pass --modify cmake complains, but I can’t reproduce that now, so I’ll update the example, thanks. I think my concern about the correctness of that code and the complexity of the correct code stands, unless… wait, I see a VERBATIM argument to add_custom_command. Could that be used to handle the quoting problem? /cc @ben.boeckel
Possibly. I’ve never found VERBATIM to be necessary, but it apparently solves problems. Maybe shell/CMake quoting rules are just too deeply ingrained in me at this point…
. Can you file an issue to improve wording to at least encourage looking there?
.