HDF5 not found if using hdf5-config.cmake

[jsharpe]

netcdf-c/CMakeLists.txt

Line 678 in db41b2a

# HDF5_C_LIBRARIES may be defined. We must check for either.

In this case only HDF5_hdf5_LIBRARY_RELEASE/HDF5_hdf5_LIBRARY_DEBUG is set.

Also the logic to detect if HDF5 was built in parallel is incorrect. In the case of using the config file this information is available via the variable HDF5_IS_PARALLEL. In the case that the library is named in the variable HDF5_hdf5_LIBRARY.

The correct fix now that the minimum cmake version has been bumped is to simply do find_package(hdf5 REQUIRED) and then link against the meta target hdf5::hdf5-shared or hdf5::hdf5-static as appropriate. This then sets up all the transitive link dependencies and propagates the correct defines for H5_BUILD_AS_DYNAMIC_LIB based upon the options the library was built with.

See the talk at C++Now available here: https://www.youtube.com/watch?v=bsXLMQ6WgIk for more details on this modern style of CMake usage.

[WardF]

Does this work with hdf5 installs not built with cmake? Do you happen to know the minimum version of hdf5 which works with this? We have had issues with detecting hdf5 across multiple versions and platforms, which is why it is the kludge it currently is. I'm happy to revisit it, thank you for the link, I will review it!

[jsharpe]

I partially implemented a fix for this issue:
zenotech@fa98926
Note I removed the 16 compatibility macro and explicitly changed the code to use the *1 variant of the calls explicitly so that I didn't have to generate configuration files with defines within cmake.

This resolves it for our use case but I don't have the time to or the list of complete set of configurations to test against to get this integrated back to the mainline.

[WardF]

Thanks! I will take a look at your fix, I appreciate you taking the time to follow up!

[jschueller]

@jsharpe your changes are interesting, any chance you'd take another try at it ?