[cmake-developers] Volunteering to maintain a new module: FindGSL.cmake

[Brad King]

On 12/02/2014 07:52 PM, Thompson, KT wrote:
> 1. The command "add_library(GSL::gsl UNKOWN IMPORTED)" generates
> warnings on platforms that do not support shared libraries.

That is a bug in the warning.  Fixed:

 add_library: Fix target type check for non-shared-lib platforms
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=bd360ee3

The check can be dropped from the version of FindGSL that goes in CMake.

> revised FindGSL.cmake continues to use GSL_ROOT_DIR.

That is fine.  The discussion Eike and I had was heading toward using them.

> The decision to eliminate XXX_ROOT_DIR type variables from all

It was not so much a decision to eliminate as an historical tendency
to not add them in the first place.  Some modules already have them
anyway though.

> # === Imported Targets ===

Please try installing sphinx and enabling the SPHINX_HTML build option of
CMake.  Then look at the Utilities/Sphinx/html documentation generated
for this module.  Update the reST syntax accordingly.

> #  GSL_ROOT_DIR       - The top level directory of the discovered GSL 
> #                       install (useful if GSL is not in a standard location)

My comment was not about removing GSL_ROOT_DIR altogether, but about not
documenting it as a guaranteed result variable that applications can use.
Instead it should be treated as an input/hint variable that may not be set
but will be used if it is set.  It's just a matter of which section documents
it.

>     file( STRINGS "${GSL_INCLUDE_DIRS}/gsl/gsl_version.h" gsl_version_h_contents )

You could use the REGEX option to file(STRINGS) to shorten the amount of data loaded.

>       if( ${entry} MATCHES "define GSL_VERSION")
>          string( REGEX REPLACE ".*([0-9].[0-9][0-9]).*" "\\1" GSL_VERSION ${entry} )
>       endif()

With the proper expression in the if(MATCHES) test you can get the results from
CMAKE_MATCH_1, CMAKE_MATCH_2, etc. variables without a separate string(REGEX)
command.

>   REQUIRED_VARS 
>     GSL_INCLUDE_DIRS 
>     GSL_LIBRARIES

The singular names (find_* command results) should be listed here because
the error messages generated when they are not set are intended to ask the
user to set the values in the cache.  The plural names are not what the
user would set.

>     GSL_ROOT_DIR

The GSL_ROOT_DIR should not be required as mentioned above.

Thanks,
-Brad