diff --git a/CHANGELOG.md b/CHANGELOG.md index f070cd5..2516491 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,11 @@ - `resize`: grow a chunked dataset up to its maximum dimensions (h5py's `Dataset.resize`). - `set_attr`: add or replace an attribute in an object header, in free - space or in a new continuation chunk at the end of the file. + space or in a new continuation chunk at the end of the file. A + version-2 header (h5py `libver='v110'` and later) without an Attribute + Info message gets one, as libhdf5's `H5O__attr_create` adds it: libhdf5 + counts such a header's attributes through that message, and without it + h5py reported `len(obj.attrs) == 0` while listing them. - Each edit is planned in memory and refused as a whole (`Error::Unsupported`, file untouched) when any part is not supported: new chunks in a version-2 B-tree index (two or more unlimited diff --git a/crates/clawhdf5-tools/tests/edit_interop.rs b/crates/clawhdf5-tools/tests/edit_interop.rs index fe44aa8..7587126 100644 --- a/crates/clawhdf5-tools/tests/edit_interop.rs +++ b/crates/clawhdf5-tools/tests/edit_interop.rs @@ -1315,3 +1315,90 @@ fn optional_filters_that_fail_are_skipped() { assert!(o.status.success(), "h5rs check --data {p}:\n{}", text(&o)); } } + +/// libhdf5 counts a version-2 object header's attributes through its +/// Attribute Info message and reports none without one. The editor adds +/// that message, as `H5O__attr_create` does, when it gives a version-2 +/// header (h5py `libver='v110'`/`'latest'`) its first attribute: afterwards +/// h5py lists, counts and reads every attribute, new and existing, h5dump +/// agrees, and h5py `r+` can add more (past the compact limit, into dense +/// storage) with the count still right. +#[test] +fn attribute_count_in_version_2_headers() { + if !tools_ok() { + return; + } + for (lv, dump) in [("'v110'", true), ("'latest'", false)] { + let dir = tmpdir(); + let path = dir.path().join("acount.h5"); + let p = path.to_str().unwrap(); + py(&format!( + "import h5py, numpy as np\n\ + with h5py.File({p:?}, 'w', libver={lv}) as f:\n\ + \x20 f.create_group('g')\n\ + \x20 f.create_group('has').attrs['old'] = 7\n\ + \x20 f.create_dataset('d', data=np.arange(3, dtype=' f.root().attrs().unwrap(), + "d" | "e" => f.dataset(o).unwrap().attrs().unwrap(), + _ => f.group(o).unwrap().attrs().unwrap(), + }; + assert_eq!(attrs.len(), n + extra, "{o}"); + assert!(matches!(attrs.get("a1"), Some(AttrValue::F64(x)) if *x == 1.5)); + } + if dump { + // Every object's attributes: 3 set by the editor on each of + // the five, 2 existing, `extra` from h5py on each. + let out = Command::new("h5dump").args(["-A", p]).output().unwrap(); + assert!(out.status.success(), "h5dump -A:\n{}", text(&out)); + let s = String::from_utf8_lossy(&out.stdout); + assert_eq!( + s.matches("ATTRIBUTE \"").count(), + 5 * (3 + extra) + 2, + "h5dump -A:\n{s}" + ); + } + }; + check(0); + // libhdf5 adds more: to 7 or 8 (compact), then past its limit. + for extra in [4usize, 10] { + py(&format!( + "import h5py\n\ + with h5py.File({p:?}, 'r+') as f:\n\ + \x20 for o in ['/', 'g', 'has', 'd', 'e']:\n\ + \x20 for i in range({extra}): f[o].attrs[f'h{{i}}'] = i\n" + )); + check(extra); + check_tools(&path, dump); + } + } +} diff --git a/crates/clawhdf5/src/edit/mod.rs b/crates/clawhdf5/src/edit/mod.rs index c0a5d07..15237fc 100644 --- a/crates/clawhdf5/src/edit/mod.rs +++ b/crates/clawhdf5/src/edit/mod.rs @@ -47,6 +47,8 @@ const MSG_EXTERNAL: u16 = 0x07; const MSG_ATTR_INFO: u16 = 0x15; /// Message flag: the message is shared (stored elsewhere). const MSG_FLAG_SHARED: u8 = 0x02; +/// Message flag: the message must not be shared (`H5O_MSG_FLAG_DONTSHARE`). +const MSG_FLAG_DONTSHARE: u8 = 0x04; /// An HDF5 file opened for in-place modification. /// @@ -815,7 +817,24 @@ impl FileEditor { if let Some(i) = existing { hdr.delete(img, i)?; } + // libhdf5 counts a version-2 header's attributes through its + // Attribute Info message and reports none without one; like + // H5O__attr_create, add it when missing: before the attribute + // when free space holds both (libhdf5's order), else after it, + // so that a new continuation chunk made for the attribute has + // room for it too. + let ainfo = (hdr.version == 2 && hdr.find(MSG_ATTR_INFO).is_none()) + .then(|| attr_info_message(hdr.flags, img.os)); + let ainfo_first = ainfo + .as_ref() + .is_some_and(|a| hdr.has_free(a.len() + hdr.hsize() + body.len())); + if let Some(a) = ainfo.as_ref().filter(|_| ainfo_first) { + hdr.insert(img, MSG_ATTR_INFO, MSG_FLAG_DONTSHARE, a, None)?; + } hdr.insert(img, MSG_ATTRIBUTE, 0, &body, None)?; + if let Some(a) = ainfo.as_ref().filter(|_| !ainfo_first) { + hdr.insert(img, MSG_ATTR_INFO, MSG_FLAG_DONTSHARE, a, None)?; + } hdr.finish(img) }) } @@ -900,6 +919,26 @@ fn attr_name(d: &[u8]) -> Result<&[u8], Error> { Ok(name.split(|&b| b == 0).next().unwrap_or(name)) } +/// A new Attribute Info message for a version-2 header with flags +/// `hdr_flags`, as `H5O__attr_create` makes it: version 0, creation order +/// tracked / indexed as the header's flags say, maximum creation index 0, +/// and no dense storage (undefined fractal heap and B-tree addresses). +fn attr_info_message(hdr_flags: u8, os: u8) -> Vec { + let track = hdr_flags & 0x04 != 0; + let index = hdr_flags & 0x08 != 0; + let mut b = vec![0u8, u8::from(track) | (u8::from(index) << 1)]; + if track { + b.extend_from_slice(&0u16.to_le_bytes()); + } + let undef_addr = vec![0xffu8; os as usize]; + b.extend_from_slice(&undef_addr); + b.extend_from_slice(&undef_addr); + if index { + b.extend_from_slice(&undef_addr); + } + b +} + /// A version-2 header's limit on compact attributes: stored when its flags /// say so, else libhdf5's default of 8. fn max_compact_attrs(img: &Image<'_>, hdr: &Header) -> Result { diff --git a/crates/clawhdf5/src/edit/ohdr.rs b/crates/clawhdf5/src/edit/ohdr.rs index 5446702..f3abfe0 100644 --- a/crates/clawhdf5/src/edit/ohdr.rs +++ b/crates/clawhdf5/src/edit/ohdr.rs @@ -326,6 +326,11 @@ impl Header { .map(|(i, _)| i) } + /// Whether free space in the header can take a body of `len` bytes. + pub(crate) fn has_free(&self, len: usize) -> bool { + self.best_nil(self.padded(len)).is_some() + } + /// Put a message into slot `i` (a NIL message, or a message being /// moved away), splitting off the rest as a NIL message. fn place(