CMake: handle multiple paths stored in XXX__INCLUDE_DIR when checking for version strings

[jmckenna]

Feature description

Packagers must often specify multiple paths inside a single CMake variable, such as for multiple includes, or libs, for a single driver.

However GDAL's CMake system assumes that the variable contains only one value (path), when looking for version strings. For example:

• in FindLibKML.cmake see the line
  • file(STRINGS ${LIBKML_INCLUDE_DIR}/kml/base/version.h libkml_version_h_string
• a packager might have set LIBKML_INCLUDE_DIR = "LIB1/include;LIB1/install/include;LIB3/include"
• that causes warnings of
  • LIBXXX has unknown version. Assuming it is at least matching the minimum version required of X.X

Additional context

I've been modifying the FindXXX.cmake paths for the version strings by hand for many GDAL drivers, but I am sure that it would be beneficial to all packagers if this was handled somehow out of the box. thanks.

[rouault]

However GDAL's CMake system assumes that the variable contains only one value (path), when looking for version strings

@dg0yt Any opinion on that?

It seems to me that the (perhaps implicit) convention for a FOO_INCLUDE_DIR CMake variable to be single path. At least this is the case when looking at most FindFOO.cmake files provided by CMake itself, like https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/FindLibXml2.cmake?ref_type=heads#L84 , https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/FindXercesC.cmake?ref_type=heads#L84 or https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/FindZLIB.cmake?ref_type=heads#L177 . https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/FindHDF5.cmake?ref_type=heads#L975 is an exception where it actually supports a list of files, but it does that by iterating over the HDF5_INCLUDE_DIRS plural variable, potentially populated by other means that by setting HDF5_INCLUDE_DIR singular.

[dg0yt]

This is legacy CMake, with all its downsides and exceptions. CMake's Find modules cannot be generalized too much because they use different styles.

From general experience:

• <Pkg>_INCLUDE_DIR (singular) should normally just be a single path, as returned by find_path.
• OTOH it is often used to initialize <Pkg>_INCLUDE_DIRS (plural).
• Sometimes it is necesary to provide a list as input, due to public dependencies (libkml: zlib/miniz?).
• So it is better to be prepared for the variable to contain lists for non-trivial packages.

For the concrete problem, I would suggest a simple solution

find_path(LIBKML_VERSION_H kml/base/version.h PATHS ${LIBKML_INCLUDE_DIR} NO_DEFAULT_PATH)
if(LIBKML_VERSION_H)
    file(STRINGS "${LIBKML_VERSION_H}" libkml_version_h_string)
endif()

OTOH I can just ignore the version warning if I know that my libkml is new enough. Soft problem. Find modules can't cover static linkage and varying transitive dependencies very well, that's my hard struggle.

[jmckenna]

@dg0yt agreed, I was going to respond earlier that packagers use FOO_INCLUDE_DIR to trigger CMake's internal FOO_INCLUDE_DIRS parameter. Sometimes indeed it is necessary to set multiple paths in these variables. My point I guess, was to point this out, as all of GDAL's FindXX.cmake files in /master/cmake/modules/packages heavily rely on those variables to set the version string (and my CMakeCache.txt FIND_PACKAGE_MESSAGE_DETAILS_FOO contains so many empty v() string version values, and values of vSOME TEXT x.x.x), so I think when we are pointing GDAL devs to a template to use to create a new driver, we assume that those variables include multiple paths, and when checking for version strings to iterate through the path values.

In other words, I wasn't trying to add more work for us, as you said it is a soft problem, but one that packagers must watch closely, [for instance recently changes were made to FindECW.cmake leveraging this important string version], instead I was hoping to fix this for any new drivers that are created.

thanks for listening

[jmckenna]

More background for other readers: this computed version string is used through the CMake build process in GDAL for specific logic (eg. if version string > 4 then do something).

[rouault]

@jmckenna can you list the modules/dependencies for which this would be needed at least for your use case? @dg0yt above propose trick looks very reasonable

[dg0yt]

I think when we are pointing GDAL devs to a template to use to create a new driver, we assume that those variables include multiple paths, and when checking for version strings to iterate through the path values.

I think we should encourage devs to help upstream libs to provide CMake config packages. Once done right upstream, all downstreams benefit, not just GDAL. A config package can be written like a find module. But the key point is that it comes with that lib, and search procedure, version and component checks are handled by CMake, without custom code in GDAL.

above propose trick looks very reasonable

However it creates another cache variable. Good for performance, but something to remember when using the same build dir over a longer time. And it continues to read the file every time. At least it follows -ULIBKML*.