Issue 699: add a workaround for read_chunk in hdf5 2.0

[jkotan]

It adds a workaround for read_chunk in hdf5 2.0 and adds missing V200 definition.
To make full support of read_chunk we would need to add either

• size to dataspace and datatype traits
• or adds read_chunk with additional size argument

[ggonei]

I don't see any issue with the HDF2 stuff because it's all conditional, but maybe it would be good to pull out the PIXI stuff into a different MR to make either easier to revert? Would be good to get @milnowak or André to add some thoughts there on the CI

[jkotan]

I don't see any issue with the HDF2 stuff because it's all conditional, but maybe it would be good to pull out the PIXI stuff into a different MR to make either easier to revert? Would be good to get @milnowak or André to add some thoughts there on the CI

I've added issue_699c branch to illustrate the problem. After commenting workaround you get https://github.com/ess-dmsc/h5cpp/actions/runs/23888511368/job/69656363675

 [ 90%] Building CXX object test/node/CMakeFiles/node_test.dir/dataset_direct_chunk_test.cpp.o
 │ │ In file included from $SRC_DIR/src/h5cpp/hdf5.hpp:88,
 │ │                  from $SRC_DIR/test/node/dataset_direct_chunk_test.cpp:32:
 │ │ $SRC_DIR/src/h5cpp/node/dataset.hpp: In instantiation of 'uint32_t hdf5::node::Dataset::read_chunk(T&, const hdf5::datatype::Datatype&, std::vector<long unsigned int>&, const hdf5::property::DatasetTransferList&) const [with T = std::vector<short unsigned int>; uint32_t = unsigned int]':
 │ │ $SRC_DIR/src/h5cpp/node/dataset.hpp:940:20:   required from 'uint32_t hdf5::node::Dataset::read_chunk(T&, std::vector<long unsigned int>, const hdf5::property::DatasetTransferList&) const [with T = std::vector<short unsigned int>; uint32_t = unsigned int]'
 │ │   940 |   return read_chunk(data, mem_type_holder.get(data), offset, dtpl);
 │ │       |          ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 │ │ $SRC_DIR/test/node/dataset_direct_chunk_test.cpp:117:24:   required from here
 │ │   117 |             dataset.read_chunk(read_chunk_value, {i, 0, 0});
 │ │       |             ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 │ │ $SRC_DIR/src/h5cpp/node/dataset.hpp:953:23: error: too few arguments to function 'herr_t H5Dread_chunk2(hid_t, hid_t, const hsize_t*, uint32_t*, void*, size_t*)'
 │ │   953 |       if(H5Dread_chunk(static_cast<hid_t>(*this),
 │ │       |                       ^
 │ │ In file included from $BUILD_PREFIX/include/hdf5.h:24,
 │ │                  from $SRC_DIR/src/h5cpp/core/hdf5_capi.hpp:44,
 │ │                  from $SRC_DIR/src/h5cpp/hdf5.hpp:27:
 │ │ $BUILD_PREFIX/include/H5Dpublic.h:1320:15: note: declared here
 │ │  1320 | H5_DLL herr_t H5Dread_chunk2(hid_t dset_id, hid_t dxpl_id, const hsize_t *offset, uint32_t *filters,
 │ │       |               ^~~~~~~~~~~~~~
 │ │ make[2]: *** [test/node/CMakeFiles/node_test.dir/build.make:457: test/node/CMakeFiles/node_test.dir/dataset_direct_chunk_test.cpp.o] Error 1
 │ │ make[1]: *** [CMakeFiles/Makefile2:1394: test/node/CMakeFiles/node_test.dir/all] Error 2
 │ │ make: *** [Makefile:146: all] Error 2

Basically, in the new version of read_chunk i.e. read_chunk2 there is an additional parameter size_t* which contains byte size of the buffer which should protect overwriting in memory (if the buffer is too small the data is not read)

[jkotan]

I've just added the read_chunk implementation with the byte_size parameter. In case we add the size method we can replace the H5Dread_chunk1 call by read_chunk with the byte_size calculated from the corresponding traits

[jkotan]

The pixi stuff was added to test the current PR (with hdf5 2.10). The pixi tests would fail separetrly. On the other hand the PR cannot be fully tested without pixi test. Other option would be to upgrade conan tests to hdf5 2

[ggoneiESS]

yeh that all makes sense then, maybe this @ggonei guy wasn't thinking things through very well ;)

[milnowak]

I'm not super familiar with pixi but this looks good to me. It actually gave me a few ideas for how we could use it in our internal pipelines thanks!