changes to detect HDF5 Hl, build with CRT and detect ZLIB in HDF5

[pedro-vicente]

I made a pull request to netcdf-c with the following:
changes to detect HDF5 HL, build with CRT and detect ZLIB in HDF5
this was explained in more detail in the mailing list

@czender
at the moment I made a temporary fork in the NCO org that will be deleted once this is merged

https://github.com/nco/netcdf-c

these changes allow to build netcdf-c in Windows without any manual changes to the file CMakeLists.txt of netcdf-c
script is in

/nco/cmake/bld.bat

run with

bld.bat

I made the changes to a branch called "prep-v4.6.0" because for some some reason I could not build the master branch

[DennisHeimbigner]

I am a bit confused. There appears to be a check to ensure that HDF5
is built with zlib already in our CMakeLists.txt. Am I looking at the wrong
thing?

[pedro-vicente]

Hi Dennis

what you have now is

CHECK_LIBRARY_EXISTS(${HDF5_C_LIBRARY_hdf5} H5Pset_deflate "" HAVE_H5PSET_DEFLATE)

H5Pset_deflate is a function in the HDF5 source code that is always defined
regardless of HDF5 being linked with ZLIB or not.

what you want is this

CHECK_LIBRARY_EXISTS(${HDF5_C_LIBRARY_hdf5} H5Z_DEFLATE "" HAVE_H5PSET_DEFLATE)

H5Z_DEFLATE is actually a structure, not a function

it is defined in the file H5Zdeflate.c, and only defined if this happens

#ifdef H5_HAVE_FILTER_DEFLATE

meaning if HDF5 is really using the ZLIB filter

so, what was happening was that netcdf was always assuming that ZLIB was present

these are the comments I added to my pull request in the CMakeLists.txt script of netcdf-c

#HDF5 can be optionally linked with the SZIP library (Science Data Lossless Compression Program) and ZLIB
#Symbol to detect in ZLIB can be only H5Z_DEFLATE, a structure only defined in H5Zdeflate.c if the filter is enabled
#For SZIP the structure can be only H5Z_SZIP, defined in H5Zszip.c if the filter is enabled
#check_library_exists() tries to link a temporary program with these symbols
#On MSVC for this detection to work, the HDF5 library MUST HAVE as additional dependencies the ZLIB and SZIP libraries,
#which is not a requirement for the library to build successfully

as you can see, there is more to the story for the MSVC case.
In the next few days, I'll try to explain here what the problem is in more detail

[pedro-vicente]

also...

the return value of the macro, HAVE_H5PSET_DEFLATE, is not actually being used anywhere

CHECK_LIBRARY_EXISTS(${HDF5_C_LIBRARY_hdf5} H5Pset_deflate "" HAVE_H5PSET_DEFLATE)

the following lines show whats is made for the SZIP case

CHECK_LIBRARY_EXISTS(${HDF5_C_LIBRARY_hdf5} H5Z_SZIP "" USE_SZIP)
  IF(USE_SZIP)
    FIND_LIBRARY(SZIP NAMES szip sz)

[DennisHeimbigner]

Got it, thanks!

[pedro-vicente]

probably a good idea to change the names to "NEED_ZLIB" or "HAVE_ZLIB"

