search

ukoethe / vigra

a generic C++ library for image analysis
http://ukoethe.github.io/vigra/
Other
407 stars 191 forks source link

Add compatibility for HDF5 1.12 #499

Closed hmaarrfk closed 2 years ago

hmaarrfk commented 2 years ago

Closes https://github.com/ukoethe/vigra/issues/476

See builds: https://github.com/conda-forge/vigra-feedstock/pull/77

The relevant code in H5version.h is:

#if !defined(H5Oget_info_by_name_vers) || H5Oget_info_by_name_vers == 3
  #ifndef H5Oget_info_by_name_vers
    #define H5Oget_info_by_name_vers 3
  #endif /* H5Oget_info_by_name_vers */
  #define H5Oget_info_by_name H5Oget_info_by_name3
#elif H5Oget_info_by_name_vers == 2
  #define H5Oget_info_by_name H5Oget_info_by_name2
#elif H5Oget_info_by_name_vers == 1
  #define H5Oget_info_by_name H5Oget_info_by_name1
#else /* H5Oget_info_by_name_vers */
  #error "H5Oget_info_by_name_vers set to invalid value"
#endif /* H5Oget_info_by_name_vers */
hmaarrfk commented 2 years ago

I'll remove the WIP once the conda forge builds pass. I hope I didn't get any c-preprocessor macro syntax wrong.

hmeine commented 2 years ago

This change looks great, because it introduces support for 1.12 while still working with older versions.

hmeine commented 2 years ago

I would like to merge this, but I though I'd test a build without it first. Interestingly, that worked, although I have hdf5 1.12.1. After some digging, I found that my H5pubconf.h contains #define H5_USE_110_API_DEFAULT 1.

I learned enough about hdf5 headers now, though, that I trust your patch. ;-)

hmaarrfk commented 2 years ago

If H4pubconf.h is a private header, then it is likely OK to keep that define. I suspect that if it is a public header, it will give some poor developer that depends on vigra a hard time.

Is H5pubconf.h not inlcuded as part of this file?

hmaarrfk commented 2 years ago

I found that my H5pubconf.h contains #define H5_USE_110_API_DEFAULT 1.

Is this referring to a private header of yours, or to a header of Vigra's? I can't find it in the source.