Skip to content

Revamping the HDF5_VERSION logic. #2095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
WardF merged 6 commits into from
Aug 31, 2021
Merged

Revamping the HDF5_VERSION logic. #2095

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ebbe3b4
Revamping the HDF5_VERSION logic.
WardF Aug 30, 2021
a76245d
Plugged a hole where HDF5_VERSION wouldn't be checked against minimum…
WardF Aug 30, 2021
a777d97
Added logic for checking the version when the HDF5 libraries and incl…
WardF Aug 30, 2021
d0561d6
Caching HDF5_VERSION string for diagnostic purposes.
WardF Aug 30, 2021
5d5a7c4
Correct an issue with appveyor builds, specific to systems where a ba…
WardF Aug 31, 2021
9e6b6bc
Correct logic error.
WardF Aug 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 45 additions & 12 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,12 +670,28 @@ IF(USE_HDF5)
##
SET(HDF5_VERSION_REQUIRED 1.8.10)

# Some versions of HDF5 set HDF5_VERSION_STRING instead of HDF5_VERSION
IF(HDF5_VERSION_STRING AND NOT HDF5_VERSION)
SET(HDF5_VERSION ${HDF5_VERSION_STRING})
ENDIF()


###
# If HDF5_VERSION is undefined, attempt to determine it programatically.
###
IF("${HDF5_VERSION}" STREQUAL "")
MESSAGE(STATUS "Unable to determine hdf5 version. NetCDF requires at least version ${HDF5_VERSION_REQUIRED}")
ELSE()
MESSAGE(STATUS "HDF5_VERSION not detected. Attempting to determine programatically.")
IF (EXISTS "${HDF5_INCLUDE_DIR}/H5pubconf.h")
file(READ "${HDF5_INCLUDE_DIR}/H5pubconf.h" _hdf5_version_lines
REGEX "#define[ \t]+H5_VERSION")
string(REGEX REPLACE ".*H5_VERSION .*\"\(.*\)\".*" "\\1" _hdf5_version "${_hdf5_version_lines}")
set(HDF5_VERSION "${_hdf5_version}")
unset(_hdf5_version)
unset(_hdf5_version_lines)
MESSAGE(STATUS "Found HDF5 libraries version ${HDF5_VERSION}")
ENDIF()

ELSE() # Original HDF5_VERSION blank check.
IF(${HDF5_VERSION} VERSION_LESS ${HDF5_VERSION_REQUIRED})
MESSAGE(FATAL_ERROR
"netCDF requires at least HDF5 ${HDF5_VERSION_REQUIRED}. Found ${HDF5_VERSION}.")
Expand All @@ -684,6 +700,29 @@ IF(USE_HDF5)
ENDIF()
ENDIF()



###
# If HDF5_VERSION is still empty, we have a problem.
# Error out.
###
IF("${HDF5_VERSION}" STREQUAL "")
MESSAGE(FATAL_ERR "Unable to determine HDF5 version. NetCDF requires at least version ${HDF5_VERSION_REQUIRED}. Please ensure that libhdf5 is installed and accessible.")
ENDIF()

####
# Check to see if HDF5 library is 1.10.6 or greater.
# Used to control path name conversion
####
IF(${HDF5_VERSION} VERSION_GREATER "1.10.5")
Copy link
Copy Markdown
Contributor

@edwardhartnett edwardhartnett Aug 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 1.8.22 version of HDF5 works fine for me in CI testing with netCDF.

Copy link
Copy Markdown
Member Author

@WardF WardF Aug 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edwardhartnett Do you mean 1.8.22 works for you in regards to the HDF5_UTF8_PATHS toggle in this block? Or do you mean in general? Taking a look back, the HDF5_VERSION_REQUIRED "minimum version gate" is set to 1.8.10. Our CI tests against 1.8.21, I'll take a look at bumping that to 1.8.22. I've just noticed a path by which HDF5_VERSION might not be checked to see if it meets the required version, so I'll be pushing that change as well.

SET(HDF5_UTF8_PATHS ON)
ELSE()
SET(HDF5_UTF8_PATHS OFF)
ENDIF()

MESSAGE("-- HDF5_UTF8_PATHS (HDF5 version 1.10.6+): ${HDF5_UTF8_PATHS}")


##
# Include the HDF5 include directory.
##
Expand Down Expand Up @@ -718,6 +757,10 @@ IF(USE_HDF5)
ENDIF(${HDF5_VERSION} VERSION_GREATER "1.8.15")

ELSE(MSVC)
####
# Environmental variables in Windows when using MSVC
# are a hot mess between versions.
####

IF(HDF5_hdf5_LIBRARY AND NOT HDF5_C_LIBRARY)
SET(HDF5_C_LIBRARY ${HDF5_hdf5_LIBRARY})
Expand Down Expand Up @@ -816,16 +859,6 @@ IF(USE_HDF5)
SET(HAS_PAR_FILTERS no CACHE STRING "")
ENDIF()

# Check to see if HDF5 library is 1.10.6 or greater.
# Used to control path name conversion
IF(${HDF5_VERSION} VERSION_GREATER "1.10.5")
SET(HDF5_UTF8_PATHS ON)
ELSE()
SET(HDF5_UTF8_PATHS OFF)
ENDIF()

MESSAGE("-- Checking for HDF5 version 1.10.6 or later: ${HDF5_UTF8_PATHS}")

FIND_PATH(HAVE_HDF5_H hdf5.h PATHS ${HDF5_INCLUDE_DIR} NO_DEFAULT_PATH)
FIND_PATH(HAVE_HDF5_H hdf5.h)
IF(NOT HAVE_HDF5_H)
Expand Down
⚡