instead of "HAVE_H5PSET_DEFLATE" and "USE_SZIP" ("use_szip" is misleading, it can mean that is an option to use or not, while "need" means you need the library to link and "have" means that the library has those symbols

this is what I have in the NCO detection of ZLIB

https://github.com/nco/nco/blob/master/CMakeLists.txt

message("-- Detecting if HDF5 library ${HDF5_LIBRARY} needs the ZLIB library...")
check_library_exists(${HDF5_LIBRARY} H5Z_DEFLATE "" NEED_ZLIB)
if (NEED_ZLIB)
  message("${color_blue}-- ZLIB library is needed...${color_reset}")
else()
  message("${color_blue}-- ZLIB library is not needed...${color_reset}")
endif()

[pedro-vicente]

regarding the Windows static CRT issue also in the pull request.

if you try the netcdf option to use CRT

cmake .. -DNC_USE_STATIC_CRT=ON

you'll get this error

CMake Error at CMakeLists.txt:333 (specify_static_crt_flag):
  Unknown CMake command "specify_static_crt_flag".

the macro "specify_static_crt_flag" needs to be defined before its call at line 333

If it was me I would just get rid of the macro and keep it simple like this, again what we did for NCO

set(MSVC_USE_STATIC_CRT off CACHE BOOL "Use MT flags when compiling in MSVC")
if (MSVC)
  if (MSVC_USE_STATIC_CRT)
     message("-- Using static CRT ${MSVC_USE_STATIC_CRT}")
     foreach(flag_var CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE
                          CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO
                          CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE
                          CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO)
       string(REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}")
     endforeach()
  endif()
endif()

[pedro-vicente]

finally, the last thing is a request if you could please add this option to netcdf

cmake .. -DHDF5_HL_INCLUDE_DIR=<path of HDF5 HL>

by adding this line in the script

INCLUDE_DIRECTORIES(${HDF5_HL_INCLUDE_DIR})

the issue is, if we don't do a "make install" then the HDF5 High Level header (hdf5_hl.h) and the HDF5 header (hdf5.h) reside in different places and then the compiler complains it cannot find the HDF5 HL header

I know if we do a "make install" (which actually for this case just copies the headers to a single place) this would not happen, but we are trying to keep things as simple as possible (no copying files around) to use this in a automated build script

you can keep the option an undocumented "super-developer" option ...

I use this to build netcdf in the NCO build script that clones and builds all the NCO dependencies

https://github.com/nco/nco/blob/master/cmake/bld.bat

cmake .. -G %MSVC_VERSION% ^
           -DNC_USE_STATIC_CRT=%STATIC_CRT% ^
           -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON ^
           -DENABLE_TESTS=OFF ^
           -DBUILD_SHARED_LIBS=OFF ^
           -DHDF5_HL_LIBRARY=%root%/hdf5/build/bin/Debug/libhdf5_hl_D.lib ^
           -DHDF5_C_LIBRARY=%root%/hdf5/build/bin/Debug/libhdf5_D.lib ^
           -DHDF5_INCLUDE_DIR=%root%/hdf5/src ^
           -DZLIB_LIBRARY:FILE=%root%/zlib/build/Debug/zlibstaticd.lib ^
           -DSZIP_LIBRARY:FILE=%root%/szip/build/bin/Debug/libszip_D.lib ^
           -DZLIB_INCLUDE_DIR:PATH=%root%/zlib ^
           -DHAVE_HDF5_H=%root%/hdf5/build ^
           -DHDF5_HL_INCLUDE_DIR=%root%/hdf5/hl/src ^
           -DCURL_LIBRARY=%root%/curl/builds/libcurl-vc14-x64-debug-static-ipv6-sspi-winssl/lib/libcurl_a_debug.lib ^
           -DCURL_INCLUDE_DIR=%root%/curl/include

as you can see, it includes

-DHDF5_HL_INCLUDE_DIR

[pedro-vicente]

thanks for the merge

more info on this

On MSVC for this detection to work, the HDF5 library MUST HAVE as additional dependencies the ZLIB and SZIP libraries,

this is explained here

https://github.com/nco/nco/blob/master/cmake/README.md

see section

Changes needed for ZLIB and SZIP detection in NCO

what this means is that the CMake generated Visual Studio projects must be MANUALLY edited and then the HDF5 library relinked from within Visual Studio

for example

edit hdf5-static.vcxproj and add full path of ZLIB and SZIP libraries as dependencies

<Lib>
<AdditionalOptions>%(AdditionalOptions) /machine:x64</AdditionalOptions>
<AdditionalDependencies>E:\nco\cmake\zlib\build\Debug\zlibstaticd.lib;E:\nco\cmake\szip\build\bin\Debug\libszip_D.lib</AdditionalDependencies>
</Lib>

having to manually edit the HDF5 /netCDF generated projects is a not a good option for an automated script; at the moment I don't have a solution to have CMake do this automatically

Note that for Linux this problem does not happen

[WardF]

You can use the NC_EXTRA_DEPS option when building in order to pass additional libraries, e.g.

-DNC_EXTRA_DEPS=szlib

or

-DNC_EXTRA_DEPS=-lszlib

Of course, I've just noticed a problem with the syntax in CMakeLists.txt; I've just pushed a fix for this and it will be in the upcoming release.