Skip to content
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.

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

hdf5 1.12.0 support #476

Closed
ArchangeGabriel opened this issue Apr 12, 2020 · 5 comments · Fixed by #499
Closed

hdf5 1.12.0 support #476

ArchangeGabriel opened this issue Apr 12, 2020 · 5 comments · Fixed by #499

Comments

@ArchangeGabriel
Copy link

hdf5 1.12.0 introduces some API changes. This results in build error:

/build/vigra/src/vigra-1.11.1/src/impex/hdf5impex.cxx: In function ‘H5O_type_t vigra::HDF5_get_type(hid_t, const char*)’:
/build/vigra/src/vigra-1.11.1/src/impex/hdf5impex.cxx:193:60: error: too few arguments to function ‘herr_t H5Oget_info_by_name3(hid_t, const char*, H5O_info2_t*, unsigned int, hid_t)’                                                                                        
  193 |     H5Oget_info_by_name(loc_id, name, &infobuf, H5P_DEFAULT);
      |                                                            ^
In file included from /usr/include/H5Apublic.h:22,
                 from /usr/include/hdf5.h:23,
                 from /build/vigra/src/vigra-1.11.1/include/vigra/hdf5impex.hxx:51,
                 from /build/vigra/src/vigra-1.11.1/src/impex/hdf5impex.cxx:38:
/usr/include/H5Opublic.h:188:15: note: declared here
  188 | H5_DLL herr_t H5Oget_info_by_name3(hid_t loc_id, const char *name, H5O_info2_t *oinfo,
      |               ^~~~~~~~~~~~~~~~~~~~
make[2]: *** [src/impex/CMakeFiles/vigraimpex.dir/build.make:174: src/impex/CMakeFiles/vigraimpex.dir/hdf5impex.cxx.o] Error 1

There might be others, see the migrating help.

@hmeine
Copy link
Collaborator

hmeine commented Apr 14, 2020

What do you suggest?

  1. Should the configuration check that hdf5 version is <1.12?
  2. Should the code be adapted to version 1.12 and the CMakeFiles require that version?
  3. Should the code support older and newer versions and the CMakeFiles create a conditional #define?

Finally, would you be willing to supply a pull request?

@ArchangeGabriel
Copy link
Author

If I’d care only about my use case (packaging for Arch Linux), I’d say 2., but 3. might be generally better.

I’d love to do a PR, but I definitively lack the skills for it (I only have very superficial knowledge of C/C++ and never wrote something else than a simple exercise in either of those languages). So while I can see the issue are changes in some H5 functions arguments, I would not trust my ability to fix them correctly.

@dvzrv
Copy link
Contributor

dvzrv commented Apr 23, 2020

@hmeine I fixed a similar issue for csound, it was relatively trivial:
csound/csound#1314

@ArchangeGabriel
Copy link
Author

ArchangeGabriel commented Apr 24, 2020

As a temporary measure I got it to build passing -DCMAKE_C_FLAGS="-DH5_USE_110_API" as a cmake arg. ;)

EDIT: Scratch that, I did not build against 1.12 actually, it still fails as above…

EDIT2: Of course, -DCMAKE_CXX_FLAGS="-DH5_USE_110_API" in addition seems to work.

@FRidh FRidh mentioned this issue Apr 4, 2021
10 tasks
LeSuisse added a commit to LeSuisse/nixpkgs that referenced this issue Apr 4, 2021
vigra is not yet compatible with the v1.12 API [0][1].

[0] ukoethe/vigra#476
[1] ukoethe/vigra#476
@hmaarrfk
Copy link
Collaborator

diff --git a/src/impex/hdf5impex.cxx b/src/impex/hdf5impex.cxx
index 2c68342e..682e0126 100644
--- a/src/impex/hdf5impex.cxx
+++ b/src/impex/hdf5impex.cxx
@@ -190,7 +190,7 @@ H5O_type_t HDF5_get_type(hid_t loc_id, const char* name)
 {
     // get information about object
     H5O_info_t infobuf;
-    H5Oget_info_by_name(loc_id, name, &infobuf, H5P_DEFAULT);
+    H5Oget_info_by_name(loc_id, name, &infobuf, H5O_INFO_BASIC, H5P_DEFAULT);
     return infobuf.type;
 }

might work, the other alternative is to use
H5Oget_info_by_name1 instead of H5Oget_info_by_name

https://portal.hdfgroup.org/display/HDF5/Migrating+from+HDF5+1.10+to+HDF5+1.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants