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

Attachment: FindGSL.cmake
Description: FindGSL.cmake

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to