cmake_path: Suggestions for renaming/removing some subcommands/keywords
Apologies for the length, all of this should have been discussed in the original merge request, but I didn't get to review it in time.
For background context, see the following forum posts:
- https://discourse.cmake.org/t/poorly-explained-cmake-path-subcommands-cmake-3-19
- https://discourse.cmake.org/t/inconsistent-behavior-of-cmake-path-subcommands-taking-a-variable-name-instead-of-a-string
A new cmake_path()
command is being added for CMake 3.19. Some of the subcommand names are a bit confusing, sometimes further sub options of the subcommands add to the confusion or offer opportunities for improved consistency across a set of subcommands. I'm also a bit concerned that the wording and set of functionality leans too heavily on C++ std::filesystem::path
and strays from more familiar CMake terminology and style in some cases.
I'm raising the following proposed changes for discussion. We would need to resolve these discussions quickly, since any changes should be part of the 3.19.0 official release and we are already well into the release candidate phase.
cmake_path(CONCAT)
Remove Having both APPEND
and CONCAT
is confusing. The only difference seems to be that APPEND
inserts a /
path separator between appended items and CONCAT
does not. Based on this, CONCAT
seems more like a string operation than a path operation. That being the case, CONCAT
seems both unnecessary and inappropriate as a cmake_path()
subcommand. Therefore, I propose that we remove the cmake_path(CONCAT)
subcommand.
cmake_path(CONVERT)
using <input>
instead of <path-var>
Inconsistency of The two cmake_path(CONVERT)
subcommands are the only ones that take an <input>
instead of a <path-var>
as the first argument after the subcommand keyword. It appears this is to support input strings of the form $ENV{PATH}
without having to first put them in a regular CMake variable. In this forum comment, an argument was put forward that using <path-var>
for (almost) every subcommand was more consistent and desirable than using <input>
. The CONVERT
subcommands seem to stand out in contradiction to that. If using $ENV{PATH}
as justification for using <input>
instead of <path-var>
for CONVERT
subcommands, the same justification could also be applied to other cmake_path()
subcommands which could also potentially source their input from environment variables.
Conversion subcommands seem inconsistent
The four existing conversion subcommands have the following form:
-
cmake_path(CMAKE_PATH <path-var> [NORMALIZE] <input>)
<-- This looks like an error, should it be<out-var>
? -
cmake_path(NATIVE_PATH <path-var> [NORMALIZE] <out-var>)
<-- Or is this one wrong? cmake_path(CONVERT <input> TO_CMAKE_PATH_LIST <out-var>)
cmake_path(CONVERT <input> TO_NATIVE_PATH_LIST <out-var>)
Following on from the previous section, a potentially more consistent set of subcommands that uses <path-var>
consistently would be the following:
cmake_path(CONVERT <path-var> TO_CMAKE_PATH [OUTPUT_VARIABLE <out-var>] [NORMALIZE])
cmake_path(CONVERT <path-var> TO_CMAKE_PATH_LIST [OUTPUT_VARIABLE <out-var>] [NORMALIZE])
cmake_path(CONVERT <path-var> TO_NATIVE_PATH [OUTPUT_VARIABLE <out-var>] [NORMALIZE])
cmake_path(CONVERT <path-var> TO_NATIVE_PATH_LIST [OUTPUT_VARIABLE <out-var>] [NORMALIZE])
Some advantages of this alternative set of forms include:
- It avoids the odd-looking
cmake_path(CMAKE_PATH)
form. - Much more consistent and predictable forms for converting to cmake or native paths. It is unclear whether both
TO_XXX_PATH
andTO_XXX_PATH_LIST
forms are both needed or whether a single keyword could cover both use cases. - Allows in-place modification or assigning the result to a separate variable (something that was put forward as a design goal in the forum discussion).
A drawback to the alternative forms is that for the path list case, the <path-var>
might now hold a list of paths rather than a single path. The docs currently state that <path-var>
never holds a list. This may just be a doc issue rather than actual functionality though (I'm not worried about this one myself, I'm pretty sure we can address this with appropriate changes to the docs and I'm happy to do the required doc changes).
<path-var>
and <input>
Comparison mixes The signature for comparison is:
cmake_path(COMPARE <path-var> <OP> <input> <out-var>)
As a user, I immediately ask myself why does the left-hand-side of the <OP>
have to be a variable and the right-hand-side have to be a string? It would be much clearer if this was just:
cmake_path(COMPARE <input1> <OP> <input2> <out-var>)
But that would be inconsistent with the stated design goal of having most/all subcommands use <path-var>
consistently for the first argument after the subcommand keyword. Personally, I think this is one example of where that design goal hurts us as much as it helps us.
NORMAL_PATH
subcommand
Inconsistent naming of This seems like it should be named NORMALIZE_PATH
. The NORMALIZE
keyword is used in other subcommands and it seems strange to use NORMAL
instead of NORMALIZE
here. I'm also wondering if this is closer to a conversion operation than a generation operation. Maybe the "generation" heading is not capturing what that set of subcommands is doing. They seem to be more about converting between forms, whereas "generation" implies creating something.
PROXIMATE
subcommand is unclear
Use for I can't tell what purpose PROXIMATE
serves. I see it mentioned in the C++ std::filesystem::path
docs in the context of lexical-related things, but for CMake users, this subcommand seems to be of dubious usefulness. I propose we remove it, or at least rename it to something that makes its purpose much clearer.
Other remarks
I find myself wanting to press for reconsideration of the dominant use of <path-var>
instead of <input>
for some subcommands, but this issue already covers a lot. We may ultimately have to revisit that area, but first let's see how far we can get with focusing on just the above points. We have to keep this discussion relatively short and implement any changes fairly quickly, so we can't afford to drag this out.