[cmake-developers] Topic hdf5-module-bug-fix

[Will Dicharry]

Brad King wrote:
> Hi Will,
>
> We reviewed this topic today during our merge-to-master meeting.  Thanks
> for working on this.  These are good improvements.  However, there are
> two problems we noticed:
>
> (1) The first patch http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=34ea1790
> contains this hunk:
>
>   -include(${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake)
>   +include(FindPackageHandleStandardArgs)
>
> The reference to CMAKE_CURRENT_LIST_DIR is intentional and must remain.
> See here for an explanation:
>
>   http://cmake.org/gitweb?p=cmake.git;a=commit;h=c4275592
>
>   

I should have read the commit message that went with that line, I 
thought it was a little strange.

> (2) The third patch http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=3978f322
> has this hunk:
>
>   +# Try to find HDF5 using an installed hdf5-config.cmake
>   +find_package( HDF5 QUIET NO_MODULE )
>   +if( HDF5_INCLUDE_DIR )
>   +    set( HDF5_INCLUDE_DIRS ${HDF5_INCLUDE_DIR} )
>   ...
>
> but the module ends in
>
>   # For backwards compatibility we set HDF5_INCLUDE_DIR to the value of
>   # HDF5_INCLUDE_DIRS
>   set( HDF5_INCLUDE_DIR "${HDF5_INCLUDE_DIRS}" )
>
> Therefore if someone finds HDF5 twice the second time may take the wrong
> logic path.  The proper value to test after the find_package call is
> HDF5_FOUND to know whether or not the config file was found and loaded.
>
>   

Won't this have the same problem? HDF5_FOUND gets set by 
FindPackageHandleStandardArgs, so the second time find_package(HDF5) 
gets called on a system with the autoconf HDF5 build we'll take the 
wrong control path and call get_target_properties on targets that don't 
exist. That's why I used the INCLUDE_DIR in the first place (forgetting 
about the backwards compatibility thing at the end).  Is there something 
else I can branch on?

Thanks,
Will

> Please add commits to the end of your topic to address these problems and
> merge it to next again through the topic stage.
>
> Thanks,
> -Brad
>   

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5852 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20110524/582d7faa/attachment-0002.bin>