Brad King wrote:
Will Dicharry wrote:Sorry for the month of delay, but I've addressed Mike Jackson's concerns below and I think I'm close to having the HDF5 find module ready for submission.Excellent. I have a few comments from quickly glancing at them, but I don't have time for thorough testing. Overall it looks good.There are two modules attached to this message: The FindHDF5.cmake module and an AdjustLibraryVariables.cmake module, which is essentially a copy of what the FindQt4 module does. It seems useful enough to incorporate into the CMake Modules, and I can maintain it if you need a maintainer.I'd like to choose a better name for AdjustLibraryVariables. Perhaps "SelectLibraryConfigurations"? Does it have all the functionality needed to update FindQt4 to use it too (you don't need to do this but it should be easy for the FindQt4 maintainer to do it)?
I agree, SelectLibraryConfigurations is better. I'll rename it. It looks like I need to set ${basename}_LIBRARIES (plural) too in order for the Qt4 module to use it, I'll go ahead and do that.
The find_path and find_library calls need some tweaking. Please read the documentation of these commands to distinguish the cases of PATHS and HINTS keywords. The PATHS should only be last-resort guesses. The HINTS should be locations computed from the system, such as those reported by the hdf5 compiler wrapper tools. Also, paths like /usr/local/include /usr/include are searched automatically and need not be listed.
I'll clean that up, I think the only path I'm specifying that should be in the PATH section is the $HOME/.local/ guess. It seems everything else should be a HINT. Thanks for the tip.
How I addressed Mike Jackson's concerns is addressed in the module documentation at the top of the file, please let me know if anyone has any other concerns.Try placing these modules in the CMake/Modules source tree and running cmake --help-module FindHDF5 cmake --help-module AdjustLibraryVariables to make sure the documentation formats correctly. Also, any macro in the public interface of the module should be documented using a format similar to the CMake command documentation.
What is the convention for keeping a macro out of the public interface? Thanks for your help, Will
Thanks, -Brad
-- Will Dicharry Software Developer Stellar Science Ltd Co
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake
