Skip to content

GNUInstallDirs: Factor out logic into macros and convert PATHs

Roger Leigh requested to merge rleigh/cmake:installdir-cache-type into master

This change:

  • Simplifies the module logic by factoring out the repeated logic into two macros (for the ways of specifying the default)
  • Forces the cache variable type to be set to PATH

The latter change might be more controversial, and I don't know if there are any pre-existing cmake policies for this type of thing. Essentially, there are some usability problems with the current approach. The existing approach:

  • creates cache variables of type PATH for all paths
  • these appear in the cmake GUI
  • if variables are set on the command-line, they are of type UNINITIALIZED and are (1) not PATHs and have no path normalisation and (2) are excluded from the GUI

The patch here forces any UNINITIALIZED types to PATH, which has the following benefits:

  • it converts any system-specific paths to normalised cmake paths, which prevents any downstream breakage with backslashes on Windows
  • it makes any explicitly set paths visible in the GUI when the type was not specified; previously any unset paths would be present, while set (but not PATH type) paths would be hidden, which is a somewhat counter-intuitive
  • it adds the appropriate help string (previously would be No help, variable specified on the command line.)

These are not problems for users intimately familiar with the internal workings of cmake, who know to set with :PATH, but for an end user who just tries to set the path and has the build blow up, or is unable to effect a change in the GUI, this makes the process robust and a bit more friendly.

(It strikes me that this approach could be used in other places as well. For example, testing just now with CMAKE_MODULE_PATH on Windows, a path with backslashes makes cmake blow up while explictly setting the PATH type makes it work. An automatic conversion to the required type for all path cache variables would be quite nice. While the macro I wrote here could be generalised and moved elsewhere, I wonder if when set(... CACHE ...) is used (with or without FORCE), an implicit conversion of the cache type would be in order, since for most cache variables this is the only place the type is set, and it's not known up-front when initially consuming the variables from the command-line. Maybe only in the case where the type is UNINITIALIZED for safety, and to not override the user when they specify the type explicitly. This would allow modules to enforce their cache variables to be of the type they they need, since it's not known up front until the cache setting commands are evaluated.)

Merge request reports