Refactor Z library detection #2121
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HDF5 can depend on the Z library (in fact required for netCDF). Moved the detection of whether hdf5 was built with zlib up before any other tests that may require linking of the hdf5 library to determine presence/absence of symbols. These tests require that the link line include "-lz" if the hdf5 library was built with libz support. This is typically handled somewhat automatically if shared libraries are being used, but in the static library case, the explicit dependency needs to be specified. For internal CMake checks, it uses the `CMAKE_REQUIRED_LIBRARIES` list to specify the libraries that should be used in a `CHECK_C_SOURCE_COMPILE` or a `CHECK_LIBRARY_EXISTS` call. In the current CMakeLists.txt ordering, the zlib detection is done _after_ the `CHECK_LIBRARY_EXISTS` calls which can cause them to fail and give an incorrect result about whether the function being tested for exists. With the reordering in this PR, I am able to correctly configure netCDF on a CRAY HPC system that uses static libraries by default.
Moving the threads find_package affects the windows build.
Not sure where the almost duplicate `find_path` line came from. Removed it.
There was another almost duplicate `find_path` call in the code that is now removed. It wasn't a part of this PR, but removing just for completeness
Instead of just specifying "z" as a dependent library, use `find_package`
Remove the double quotes I had used in the find_package...
A previous edit added some extraneous whitespace that should not be there...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
HDF5 can depend on the Z library (in fact required for netCDF). Moved the detection of whether hdf5 was built with zlib up before any other tests that may require linking of the hdf5 library to determine presence/absence of symbols. These tests require that the link line include "-lz" if the hdf5 library was built with libz support.
This is typically handled somewhat automatically if shared libraries are being used, but in the static library case, the explicit dependency needs to be specified. For internal CMake checks, it uses the
CMAKE_REQUIRED_LIBRARIESlist to specify the libraries that should be used in aCHECK_C_SOURCE_COMPILEor aCHECK_LIBRARY_EXISTScall. In the current CMakeLists.txt ordering, the zlib detection is done after theCHECK_LIBRARY_EXISTScalls which can cause them to fail and give an incorrect result about whether the function being tested for exists. With the reordering in this PR, I am able to correctly configure netCDF on a CRAY HPC system that uses static libraries by default.I moved the
find_package(threads)out of the HDF5 block since it doesn't seem to be related to HDF5 and nothing is done whether it is found or not... Maybe it can be removed entirely?