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
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ cmake-developers mailing list [email protected] http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
