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

[Thompson, KT]

Brad,

Here is a repeat and reformatted version of the email I sent off list (sorry about that).  The attached file also addresses comments from Stephen Kelly concerning IMPORTED_CONFIGURATIONS.

Brad King wrote:

> > 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.

I rebased to the git HEAD to test your fix (bd360ee3 works for my systems)
and removed the unnecessary code that checked if the system supported
shared libraries.

> 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.

I corrected the documentation formatting after building it with
Sphinx.  I will comment that I had a little trouble installing
Sphinx since we do not have root access to our Linux machines.  I
can only install software that I can install at $HOME. (Can you
point me to any instructions for installing Sphynx w/o root
access?)  However, I was able to get it working on a Windows
machine and was able to clean up the documentation a bit.  I also
modified Help/manual/cmake-modules.7.rst and added
Help/module/FindGSL.rst as required.

> 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.

Comments about GSL_ROOT_DIR were moved to the HINTS section only.
I also removed this variable from
find_package_handle_standard_args's REQUIRED_VARS section.  (I
understand your comment now).

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

I simplified the search for GSL_VERSION as you suggested.  Thanks
for the pointer, this code was copied from some very old cmake
scripts w/o much thought -- but your way is clearly better.

> >   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.

I have a better understanding of the REQUIRED_VARS section of
find_package_handle_standard_args now and have made the updates
that you recommended.

A new versions of FindGSL is attached.  The other modified files
are very simple and I will not bother you with those changes.

-kt

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20141204/4c3b9eea/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FindGSL.cmake
Type: application/octet-stream
Size: 9878 bytes
Desc: FindGSL.cmake
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20141204/4c3b9eea/attachment-0001.obj>