Use only one find_package call for zlib. #5280
Conversation
byrnHDF
added
Priority - 2. Medium ⏹
byrnHDF
requested review from
lrknox,
derobins,
fortnern,
jhendersonHDF,
qkoziol,
vchoi-hdfgroup,
bmribler,
glennsong09,
mattjala and
brtnfld
as code owners
|
Tested minimal cmake configure commands, e.g. cmake -G "Unix Makefiles" -DHDF5_ENABLE_ZLIB_SUPPORT=ON ../hdf5, on jelly, ec2-ubuntu24.04, and macmini-m1. In a spack environment and outside the spack environment on jelly and macmini-m1. In the spack environment the previous zlib build in spack is added; outside it picks up the one beneath /usr. On Windows 11 I tested with a VCPkg installed zlib, szip(libaec), and curl and ssl for ros3. I added CMake ROOT variables for these since they were not system installs. I ran builds and tests with no errors/failures on all the machines. |
| message (VERBOSE "Filter HDF5_ZLIB package name:${PACKAGE_NAME}") | ||
| if (HDF5_MODULE_MODE_ZLIB) | ||
| # Expect that the default shared library is expected with FindZLIB.cmake | ||
| find_package (ZLIB MODULE) |
There was a problem hiding this comment.
| find_package (ZLIB MODULE) | |
| find_package (ZLIB MODULE REQUIRED) |
There was a problem hiding this comment.
Issue #5306 created
| else () | ||
| set(ZLIB_SEARCH_TYPE shared) | ||
| endif () | ||
| find_package (ZLIB NAMES ${PACKAGE_NAME} CONFIG OPTIONAL_COMPONENTS ${ZLIB_SEARCH_TYPE}) |
There was a problem hiding this comment.
| find_package (ZLIB NAMES ${PACKAGE_NAME} CONFIG OPTIONAL_COMPONENTS ${ZLIB_SEARCH_TYPE}) | |
| find_package (ZLIB NAMES ${PACKAGE_NAME} CONFIG REQUIRED OPTIONAL_COMPONENTS ${ZLIB_SEARCH_TYPE}) |
There was a problem hiding this comment.
Issue #5306 created
|
Can you use |
I understand the reasoning, however, there is still a group of devs, which are not convinced that a FATAL_ERROR should happen for not finding a compression library - also, if zlib is built in-line then that process also would need to be changed. |
Right now it's already a fatal error: if (HDF5_ENABLE_ZLIB_SUPPORT)
... find_package calls
if (H5_ZLIB_FOUND)
...
else ()
set (HDF5_ENABLE_ZLIB_SUPPORT OFF CACHE BOOL "" FORCE)
message (FATAL_ERROR " ZLib support in HDF5 was enabled but not found")
endif ()
endif ()
|
|
|
Issue #5306 created to deal with the REQUIRED argument of find_package. |
There was a problem hiding this comment.
homebrew-installed libaec and szip were found with command "cmake -DHDF5_ENABLE_SZIP_SUPPORT -DHDF5_ENABLE_ZLIB_SUPPORT ../byrn-hdf5", which contained changes for PRs #5280 and #5298. ctest ran deflate and szip tests; all tests passed. This was on an older mac 10.13, the only one where I have privileges for system installs.
Addresses #5155, and related #4614 #4904
zlib system installs tend to not have CMake config files and therefore require that a plain vanilla find_package call be used. This uses the find module for zlib. For more control over what gets found, an option to use config mode of find package is added. Which allows the use of static libs and better target properties. Also native zlib-ng, which usually has config files, would be better served with the config mode.