From 8cbbef3fae6a11286aa096f47f0168ddfcd96463 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:08:31 -0500 Subject: [PATCH 01/10] fix(format): write a Group Info message in every group libhdf5 reads a group's Group Info message before it inserts a link, and FileWriter wrote none, so h5py in "r+" mode could not add a link to any group we wrote: "Unable to create link (message type not found)". Each group header now carries a version 0 Group Info message with the default link-phase thresholds, as libhdf5 writes for a new group. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 9 ++ crates/clawhdf5-format/src/file_writer.rs | 7 + .../clawhdf5/tests/writer_groups_interop.rs | 136 ++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 crates/clawhdf5/tests/writer_groups_interop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ef4bd2f..cc1010f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## Unreleased +### Writer: groups and links (2026-09-26) +- **libhdf5 could not add links to groups we wrote.** h5py in `"r+"` mode + failed with "Unable to create link (message type not found)" on every + group `FileWriter` wrote: libhdf5 reads a group's Group Info message before + inserting a link, and none was written. Every group now carries one + (version 0, default thresholds: 6 more bytes per group header, so files + are not byte-identical to earlier versions). Regression test: + `crates/clawhdf5/tests/writer_groups_interop.rs`. + ### Plugin filters (2026-09-26) - **LZF, bitshuffle, bzip2 and Blosc read and write, in pure Rust.** Files written by h5py with `compression="lzf"`, or with hdf5plugin's diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index dbf59f2..36c3fbb 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -196,6 +196,13 @@ pub(crate) fn build_group_oh( li.extend_from_slice(&u64::MAX.to_le_bytes()); // fractal heap addr = UNDEF li.extend_from_slice(&u64::MAX.to_le_bytes()); // btree name index addr = UNDEF w.add_message(MessageType::LinkInfo, li); + } + // Group Info (version 0, default link-phase thresholds, no estimates). + // Readers don't need it, but libhdf5 reads it before inserting a link: + // without one, adding a link to a group we wrote (h5py in "r+" mode) + // failed with "message type not found". + w.add_message(MessageType::GroupInfo, vec![0, 0]); + if dense_link_info.is_none() { for link in links { w.add_message(MessageType::Link, link.serialize(OFFSET_SIZE)); } diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs new file mode 100644 index 0000000..9ea3053 --- /dev/null +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -0,0 +1,136 @@ +//! Groups and links written by `FileBuilder`, read back by h5py (libhdf5) +//! and h5dump, and by clawhdf5 itself. +//! +//! Skipped when python3 with h5py is unavailable, unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::process::Command; + +use clawhdf5::{File, FileBuilder}; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn interop_required() -> bool { + std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1") +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +fn h5dump_available() -> bool { + Command::new("h5dump") + .arg("--version") + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +macro_rules! skip_if_no_python { + () => { + if !python_available() { + assert!( + !interop_required(), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + }; +} + +fn run_python(script: &str) -> String { + let output = Command::new(python()) + .args(["-c", script]) + .output() + .expect("failed to run python"); + if !output.status.success() { + panic!( + "Python script failed:\nSTDOUT: {}\nSTDERR: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + String::from_utf8_lossy(&output.stdout).trim().to_string() +} + +/// Run `body` under h5py with `path` bound to the file's path. +fn h5py(path: &str, body: &str) -> String { + run_python(&format!( + "import h5py, numpy as np, json\npath = r'{path}'\n{body}" + )) +} + +fn write(dir: &tempfile::TempDir, name: &str, b: FileBuilder) -> String { + let path = dir.path().join(name).display().to_string(); + b.write(&path).unwrap(); + path +} + +/// h5dump must read the whole file without an error. +fn h5dump_ok(path: &str) -> String { + if !h5dump_available() { + assert!(!interop_required(), "h5dump is not available"); + return String::new(); + } + let o = Command::new("h5dump").arg(path).output().unwrap(); + let out = String::from_utf8_lossy(&o.stdout).to_string(); + assert!( + o.status.success(), + "h5dump failed:\n{out}{}", + String::from_utf8_lossy(&o.stderr) + ); + out +} + +// ---- libhdf5 can modify the groups we write ---- + +#[test] +fn h5py_can_add_links_to_groups_we_wrote() { + skip_if_no_python!(); + // Measured before the fix: h5py in "r+" mode could not add a link to any + // group we wrote ("Unable to create link (message type not found)"): + // libhdf5 reads a group's Group Info message before inserting a link, and + // the writer wrote none. + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.create_dataset("x").with_f64_data(&[1.0, 2.0]); + let mut g = b.create_group("small"); + g.create_dataset("a").with_i32_data(&[1]); + b.add_group(g.finish()); + let mut g = b.create_group("big"); // dense link storage + for i in 0..20 { + g.create_dataset(&format!("d{i:02}")).with_i32_data(&[i]); + } + b.add_group(g.finish()); + let path = write(&dir, "modify.h5", b); + + let out = h5py( + &path, + "with h5py.File(path, 'r+') as f:\n\ + \x20 f['alias'] = f['x']\n\ + \x20 f['small']['new'] = np.arange(3)\n\ + \x20 f['big']['new'] = np.arange(4)\n\ + \x20 f.create_group('added/deeper')\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([sorted(f), sorted(f['small']), len(f['big']),\n\ + \x20 f['alias'][()].tolist(), f['big/new'][()].tolist(), f['big/d07'][()].tolist()]))", + ); + assert_eq!( + out, + r#"[["added", "alias", "big", "small", "x"], ["a", "new"], 21, [1.0, 2.0], [0, 1, 2, 3], [7]]"# + ); + h5dump_ok(&path); + let f = File::open(&path).unwrap(); + assert_eq!( + f.dataset("big/new").unwrap().read_i64().unwrap(), + [0, 1, 2, 3] + ); + assert_eq!(f.dataset("alias").unwrap().read_f64().unwrap(), [1.0, 2.0]); +} From d102c063064d369c4764f1e92d349a25b7dafc15 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:33:46 -0500 Subject: [PATCH 02/10] feat(format): nested groups, soft/hard/external links and creation order in the writer FileWriter wrote the root group plus one level of groups, and refused path-like names. The writer now flattens its builders into a group tree (writer_tree.rs) before layout: - A name may be a path ("a/b/x", "/a/b/x" at the root); missing intermediate groups are created as h5py does, and GroupBuilder gains create_group/add_group so builders nest to any depth. A group added at a path that already holds a group is merged into it (require_group); any other repeated name, an empty or "." component, or an absolute path below the root is an error. - add_soft_link, add_hard_link and add_external_link on FileWriter, FileBuilder and GroupBuilder. Hard-link targets are resolved to objects at finish (through other hard links; a missing target, a soft link on the way or a cycle of paths is an error). Objects with several hard links get an Object Reference Count message so libhdf5 can delete one link without freeing the object. - track_order(true) per group, or as the file default, tracks and indexes link creation order: Link Info flags and max order, the order in each Link message, and a type-6 creation-order B-tree for dense groups. - A group's link index is one B-tree leaf; more than 65535 links is an error. Groups are laid out depth-first from the root, datasets group by group, and untracked groups keep writing datasets, then groups, then other links: files with one level of groups are byte-identical to before. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 38 + README.md | 24 + crates/clawhdf5-format/src/file_writer.rs | 848 ++++++++++-------- crates/clawhdf5-format/src/lib.rs | 1 + crates/clawhdf5-format/src/type_builders.rs | 122 ++- crates/clawhdf5-format/src/writer_tree.rs | 446 +++++++++ .../tests/writer_meta_tests.rs | 47 +- crates/clawhdf5-tools/tests/h5rs_interop.rs | 85 ++ crates/clawhdf5/src/writer.rs | 43 +- .../clawhdf5/tests/writer_groups_interop.rs | 460 +++++++++- docs/known-issues.md | 13 +- 11 files changed, 1694 insertions(+), 433 deletions(-) create mode 100644 crates/clawhdf5-format/src/writer_tree.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index cc1010f..bb939de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,44 @@ ## Unreleased ### Writer: groups and links (2026-09-26) +- **Nested groups, to any depth.** `FileWriter`/`FileBuilder` wrote the root + group plus one level, and refused path-like names. Now a name may be a path + (`create_dataset("a/b/x")`, `create_group("a/b")`, a leading `/` at the + root) and missing intermediate groups are created, as h5py does; groups + also nest through the new `GroupBuilder::create_group`/`add_group`. A group + added at a path that already holds a group is merged into it (h5py's + `require_group`); a name used twice otherwise, an empty or `"."` + component (`"a//b"`, `"a/"`) or an absolute path below the root is an + error. Datasets, attributes, dense attribute storage and dense link + storage work at every level. +- **Soft, hard and external links at any depth:** `add_soft_link(name, + target)` (h5py's `SoftLink`; the target may dangle), + `add_hard_link(name, target)` (h5py's `f[name] = f[target]`; the target + path is resolved when the file is written, may go through other hard + links, and a missing target, a soft link on the way or a cycle of + hard-link paths is an error) and `add_external_link`, on `FileWriter`, + `FileBuilder` and `GroupBuilder`. An object with several hard links gets + an Object Reference Count message, so libhdf5 can delete one of the links + without freeing the object. +- **Link creation order:** `track_order(true)` on a `GroupBuilder`, or on + `FileWriter`/`FileBuilder` for every group that does not set its own, + tracks and indexes link creation order (h5py's `track_order=True`): the + Link Info message carries the flags, each link its order, and a dense + group a creation-order B-tree (type 6). h5py then lists members in + insertion order. Attribute creation order is not tracked. +- A group holds at most 65 535 links (its link index is one B-tree leaf); + more is an error. `GroupBuilder`'s fields changed (they were + crate-private); `FinishedGroup` is unchanged for callers. +- Files that use one level of groups and no new link kinds are laid out as + before: byte-identical to the writer with the Group Info fix below + (compared on simple, mixed dense/chunked/compact/external-link and paged + files). Tests: h5py and clawhdf5 read the same + tree (every path, attribute and value) from a 5-level file; soft, hard, + external and cyclic hard links; 10 000 links in one group, with and + without creation order; libhdf5 adding and deleting links in our groups; + `h5rs check` passes and `h5rs dump` equals h5dump + (`crates/clawhdf5/tests/writer_groups_interop.rs`, + `crates/clawhdf5-tools/tests/h5rs_interop.rs`). - **libhdf5 could not add links to groups we wrote.** h5py in `"r+"` mode failed with "Unable to create link (message type not found)" on every group `FileWriter` wrote: libhdf5 reads a group's Group Info message before diff --git a/README.md b/README.md index b9e3aa1..3b3fd22 100644 --- a/README.md +++ b/README.md @@ -407,6 +407,30 @@ let values = ds.read_f64()?; assert_eq!(values, vec![22.5, 23.1, 21.8]); ``` +### Groups and links + +```rust +use clawhdf5::{AttrValue, FileBuilder}; + +let mut b = FileBuilder::new(); +// A path creates its missing intermediate groups, as in h5py. +b.create_dataset("run/2026/temps").with_f64_data(&[22.5, 23.1]); +// Builders nest; a group added at an existing path is merged into it. +let mut run = b.create_group("run"); +run.set_attr("operator", AttrValue::String("ana".into())); +let mut cal = run.create_group("calibration"); +cal.track_order(true); // h5py lists members in insertion order +cal.create_dataset("offset").with_f64_data(&[0.1]); +run.add_group(cal.finish()); +b.add_group(run.finish()); +b.add_soft_link("latest", "/run/2026"); // h5py.SoftLink +b.add_hard_link("temps", "/run/2026/temps"); // f["temps"] = f["run/2026/temps"] +b.add_external_link("raw", "raw.h5", "/data"); +b.write("groups.h5")?; +``` + +A group holds at most 65 535 links; more is an error. + ### Agent Memory ```rust diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 36c3fbb..b4a90aa 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -4,7 +4,7 @@ //! link messages, contiguous datasets, inline and dense attributes. #[cfg(not(feature = "std"))] -use alloc::{format, string::String, string::ToString, vec, vec::Vec}; +use alloc::{format, vec, vec::Vec}; use crate::attribute::AttributeMessage; use crate::chunked_write::{ @@ -21,6 +21,7 @@ use crate::superblock::Superblock; use crate::type_builders::{ DatasetBuilder, FinishedGroup, GroupBuilder, build_attr_message, fill_value_message, }; +use crate::writer_tree::{self, LinkTo}; // Re-export public types that moved to type_builders for API compatibility. #[cfg(feature = "provenance")] @@ -63,19 +64,6 @@ fn build_paged_superblock_extension(page_size: u32) -> Result, FormatErr w.serialize() } -/// A group or dataset name must be one path component: not empty, not ".", -/// and without '/'. `FileWriter` writes a root group plus one level of -/// groups, and cannot create intermediate groups for a path. -fn check_link_name(name: &str) -> Result<(), FormatError> { - if name.is_empty() || name == "." || name.contains('/') { - return Err(FormatError::SerializationError(format!( - "invalid object name {name:?}: names must be a single path component \ - (FileWriter does not create nested groups)" - ))); - } - Ok(()) -} - /// Threshold for switching from compact (inline) to dense attribute storage. const DENSE_ATTR_THRESHOLD: usize = 8; @@ -86,6 +74,7 @@ const DENSE_LINK_THRESHOLD: usize = 8; // ---- OH builders ---- +#[allow(clippy::too_many_arguments)] pub(crate) fn build_chunked_dataset_oh( dt: &Datatype, ds: &Dataspace, @@ -94,6 +83,7 @@ pub(crate) fn build_chunked_dataset_oh( attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, fill_message: &[u8], + refcount: u32, ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); @@ -110,9 +100,11 @@ pub(crate) fn build_chunked_dataset_oh( w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); } } + add_refcount(&mut w, refcount); w.serialize() } +#[allow(clippy::too_many_arguments)] pub(crate) fn build_dataset_oh( dt: &Datatype, ds: &Dataspace, @@ -121,6 +113,7 @@ pub(crate) fn build_dataset_oh( attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, fill_message: &[u8], + refcount: u32, ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); @@ -145,6 +138,7 @@ pub(crate) fn build_dataset_oh( w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); } } + add_refcount(&mut w, refcount); w.serialize() } @@ -156,6 +150,7 @@ pub(crate) fn build_compact_dataset_oh( attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, fill_message: &[u8], + refcount: u32, ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); @@ -175,34 +170,29 @@ pub(crate) fn build_compact_dataset_oh( w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); } } + add_refcount(&mut w, refcount); w.serialize() } +/// Build a group's object header. `link_info` is its Link Info message; +/// with `dense_links` the links live in the fractal heap it points at and are +/// not written inline. pub(crate) fn build_group_oh( links: &[LinkMessage], - dense_link_info: Option<&[u8]>, + link_info: &[u8], + dense_links: bool, attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, + refcount: u32, ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); - if let Some(li) = dense_link_info { - // Dense link storage: a LinkInfo pointing at the fractal heap + name - // B-tree, and no inline Link messages. - w.add_message(MessageType::LinkInfo, li.to_vec()); - } else { - let mut li = Vec::new(); - li.push(0); // version - li.push(0); // flags - li.extend_from_slice(&u64::MAX.to_le_bytes()); // fractal heap addr = UNDEF - li.extend_from_slice(&u64::MAX.to_le_bytes()); // btree name index addr = UNDEF - w.add_message(MessageType::LinkInfo, li); - } + w.add_message(MessageType::LinkInfo, link_info.to_vec()); // Group Info (version 0, default link-phase thresholds, no estimates). // Readers don't need it, but libhdf5 reads it before inserting a link: // without one, adding a link to a group we wrote (h5py in "r+" mode) // failed with "message type not found". w.add_message(MessageType::GroupInfo, vec![0, 0]); - if dense_link_info.is_none() { + if !dense_links { for link in links { w.add_message(MessageType::Link, link.serialize(OFFSET_SIZE)); } @@ -214,30 +204,58 @@ pub(crate) fn build_group_oh( w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); } } + add_refcount(&mut w, refcount); w.serialize() } -pub(crate) fn make_link(name: &str, addr: u64) -> LinkMessage { - LinkMessage { - name: name.to_string(), - link_target: LinkTarget::Hard { - object_header_address: addr, +/// An object with more than one hard link records the count in an Object +/// Reference Count message (libhdf5 omits it for a count of one). Without +/// it, libhdf5 deleting one of the links would free an object that is still +/// linked. +fn add_refcount(w: &mut ObjectHeaderWriter, refcount: u32) { + if refcount > 1 { + let mut msg = vec![0u8]; // version + msg.extend_from_slice(&refcount.to_le_bytes()); + w.add_message(MessageType::ObjectReferenceCount, msg); + } +} + +/// The Link message for `link`, whose group and dataset targets are at the +/// given addresses (indexed as in the writer tree). +fn link_message(link: &writer_tree::Link, group_addrs: &[u64], ds_addrs: &[u64]) -> LinkMessage { + let link_target = match &link.to { + LinkTo::Group(g) => LinkTarget::Hard { + object_header_address: group_addrs.get(*g).copied().unwrap_or(0), }, - creation_order: None, + LinkTo::Dataset(d) => LinkTarget::Hard { + object_header_address: ds_addrs.get(*d).copied().unwrap_or(0), + }, + LinkTo::Soft(target_path) => LinkTarget::Soft { + target_path: target_path.clone(), + }, + LinkTo::External { file, path } => LinkTarget::External { + filename: file.clone(), + object_path: path.clone(), + }, + }; + LinkMessage { + name: link.name.clone(), + link_target, + creation_order: link.creation_order, charset: CharacterSet::Ascii, } } -pub(crate) fn make_external_link(name: &str, filename: &str, object_path: &str) -> LinkMessage { - LinkMessage { - name: name.to_string(), - link_target: LinkTarget::External { - filename: filename.to_string(), - object_path: object_path.to_string(), - }, - creation_order: None, - charset: CharacterSet::Ascii, - } +/// Link Info message for a group with compact (inline) links: no heap, no +/// B-trees. A group tracking creation order records the next order to use. +fn compact_link_info(track_order: bool, nlinks: usize) -> Vec { + let max_corder = track_order.then_some(nlinks as u64); + serialize_link_info( + max_corder, + u64::MAX, + u64::MAX, + track_order.then_some(u64::MAX), + ) } // ---- Dense attribute blob ---- @@ -765,97 +783,181 @@ pub(crate) fn build_dense_attrs(attrs: &[AttributeMessage], base_address: u64) - pub(crate) struct DenseLinkBlob { /// Serialized LinkInfo message (to embed in the group's object header). pub(crate) link_info_message: Vec, - /// The combined fractal heap header + direct block + B-tree v2 bytes. + /// The combined fractal heap, name-index B-tree and (when creation order + /// is tracked) creation-order-index B-tree bytes. pub(crate) blob: Vec, } +/// A v2 B-tree of `btree_type` holding `records` (already in key order) in a +/// single leaf, laid out at `addr`: the header, then the leaf. +fn single_leaf_v2_btree( + btree_type: u8, + record_size: u16, + records: &[Vec], + addr: u64, +) -> Result, FormatError> { + let os = OFFSET_SIZE as usize; + let ls = LENGTH_SIZE as usize; + // The root node's record count is a 2-byte field; more records need + // internal nodes, which the writer does not build. + let num_records = u16::try_from(records.len()).map_err(|_| { + FormatError::SerializationError(format!( + "{} links in one group: a group holds at most {} links \ + (a deeper link index is not implemented)", + records.len(), + u16::MAX + )) + })?; + let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4; + let btlf_size = 4 + 1 + 1 + (records.len() * record_size as usize) + 4; + let node_size = btlf_size.next_power_of_two().max(512) as u32; + let btlf_addr = addr + bthd_size as u64; + + let mut out = Vec::with_capacity(bthd_size + node_size as usize); + out.extend_from_slice(b"BTHD"); + out.push(0); // version + out.push(btree_type); + out.extend_from_slice(&node_size.to_le_bytes()); + out.extend_from_slice(&record_size.to_le_bytes()); + out.extend_from_slice(&0u16.to_le_bytes()); // depth = 0 (single leaf) + out.push(100); // split_percent + out.push(40); // merge_percent + write_offset(&mut out, btlf_addr, OFFSET_SIZE); + out.extend_from_slice(&num_records.to_le_bytes()); + write_length(&mut out, records.len() as u64, LENGTH_SIZE); + let checksum = crate::checksum::jenkins_lookup3(&out); + out.extend_from_slice(&checksum.to_le_bytes()); + debug_assert_eq!(out.len(), bthd_size); + + let mut btlf = Vec::with_capacity(node_size as usize); + btlf.extend_from_slice(b"BTLF"); + btlf.push(0); // version + btlf.push(btree_type); + for rec in records { + debug_assert_eq!(rec.len(), record_size as usize); + btlf.extend_from_slice(rec); + } + // The checksum follows the records, not the end of the node. + let checksum = crate::checksum::jenkins_lookup3(&btlf); + btlf.extend_from_slice(&checksum.to_le_bytes()); + btlf.resize(node_size as usize, 0); + out.extend_from_slice(&btlf); + Ok(out) +} + /// Build dense link storage for a group's links, laid out at `base_address`. /// /// Mirrors [`build_dense_attrs`]: each link is stored as a serialized Link -/// message in a single-direct-block fractal heap, indexed by a v2 B-tree of -/// **type 5** (link-name index, record = name hash + heap ID). The returned -/// LinkInfo message points at the heap and the name B-tree. -pub(crate) fn build_dense_links(links: &[LinkMessage], base_address: u64) -> DenseLinkBlob { +/// message in a fractal heap, indexed by a v2 B-tree of **type 5** (link-name +/// index, record = name hash + heap ID). With `track_order` (every link then +/// carries its creation order) a **type 6** B-tree (creation-order index, +/// record = creation order + heap ID) follows, as libhdf5 writes for a group +/// created with an indexed creation order. The returned LinkInfo message +/// points at the heap and the B-trees. +pub(crate) fn build_dense_links( + links: &[LinkMessage], + base_address: u64, + track_order: bool, +) -> Result { let serialized: Vec> = links.iter().map(|l| l.serialize(OFFSET_SIZE)).collect(); - let name_hashes: Vec = links - .iter() - .map(|l| crate::checksum::jenkins_lookup3(l.name.as_bytes())) - .collect(); - - let os = OFFSET_SIZE as usize; - let ls = LENGTH_SIZE as usize; // libhdf5's link heap uses max_heap_size 32 / heap ID length 7 (vs 40/8 for // attributes), giving a 7-byte heap ID and an 11-byte type-5 record. let heap = build_single_block_fractal_heap(&serialized, base_address, 32, 7); let heap_id_length = heap.heap_id_length; - // B-tree v2 type 5 records: hash(4) + heap_id(heap_id_length). The B-tree - // search key is the name hash, so records are sorted by (hash, order). - let record_size: u16 = 4 + heap_id_length; - let mut records: Vec<(u32, u32, Vec)> = Vec::with_capacity(links.len()); - for (i, heap_id) in heap.heap_ids.iter().enumerate() { - let mut rec = Vec::with_capacity(record_size as usize); - rec.extend_from_slice(&name_hashes[i].to_le_bytes()); // hash - rec.extend_from_slice(heap_id); // heap ID - records.push((name_hashes[i], i as u32, rec)); - } - records.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.cmp(&b.1))); + // Type 5 records: hash(4) + heap_id. The B-tree's key is the name hash, + // so records are sorted by (hash, order). + let mut by_name: Vec<(u32, usize)> = links + .iter() + .enumerate() + .map(|(i, l)| (crate::checksum::jenkins_lookup3(l.name.as_bytes()), i)) + .collect(); + by_name.sort_unstable(); + let name_records: Vec> = by_name + .iter() + .map(|&(hash, i)| { + let mut rec = hash.to_le_bytes().to_vec(); + rec.extend_from_slice(&heap.heap_ids[i]); + rec + }) + .collect(); + let name_bt_addr = heap.btree_addr; + let mut blob = heap.blob; + blob.extend_from_slice(&single_leaf_v2_btree( + 5, + 4 + heap_id_length, + &name_records, + name_bt_addr, + )?); - let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4; - let num_records = links.len(); - let btlf_size = 4 + 1 + 1 + (num_records * record_size as usize) + 4; - let node_size = btlf_size.next_power_of_two().max(512) as u32; + let link_info_message = if track_order { + // Type 6 records: creation order(8) + heap_id, sorted by order. + let mut by_order: Vec<(u64, usize)> = links + .iter() + .enumerate() + .map(|(i, l)| (l.creation_order.unwrap_or(i as u64), i)) + .collect(); + by_order.sort_unstable(); + let order_records: Vec> = by_order + .iter() + .map(|&(order, i)| { + let mut rec = order.to_le_bytes().to_vec(); + rec.extend_from_slice(&heap.heap_ids[i]); + rec + }) + .collect(); + let order_bt_addr = base_address + blob.len() as u64; + blob.extend_from_slice(&single_leaf_v2_btree( + 6, + 8 + heap_id_length, + &order_records, + order_bt_addr, + )?); + let next_order = by_order.last().map_or(0, |&(o, _)| o + 1); + serialize_link_info( + Some(next_order), + heap.frhp_addr, + name_bt_addr, + Some(order_bt_addr), + ) + } else { + serialize_link_info(None, heap.frhp_addr, name_bt_addr, None) + }; - let bthd_addr = heap.btree_addr; - let btlf_addr = bthd_addr + bthd_size as u64; - - let mut bthd = Vec::with_capacity(bthd_size); - bthd.extend_from_slice(b"BTHD"); - bthd.push(0); // version - bthd.push(5); // type = link name index - bthd.extend_from_slice(&node_size.to_le_bytes()); - bthd.extend_from_slice(&record_size.to_le_bytes()); - bthd.extend_from_slice(&0u16.to_le_bytes()); // depth = 0 (single leaf) - bthd.push(100); // split_percent - bthd.push(40); // merge_percent - write_offset(&mut bthd, btlf_addr, OFFSET_SIZE); - bthd.extend_from_slice(&(num_records as u16).to_le_bytes()); - write_length(&mut bthd, num_records as u64, LENGTH_SIZE); - let bthd_checksum = crate::checksum::jenkins_lookup3(&bthd); - bthd.extend_from_slice(&bthd_checksum.to_le_bytes()); - debug_assert_eq!(bthd.len(), bthd_size); - - let mut btlf = Vec::with_capacity(node_size as usize); - btlf.extend_from_slice(b"BTLF"); - btlf.push(0); // version - btlf.push(5); // type - for (_, _, rec) in &records { - btlf.extend_from_slice(rec); - } - let btlf_checksum = crate::checksum::jenkins_lookup3(&btlf); - btlf.extend_from_slice(&btlf_checksum.to_le_bytes()); - btlf.resize(node_size as usize, 0); - - let mut blob = Vec::with_capacity(heap.blob.len() + bthd.len() + btlf.len()); - blob.extend_from_slice(&heap.blob); - blob.extend_from_slice(&bthd); - blob.extend_from_slice(&btlf); - - DenseLinkBlob { - link_info_message: serialize_link_info(heap.frhp_addr, bthd_addr), + Ok(DenseLinkBlob { + link_info_message, blob, - } + }) } -/// Serialize a LinkInfo message (version 0, no creation-order index) pointing -/// at a fractal heap and a v2 B-tree name index. -fn serialize_link_info(fh_addr: u64, btree_name_addr: u64) -> Vec { +/// Serialize a LinkInfo message (version 0). `max_creation_order` (the next +/// creation order to assign) is present when creation order is tracked, and +/// `btree_corder_addr` when it is indexed; both set flag bits. +fn serialize_link_info( + max_creation_order: Option, + fh_addr: u64, + btree_name_addr: u64, + btree_corder_addr: Option, +) -> Vec { let mut data = Vec::new(); data.push(0); // version - data.push(0x00); // flags: no creation-order tracking + let mut flags = 0u8; + if max_creation_order.is_some() { + flags |= 0x01; // creation order tracked + } + if btree_corder_addr.is_some() { + flags |= 0x02; // creation order indexed + } + data.push(flags); + if let Some(m) = max_creation_order { + data.extend_from_slice(&m.to_le_bytes()); + } write_offset(&mut data, fh_addr, OFFSET_SIZE); write_offset(&mut data, btree_name_addr, OFFSET_SIZE); + if let Some(a) = btree_corder_addr { + write_offset(&mut data, a, OFFSET_SIZE); + } data } @@ -953,6 +1055,7 @@ pub(crate) fn build_vds_dataset_oh( attrs: &[AttributeMessage], dense_blob: Option<&DenseAttrBlob>, fill_message: &[u8], + refcount: u32, ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); @@ -972,6 +1075,7 @@ pub(crate) fn build_vds_dataset_oh( w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); } } + add_refcount(&mut w, refcount); w.serialize() } @@ -997,10 +1101,16 @@ fn write_undef_offset(buf: &mut Vec, offset_size: u8) { // ---- FileWriter ---- /// The main file creation API. +/// +/// Groups nest to any depth: a name may be a path (`"a/b/x"`), and missing +/// intermediate groups are created, as h5py does; groups also nest through +/// [`GroupBuilder::add_group`]. See [`GroupBuilder`] for how names and +/// repeated groups are handled, and for soft, hard and external links. pub struct FileWriter { - root_datasets: Vec, - root_attrs: Vec<(String, AttrValue)>, - groups: Vec, + /// The root group's contents (its name is unused). + root: GroupBuilder, + /// Default for groups that do not call [`GroupBuilder::track_order`]. + track_order: bool, /// Global alignment threshold: datasets with raw data >= this many bytes /// will have their data aligned to `alignment_bytes`. alignment_threshold: usize, @@ -1018,12 +1128,98 @@ impl Default for FileWriter { } } +/// A dataset ready for layout. +struct DsFlat { + dt: Datatype, + ds: Dataspace, + raw: Vec, + attrs: Vec, + chunk_options: ChunkOptions, + maxshape: Option>, + /// Serialized Fill Value message. + fill_message: Vec, + compact: bool, + alignment: usize, + /// VDS source mappings (set for Virtual datasets). + virtual_sources: Option>, + /// Number of hard links to the dataset. + refcount: u32, +} + +/// Convert a DatasetBuilder into a DsFlat, handling VDS (which does not +/// require a `data` field). +fn flatten_ds(db: DatasetBuilder, refcount: u32) -> Result { + let dt = db.datatype.ok_or(FormatError::DatasetMissingData)?; + let shape = db.shape.ok_or(FormatError::DatasetMissingShape)?; + let is_vds = db.virtual_sources.is_some(); + let raw = if is_vds { + // VDS datasets have no raw data stored in this file. + db.data.unwrap_or_default() + } else { + db.data.ok_or(FormatError::DatasetMissingData)? + }; + let max_dimensions = db.maxshape.clone(); + let dspace = Dataspace { + space_type: if shape.is_empty() { + DataspaceType::Scalar + } else { + DataspaceType::Simple + }, + rank: shape.len() as u8, + dimensions: shape, + max_dimensions, + }; + let mut attrs = Vec::new(); + for (n, v) in &db.attrs { + attrs.push(build_attr_message(n, v)); + } + #[cfg(feature = "provenance")] + if let Some(ref prov) = db.provenance { + let p = crate::provenance::Provenance { + creator: prov.creator.clone(), + timestamp: prov.timestamp.clone(), + source: prov.source.clone(), + }; + attrs.extend(p.build_attrs(&raw)); + } + let fill_message = fill_value_message(db.fill_time, db.fill_value.as_deref(), &dt)?; + Ok(DsFlat { + dt, + ds: dspace, + raw, + attrs, + chunk_options: db.chunk_options, + maxshape: db.maxshape, + fill_message, + compact: db.compact, + alignment: db.alignment, + virtual_sources: db.virtual_sources, + refcount, + }) +} + +/// A group ready for layout. +struct GrpFlat { + attrs: Vec, + links: Vec, + track_order: bool, + refcount: u32, +} + +impl GrpFlat { + fn link_messages(&self, group_addrs: &[u64], ds_addrs: &[u64]) -> Vec { + self.links + .iter() + .map(|l| link_message(l, group_addrs, ds_addrs)) + .collect() + } +} + impl FileWriter { pub fn new() -> Self { Self { - root_datasets: Vec::new(), - root_attrs: Vec::new(), - groups: Vec::new(), + root: GroupBuilder::new("/"), + track_order: false, alignment_threshold: 0, alignment_bytes: 0, page_size: None, @@ -1055,21 +1251,61 @@ impl FileWriter { self } + /// Track (and index) link creation order in every group that does not + /// set its own [`GroupBuilder::track_order`], the root included — as + /// h5py's `track_order=True`: libhdf5 then lists members in the order + /// they were added. Off by default (members are listed by name). + pub fn track_order(&mut self, track: bool) -> &mut Self { + self.track_order = track; + self + } + + /// Start a group. The builder is detached: fill it, then pass + /// `finish()`'s result to [`Self::add_group`]. `name` may be a path + /// (`"a/b"`); missing intermediate groups are created. pub fn create_group(&mut self, name: &str) -> GroupBuilder { GroupBuilder::new(name) } + /// Add a finished group to the root group. pub fn add_group(&mut self, group: FinishedGroup) { - self.groups.push(group); + self.root.add_group(group); } + /// Create a dataset. `name` may be a path (`"a/b/x"`, or `"/a/b/x"`); + /// missing intermediate groups are created. pub fn create_dataset(&mut self, name: &str) -> &mut DatasetBuilder { - self.root_datasets.push(DatasetBuilder::new(name)); - self.root_datasets.last_mut().unwrap() + self.root.create_dataset(name) } pub fn set_root_attr(&mut self, name: &str, value: AttrValue) { - self.root_attrs.push((name.to_string(), value)); + self.root.set_attr(name, value); + } + + /// Add a soft link `name` (a path from the root) to `target`. See + /// [`GroupBuilder::add_soft_link`]. + pub fn add_soft_link(&mut self, name: &str, target: &str) -> &mut Self { + self.root.add_soft_link(name, target); + self + } + + /// Add another hard link `name` (a path from the root) to the object at + /// `target`. See [`GroupBuilder::add_hard_link`]. + pub fn add_hard_link(&mut self, name: &str, target: &str) -> &mut Self { + self.root.add_hard_link(name, target); + self + } + + /// Add an external link `name` (a path from the root) to `target_path` + /// in the file `target_file`. + pub fn add_external_link( + &mut self, + name: &str, + target_file: &str, + target_path: &str, + ) -> &mut Self { + self.root.add_external_link(name, target_file, target_path); + self } pub fn finish(self) -> Result, FormatError> { @@ -1082,131 +1318,35 @@ impl FileWriter { {MIN_FILE_SPACE_PAGE_SIZE}..={MAX_FILE_SPACE_PAGE_SIZE} bytes" ))); } - struct DsFlat { - name: String, - dt: Datatype, - ds: Dataspace, - raw: Vec, - attrs: Vec, - chunk_options: ChunkOptions, - maxshape: Option>, - /// Serialized Fill Value message. - fill_message: Vec, - compact: bool, - alignment: usize, - /// VDS source mappings (set for Virtual datasets). - virtual_sources: Option>, - } - struct GrpFlat { - name: String, - attrs: Vec, - ds_indices: Vec, - /// (link_name, target_file, target_path) - external_links: Vec<(String, String, String)>, - } - // Helper: convert a DatasetBuilder into DsFlat, handling VDS (which - // does not require a `data` field). - let flatten_ds = |db: DatasetBuilder| -> Result { - let dt = db.datatype.ok_or(FormatError::DatasetMissingData)?; - let shape = db.shape.ok_or(FormatError::DatasetMissingShape)?; - let is_vds = db.virtual_sources.is_some(); - let raw = if is_vds { - // VDS datasets have no raw data stored in this file. - db.data.unwrap_or_default() - } else { - db.data.ok_or(FormatError::DatasetMissingData)? - }; - let max_dimensions = db.maxshape.clone(); - let dspace = Dataspace { - space_type: if shape.is_empty() { - DataspaceType::Scalar - } else { - DataspaceType::Simple - }, - rank: shape.len() as u8, - dimensions: shape, - max_dimensions, - }; - let mut attrs = Vec::new(); - for (n, v) in &db.attrs { - attrs.push(build_attr_message(n, v)); - } - #[cfg(feature = "provenance")] - if let Some(ref prov) = db.provenance { - let p = crate::provenance::Provenance { - creator: prov.creator.clone(), - timestamp: prov.timestamp.clone(), - source: prov.source.clone(), - }; - attrs.extend(p.build_attrs(&raw)); - } - let fill_message = fill_value_message(db.fill_time, db.fill_value.as_deref(), &dt)?; - Ok(DsFlat { - name: db.name, - dt, - ds: dspace, - raw, - attrs, - chunk_options: db.chunk_options, - maxshape: db.maxshape, - fill_message, - compact: db.compact, - alignment: db.alignment, - virtual_sources: db.virtual_sources, + // The group tree, in layout order: groups depth-first from the root, + // then every group's datasets in the same order. + let tree = writer_tree::build(self.root, self.track_order)?; + let all_ds: Vec = tree + .datasets + .into_iter() + .map(|(db, refcount)| flatten_ds(db, refcount)) + .collect::>()?; + let groups: Vec = tree + .groups + .into_iter() + .map(|g| GrpFlat { + attrs: g + .attrs + .iter() + .map(|(n, v)| build_attr_message(n, v)) + .collect(), + links: g.links, + track_order: g.track_order, + refcount: g.refcount, }) - }; - - // Every name becomes a single link in its parent group. The writer - // has no nested groups, so a path like "a/b" would be stored as one - // link literally named "a/b" — which no HDF5 reader can resolve. - let root_names = self.root_datasets.iter().map(|d| d.name.as_str()); - let group_names = self.groups.iter().flat_map(|g| { - core::iter::once(g.name.as_str()) - .chain(g.datasets.iter().map(|d| d.name.as_str())) - .chain(g.external_links.iter().map(|l| l.0.as_str())) - }); - for name in root_names.chain(group_names) { - check_link_name(name)?; - } - - let mut all_ds: Vec = Vec::new(); - let mut groups: Vec = Vec::new(); - let mut root_ds_indices: Vec = Vec::new(); - - for db in self.root_datasets { - root_ds_indices.push(all_ds.len()); - all_ds.push(flatten_ds(db)?); - } - - for g in self.groups.into_iter() { - let mut gattrs = Vec::new(); - for (n, v) in &g.attrs { - gattrs.push(build_attr_message(n, v)); - } - let mut ds_idx = Vec::new(); - for db in g.datasets { - ds_idx.push(all_ds.len()); - all_ds.push(flatten_ds(db)?); - } - groups.push(GrpFlat { - name: g.name, - attrs: gattrs, - ds_indices: ds_idx, - external_links: g.external_links, - }); - } - - let mut root_attrs: Vec = Vec::new(); - for (n, v) in &self.root_attrs { - root_attrs.push(build_attr_message(n, v)); - } + .collect(); // Every datatype must have an on-disk encoding before anything is laid // out: `Datatype::serialize` itself cannot report a failure. let group_attrs = groups.iter().flat_map(|g| &g.attrs); let ds_attrs = all_ds.iter().flat_map(|d| &d.attrs); - for a in root_attrs.iter().chain(group_attrs).chain(ds_attrs) { + for a in group_attrs.chain(ds_attrs) { a.datatype.check_encodable()?; } for d in &all_ds { @@ -1232,7 +1372,6 @@ impl FileWriter { !is_vds[i] && !is_chunked[i] && d.compact && d.raw.len() <= MAX_COMPACT_DATA_SIZE }) .collect(); - let root_dense = root_attrs.len() > DENSE_ATTR_THRESHOLD; let group_dense: Vec = groups .iter() .map(|g| g.attrs.len() > DENSE_ATTR_THRESHOLD) @@ -1244,51 +1383,41 @@ impl FileWriter { // Dense link decision: a group with more than the compact threshold of // links stores them in a fractal heap + v2 B-tree instead of inline. - let root_link_count = root_ds_indices.len() + groups.len(); - let root_links_dense = root_link_count > DENSE_LINK_THRESHOLD; let group_links_dense: Vec = groups .iter() - .map(|g| g.ds_indices.len() + g.external_links.len() > DENSE_LINK_THRESHOLD) + .map(|g| g.links.len() > DENSE_LINK_THRESHOLD) .collect(); - // The dense LinkInfo message is a fixed size regardless of address, so a - // dummy is sufficient for OH size computation. - let dummy_link_info = serialize_link_info(0, 0); - // Pass 1: compute OH sizes with dummy addresses + // Pass 1: compute OH sizes with dummy addresses. Link messages and + // the Link Info message are the same size whatever the addresses. let group_oh_sizes: Vec = groups .iter() .enumerate() .map(|(gi, g)| { - let mut dummy_links: Vec = g - .ds_indices - .iter() - .map(|&i| make_link(&all_ds[i].name, 0)) - .collect(); - for (lname, fname, opath) in &g.external_links { - dummy_links.push(make_external_link(lname, fname, opath)); - } + let dummy_links = g.link_messages(&[], &[]); let attr_blob = group_dense[gi].then(|| build_dense_attrs(&g.attrs, 0)); - let dl = group_links_dense[gi].then_some(dummy_link_info.as_slice()); - build_group_oh(&dummy_links, dl, &g.attrs, attr_blob.as_ref()).map(|oh| oh.len()) + let li = if group_links_dense[gi] { + serialize_link_info( + g.track_order.then_some(0), + 0, + 0, + g.track_order.then_some(0), + ) + } else { + compact_link_info(g.track_order, g.links.len()) + }; + build_group_oh( + &dummy_links, + &li, + group_links_dense[gi], + &g.attrs, + attr_blob.as_ref(), + g.refcount, + ) + .map(|oh| oh.len()) }) .collect::>()?; - let root_dummy_links: Vec = { - let mut links = Vec::new(); - for &i in &root_ds_indices { - links.push(make_link(&all_ds[i].name, 0)); - } - for g in &groups { - links.push(make_link(&g.name, 0)); - } - links - }; - let root_oh_size = { - let attr_blob = root_dense.then(|| build_dense_attrs(&root_attrs, 0)); - let dl = root_links_dense.then_some(dummy_link_info.as_slice()); - build_group_oh(&root_dummy_links, dl, &root_attrs, attr_blob.as_ref())?.len() - }; - struct DataBlob { data: Vec, oh_bytes: Vec, @@ -1300,14 +1429,10 @@ impl FileWriter { let mut dummy_blobs: Vec = Vec::new(); let mut dummy_cursor = 0u64; for (i, d) in all_ds.iter().enumerate() { + let dense_blob = ds_dense[i].then(|| build_dense_attrs(&d.attrs, 0)); if is_vds[i] { // VDS: dummy OH with address 0 to get the OH size. The global // heap blob will be placed after the OHs in pass 2. - let dense_blob = if ds_dense[i] { - Some(build_dense_attrs(&d.attrs, 0)) - } else { - None - }; let oh = build_vds_dataset_oh( &d.dt, &d.ds, @@ -1315,6 +1440,7 @@ impl FileWriter { &d.attrs, dense_blob.as_ref(), &d.fill_message, + d.refcount, )?; // Global heap blob size is address-independent; compute it now // so pass 2 can place it correctly. @@ -1346,11 +1472,6 @@ impl FileWriter { d.maxshape.as_deref(), )?; dummy_cursor += result.data_bytes.len() as u64; - let dense_blob = if ds_dense[i] { - Some(build_dense_attrs(&d.attrs, 0)) - } else { - None - }; let oh = build_chunked_dataset_oh( &d.dt, &d.ds, @@ -1359,6 +1480,7 @@ impl FileWriter { &d.attrs, dense_blob.as_ref(), &d.fill_message, + d.refcount, )?; dummy_blobs.push(DataBlob { data: result.data_bytes, @@ -1366,11 +1488,6 @@ impl FileWriter { precompressed: Some(pre), }); } else if is_compact[i] { - let dense_blob = if ds_dense[i] { - Some(build_dense_attrs(&d.attrs, 0)) - } else { - None - }; let oh = build_compact_dataset_oh( &d.dt, &d.ds, @@ -1378,6 +1495,7 @@ impl FileWriter { &d.attrs, dense_blob.as_ref(), &d.fill_message, + d.refcount, )?; dummy_blobs.push(DataBlob { data: vec![], @@ -1385,11 +1503,6 @@ impl FileWriter { precompressed: None, }); } else { - let dense_blob = if ds_dense[i] { - Some(build_dense_attrs(&d.attrs, 0)) - } else { - None - }; let oh = build_dataset_oh( &d.dt, &d.ds, @@ -1398,9 +1511,10 @@ impl FileWriter { &d.attrs, dense_blob.as_ref(), &d.fill_message, + d.refcount, )?; dummy_blobs.push(DataBlob { - data: d.raw.clone(), + data: vec![], oh_bytes: oh, precompressed: None, }); @@ -1416,61 +1530,37 @@ impl FileWriter { .map(build_paged_superblock_extension) .transpose()?; let superblock_size = SUPERBLOCK_SIZE + sb_ext.as_ref().map_or(0, Vec::len); - let root_group_addr = superblock_size as u64; - let mut cursor2 = superblock_size + root_oh_size; - - // Each group is laid out as: object header, then (if dense) its link - // blob, then (if dense) its attribute blob. Link blobs are sized with - // dummy target addresses here — link message size is address-independent - // — and rebuilt with real addresses in the final pass. - let root_link_blob_addr = if root_links_dense { - let addr = cursor2 as u64; - cursor2 += build_dense_links(&root_dummy_links, addr).blob.len(); - Some(addr) - } else { - None - }; - let root_dense_blob = if root_dense { - let blob = build_dense_attrs(&root_attrs, cursor2 as u64); - cursor2 += blob.blob.len(); - Some(blob) - } else { - None - }; + let mut cursor2 = superblock_size; + // Each group (the root first) is laid out as: object header, then (if + // dense) its link blob, then (if dense) its attribute blob. Link blobs + // are sized with dummy target addresses here — link message size is + // address-independent — and rebuilt with real addresses when written. let mut group_link_blob_addrs: Vec> = Vec::new(); let mut group_dense_blobs: Vec> = Vec::new(); - let group_addrs2: Vec = group_oh_sizes - .iter() - .enumerate() - .map(|(gi, &sz)| { - let addr = cursor2 as u64; - cursor2 += sz; - if group_links_dense[gi] { - let mut dummy_links: Vec = groups[gi] - .ds_indices - .iter() - .map(|&i| make_link(&all_ds[i].name, 0)) - .collect(); - for (lname, fname, opath) in &groups[gi].external_links { - dummy_links.push(make_external_link(lname, fname, opath)); - } - let blob_addr = cursor2 as u64; - cursor2 += build_dense_links(&dummy_links, blob_addr).blob.len(); - group_link_blob_addrs.push(Some(blob_addr)); - } else { - group_link_blob_addrs.push(None); - } - if group_dense[gi] { - let blob = build_dense_attrs(&groups[gi].attrs, cursor2 as u64); - cursor2 += blob.blob.len(); - group_dense_blobs.push(Some(blob)); - } else { - group_dense_blobs.push(None); - } - addr - }) - .collect(); + let mut group_addrs2: Vec = Vec::with_capacity(groups.len()); + for (gi, g) in groups.iter().enumerate() { + group_addrs2.push(cursor2 as u64); + cursor2 += group_oh_sizes[gi]; + if group_links_dense[gi] { + let blob_addr = cursor2 as u64; + let dummy = g.link_messages(&[], &[]); + cursor2 += build_dense_links(&dummy, blob_addr, g.track_order)? + .blob + .len(); + group_link_blob_addrs.push(Some(blob_addr)); + } else { + group_link_blob_addrs.push(None); + } + if group_dense[gi] { + let blob = build_dense_attrs(&g.attrs, cursor2 as u64); + cursor2 += blob.blob.len(); + group_dense_blobs.push(Some(blob)); + } else { + group_dense_blobs.push(None); + } + } + let root_group_addr = group_addrs2[0]; let mut ds_dense_blobs: Vec> = Vec::new(); let ds_oh_addrs2: Vec = actual_ds_oh_sizes @@ -1507,6 +1597,7 @@ impl FileWriter { &d.attrs, ds_dense_blobs[i].as_ref(), &d.fill_message, + d.refcount, )?; ds_blobs2.push(DataBlob { data: gcol_bytes.clone(), @@ -1534,6 +1625,7 @@ impl FileWriter { &d.attrs, ds_dense_blobs[i].as_ref(), &d.fill_message, + d.refcount, )?; ds_blobs2.push(DataBlob { data: result.data_bytes, @@ -1549,6 +1641,7 @@ impl FileWriter { &d.attrs, ds_dense_blobs[i].as_ref(), &d.fill_message, + d.refcount, )?; ds_blobs2.push(DataBlob { data: vec![], @@ -1574,6 +1667,7 @@ impl FileWriter { &d.attrs, ds_dense_blobs[i].as_ref(), &d.fill_message, + d.refcount, )?; let mut data = vec![0u8; padding]; data.extend_from_slice(&d.raw); @@ -1623,51 +1717,29 @@ impl FileWriter { buf.extend_from_slice(ext); } - // Root group OH - let mut root_links: Vec = Vec::new(); - for &i in &root_ds_indices { - root_links.push(make_link(&all_ds[i].name, ds_oh_addrs2[i])); - } - for (gi, g) in groups.iter().enumerate() { - root_links.push(make_link(&g.name, group_addrs2[gi])); - } - // Rebuild the root link blob with real target addresses (same size as - // the dummy used for layout); its LinkInfo goes in the OH. - let root_link_blob = root_link_blob_addr.map(|addr| build_dense_links(&root_links, addr)); - let root_dl = root_link_blob - .as_ref() - .map(|b| b.link_info_message.as_slice()); - buf.extend_from_slice(&build_group_oh( - &root_links, - root_dl, - &root_attrs, - root_dense_blob.as_ref(), - )?); - if let Some(ref b) = root_link_blob { - buf.extend_from_slice(&b.blob); - } - if let Some(ref blob) = root_dense_blob { - buf.extend_from_slice(&blob.blob); - } - // Group OHs + dense blobs (link blob, then attr blob, matching pass 2) for (gi, g) in groups.iter().enumerate() { - let mut links: Vec = g - .ds_indices - .iter() - .map(|&i| make_link(&all_ds[i].name, ds_oh_addrs2[i])) - .collect(); - for (lname, fname, opath) in &g.external_links { - links.push(make_external_link(lname, fname, opath)); - } - let link_blob = group_link_blob_addrs[gi].map(|addr| build_dense_links(&links, addr)); - let dl = link_blob.as_ref().map(|b| b.link_info_message.as_slice()); - buf.extend_from_slice(&build_group_oh( + let links = g.link_messages(&group_addrs2, &ds_oh_addrs2); + // Rebuild the link blob with real target addresses (same size as + // the dummy used for layout); its LinkInfo goes in the OH. + let link_blob = group_link_blob_addrs[gi] + .map(|addr| build_dense_links(&links, addr, g.track_order)) + .transpose()?; + let li = match &link_blob { + Some(b) => b.link_info_message.clone(), + None => compact_link_info(g.track_order, links.len()), + }; + let oh = build_group_oh( &links, - dl, + &li, + link_blob.is_some(), &g.attrs, group_dense_blobs[gi].as_ref(), - )?); + g.refcount, + )?; + debug_assert_eq!(oh.len(), group_oh_sizes[gi]); + debug_assert_eq!(buf.len() as u64, group_addrs2[gi]); + buf.extend_from_slice(&oh); if let Some(ref b) = link_blob { buf.extend_from_slice(&b.blob); } diff --git a/crates/clawhdf5-format/src/lib.rs b/crates/clawhdf5-format/src/lib.rs index 905ce84..180cf07 100644 --- a/crates/clawhdf5-format/src/lib.rs +++ b/crates/clawhdf5-format/src/lib.rs @@ -130,6 +130,7 @@ mod test_fuzz; pub mod type_builders; pub mod vds; pub mod vl_data; +mod writer_tree; #[cfg(feature = "provenance")] pub mod provenance; diff --git a/crates/clawhdf5-format/src/type_builders.rs b/crates/clawhdf5-format/src/type_builders.rs index f055a88..fa64465 100644 --- a/crates/clawhdf5-format/src/type_builders.rs +++ b/crates/clawhdf5-format/src/type_builders.rs @@ -903,34 +903,117 @@ impl DatasetBuilder { // ---- Group builder ---- -/// Builder for groups. +/// One entry of a [`GroupBuilder`], kept in the order it was added (the +/// order a group that tracks creation order lists its links in). +pub(crate) enum GroupItem { + Dataset(Box), + Group(GroupBuilder), + /// A soft link: `name` resolves to whatever `target` names when read. + Soft { + name: String, + target: String, + }, + /// An extra hard link to the object at `target` (a path in this file). + Hard { + name: String, + target: String, + }, + /// An external link to `path` in the file `file`. + External { + name: String, + file: String, + path: String, + }, +} + +/// Builder for a group: its datasets, subgroups, links and attributes. +/// +/// Names are paths relative to the group: `create_dataset("a/b/x")` creates +/// the groups `a` and `a/b` as needed, as h5py does. A group added where a +/// group of the same path already exists (added by another builder, or +/// created as an intermediate group) is merged into it, like h5py's +/// `require_group`; any other name used twice in a group is an error when the +/// file is written. A path component must not be empty or `"."`. pub struct GroupBuilder { pub(crate) name: String, - pub(crate) datasets: Vec, + pub(crate) items: Vec, pub(crate) attrs: Vec<(String, AttrValue)>, - /// (link_name, target_file, target_path) - pub(crate) external_links: Vec<(String, String, String)>, + /// Track (and index) link creation order; `None` follows the file's + /// default (`FileWriter::track_order`). + pub(crate) track_order: Option, } impl GroupBuilder { pub(crate) fn new(name: &str) -> Self { Self { name: name.to_string(), - datasets: Vec::new(), + items: Vec::new(), attrs: Vec::new(), - external_links: Vec::new(), + track_order: None, } } + /// Create a dataset in this group. `name` may be a relative path + /// (`"a/b/x"`); missing intermediate groups are created. pub fn create_dataset(&mut self, name: &str) -> &mut DatasetBuilder { - self.datasets.push(DatasetBuilder::new(name)); - self.datasets.last_mut().unwrap() + self.items + .push(GroupItem::Dataset(Box::new(DatasetBuilder::new(name)))); + match self.items.last_mut() { + Some(GroupItem::Dataset(d)) => d, + _ => unreachable!("just pushed a dataset"), + } + } + + /// Start a subgroup of this group. Like `FileWriter::create_group`, the + /// builder is detached: fill it, then pass `finish()`'s result to + /// [`Self::add_group`]. `name` may be a relative path. + pub fn create_group(&self, name: &str) -> GroupBuilder { + GroupBuilder::new(name) + } + + /// Add a finished subgroup to this group. + pub fn add_group(&mut self, group: FinishedGroup) -> &mut Self { + self.items.push(GroupItem::Group(group.group)); + self } pub fn set_attr(&mut self, name: &str, value: AttrValue) { self.attrs.push((name.to_string(), value)); } + /// Track the creation order of this group's links, and index it, as + /// h5py's `track_order=True` does: libhdf5 (and h5py) then list the + /// group's members in the order they were added rather than by name. + /// Applies to links only, not to attributes. + pub fn track_order(&mut self, track: bool) -> &mut Self { + self.track_order = Some(track); + self + } + + /// Add a soft link `name` to the path `target` (absolute, or relative to + /// this group), like h5py's `grp[name] = h5py.SoftLink(target)`. The + /// target need not exist. + pub fn add_soft_link(&mut self, name: &str, target: &str) -> &mut Self { + self.items.push(GroupItem::Soft { + name: name.to_string(), + target: target.to_string(), + }); + self + } + + /// Add another hard link `name` to the group or dataset at `target` + /// (absolute, or relative to this group), like h5py's + /// `grp[name] = f[target]`. The target must be written in the same file; + /// its path may go through other hard links, but not through soft or + /// external links. + pub fn add_hard_link(&mut self, name: &str, target: &str) -> &mut Self { + self.items.push(GroupItem::Hard { + name: name.to_string(), + target: target.to_string(), + }); + self + } + /// Add an external link: a named pointer to an object in another HDF5 file. pub fn add_external_link( &mut self, @@ -938,30 +1021,21 @@ impl GroupBuilder { target_file: &str, target_path: &str, ) -> &mut Self { - self.external_links.push(( - name.to_string(), - target_file.to_string(), - target_path.to_string(), - )); + self.items.push(GroupItem::External { + name: name.to_string(), + file: target_file.to_string(), + path: target_path.to_string(), + }); self } /// Consume the builder, returning a FinishedGroup to add to FileWriter. pub fn finish(self) -> FinishedGroup { - FinishedGroup { - name: self.name, - datasets: self.datasets, - attrs: self.attrs, - external_links: self.external_links, - } + FinishedGroup { group: self } } } /// A finished group ready for the file writer. pub struct FinishedGroup { - pub(crate) name: String, - pub(crate) datasets: Vec, - pub(crate) attrs: Vec<(String, AttrValue)>, - /// (link_name, target_file, target_path) - pub(crate) external_links: Vec<(String, String, String)>, + pub(crate) group: GroupBuilder, } diff --git a/crates/clawhdf5-format/src/writer_tree.rs b/crates/clawhdf5-format/src/writer_tree.rs new file mode 100644 index 0000000..284867e --- /dev/null +++ b/crates/clawhdf5-format/src/writer_tree.rs @@ -0,0 +1,446 @@ +//! The group hierarchy `FileWriter` writes: builders flattened into a tree +//! of groups, datasets and links, with path names expanded into +//! intermediate groups, hard links resolved to objects, reference counts +//! counted, and everything put in layout order. + +#[cfg(not(feature = "std"))] +use alloc::{ + collections::BTreeMap, + format, + string::{String, ToString}, + vec, + vec::Vec, +}; +#[cfg(feature = "std")] +use std::collections::BTreeMap; + +use crate::error::FormatError; +use crate::type_builders::{AttrValue, DatasetBuilder, GroupBuilder, GroupItem}; + +/// Hard links followed while resolving one hard-link target path. Guards +/// against hard links whose targets name each other. +const MAX_LINK_DEPTH: usize = 64; + +fn err(msg: String) -> FormatError { + FormatError::SerializationError(msg) +} + +/// A link name must be one path component: not empty, not ".", and without +/// '/' (a '/' separates components, so it cannot be part of a name). +fn check_link_name(name: &str, path: &str) -> Result<(), FormatError> { + if name.is_empty() || name == "." || name.contains('/') { + return Err(err(format!( + "invalid object name {path:?}: every path component must be a \ + non-empty name other than \".\"" + ))); + } + Ok(()) +} + +/// What a link in the final tree points at. +#[derive(Debug, Clone, PartialEq)] +pub(crate) enum LinkTo { + /// A group, by index into [`Tree::groups`] (layout order). + Group(usize), + /// A dataset, by index into [`Tree::datasets`] (layout order). + Dataset(usize), + Soft(String), + External { + file: String, + path: String, + }, +} + +pub(crate) struct Link { + pub(crate) name: String, + pub(crate) to: LinkTo, + /// Set when the group tracks creation order. + pub(crate) creation_order: Option, +} + +pub(crate) struct Group { + pub(crate) attrs: Vec<(String, AttrValue)>, + /// Links in the order they are written. + pub(crate) links: Vec, + pub(crate) track_order: bool, + /// Number of hard links to this group (the root counts one for the + /// superblock's reference). + pub(crate) refcount: u32, +} + +/// The flattened file: groups (root first) and datasets, both in the order +/// they are laid out in the file. +pub(crate) struct Tree { + pub(crate) groups: Vec, + pub(crate) datasets: Vec<(DatasetBuilder, u32)>, +} + +// ---- construction ---- + +enum Target { + Group(usize), + Dataset(usize), + Soft(String), + Hard(String), + External { file: String, path: String }, +} + +struct BuildGroup { + /// Full path, for messages. + path: String, + attrs: Vec<(String, AttrValue)>, + links: Vec<(String, Target)>, + by_name: BTreeMap, + track_order: Option, +} + +struct Builder { + groups: Vec, + datasets: Vec, +} + +fn join(parent: &str, name: &str) -> String { + if parent == "/" { + format!("/{name}") + } else { + format!("{parent}/{name}") + } +} + +impl Builder { + fn new_group(&mut self, path: String) -> usize { + self.groups.push(BuildGroup { + path, + attrs: Vec::new(), + links: Vec::new(), + by_name: BTreeMap::new(), + track_order: None, + }); + self.groups.len() - 1 + } + + /// Split `path` (relative to group `g`) into the group holding its last + /// component, creating missing intermediate groups, and that component. + fn parent_of<'p>(&mut self, g: usize, path: &'p str) -> Result<(usize, &'p str), FormatError> { + // An absolute path is accepted at the root only. + let rel = match path.strip_prefix('/') { + Some(rest) if g == 0 => rest, + Some(_) => { + return Err(err(format!( + "invalid object name {path:?} in {}: absolute paths are accepted \ + only at the root", + self.groups[g].path + ))); + } + None => path, + }; + let mut comps: Vec<&str> = rel.split('/').collect(); + let last = comps.pop().unwrap_or(""); + check_link_name(last, path)?; + let mut cur = g; + for c in comps { + check_link_name(c, path)?; + cur = match self.groups[cur].by_name.get(c).copied() { + Some(i) => match self.groups[cur].links[i].1 { + Target::Group(child) => child, + _ => { + return Err(err(format!( + "cannot create {path:?} in {}: {c:?} exists and is not a group", + self.groups[g].path + ))); + } + }, + None => { + let child = self.new_group(join(&self.groups[cur].path, c)); + self.push_link(cur, c, Target::Group(child))?; + child + } + }; + } + Ok((cur, last)) + } + + fn push_link(&mut self, g: usize, name: &str, to: Target) -> Result<(), FormatError> { + let grp = &mut self.groups[g]; + if grp.by_name.contains_key(name) { + return Err(err(format!("{:?} already exists", join(&grp.path, name)))); + } + grp.by_name.insert(name.to_string(), grp.links.len()); + grp.links.push((name.to_string(), to)); + Ok(()) + } + + /// Add `item` to group `g`. + fn add_item(&mut self, g: usize, item: GroupItem) -> Result<(), FormatError> { + match item { + GroupItem::Dataset(db) => { + let (parent, name) = self.parent_of(g, &db.name)?; + let name = name.to_string(); + self.push_link(parent, &name, Target::Dataset(self.datasets.len()))?; + self.datasets.push(*db); + } + GroupItem::Group(gb) => self.add_group(g, gb)?, + GroupItem::Soft { name, target } => { + if target.is_empty() { + return Err(err(format!("soft link {name:?} has an empty target"))); + } + let (parent, last) = self.parent_of(g, &name)?; + self.push_link(parent, last, Target::Soft(target))?; + } + GroupItem::Hard { name, target } => { + let (parent, last) = self.parent_of(g, &name)?; + self.push_link(parent, last, Target::Hard(target))?; + } + GroupItem::External { name, file, path } => { + if file.is_empty() || path.is_empty() { + return Err(err(format!( + "external link {name:?} needs a file name and an object path" + ))); + } + let (parent, last) = self.parent_of(g, &name)?; + self.push_link(parent, last, Target::External { file, path })?; + } + } + Ok(()) + } + + /// Add the group `gb` (named by a path relative to group `g`), merging it + /// into a group already at that path. + fn add_group(&mut self, g: usize, gb: GroupBuilder) -> Result<(), FormatError> { + let (parent, last) = self.parent_of(g, &gb.name)?; + let idx = match self.groups[parent].by_name.get(last).copied() { + Some(i) => match self.groups[parent].links[i].1 { + Target::Group(child) => child, + _ => { + return Err(err(format!( + "{:?} already exists and is not a group", + join(&self.groups[parent].path, last) + ))); + } + }, + None => { + let child = self.new_group(join(&self.groups[parent].path, last)); + self.push_link(parent, last, Target::Group(child))?; + child + } + }; + self.merge_into(idx, gb) + } + + /// Merge a builder's attributes, setting and items into group `idx`. + fn merge_into(&mut self, idx: usize, gb: GroupBuilder) -> Result<(), FormatError> { + for (name, value) in gb.attrs { + if self.groups[idx].attrs.iter().any(|(n, _)| *n == name) { + return Err(err(format!( + "attribute {name:?} set twice on {}", + self.groups[idx].path + ))); + } + self.groups[idx].attrs.push((name, value)); + } + if let Some(t) = gb.track_order { + match self.groups[idx].track_order { + Some(old) if old != t => { + return Err(err(format!( + "conflicting track_order settings for {}", + self.groups[idx].path + ))); + } + _ => self.groups[idx].track_order = Some(t), + } + } + for item in gb.items { + self.add_item(idx, item)?; + } + Ok(()) + } + + /// The object a hard link's `target` path names, from group `from`. + fn resolve(&self, from: usize, target: &str, depth: usize) -> Result { + if depth > MAX_LINK_DEPTH { + return Err(err(format!( + "hard link target {target:?}: too many hard links to follow (a cycle?)" + ))); + } + let (mut cur, rest) = match target.strip_prefix('/') { + Some(rest) => (0, rest), + None => (from, target), + }; + if target.is_empty() { + return Err(err("a hard link needs a target path".to_string())); + } + let comps: Vec<&str> = rest + .split('/') + .filter(|c| !c.is_empty() && *c != ".") + .collect(); + let mut obj = Obj::Group(cur); + for (i, c) in comps.iter().enumerate() { + let Obj::Group(g) = obj else { + return Err(err(format!( + "hard link target {target:?}: {:?} is not a group", + comps[..i].join("/") + ))); + }; + cur = g; + let grp = &self.groups[cur]; + let Some(&li) = grp.by_name.get(*c) else { + return Err(err(format!( + "hard link target {target:?} does not exist in the file" + ))); + }; + obj = match &grp.links[li].1 { + Target::Group(child) => Obj::Group(*child), + Target::Dataset(d) => Obj::Dataset(*d), + Target::Hard(p) => self.resolve(cur, p, depth + 1)?, + Target::Soft(_) | Target::External { .. } => { + return Err(err(format!( + "hard link target {target:?} goes through a soft or external \ + link ({:?}); name the object by its hard-link path", + join(&grp.path, c) + ))); + } + }; + } + Ok(obj) + } +} + +#[derive(Clone, Copy)] +enum Obj { + Group(usize), + Dataset(usize), +} + +/// Flatten the root group builder into a [`Tree`]. `default_track_order` +/// applies to every group that does not set its own. +pub(crate) fn build(root: GroupBuilder, default_track_order: bool) -> Result { + let mut b = Builder { + groups: Vec::new(), + datasets: Vec::new(), + }; + b.new_group("/".to_string()); + b.merge_into(0, root)?; + + // Resolve hard links and count references. + let mut group_refs = vec![0u32; b.groups.len()]; + let mut ds_refs = vec![0u32; b.datasets.len()]; + group_refs[0] = 1; // the superblock's reference to the root + let mut resolved: Vec>> = Vec::with_capacity(b.groups.len()); + for (gi, g) in b.groups.iter().enumerate() { + let mut row = Vec::with_capacity(g.links.len()); + for (_, t) in &g.links { + let obj = match t { + Target::Group(i) => Some(Obj::Group(*i)), + Target::Dataset(d) => Some(Obj::Dataset(*d)), + Target::Hard(p) => Some(b.resolve(gi, p, 0)?), + Target::Soft(_) | Target::External { .. } => None, + }; + match obj { + Some(Obj::Group(i)) => group_refs[i] += 1, + Some(Obj::Dataset(d)) => ds_refs[d] += 1, + None => {} + } + row.push(obj); + } + resolved.push(row); + } + + // The order each group's links are written in: creation order when + // tracked; otherwise datasets, then groups, then other links (the order + // earlier versions wrote, so one-level files keep their layout). + let tracked: Vec = b + .groups + .iter() + .map(|g| g.track_order.unwrap_or(default_track_order)) + .collect(); + let link_order: Vec> = b + .groups + .iter() + .enumerate() + .map(|(gi, g)| { + let mut idx: Vec = (0..g.links.len()).collect(); + if !tracked[gi] { + idx.sort_by_key(|&i| match g.links[i].1 { + Target::Dataset(_) => 0, + Target::Group(_) => 1, + _ => 2, + }); + } + idx + }) + .collect(); + + // Layout order: groups depth-first from the root, following the links + // that created them; datasets group by group in that order. + let mut group_order = Vec::with_capacity(b.groups.len()); + let mut stack = vec![0usize]; + while let Some(g) = stack.pop() { + group_order.push(g); + let children: Vec = link_order[g] + .iter() + .filter_map(|&i| match b.groups[g].links[i].1 { + Target::Group(c) => Some(c), + _ => None, + }) + .collect(); + stack.extend(children.into_iter().rev()); + } + let mut ds_order = Vec::with_capacity(b.datasets.len()); + for &g in &group_order { + for &i in &link_order[g] { + if let Target::Dataset(d) = b.groups[g].links[i].1 { + ds_order.push(d); + } + } + } + let mut group_pos = vec![0usize; b.groups.len()]; + for (pos, &g) in group_order.iter().enumerate() { + group_pos[g] = pos; + } + let mut ds_pos = vec![0usize; b.datasets.len()]; + for (pos, &d) in ds_order.iter().enumerate() { + ds_pos[d] = pos; + } + + let mut groups_by_id: Vec> = b.groups.into_iter().map(Some).collect(); + let mut groups = Vec::with_capacity(group_order.len()); + for &g in &group_order { + let bg = groups_by_id[g].take().expect("each group is laid out once"); + let mut targets: Vec> = bg.links.into_iter().map(Some).collect(); + let links = link_order[g] + .iter() + .map(|&i| { + let (name, t) = targets[i].take().expect("each link is written once"); + let to = match (resolved[g][i], t) { + (Some(Obj::Group(c)), _) => LinkTo::Group(group_pos[c]), + (Some(Obj::Dataset(d)), _) => LinkTo::Dataset(ds_pos[d]), + (None, Target::Soft(s)) => LinkTo::Soft(s), + (None, Target::External { file, path }) => LinkTo::External { file, path }, + (None, _) => unreachable!("hard links are resolved"), + }; + Link { + name, + to, + creation_order: tracked[g].then_some(i as u64), + } + }) + .collect(); + groups.push(Group { + attrs: bg.attrs, + links, + track_order: tracked[g], + refcount: group_refs[g], + }); + } + let mut ds_by_id: Vec> = b.datasets.into_iter().map(Some).collect(); + let datasets = ds_order + .iter() + .map(|&d| { + ( + ds_by_id[d].take().expect("each dataset is laid out once"), + ds_refs[d], + ) + }) + .collect(); + Ok(Tree { groups, datasets }) +} diff --git a/crates/clawhdf5-format/tests/writer_meta_tests.rs b/crates/clawhdf5-format/tests/writer_meta_tests.rs index a79e52d..3e907ab 100644 --- a/crates/clawhdf5-format/tests/writer_meta_tests.rs +++ b/crates/clawhdf5-format/tests/writer_meta_tests.rs @@ -528,33 +528,50 @@ fn h5py_reads_all_attributes_next_to_an_empty_string() { // ---- 6. path-like names ---- #[test] -fn slash_in_a_group_or_dataset_name_is_an_error() { - // Measured: create_group("a/b") wrote one link literally named "a/b", - // which h5py cannot reach ("component not found"). The writer has no - // nested groups, so such names are refused. +fn path_names_create_nested_groups() { + // create_group("a/b") used to write one link literally named "a/b", + // which h5py cannot reach ("component not found"); then such names were + // refused. Now a path creates its missing intermediate groups, as h5py + // does. let mut fw = FileWriter::new(); let mut g = fw.create_group("a/b"); g.create_dataset("c").with_f64_data(&[1.0]); fw.add_group(g.finish()); - assert!(fw.finish().is_err()); - - let mut fw = FileWriter::new(); - fw.create_dataset("x/y").with_f64_data(&[1.0]); - assert!(fw.finish().is_err()); - - let mut fw = FileWriter::new(); + fw.create_dataset("x/y").with_f64_data(&[2.0]); + fw.create_dataset("/a/b/z").with_f64_data(&[3.0]); let mut g = fw.create_group("g"); - g.create_dataset("x/y").with_f64_data(&[1.0]); + g.create_dataset("x/y").with_f64_data(&[4.0]); fw.add_group(g.finish()); - assert!(fw.finish().is_err()); + let bytes = fw.finish().unwrap(); + for path in ["a", "a/b", "a/b/c", "a/b/z", "x", "x/y", "g/x", "g/x/y"] { + header_at(&bytes, path); + } +} - for bad in ["", "."] { +#[test] +fn names_that_are_not_valid_link_names_are_errors() { + for bad in ["", ".", "a//b", "a/", "a/./b", "/"] { let mut fw = FileWriter::new(); fw.create_dataset(bad).with_f64_data(&[1.0]); assert!(fw.finish().is_err(), "{bad:?}"); } + // An absolute path inside a group, and a name used twice. + let mut fw = FileWriter::new(); + let mut g = fw.create_group("g"); + g.create_dataset("/x").with_f64_data(&[1.0]); + fw.add_group(g.finish()); + assert!(fw.finish().is_err()); + let mut fw = FileWriter::new(); + fw.create_dataset("x").with_f64_data(&[1.0]); + fw.create_dataset("x").with_f64_data(&[1.0]); + assert!(fw.finish().is_err()); + // A dataset in the way of a path. + let mut fw = FileWriter::new(); + fw.create_dataset("x").with_f64_data(&[1.0]); + fw.create_dataset("x/y").with_f64_data(&[1.0]); + assert!(fw.finish().is_err()); - // One level of groups still works, and '/' stays legal in attribute names. + // '/' stays legal in attribute names. let mut fw = FileWriter::new(); let mut g = fw.create_group("g"); g.create_dataset("c").with_f64_data(&[1.0]); diff --git a/crates/clawhdf5-tools/tests/h5rs_interop.rs b/crates/clawhdf5-tools/tests/h5rs_interop.rs index 7d97cc5..519bc6d 100644 --- a/crates/clawhdf5-tools/tests/h5rs_interop.rs +++ b/crates/clawhdf5-tools/tests/h5rs_interop.rs @@ -773,3 +773,88 @@ fn every_subcommand_rejects_a_non_hdf5_file_cleanly() { assert_eq!(code(&h5rs(&["ls"])), 2); assert_eq!(code(&h5rs(&["--help"])), 0); } + +// --------------------------------------------------------------------------- +// files clawhdf5 writes: nested groups and links +// --------------------------------------------------------------------------- + +/// Nested groups (4 levels, by builders and by path names), soft, hard and +/// external links, creation-order tracking, and dense link and attribute +/// storage, as `FileBuilder` writes them. +fn write_nested_links(dir: &Path) -> Vec { + use clawhdf5::{AttrValue, FileBuilder}; + let mut b = FileBuilder::new(); + b.set_attr("title", AttrValue::String("links".into())); + b.create_dataset("x/y").with_f64_data(&[1.0, 2.0]); + b.create_dataset("a/b/c/d/leaf") + .with_i32_data(&[4, 5]) + .set_attr("depth", AttrValue::I64(5)); + b.add_soft_link("soft", "/x/y"); + b.add_soft_link("dangling", "/nowhere"); + b.add_hard_link("alias", "/x/y"); + b.add_external_link("ext", "other.h5", "/data"); + let mut g = b.create_group("a/b"); + g.set_attr("merged", AttrValue::I64(1)); + for i in 0..10 { + g.set_attr(&format!("attr{i}"), AttrValue::F64(i as f64)); + } + b.add_group(g.finish()); + let mut g = b.create_group("ordered"); + g.track_order(true); + for i in (0..40).rev() { + g.create_dataset(&format!("n{i:02}")).with_i32_data(&[i]); + } + g.add_hard_link("back", "/a/b/c"); + b.add_group(g.finish()); + let mut g = b.create_group("compact_ordered"); + g.track_order(true); + g.create_dataset("z").with_i32_data(&[1]); + g.create_dataset("a").with_i32_data(&[2]); + b.add_group(g.finish()); + let nested = dir.join("nested.h5"); + b.write(&nested).unwrap(); + + let mut b = FileBuilder::new(); + let mut g = b.create_group("many"); + for i in 0..10_000 { + g.create_dataset(&format!("d{i:05}")).with_i32_data(&[i]); + } + b.add_group(g.finish()); + let many = dir.join("many.h5"); + b.write(&many).unwrap(); + [nested, many] + .iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect() +} + +#[test] +fn check_and_dump_files_with_nested_groups_and_links() { + let dir = tempfile::tempdir().unwrap(); + let files = write_nested_links(dir.path()); + // `--data` reads every dataset by path, and a lookup in a dense group + // scans all of its links: 10 000 datasets take minutes in a debug + // build, so the big file is checked structurally only (and not dumped). + for (p, data) in [(&files[0], true), (&files[1], false)] { + let args: &[&str] = if data { + &["check", "--data", p] + } else { + &["check", p] + }; + let o = h5rs(args); + assert_eq!(code(&o), 0, "{p}:\n{}", stdout(&o)); + assert!(stdout(&o).contains("no problems found"), "{}", stdout(&o)); + } + if missing(tool_available("h5dump"), "h5dump") { + return; + } + for p in &files[..1] { + let name = Path::new(p).file_name().unwrap().to_string_lossy(); + let ours = h5rs(&["dump", p]); + assert!(ours.status.success(), "{p}: {ours:?}"); + let reference = run("h5dump", &[p]); + assert!(reference.status.success(), "h5dump {p}: {reference:?}"); + let r = stdout(&reference).replacen(p.as_str(), &name, 1); + assert_eq!(stdout(&ours), r, "{name}"); + } +} diff --git a/crates/clawhdf5/src/writer.rs b/crates/clawhdf5/src/writer.rs index 59904ba..bfa9393 100644 --- a/crates/clawhdf5/src/writer.rs +++ b/crates/clawhdf5/src/writer.rs @@ -42,14 +42,17 @@ impl FileBuilder { } } - /// Create a dataset at the root level. Returns a mutable reference to - /// a `DatasetBuilder` for configuring data, shape, and attributes. + /// Create a dataset. Returns a mutable reference to a `DatasetBuilder` + /// for configuring data, shape, and attributes. `name` may be a path + /// (`"a/b/x"`): missing intermediate groups are created, as in h5py. pub fn create_dataset(&mut self, name: &str) -> &mut FormatDatasetBuilder { self.writer.create_dataset(name) } /// Create a group builder. Call `.finish()` on the returned builder - /// to complete it, then pass to `add_group()`. + /// to complete it, then pass to `add_group()`. `name` may be a path; + /// groups nest to any depth (see `GroupBuilder::add_group`), and a group + /// added at a path that already holds a group is merged into it. pub fn create_group(&mut self, name: &str) -> FormatGroupBuilder { self.writer.create_group(name) } @@ -59,6 +62,40 @@ impl FileBuilder { self.writer.add_group(group); } + /// Add a soft link `name` to the path `target`, like h5py's + /// `f[name] = h5py.SoftLink(target)`. The target need not exist. + pub fn add_soft_link(&mut self, name: &str, target: &str) -> &mut Self { + self.writer.add_soft_link(name, target); + self + } + + /// Add another hard link `name` to the object at `target`, like h5py's + /// `f[name] = f[target]`. The target must be written in this file. + pub fn add_hard_link(&mut self, name: &str, target: &str) -> &mut Self { + self.writer.add_hard_link(name, target); + self + } + + /// Add an external link `name` to `target_path` in `target_file`. + pub fn add_external_link( + &mut self, + name: &str, + target_file: &str, + target_path: &str, + ) -> &mut Self { + self.writer + .add_external_link(name, target_file, target_path); + self + } + + /// Track link creation order in every group that does not set its own + /// (`GroupBuilder::track_order`), as h5py's `track_order=True`: libhdf5 + /// then lists members in the order they were added. + pub fn track_order(&mut self, track: bool) -> &mut Self { + self.writer.track_order(track); + self + } + /// Set an attribute on the root group. pub fn set_attr(&mut self, name: &str, value: AttrValue) { self.writer.set_root_attr(name, value); diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 9ea3053..1e8f879 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -6,7 +6,7 @@ use std::process::Command; -use clawhdf5::{File, FileBuilder}; +use clawhdf5::{AttrValue, File, FileBuilder, Group}; fn python() -> String { std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) @@ -134,3 +134,461 @@ fn h5py_can_add_links_to_groups_we_wrote() { ); assert_eq!(f.dataset("alias").unwrap().read_f64().unwrap(), [1.0, 2.0]); } + +// ---- the whole tree, as h5py and as clawhdf5 read it ---- + +fn fmt_num(x: f64) -> String { + format!("{x:.6}") +} + +fn fmt_attr(v: &AttrValue) -> String { + let join = |v: Vec| v.join(","); + match v { + AttrValue::F64(x) => fmt_num(*x), + AttrValue::I64(x) => fmt_num(*x as f64), + AttrValue::U64(x) => fmt_num(*x as f64), + AttrValue::F64Array(a) => join(a.iter().map(|x| fmt_num(*x)).collect()), + AttrValue::I64Array(a) => join(a.iter().map(|x| fmt_num(*x as f64)).collect()), + AttrValue::U64Array(a) => join(a.iter().map(|x| fmt_num(*x as f64)).collect()), + AttrValue::String(s) => s.clone(), + AttrValue::StringArray(a) => a.join(","), + AttrValue::Raw { .. } => "raw".to_string(), + } +} + +fn fmt_attrs(attrs: std::collections::HashMap) -> String { + let mut v: Vec<_> = attrs.into_iter().collect(); + v.sort_by(|a, b| a.0.cmp(&b.0)); + v.iter() + .map(|(k, a)| format!("{k}={}", fmt_attr(a))) + .collect::>() + .join(";") +} + +fn child_path(path: &str, name: &str) -> String { + if path == "/" { + format!("/{name}") + } else { + format!("{path}/{name}") + } +} + +/// Every group and dataset reachable from `g` (following hard and soft +/// links; the tree must be acyclic), one line each: path, kind, attributes +/// and (datasets) values. +fn walk(g: &Group<'_>, path: &str, out: &mut Vec) { + out.push(format!("{path}|group|{}", fmt_attrs(g.attrs().unwrap()))); + let mut names: Vec<(String, bool)> = g + .datasets() + .unwrap() + .into_iter() + .map(|n| (n, false)) + .chain(g.groups().unwrap().into_iter().map(|n| (n, true))) + .collect(); + names.sort(); + for (name, is_group) in names { + let p = child_path(path, &name); + if is_group { + walk(&g.group(&name).unwrap(), &p, out); + } else { + let ds = g.dataset(&name).unwrap(); + let values: Vec = ds.read_f64().unwrap().into_iter().map(fmt_num).collect(); + out.push(format!( + "{p}|dataset|{}|{}", + fmt_attrs(ds.attrs().unwrap()), + values.join(",") + )); + } + } +} + +fn clawhdf5_tree(path: &str) -> String { + let f = File::open(path).unwrap(); + let mut out = Vec::new(); + walk(&f.root(), "/", &mut out); + out.join("\n") +} + +/// The same listing as [`walk`], from h5py. External and dangling soft +/// links are skipped, as clawhdf5's group listings skip them. +const H5PY_WALK: &str = r#" +def fmt(v): + if isinstance(v, bytes): return v.decode() + if isinstance(v, str): return v + a = np.asarray(v) + if a.dtype.kind in 'SUO': + return ','.join(x.decode() if isinstance(x, bytes) else str(x) for x in a.ravel()) + if a.ndim == 0: return '%.6f' % float(a) + return ','.join('%.6f' % float(x) for x in a.ravel()) +def attrs(o): return ';'.join(f'{k}={fmt(o.attrs[k])}' for k in sorted(o.attrs)) +out = [] +def walk(g, path): + out.append(f'{path}|group|{attrs(g)}') + for k in sorted(g.keys()): + if isinstance(g.get(k, getlink=True), h5py.ExternalLink): continue + o = g.get(k) + if o is None: continue + p = '/' + k if path == '/' else path + '/' + k + if isinstance(o, h5py.Group): walk(o, p) + else: + vals = ','.join('%.6f' % float(x) for x in np.asarray(o[()]).ravel()) + out.append(f'{p}|dataset|{attrs(o)}|{vals}') +with h5py.File(path, 'r') as f: + walk(f, '/') +print('\n'.join(out)) +"#; + +fn h5py_tree(path: &str) -> String { + h5py(path, H5PY_WALK) +} + +/// A four-level tree with attributes on every object: nested builders, +/// path names (with intermediate groups made on the way) and a group added +/// twice (merged), with dense attribute storage at one level and dense link +/// storage at another. +fn nested_builder() -> FileBuilder { + let mut b = FileBuilder::new(); + b.set_attr("title", AttrValue::String("nested".into())); + let mut l1 = b.create_group("l1"); + l1.set_attr("depth", AttrValue::I64(1)); + l1.create_dataset("d1") + .with_f64_data(&[1.0, 1.5]) + .set_attr("unit", AttrValue::String("m".into())); + let mut l2 = l1.create_group("l2"); + l2.set_attr("depth", AttrValue::I64(2)); + l2.create_dataset("d2").with_i32_data(&[2, 3, 4]); + let mut l3 = l2.create_group("l3"); + for i in 0..10 { + l3.set_attr(&format!("a{i}"), AttrValue::F64(i as f64 / 4.0)); // dense + } + for i in 0..12 { + l3.create_dataset(&format!("x{i:02}")) // dense links + .with_i64_data(&[i, -i]) + .set_attr("i", AttrValue::I64(i)); + } + let mut l4 = l3.create_group("l4"); + l4.set_attr("depth", AttrValue::I64(4)); + l4.create_dataset("leaf") + .with_f64_data(&[4.0, 4.25, 4.5]) + .set_attr( + "tags", + AttrValue::StringArray(vec!["a".into(), "bc".into()]), + ); + l3.add_group(l4.finish()); + l2.add_group(l3.finish()); + l1.add_group(l2.finish()); + b.add_group(l1.finish()); + // Path names: /p, /p/q and /p/q/r are made on the way to the dataset. + b.create_dataset("p/q/r/s") + .with_f64_data(&[7.0]) + .set_attr("deep", AttrValue::I64(4)); + // A group at an existing path is merged into it. + let mut pq = b.create_group("p/q"); + pq.set_attr("merged", AttrValue::I64(1)); + pq.create_dataset("t").with_i32_data(&[8]); + b.add_group(pq.finish()); + let mut l1b = b.create_group("l1/l2/l3/l4/l5"); + l1b.set_attr("depth", AttrValue::I64(5)); + b.add_group(l1b.finish()); + b +} + +const NESTED_TREE: &str = "\ +/|group|title=nested +/l1|group|depth=1.000000 +/l1/d1|dataset|unit=m|1.000000,1.500000 +/l1/l2|group|depth=2.000000 +/l1/l2/d2|dataset||2.000000,3.000000,4.000000"; + +#[test] +fn nested_groups_read_the_same_in_h5py_and_clawhdf5() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = write(&dir, "nested.h5", nested_builder()); + let ours = clawhdf5_tree(&path); + let theirs = h5py_tree(&path); + assert_eq!(ours, theirs); + assert!(ours.starts_with(NESTED_TREE), "{ours}"); + for line in [ + "/l1/l2/l3|group|a0=0.000000;a1=0.250000;a2=0.500000;a3=0.750000;a4=1.000000;\ + a5=1.250000;a6=1.500000;a7=1.750000;a8=2.000000;a9=2.250000", + "/l1/l2/l3/l4|group|depth=4.000000", + "/l1/l2/l3/l4/l5|group|depth=5.000000", + "/l1/l2/l3/l4/leaf|dataset|tags=a,bc|4.000000,4.250000,4.500000", + "/l1/l2/l3/x11|dataset|i=11.000000|11.000000,-11.000000", + "/p|group|", + "/p/q|group|merged=1.000000", + "/p/q/r/s|dataset|deep=4.000000|7.000000", + "/p/q/t|dataset||8.000000", + ] { + assert!( + ours.lines().any(|l| l == line), + "missing {line:?} in\n{ours}" + ); + } + assert_eq!(ours.lines().count(), 26, "{ours}"); + let dump = h5dump_ok(&path); + if !dump.is_empty() { + assert!(dump.contains("GROUP \"l5\""), "{dump}"); + assert!(dump.contains("DATASET \"leaf\""), "{dump}"); + } +} + +#[test] +fn soft_hard_and_external_links() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let mut other = FileBuilder::new(); + other.create_dataset("data").with_i32_data(&[42, 43]); + write(&dir, "other.h5", other); + + let mut b = FileBuilder::new(); + b.create_dataset("x/y").with_f64_data(&[1.0, 2.0, 3.0]); + b.create_dataset("x/z").with_i32_data(&[9]); + b.add_soft_link("soft_abs", "/x/y"); + b.add_soft_link("dangling", "/nowhere"); + b.add_hard_link("alias", "/x/y"); + b.add_hard_link("x_again", "x"); + b.add_external_link("ext", "other.h5", "/data"); + let mut g = b.create_group("a/b/c"); + g.add_soft_link("rel", "sib"); // relative to /a/b/c + g.create_dataset("sib").with_i32_data(&[5]); + g.add_hard_link("deep_alias", "/x_again/z"); // through a hard link + g.add_soft_link("to_group", "/x"); + b.add_group(g.finish()); + let path = write(&dir, "links.h5", b); + + let out = h5py( + &path, + "import os\nos.chdir(os.path.dirname(path))\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 def kind(g, k):\n\ + \x20 l = g.get(k, getlink=True)\n\ + \x20 if isinstance(l, h5py.SoftLink): return 'soft:' + l.path\n\ + \x20 if isinstance(l, h5py.ExternalLink): return 'ext:' + l.filename + ':' + l.path\n\ + \x20 return 'hard'\n\ + \x20 print(json.dumps({\n\ + \x20 'root': {k: kind(f, k) for k in f},\n\ + \x20 'abc': {k: kind(f['a/b/c'], k) for k in f['a/b/c']},\n\ + \x20 'same': [f['alias'].id == f['x/y'].id, f['x_again'].id == f['x'].id,\n\ + \x20 f['a/b/c/deep_alias'].id == f['x/z'].id],\n\ + \x20 'rc': [h5py.h5o.get_info(f['x/y'].id).rc, h5py.h5o.get_info(f['x'].id).rc,\n\ + \x20 h5py.h5o.get_info(f['x/z'].id).rc, h5py.h5o.get_info(f['a'].id).rc],\n\ + \x20 'vals': [f['soft_abs'][()].tolist(), f['ext'][()].tolist(),\n\ + \x20 f['a/b/c/rel'][()].tolist(), sorted(f['a/b/c/to_group'])],\n\ + \x20 'dangling': f.get('dangling') is None,\n\ + \x20 }, sort_keys=True))", + ); + assert_eq!( + out, + r#"{"abc": {"deep_alias": "hard", "rel": "soft:sib", "sib": "hard", "to_group": "soft:/x"}, "dangling": true, "rc": [2, 2, 2, 1], "root": {"a": "hard", "alias": "hard", "dangling": "soft:/nowhere", "ext": "ext:other.h5:/data", "soft_abs": "soft:/x/y", "x": "hard", "x_again": "hard"}, "same": [true, true, true], "vals": [[1.0, 2.0, 3.0], [42, 43], [5], ["y", "z"]]}"# + ); + assert_eq!(clawhdf5_tree(&path), h5py_tree(&path)); + h5dump_ok(&path); + + let f = File::open(&path).unwrap(); + assert_eq!( + f.dataset("alias").unwrap().read_f64().unwrap(), + [1.0, 2.0, 3.0] + ); + assert_eq!( + f.dataset("soft_abs").unwrap().read_f64().unwrap(), + [1.0, 2.0, 3.0] + ); + assert_eq!(f.dataset("a/b/c/rel").unwrap().read_i32().unwrap(), [5]); + assert_eq!( + f.dataset("a/b/c/deep_alias").unwrap().read_i32().unwrap(), + [9] + ); + assert_eq!( + f.dataset("x_again/y").unwrap().read_f64().unwrap(), + [1.0, 2.0, 3.0] + ); + drop(f); + + // The reference counts let libhdf5 delete one of two hard links and + // keep the object; with a count of 1 it would free an object still + // linked from elsewhere. + let out = h5py( + &path, + "with h5py.File(path, 'r+') as f:\n\ + \x20 del f['alias']\n\ + \x20 del f['x_again']\n\ + \x20 f.create_dataset('filler', data=np.arange(1000))\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([f['x/y'][()].tolist(), sorted(f['x']), h5py.h5o.get_info(f['x/y'].id).rc]))", + ); + assert_eq!(out, r#"[[1.0, 2.0, 3.0], ["y", "z"], 1]"#); + h5dump_ok(&path); +} + +#[test] +fn a_hard_link_can_make_a_cycle() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + let mut g = b.create_group("g"); + g.create_dataset("v").with_i32_data(&[1]); + g.add_hard_link("up", "/"); + g.add_hard_link("me", "."); + b.add_group(g.finish()); + let path = write(&dir, "cycle.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([sorted(f['g/up/g']), f['g/up/g/me/me/v'][()].tolist(),\n\ + \x20 h5py.h5o.get_info(f.id).rc, h5py.h5o.get_info(f['g'].id).rc]))", + ); + assert_eq!(out, r#"[["me", "up", "v"], [1], 2, 2]"#); + h5dump_ok(&path); + let f = File::open(&path).unwrap(); + assert_eq!(f.dataset("g/up/g/me/v").unwrap().read_i32().unwrap(), [1]); +} + +#[test] +fn bad_links_are_errors() { + for setup in [ + |b: &mut FileBuilder| { + b.add_hard_link("h", "/missing"); + }, + |b: &mut FileBuilder| { + b.create_dataset("x").with_i32_data(&[1]); + b.add_soft_link("s", "/x"); + b.add_hard_link("h", "/s"); // through a soft link + }, + |b: &mut FileBuilder| { + b.add_hard_link("h1", "/h2"); + b.add_hard_link("h2", "/h1"); + }, + |b: &mut FileBuilder| { + b.create_dataset("x").with_i32_data(&[1]); + b.add_hard_link("h", "/x/y"); // a dataset is not a group + }, + |b: &mut FileBuilder| { + b.add_soft_link("s", ""); + }, + |b: &mut FileBuilder| { + b.add_external_link("e", "", "/x"); + }, + |b: &mut FileBuilder| { + b.create_dataset("x").with_i32_data(&[1]); + b.add_soft_link("x", "/y"); // name taken + }, + ] { + let mut b = FileBuilder::new(); + setup(&mut b); + assert!(b.finish().is_err()); + } +} + +#[test] +fn ten_thousand_links_in_one_group() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + let mut g = b.create_group("many"); + for i in 0..10_000 { + g.create_dataset(&format!("d{i:05}")).with_i32_data(&[i]); + } + g.set_attr("n", AttrValue::I64(10_000)); + b.add_group(g.finish()); + // The same in creation order, added in reverse name order, with soft + // links among them. + let mut g = b.create_group("ordered"); + g.track_order(true); + for i in (0..10_000).rev() { + if i % 1000 == 0 { + g.add_soft_link(&format!("s{i:05}"), &format!("/many/d{i:05}")); + } + g.create_dataset(&format!("d{i:05}")).with_i32_data(&[i]); + } + b.add_group(g.finish()); + let path = write(&dir, "many.h5", b); + + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 m, o = f['many'], f['ordered']\n\ + \x20 names = list(m)\n\ + \x20 onames = list(o)\n\ + \x20 print(json.dumps([len(names), names == sorted(names), names[:2], int(m.attrs['n']),\n\ + \x20 [int(m['d%05d' % i][0]) for i in (0, 1, 4096, 9999)],\n\ + \x20 len(onames), onames[:3], onames[-2:], int(o['s05000'][0]),\n\ + \x20 o.id.get_create_plist().get_link_creation_order()]))", + ); + assert_eq!( + out, + r#"[10000, true, ["d00000", "d00001"], 10000, [0, 1, 4096, 9999], 10010, ["d09999", "d09998", "d09997"], ["s00000", "d00000"], 5000, 3]"# + ); + h5dump_ok(&path); + let f = File::open(&path).unwrap(); + let g = f.group("many").unwrap(); + assert_eq!(g.datasets().unwrap().len(), 10_000); + assert_eq!(g.dataset("d09999").unwrap().read_i32().unwrap(), [9999]); + assert_eq!( + f.dataset("ordered/s05000").unwrap().read_i32().unwrap(), + [5000] + ); +} + +#[test] +fn more_links_than_one_index_leaf_holds_is_an_error() { + let mut b = FileBuilder::new(); + for i in 0..70_000 { + b.add_soft_link(&format!("s{i}"), "/x"); + } + let err = b.finish().unwrap_err().to_string(); + assert!(err.contains("at most 65535 links"), "{err}"); +} + +#[test] +fn track_order_lists_members_in_creation_order() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let names = ["zeta", "alpha", "mid", "beta"]; + let mut b = FileBuilder::new(); + b.track_order(true); // the root and every group without its own setting + for n in names { + b.create_dataset(n).with_i32_data(&[1]); + } + let mut g = b.create_group("by_name"); + g.track_order(false); + for n in names { + g.create_dataset(n).with_i32_data(&[2]); + } + b.add_group(g.finish()); + let mut g = b.create_group("dense"); + for i in (0..20).rev() { + g.create_dataset(&format!("n{i:02}")).with_i32_data(&[i]); + } + g.add_soft_link("soft", "/zeta"); + b.add_group(g.finish()); + b.create_dataset("made/on/the/way").with_i32_data(&[3]); + let path = write(&dir, "order.h5", b); + + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([list(f), list(f['by_name']), list(f['dense'])[:3],\n\ + \x20 list(f['dense'])[-2:], list(f['made/on'])]))", + ); + assert_eq!( + out, + r#"[["zeta", "alpha", "mid", "beta", "by_name", "dense", "made"], ["alpha", "beta", "mid", "zeta"], ["n19", "n18", "n17"], ["n00", "soft"], ["the"]]"# + ); + h5dump_ok(&path); + assert_eq!(clawhdf5_tree(&path), h5py_tree(&path)); + + // libhdf5 keeps the order when it adds to (and converts) these groups. + let out = h5py( + &path, + "with h5py.File(path, 'r+') as f:\n\ + \x20 f['aaa'] = np.arange(2)\n\ + \x20 f['dense']['aaa'] = np.arange(2)\n\ + \x20 del f['dense/n10']\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([list(f)[-1], list(f['dense'])[-2:], len(f['dense'])]))", + ); + assert_eq!(out, r#"["aaa", ["soft", "aaa"], 21]"#); + h5dump_ok(&path); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index bf8f77a..76fc86e 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -202,8 +202,17 @@ fill-value item that did is fixed). and h5dump 1.14.6 rejects 9 of those (tank, 2026-09-26; 28 and 21 before these checks). - **Writer:** - - Nested groups beyond one level: path-like names are now refused, not - created. + - ~~Nested groups beyond one level: path-like names are now refused, not + created.~~ **Fixed 2026-09-26:** groups nest to any depth (path names + create intermediate groups, as h5py does), with soft, extra hard and + external links at any depth and optional creation-order tracking; + h5py, h5dump and `h5rs check --data` read them + (`crates/clawhdf5/tests/writer_groups_interop.rs`, + `crates/clawhdf5-tools/tests/h5rs_interop.rs`). Still missing: a group + with more than 65 535 links (its link index is one B-tree leaf) is an + error, and attribute creation order is not tracked. + - ~~libhdf5 could not add a link to a group we wrote (no Group Info + message).~~ **Fixed 2026-09-26.** - Dense attribute storage for attributes over 64 KiB. - Output that HDF5 1.8 can read. - A B-tree v2 chunk index larger than one leaf, so datasets with several From bd36fe883bfa31478a872c27bbcd55c61f87a536 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:34:04 -0500 Subject: [PATCH 03/10] fix(format): flag non-ASCII link names as UTF-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The writer marked every link name ASCII, so a name such as "größe" was stored as UTF-8 bytes under the ASCII character set (h5py reports cset 0 for it). Names that are not plain ASCII now carry the UTF-8 flag, as h5py writes them; ASCII names are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 3 +++ crates/clawhdf5-format/src/file_writer.rs | 12 +++++++++++- crates/clawhdf5/tests/writer_groups_interop.rs | 18 ++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb939de..3092ae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,9 @@ `h5rs check` passes and `h5rs dump` equals h5dump (`crates/clawhdf5/tests/writer_groups_interop.rs`, `crates/clawhdf5-tools/tests/h5rs_interop.rs`). +- **Non-ASCII link names were marked ASCII.** A group or dataset name such as + `größe` was written with the ASCII character set flag (h5py reported + `cset` 0 for it); it is now flagged UTF-8, as h5py writes it. - **libhdf5 could not add links to groups we wrote.** h5py in `"r+"` mode failed with "Unable to create link (message type not found)" on every group `FileWriter` wrote: libhdf5 reads a group's Group Info message before diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index b4a90aa..2d318f5 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -220,6 +220,16 @@ fn add_refcount(w: &mut ObjectHeaderWriter, refcount: u32) { } } +/// The character set a link name is written with: UTF-8 when it is not +/// plain ASCII, as h5py writes it. +fn name_charset(name: &str) -> CharacterSet { + if name.is_ascii() { + CharacterSet::Ascii + } else { + CharacterSet::Utf8 + } +} + /// The Link message for `link`, whose group and dataset targets are at the /// given addresses (indexed as in the writer tree). fn link_message(link: &writer_tree::Link, group_addrs: &[u64], ds_addrs: &[u64]) -> LinkMessage { @@ -242,7 +252,7 @@ fn link_message(link: &writer_tree::Link, group_addrs: &[u64], ds_addrs: &[u64]) name: link.name.clone(), link_target, creation_order: link.creation_order, - charset: CharacterSet::Ascii, + charset: name_charset(&link.name), } } diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 1e8f879..fb7aa23 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -592,3 +592,21 @@ fn track_order_lists_members_in_creation_order() { assert_eq!(out, r#"["aaa", ["soft", "aaa"], 21]"#); h5dump_ok(&path); } + +#[test] +fn non_ascii_names_are_utf8() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.create_dataset("größe/wert").with_i32_data(&[1]); + let path = write(&dir, "utf8.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 l = f.id.links.get_info('größe'.encode())\n\ + \x20 print(json.dumps([list(f), list(f['größe']), l.cset], ensure_ascii=False))", + ); + assert_eq!(out, r#"[["größe"], ["wert"], 1]"#); + let f = File::open(&path).unwrap(); + assert_eq!(f.dataset("größe/wert").unwrap().read_i32().unwrap(), [1]); +} From b0a1e4f9a61f3b0e1a925f76a31a55f9173a7278 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:35:16 -0500 Subject: [PATCH 04/10] fix(format): a group attribute set again replaces the earlier value Setting a group or root attribute twice wrote two attribute messages with the same name, and h5py read back the first value: set_attr("w", 1) then set_attr("w", "two") read as 1. The later value now replaces the earlier one, as `attrs[name] = v` does in h5py, including when a group is merged from two builders. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 5 ++++ crates/clawhdf5-format/src/writer_tree.rs | 12 ++++---- .../clawhdf5/tests/writer_groups_interop.rs | 28 +++++++++++++++++++ 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3092ae5..080045d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,11 @@ `h5rs check` passes and `h5rs dump` equals h5dump (`crates/clawhdf5/tests/writer_groups_interop.rs`, `crates/clawhdf5-tools/tests/h5rs_interop.rs`). +- **A group attribute set twice read back as its first value.** Setting a + group (or root) attribute again wrote a second attribute message with the + same name, and h5py returned the first value. The later value now replaces + the earlier one, as `attrs[name] = v` does in h5py — also when a group is + merged from two builders. - **Non-ASCII link names were marked ASCII.** A group or dataset name such as `größe` was written with the ASCII character set flag (h5py reported `cset` 0 for it); it is now flagged UTF-8, as h5py writes it. diff --git a/crates/clawhdf5-format/src/writer_tree.rs b/crates/clawhdf5-format/src/writer_tree.rs index 284867e..724ba70 100644 --- a/crates/clawhdf5-format/src/writer_tree.rs +++ b/crates/clawhdf5-format/src/writer_tree.rs @@ -229,14 +229,14 @@ impl Builder { /// Merge a builder's attributes, setting and items into group `idx`. fn merge_into(&mut self, idx: usize, gb: GroupBuilder) -> Result<(), FormatError> { + // An attribute set again (by this builder or a merged one) takes the + // new value, as assigning `attrs[name]` in h5py does. for (name, value) in gb.attrs { - if self.groups[idx].attrs.iter().any(|(n, _)| *n == name) { - return Err(err(format!( - "attribute {name:?} set twice on {}", - self.groups[idx].path - ))); + let attrs = &mut self.groups[idx].attrs; + match attrs.iter_mut().find(|(n, _)| *n == name) { + Some(slot) => slot.1 = value, + None => attrs.push((name, value)), } - self.groups[idx].attrs.push((name, value)); } if let Some(t) = gb.track_order { match self.groups[idx].track_order { diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index fb7aa23..ba2d121 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -610,3 +610,31 @@ fn non_ascii_names_are_utf8() { let f = File::open(&path).unwrap(); assert_eq!(f.dataset("größe/wert").unwrap().read_i32().unwrap(), [1]); } + +#[test] +fn a_group_attribute_set_again_takes_the_new_value() { + skip_if_no_python!(); + // Setting a group attribute twice wrote two attribute messages with one + // name. Now the later value replaces the earlier, as `attrs[name] = v` + // does in h5py — also across a group merged from two builders. + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.set_attr("v", AttrValue::I64(1)); + b.set_attr("v", AttrValue::I64(2)); + let mut g = b.create_group("g"); + g.set_attr("w", AttrValue::I64(1)); + b.add_group(g.finish()); + let mut g = b.create_group("g"); + g.set_attr("w", AttrValue::String("two".into())); + b.add_group(g.finish()); + let path = write(&dir, "attrs.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([list(f.attrs), int(f.attrs['v']), list(f['g'].attrs),\n\ + \x20 f['g'].attrs['w'].decode()]))", + ); + assert_eq!(out, r#"[["v"], 2, ["w"], "two"]"#); + let f = File::open(&path).unwrap(); + assert!(matches!(f.root().attrs().unwrap()["v"], AttrValue::I64(2))); +} From 81a0e8685db41080442dc4c4e4c4a73b92783940 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:57:01 -0500 Subject: [PATCH 05/10] fix(format): write child indirect blocks in big fractal heaps Dense link and attribute storage keeps its messages in a fractal heap. Its root indirect block holds direct blocks up to 64 KiB, 512 KiB in all; rows past that are child indirect blocks. The writer kept adding rows of direct blocks instead, and libhdf5 and h5rs read them as indirect blocks: a group with 20 000 links of 20-byte names was written without error and could not be listed ("incorrect metadata checksum"), and 150 dense attributes of up to 56 KB could not be opened. The heap writer now follows the doubling table: rows past the direct ones hold child indirect blocks, each with its own rows, nested as deep as the heap needs. Two more heap bugs are fixed on the way. An object bigger than the next block's free space was written into it anyway and cut off; the block is now left unallocated and the object goes in the first block big enough, as libhdf5 skips blocks. And the header's next-block offset was 0, so libhdf5 adding a link to such a group overwrote the heap's first block ("bad version number for message"); it is now the offset after the last block. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/file_writer.rs | 446 ++++++++++++------ crates/clawhdf5-tools/tests/h5rs_interop.rs | 31 ++ .../clawhdf5/tests/writer_groups_interop.rs | 127 +++++ 3 files changed, 467 insertions(+), 137 deletions(-) diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 2d318f5..1574dc1 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -307,7 +307,7 @@ pub(crate) fn build_single_block_fractal_heap( base_address: u64, max_heap_size: u16, heap_id_length: u16, -) -> FractalHeapBlock { +) -> Result { let os = OFFSET_SIZE as usize; let ls = LENGTH_SIZE as usize; let block_offset_bytes = (max_heap_size as usize).div_ceil(8); @@ -435,183 +435,346 @@ pub(crate) fn build_single_block_fractal_heap( blob.extend_from_slice(&frhp); blob.extend_from_slice(&dblock); - FractalHeapBlock { + Ok(FractalHeapBlock { blob, frhp_addr, btree_addr, heap_ids, heap_id_length, - } + }) } -/// Build a multi-block fractal heap: a root indirect block (FHIB) over multiple -/// direct blocks sized by the doubling table. Used when the objects don't fit -/// in a single direct block. Objects do not span blocks (no huge-object path). +/// Build a multi-block fractal heap: a root indirect block (FHIB) over direct +/// blocks sized by the doubling table. Used when the objects don't fit in a +/// single direct block. +/// +/// Rows of the doubling table whose block size exceeds the maximum direct +/// block size hold child indirect blocks, as the HDF5 spec (and libhdf5) +/// reads them: a child in row `r` spans that row's block size of heap space +/// and has `log2(size) - log2(start * width) + 1` rows of its own, which may +/// in turn hold indirect blocks. Objects are packed into direct blocks in +/// heap-offset order and never span blocks; a block too small for the next +/// object is left unallocated (an undefined address), as libhdf5 skips rows +/// when it needs a bigger block. There is no huge-object path. fn build_multiblock_fractal_heap( serialized: &[Vec], base_address: u64, max_heap_size: u16, heap_id_length: u16, -) -> FractalHeapBlock { +) -> Result { let os = OFFSET_SIZE as usize; let block_offset_bytes = (max_heap_size as usize).div_ceil(8); - let max_direct_block_size: u64 = 65536; - let table_width: u16 = 4; - let starting_block_size: u64 = 512; - let dblock_header_size = 4 + 1 + os + block_offset_bytes + 4; - let block_capacity = - |row: usize| block_size_for_row(starting_block_size, row) - dblock_header_size as u64; + let geom = HeapGeometry { + width: 4, + starting_block_size: 512, + max_direct_block_size: 65536, + dblock_header_size: 4 + 1 + os + block_offset_bytes + 4, + iblock_fixed_size: 5 + os + block_offset_bytes + 4, + max_heap_size, + }; - // ---- Pack objects into direct blocks (row-major over the doubling table) ---- - struct Blk { - row: usize, - size: u64, - heap_offset: u64, - data: Vec, - } - let mut blocks: Vec = Vec::new(); - // Each object's (heap_offset, length) for the heap ID. - let mut obj_loc: Vec<(u64, u64)> = vec![(0, 0); serialized.len()]; - - let mut row = 0usize; - let mut col = 0u16; - let mut heap_off = 0u64; - let mut cur: Option = None; - - for (idx, s) in serialized.iter().enumerate() { - loop { - if cur.is_none() { - let size = block_size_for_row(starting_block_size, row); - cur = Some(Blk { - row, - size, - heap_offset: heap_off, - data: Vec::new(), - }); - } - let blk = cur.as_mut().unwrap(); - let cap = block_capacity(blk.row) as usize; - if !blk.data.is_empty() && blk.data.len() + s.len() > cap { - // Doesn't fit; finalize this block and advance to the next slot. - let finished = cur.take().unwrap(); - heap_off += finished.size; - blocks.push(finished); - col += 1; - if col >= table_width { - col = 0; - row += 1; - } - continue; - } - // Place the object (a fresh block always accepts at least one object - // up to its capacity; objects larger than a max block are unsupported). - let pos_in_block = dblock_header_size + blk.data.len(); - obj_loc[idx] = (blk.heap_offset + pos_in_block as u64, s.len() as u64); - blk.data.extend_from_slice(s); - break; - } - } - if let Some(b) = cur.take() { - blocks.push(b); - } - - let cur_rows = (blocks.last().map(|b| b.row).unwrap_or(0) + 1) as u16; + // ---- Pack objects into the doubling table ---- + let mut packer = HeapPacker { + geom: &geom, + objects: serialized, + next: 0, + blocks: Vec::new(), + obj_loc: vec![(0, 0); serialized.len()], + }; + let root = packer.fill(0, None)?; + let HeapPacker { + blocks, obj_loc, .. + } = packer; // ---- Addresses ---- let frhp_size = frhp_header_size(os, LENGTH_SIZE as usize); let frhp_addr = base_address; let fhib_addr = frhp_addr + frhp_size as u64; - let fhib_entries = cur_rows as usize * table_width as usize; - let fhib_size = 5 + os + block_offset_bytes + fhib_entries * os + 4; - let first_dblock_addr = fhib_addr + fhib_size as u64; + let heap_len = root.subtree_size(&geom, &blocks); + let btree_addr = fhib_addr + heap_len; - // Assign each used block an address (laid out consecutively after the FHIB). - let mut blk_addrs: Vec = Vec::with_capacity(blocks.len()); - let mut a = first_dblock_addr; - for b in &blocks { - blk_addrs.push(a); - a += b.size; - } - let heap_end = a; - let btree_addr = heap_end; - - // Bookkeeping totals. - let managed_space: u64 = (0..cur_rows as usize) - .map(|r| block_size_for_row(starting_block_size, r) * table_width as u64) - .sum(); + // Bookkeeping totals, as libhdf5 keeps them: the managed space is what + // the root's rows span, the allocated space the direct blocks written, + // and the allocation iterator the heap offset after the last of them. + let cur_rows = root.nrows as u16; + let managed_space: u64 = (0..root.nrows).map(|r| geom.row_size(r) * geom.width).sum(); let alloc_space: u64 = blocks.iter().map(|b| b.size).sum(); let used: u64 = blocks .iter() - .map(|b| dblock_header_size as u64 + b.data.len() as u64) + .map(|b| geom.dblock_header_size as u64 + b.data.len() as u64) .sum(); let free_space = alloc_space.saturating_sub(used); + let alloc_iter = blocks.last().map_or(0, |b| b.heap_offset + b.size); // ---- FRHP header ---- - let max_managed = max_direct_block_size as u32 - dblock_header_size as u32; + let max_managed = geom.max_managed(); let frhp = write_frhp(WriteFrhp { heap_id_length, max_managed, free_space, managed_space, alloc_space, + alloc_iter, nobjects: serialized.len() as u64, - table_width, - starting_block_size, - max_direct_block_size, + table_width: geom.width as u16, + starting_block_size: geom.starting_block_size, + max_direct_block_size: geom.max_direct_block_size, max_heap_size, root_addr: fhib_addr, cur_rows, }); debug_assert_eq!(frhp.len(), frhp_size); - // ---- Root indirect block (FHIB) ---- - let mut fhib = Vec::with_capacity(fhib_size); - fhib.extend_from_slice(b"FHIB"); - fhib.push(0); // version - write_offset(&mut fhib, frhp_addr, OFFSET_SIZE); - fhib.extend_from_slice(&vec![0u8; block_offset_bytes]); // block offset = 0 (root) - for &addr in &blk_addrs { - write_offset(&mut fhib, addr, OFFSET_SIZE); - } - // Remaining slots within the current rows are unallocated. - for _ in blk_addrs.len()..fhib_entries { - write_undef_offset(&mut fhib, OFFSET_SIZE); - } - let fhib_checksum = crate::checksum::jenkins_lookup3(&fhib); - fhib.extend_from_slice(&fhib_checksum.to_le_bytes()); - debug_assert_eq!(fhib.len(), fhib_size); - - // ---- Direct blocks ---- + // ---- Indirect and direct blocks, depth first after the root ---- let mut blob = frhp; - blob.extend_from_slice(&fhib); - for b in &blocks { - let mut dblock = Vec::with_capacity(b.size as usize); - dblock.extend_from_slice(b"FHDB"); - dblock.push(0); // version - write_offset(&mut dblock, frhp_addr, OFFSET_SIZE); - let mut bo = b.heap_offset.to_le_bytes().to_vec(); - bo.truncate(block_offset_bytes); - dblock.extend_from_slice(&bo); - let cksum_pos = dblock.len(); - dblock.extend_from_slice(&[0u8; 4]); // checksum placeholder - dblock.extend_from_slice(&b.data); - dblock.resize(b.size as usize, 0); - let cksum = crate::checksum::jenkins_lookup3(&dblock); - dblock[cksum_pos..cksum_pos + 4].copy_from_slice(&cksum.to_le_bytes()); - blob.extend_from_slice(&dblock); - } + root.emit(&geom, &blocks, frhp_addr, fhib_addr, &mut blob); + debug_assert_eq!(blob.len() as u64, frhp_size as u64 + heap_len); let heap_ids: Vec> = obj_loc .iter() .map(|(off, len)| encode_managed_id(*off, *len, max_heap_size, heap_id_length)) .collect(); - FractalHeapBlock { + Ok(FractalHeapBlock { blob, frhp_addr, btree_addr, heap_ids, heap_id_length, + }) +} + +/// The doubling table of a heap the writer builds. +struct HeapGeometry { + width: u64, + starting_block_size: u64, + max_direct_block_size: u64, + dblock_header_size: usize, + /// An indirect block's size without its child entries. + iblock_fixed_size: usize, + max_heap_size: u16, +} + +impl HeapGeometry { + fn row_size(&self, row: usize) -> u64 { + block_size_for_row(self.starting_block_size, row) + } + + /// Rows holding direct blocks: `log2(max_direct / start) + 2`. + fn max_direct_rows(&self) -> usize { + (self.max_direct_block_size / self.starting_block_size).ilog2() as usize + 2 + } + + /// `log2(start * width)`, libhdf5's `first_row_bits`. + fn first_row_bits(&self) -> u32 { + (self.starting_block_size * self.width).ilog2() + } + + /// Rows of an indirect block spanning `size` bytes of heap space + /// (libhdf5's `H5HF__dtable_size_to_rows`). + fn rows_for_size(&self, size: u64) -> usize { + (size.ilog2() - self.first_row_bits() + 1) as usize + } + + /// Rows the root indirect block can have: enough to span the heap's + /// whole `2^max_heap_size` address space. + fn max_root_rows(&self) -> usize { + (u32::from(self.max_heap_size) - self.first_row_bits() + 1) as usize + } + + /// The largest object a direct block holds. + fn max_managed(&self) -> u32 { + (self.max_direct_block_size - self.dblock_header_size as u64) as u32 + } +} + +/// A direct block the packer filled. +struct HeapDirectBlock { + size: u64, + heap_offset: u64, + data: Vec, +} + +/// One entry of an indirect block. +enum HeapSlot { + /// Not allocated (undefined address). + Empty, + /// Index into the packer's direct blocks. + Direct(usize), + Indirect(HeapIndirectBlock), +} + +struct HeapIndirectBlock { + heap_offset: u64, + nrows: usize, + /// `nrows * width` entries, row-major. + slots: Vec, +} + +impl HeapIndirectBlock { + fn own_size(&self, geom: &HeapGeometry) -> u64 { + (geom.iblock_fixed_size + self.slots.len() * OFFSET_SIZE as usize) as u64 + } + + /// Bytes of this block and everything below it. + fn subtree_size(&self, geom: &HeapGeometry, blocks: &[HeapDirectBlock]) -> u64 { + self.own_size(geom) + + self + .slots + .iter() + .map(|s| match s { + HeapSlot::Empty => 0, + HeapSlot::Direct(i) => blocks[*i].size, + HeapSlot::Indirect(ib) => ib.subtree_size(geom, blocks), + }) + .sum::() + } + + /// Append this block at `addr` (= `out`'s current end, relative to the + /// same base as `frhp_addr`), then its children in entry order. + fn emit( + &self, + geom: &HeapGeometry, + blocks: &[HeapDirectBlock], + frhp_addr: u64, + addr: u64, + out: &mut Vec, + ) { + let block_offset_bytes = (geom.max_heap_size as usize).div_ceil(8); + let start = out.len(); + out.extend_from_slice(b"FHIB"); + out.push(0); // version + write_offset(out, frhp_addr, OFFSET_SIZE); + out.extend_from_slice(&self.heap_offset.to_le_bytes()[..block_offset_bytes]); + let mut child = addr + self.own_size(geom); + for s in &self.slots { + match s { + HeapSlot::Empty => write_undef_offset(out, OFFSET_SIZE), + HeapSlot::Direct(i) => { + write_offset(out, child, OFFSET_SIZE); + child += blocks[*i].size; + } + HeapSlot::Indirect(ib) => { + write_offset(out, child, OFFSET_SIZE); + child += ib.subtree_size(geom, blocks); + } + } + } + let checksum = crate::checksum::jenkins_lookup3(&out[start..]); + out.extend_from_slice(&checksum.to_le_bytes()); + + let mut child = addr + self.own_size(geom); + for s in &self.slots { + match s { + HeapSlot::Empty => {} + HeapSlot::Direct(i) => { + let b = &blocks[*i]; + let d = out.len(); + out.extend_from_slice(b"FHDB"); + out.push(0); // version + write_offset(out, frhp_addr, OFFSET_SIZE); + out.extend_from_slice(&b.heap_offset.to_le_bytes()[..block_offset_bytes]); + let cksum_pos = out.len(); + out.extend_from_slice(&[0u8; 4]); // checksum placeholder + out.extend_from_slice(&b.data); + out.resize(d + b.size as usize, 0); + let cksum = crate::checksum::jenkins_lookup3(&out[d..]); + out[cksum_pos..cksum_pos + 4].copy_from_slice(&cksum.to_le_bytes()); + child += b.size; + } + HeapSlot::Indirect(ib) => { + ib.emit(geom, blocks, frhp_addr, child, out); + child += ib.subtree_size(geom, blocks); + } + } + } + } +} + +/// Packs objects into a heap's doubling table in heap-offset order. +struct HeapPacker<'a> { + geom: &'a HeapGeometry, + objects: &'a [Vec], + /// The next object to place. + next: usize, + blocks: Vec, + /// Each object's (heap offset, length). + obj_loc: Vec<(u64, u64)>, +} + +impl HeapPacker<'_> { + /// Fill an indirect block at `heap_offset` with `nrows` rows, or, for the + /// root (`None`), with as many rows as the objects need. + fn fill( + &mut self, + heap_offset: u64, + nrows: Option, + ) -> Result { + let geom = self.geom; + let width = geom.width as usize; + let mut slots = Vec::new(); + let mut off = heap_offset; + let mut row = 0usize; + while self.next < self.objects.len() && nrows.is_none_or(|n| row < n) { + if nrows.is_none() && row >= geom.max_root_rows() { + return Err(FormatError::SerializationError(format!( + "fractal heap: {} objects do not fit its {}-bit address space", + self.objects.len(), + geom.max_heap_size + ))); + } + let size = geom.row_size(row); + for _ in 0..width { + if self.next == self.objects.len() { + slots.push(HeapSlot::Empty); + } else if row < geom.max_direct_rows() { + slots.push(self.fill_direct(off, size)); + } else { + let child = self.fill(off, Some(geom.rows_for_size(size)))?; + let used = child.slots.iter().any(|s| !matches!(s, HeapSlot::Empty)); + slots.push(if used { + HeapSlot::Indirect(child) + } else { + HeapSlot::Empty + }); + } + off += size; + } + row += 1; + } + let nrows = nrows.unwrap_or(row); + slots.resize_with(nrows * width, || HeapSlot::Empty); + Ok(HeapIndirectBlock { + heap_offset, + nrows, + slots, + }) + } + + /// Fill the direct block at `heap_offset` with as many of the next + /// objects as fit; leave it unallocated if not even the next one does. + fn fill_direct(&mut self, heap_offset: u64, size: u64) -> HeapSlot { + let header = self.geom.dblock_header_size; + let capacity = size as usize - header; + let mut data = Vec::new(); + while let Some(obj) = self.objects.get(self.next) { + if data.len() + obj.len() > capacity { + break; + } + self.obj_loc[self.next] = + (heap_offset + (header + data.len()) as u64, obj.len() as u64); + data.extend_from_slice(obj); + self.next += 1; + } + if data.is_empty() && self.objects.get(self.next).is_some_and(|o| !o.is_empty()) { + return HeapSlot::Empty; + } + self.blocks.push(HeapDirectBlock { + size, + heap_offset, + data, + }); + HeapSlot::Direct(self.blocks.len() - 1) } } @@ -661,6 +824,8 @@ struct WriteFrhp { free_space: u64, managed_space: u64, alloc_space: u64, + /// Heap offset of the next direct block to allocate. + alloc_iter: u64, nobjects: u64, table_width: u16, starting_block_size: u64, @@ -685,7 +850,7 @@ fn write_frhp(p: WriteFrhp) -> Vec { write_undef_offset(&mut frhp, OFFSET_SIZE); // free_space_mgr_addr write_length(&mut frhp, p.managed_space, LENGTH_SIZE); // managed_space_in_heap write_length(&mut frhp, p.alloc_space, LENGTH_SIZE); // allocated_managed_space - write_length(&mut frhp, 0, LENGTH_SIZE); // dblock_alloc_iter + write_length(&mut frhp, p.alloc_iter, LENGTH_SIZE); // dblock_alloc_iter write_length(&mut frhp, p.nobjects, LENGTH_SIZE); // managed_objects_count write_length(&mut frhp, 0, LENGTH_SIZE); // huge_objects_size write_length(&mut frhp, 0, LENGTH_SIZE); // huge_objects_count @@ -704,7 +869,10 @@ fn write_frhp(p: WriteFrhp) -> Vec { } /// Build dense attribute storage for a set of attributes. -pub(crate) fn build_dense_attrs(attrs: &[AttributeMessage], base_address: u64) -> DenseAttrBlob { +pub(crate) fn build_dense_attrs( + attrs: &[AttributeMessage], + base_address: u64, +) -> Result { // Dense attrs use v3 attribute messages (adds character set encoding byte). let serialized: Vec> = attrs.iter().map(|a| a.serialize_v3(LENGTH_SIZE)).collect(); @@ -717,7 +885,7 @@ pub(crate) fn build_dense_attrs(attrs: &[AttributeMessage], base_address: u64) - let ls = LENGTH_SIZE as usize; // Attribute heaps use max_heap_size 40 / heap ID length 8 (matching libhdf5). - let heap = build_single_block_fractal_heap(&serialized, base_address, 40, 8); + let heap = build_single_block_fractal_heap(&serialized, base_address, 40, 8)?; let frhp_addr = heap.frhp_addr; let btree_addr = heap.btree_addr; let heap_id_length = heap.heap_id_length; @@ -781,10 +949,10 @@ pub(crate) fn build_dense_attrs(attrs: &[AttributeMessage], base_address: u64) - let attr_info = serialize_attribute_info(frhp_addr, bthd_addr); - DenseAttrBlob { + Ok(DenseAttrBlob { attr_info_message: attr_info, blob, - } + }) } // ---- Dense link blob ---- @@ -873,7 +1041,7 @@ pub(crate) fn build_dense_links( // libhdf5's link heap uses max_heap_size 32 / heap ID length 7 (vs 40/8 for // attributes), giving a 7-byte heap ID and an 11-byte type-5 record. - let heap = build_single_block_fractal_heap(&serialized, base_address, 32, 7); + let heap = build_single_block_fractal_heap(&serialized, base_address, 32, 7)?; let heap_id_length = heap.heap_id_length; // Type 5 records: hash(4) + heap_id. The B-tree's key is the name hash, @@ -1405,7 +1573,9 @@ impl FileWriter { .enumerate() .map(|(gi, g)| { let dummy_links = g.link_messages(&[], &[]); - let attr_blob = group_dense[gi].then(|| build_dense_attrs(&g.attrs, 0)); + let attr_blob = group_dense[gi] + .then(|| build_dense_attrs(&g.attrs, 0)) + .transpose()?; let li = if group_links_dense[gi] { serialize_link_info( g.track_order.then_some(0), @@ -1439,7 +1609,9 @@ impl FileWriter { let mut dummy_blobs: Vec = Vec::new(); let mut dummy_cursor = 0u64; for (i, d) in all_ds.iter().enumerate() { - let dense_blob = ds_dense[i].then(|| build_dense_attrs(&d.attrs, 0)); + let dense_blob = ds_dense[i] + .then(|| build_dense_attrs(&d.attrs, 0)) + .transpose()?; if is_vds[i] { // VDS: dummy OH with address 0 to get the OH size. The global // heap blob will be placed after the OHs in pass 2. @@ -1563,7 +1735,7 @@ impl FileWriter { group_link_blob_addrs.push(None); } if group_dense[gi] { - let blob = build_dense_attrs(&g.attrs, cursor2 as u64); + let blob = build_dense_attrs(&g.attrs, cursor2 as u64)?; cursor2 += blob.blob.len(); group_dense_blobs.push(Some(blob)); } else { @@ -1580,15 +1752,15 @@ impl FileWriter { let addr = cursor2 as u64; cursor2 += sz; if ds_dense[i] { - let blob = build_dense_attrs(&all_ds[i].attrs, cursor2 as u64); + let blob = build_dense_attrs(&all_ds[i].attrs, cursor2 as u64)?; cursor2 += blob.blob.len(); ds_dense_blobs.push(Some(blob)); } else { ds_dense_blobs.push(None); } - addr + Ok(addr) }) - .collect(); + .collect::>()?; let mut ds_blobs2: Vec = Vec::new(); let global_align_threshold = self.alignment_threshold; diff --git a/crates/clawhdf5-tools/tests/h5rs_interop.rs b/crates/clawhdf5-tools/tests/h5rs_interop.rs index 519bc6d..8d5bd44 100644 --- a/crates/clawhdf5-tools/tests/h5rs_interop.rs +++ b/crates/clawhdf5-tools/tests/h5rs_interop.rs @@ -858,3 +858,34 @@ fn check_and_dump_files_with_nested_groups_and_links() { assert_eq!(stdout(&ours), r, "{name}"); } } + +#[test] +fn check_files_with_big_dense_storage() { + // Dense links and attributes past the 512 KiB the root indirect block's + // direct blocks hold: the heap then needs child indirect blocks, which + // the writer used to write as direct blocks ("fractal heap indirect + // block: bad signature"). + use clawhdf5::{AttrValue, FileBuilder}; + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + let x = b.create_dataset("x"); + x.with_i32_data(&[7]); + for i in 0..150usize { + let len = if i % 3 == 0 { 7_000 } else { 1 + i }; + x.set_attr( + &format!("a{i:03}"), + AttrValue::F64Array(vec![i as f64; len]), + ); + } + let mut g = b.create_group("g"); + for i in 0..40_000 { + g.add_hard_link(&format!("link_{i:06}_{}", "x".repeat(88)), "/x"); + } + b.add_group(g.finish()); + let p = dir.path().join("big.h5").to_string_lossy().into_owned(); + b.write(&p).unwrap(); + // Structure only: `--data` looks every link up by a linear scan. + let o = h5rs(&["check", &p]); + assert_eq!(code(&o), 0, "{p}:\n{}", stdout(&o)); + assert!(stdout(&o).contains("no problems found"), "{}", stdout(&o)); +} diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index ba2d121..3b8727d 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -638,3 +638,130 @@ fn a_group_attribute_set_again_takes_the_new_value() { let f = File::open(&path).unwrap(); assert!(matches!(f.root().attrs().unwrap()["v"], AttrValue::I64(2))); } + +// ---- big dense storage: child indirect blocks in the fractal heap ---- + +/// A name `len` bytes long, unique per `i`. +fn long_name(i: usize, len: usize) -> String { + let n = format!("link_{i:06}_"); + format!("{n}{}", "x".repeat(len - n.len())) +} + +#[test] +fn dense_links_past_the_direct_blocks_of_the_root() { + skip_if_no_python!(); + // A dense group's links live in a fractal heap whose root indirect + // block holds direct blocks up to 64 KiB: 512 KiB of link messages. + // Rows past that are child indirect blocks. The writer used to write + // them as direct blocks, which libhdf5 cannot read ("incorrect metadata + // checksum"), from about 17 000 links with 20-byte names. + // `g` crosses the first boundary (0.6 MB of links); `deep` has 65 535 + // links of about 110 bytes (7 MB), so its heap reaches the child indirect + // blocks that hold indirect blocks themselves. + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.create_dataset("x").with_i32_data(&[7]); + let mut g = b.create_group("g"); + for i in 0..20_000 { + g.create_dataset(&format!("dataset_number_{i:06}")) + .with_i32_data(&[i]); + } + b.add_group(g.finish()); + let mut g = b.create_group("deep"); + g.track_order(true); + for i in 0..usize::from(u16::MAX) { + g.add_hard_link(&long_name(i, 100), "/x"); + } + b.add_group(g.finish()); + let path = write(&dir, "big_links.h5", b); + + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 g, d = f['g'], f['deep']\n\ + \x20 names = list(g)\n\ + \x20 dn = list(d)\n\ + \x20 print(json.dumps([len(names), names[-1], int(g[names[-1]][0]),\n\ + \x20 sum(int(g[n][0]) for n in names), len(dn), dn[0][:12], dn[-1][:12],\n\ + \x20 int(d[dn[-1]][0]), h5py.h5o.get_info(f['x'].id).rc]))", + ); + assert_eq!( + out, + r#"[20000, "dataset_number_019999", 19999, 199990000, 65535, "link_000000_", "link_065534_", 7, 65536]"# + ); + h5dump_ok(&path); + let f = File::open(&path).unwrap(); + let g = f.group("g").unwrap(); + assert_eq!(g.datasets().unwrap().len(), 20_000); + assert_eq!( + g.dataset("dataset_number_019999") + .unwrap() + .read_i32() + .unwrap(), + [19999] + ); + let d = f.group("deep").unwrap(); + assert_eq!(d.datasets().unwrap().len(), usize::from(u16::MAX)); + assert_eq!( + d.dataset(&long_name(65_534, 100)) + .unwrap() + .read_i32() + .unwrap(), + [7] + ); + + // libhdf5 can add to and delete from the heap. It could not when the + // header's block allocation offset was 0: its next block overwrote the + // first ("bad version number for message"). + let out = h5py( + &path, + "with h5py.File(path, 'r+') as f:\n\ + \x20 f['g']['zz_new'] = np.arange(3)\n\ + \x20 del f['g/dataset_number_000005']\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([len(f['g']), int(f['g/zz_new'][2]),\n\ + \x20 int(f['g/dataset_number_019998'][0])]))", + ); + assert_eq!(out, r#"[20000, 2, 19998]"#); + h5dump_ok(&path); +} + +#[test] +fn dense_attributes_past_the_direct_blocks_of_the_root() { + skip_if_no_python!(); + // Dense attributes share the heap writer. 150 attributes of up to 56 KB + // (8 MB) need child indirect blocks, and a big attribute after small + // ones must skip the small blocks rather than overrun one. + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + let ds = b.create_dataset("x"); + ds.with_i32_data(&[1]); + for i in 0..150usize { + let len = if i % 3 == 0 { 7_000 } else { 1 + i }; + let v: Vec = (0..len).map(|k| (i * 100_000 + k) as f64).collect(); + ds.set_attr(&format!("a{i:03}"), AttrValue::F64Array(v)); + } + let path = write(&dir, "big_attrs.h5", b); + + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 a = f['x'].attrs\n\ + \x20 ok = all(np.array_equal(a['a%03d' % i],\n\ + \x20 np.arange(7000 if i % 3 == 0 else 1 + i) + i * 100000) for i in range(150))\n\ + \x20 print(json.dumps([len(a), ok]))", + ); + assert_eq!(out, "[150, true]"); + h5dump_ok(&path); + let f = File::open(&path).unwrap(); + let attrs = f.dataset("x").unwrap().attrs().unwrap(); + assert_eq!(attrs.len(), 150); + for i in [0usize, 1, 147, 149] { + let len = if i % 3 == 0 { 7_000 } else { 1 + i }; + let want: Vec = (0..len).map(|k| (i * 100_000 + k) as f64).collect(); + match &attrs[&format!("a{i:03}")] { + AttrValue::F64Array(v) => assert_eq!(*v, want, "a{i:03}"), + other => panic!("a{i:03}: {other:?}"), + } + } +} From 751edeb7e6fa9edcbcea6e23c0906459176aec28 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:57:52 -0500 Subject: [PATCH 06/10] fix(format): refuse a dense link or attribute too big for the heap A message in dense storage is a fractal heap object, and an object must fit one direct block: 65 515 bytes here, since the writer has no huge-object path. A bigger one (a soft link with a 80 000-byte target in a group of more than 8 links) was written without error, cut off at the end of its block, and libhdf5 could not list the group ("object overruns end of direct block"). finish() now fails with an error that names the limit, for links and for dense attributes; a 65 001-byte soft link target still works and h5py reads it back. The heap packer also skips a child indirect block whose blocks are all too small for the next object instead of walking it. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/file_writer.rs | 30 +++++++++++-- .../clawhdf5/tests/writer_groups_interop.rs | 42 +++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 1574dc1..65181a0 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -316,6 +316,18 @@ pub(crate) fn build_single_block_fractal_heap( // Direct block layout: sig(4) + ver(1) + heap_addr(os) + block_offset(bo_bytes) // + checksum(4) [when flags bit 1 set] + data... let dblock_header_size = 4 + 1 + os + block_offset_bytes + 4; // +4 for checksum + + // An object must fit one direct block: the writer has no huge-object + // path, and libhdf5 cannot read an object that overruns its block. + let max_managed = max_direct_block_size as usize - dblock_header_size; + if let Some(big) = serialized.iter().find(|s| s.len() > max_managed) { + return Err(FormatError::SerializationError(format!( + "a {}-byte message cannot go in dense storage: a fractal heap \ + object holds at most {max_managed} bytes (huge heap objects are \ + not written)", + big.len() + ))); + } let total_data_size: usize = serialized.iter().map(|s| s.len()).sum(); let dblock_content_size = dblock_header_size + total_data_size; let starting_block_size = dblock_content_size.next_power_of_two().max(512) as u64; @@ -373,8 +385,7 @@ pub(crate) fn build_single_block_fractal_heap( frhp.extend_from_slice(&heap_id_length.to_le_bytes()); frhp.extend_from_slice(&0u16.to_le_bytes()); // io_filter_encoded_length frhp.push(0x02); // flags: bit 1 = checksum direct blocks - let max_managed = max_direct_block_size as u32 - dblock_header_size as u32; - frhp.extend_from_slice(&max_managed.to_le_bytes()); + frhp.extend_from_slice(&(max_managed as u32).to_le_bytes()); write_length(&mut frhp, 0, LENGTH_SIZE); // next_huge_object_id write_undef_offset(&mut frhp, OFFSET_SIZE); // btree_huge_objects_address write_length(&mut frhp, free_space as u64, LENGTH_SIZE); // free_space_managed_blocks @@ -455,7 +466,8 @@ pub(crate) fn build_single_block_fractal_heap( /// in turn hold indirect blocks. Objects are packed into direct blocks in /// heap-offset order and never span blocks; a block too small for the next /// object is left unallocated (an undefined address), as libhdf5 skips rows -/// when it needs a bigger block. There is no huge-object path. +/// when it needs a bigger block. The caller has checked that every object +/// fits a maximum-size direct block (there is no huge-object path). fn build_multiblock_fractal_heap( serialized: &[Vec], base_address: u64, @@ -730,7 +742,17 @@ impl HeapPacker<'_> { } else if row < geom.max_direct_rows() { slots.push(self.fill_direct(off, size)); } else { - let child = self.fill(off, Some(geom.rows_for_size(size)))?; + let child_rows = geom.rows_for_size(size); + // A child whose biggest direct block cannot hold the + // next object is skipped whole, not walked. + let biggest = geom.row_size(child_rows.min(geom.max_direct_rows()) - 1); + if self.objects[self.next].len() > (biggest as usize - geom.dblock_header_size) + { + slots.push(HeapSlot::Empty); + off += size; + continue; + } + let child = self.fill(off, Some(child_rows))?; let used = child.slots.iter().any(|s| !matches!(s, HeapSlot::Empty)); slots.push(if used { HeapSlot::Indirect(child) diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 3b8727d..770b835 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -765,3 +765,45 @@ fn dense_attributes_past_the_direct_blocks_of_the_root() { } } } + +#[test] +fn a_link_too_big_for_dense_storage_is_an_error() { + // A link message must fit one fractal heap direct block (64 KiB less + // its header); the writer has no huge-object path. It used to be + // written anyway, cut off, and libhdf5 could not list the group. + let mut b = FileBuilder::new(); + for i in 0..10 { + b.create_dataset(&format!("d{i}")).with_i32_data(&[i]); + } + b.add_soft_link("s", &"/y".repeat(40_000)); + let err = b.finish().unwrap_err().to_string(); + assert!(err.contains("fractal heap object holds at most"), "{err}"); + // The same for a dense attribute. + let mut b = FileBuilder::new(); + let x = b.create_dataset("x"); + x.with_i32_data(&[1]); + for i in 0..9 { + x.set_attr(&format!("a{i}"), AttrValue::I64(i)); + } + x.set_attr("big", AttrValue::F64Array(vec![0.5; 9_000])); + let err = b.finish().unwrap_err().to_string(); + assert!(err.contains("fractal heap object holds at most"), "{err}"); + + // Just under the limit is fine, and libhdf5 reads it back. + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + for i in 0..10 { + b.create_dataset(&format!("d{i}")).with_i32_data(&[i]); + } + let target = format!("/{}", "y".repeat(65_000)); + b.add_soft_link("s", &target); + let path = write(&dir, "long_soft.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([len(f), len(f.get('s', getlink=True).path)]))", + ); + assert_eq!(out, "[11, 65001]"); + h5dump_ok(&path); +} From bd1d8f1a593d8d6e5842f9e8dffdfb981f45f245 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:58:49 -0500 Subject: [PATCH 07/10] fix(format): keep a dense index leaf within 65 535 records The link and attribute name indexes are one v2 B-tree leaf, sized to the next power of two. libhdf5 takes a leaf's capacity from that node size, but a leaf's record count is a 2-byte field. From about 47 700 links the node had room for more than 65 535 records, so adding a link in h5py overflowed the count: a group of 65 535 links crashed h5py, or could no longer be listed ("unknown link class"). The node is now capped at a full leaf of 65 535 records, so libhdf5 splits it instead. Dense attributes now go through the same index builder. Their record count was written modulo 65 536, without error; more than 65 535 attributes on one object are now refused, like links. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/file_writer.rs | 71 ++++++------------- .../clawhdf5/tests/writer_groups_interop.rs | 30 ++++++-- 2 files changed, 48 insertions(+), 53 deletions(-) diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 65181a0..0b318a9 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -903,9 +903,6 @@ pub(crate) fn build_dense_attrs( .map(|a| crate::checksum::jenkins_lookup3(a.name.as_bytes())) .collect(); - let os = OFFSET_SIZE as usize; - let ls = LENGTH_SIZE as usize; - // Attribute heaps use max_heap_size 40 / heap ID length 8 (matching libhdf5). let heap = build_single_block_fractal_heap(&serialized, base_address, 40, 8)?; let frhp_addr = heap.frhp_addr; @@ -926,48 +923,16 @@ pub(crate) fn build_dense_attrs( } records.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.cmp(&b.1))); - let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4; - let num_records = attrs.len(); - let btlf_size = 4 + 1 + 1 + (num_records * record_size as usize) + 4; - let node_size = btlf_size.next_power_of_two().max(512) as u32; - + let records: Vec> = records.into_iter().map(|(_, _, rec)| rec).collect(); let bthd_addr = btree_addr; - let btlf_addr = bthd_addr + bthd_size as u64; - - let mut bthd = Vec::with_capacity(bthd_size); - bthd.extend_from_slice(b"BTHD"); - bthd.push(0); // version - bthd.push(8); // type = attribute name index - bthd.extend_from_slice(&node_size.to_le_bytes()); - bthd.extend_from_slice(&record_size.to_le_bytes()); - bthd.extend_from_slice(&0u16.to_le_bytes()); // depth = 0 - bthd.push(100); // split_percent - bthd.push(40); // merge_percent - write_offset(&mut bthd, btlf_addr, OFFSET_SIZE); - bthd.extend_from_slice(&(num_records as u16).to_le_bytes()); - write_length(&mut bthd, num_records as u64, LENGTH_SIZE); - let bthd_checksum = crate::checksum::jenkins_lookup3(&bthd); - bthd.extend_from_slice(&bthd_checksum.to_le_bytes()); - debug_assert_eq!(bthd.len(), bthd_size); - - let mut btlf = Vec::with_capacity(node_size as usize); - btlf.extend_from_slice(b"BTLF"); - btlf.push(0); // version - btlf.push(8); // type - for (_, _, rec) in &records { - btlf.extend_from_slice(rec); - } - // Checksum goes immediately after records (NOT at end of node). - // HDF5 C library computes checksum over sig+ver+type+records only. - let btlf_checksum = crate::checksum::jenkins_lookup3(&btlf); - btlf.extend_from_slice(&btlf_checksum.to_le_bytes()); - // Pad to node_size - btlf.resize(node_size as usize, 0); - - let mut blob = Vec::with_capacity(heap.blob.len() + bthd.len() + btlf.len()); - blob.extend_from_slice(&heap.blob); - blob.extend_from_slice(&bthd); - blob.extend_from_slice(&btlf); + let mut blob = heap.blob; + blob.extend_from_slice(&single_leaf_v2_btree( + 8, + record_size, + &records, + bthd_addr, + "attributes on one object", + )?); let attr_info = serialize_attribute_info(frhp_addr, bthd_addr); @@ -989,12 +954,14 @@ pub(crate) struct DenseLinkBlob { } /// A v2 B-tree of `btree_type` holding `records` (already in key order) in a -/// single leaf, laid out at `addr`: the header, then the leaf. +/// single leaf, laid out at `addr`: the header, then the leaf. `what` names +/// the records in the error for too many ("links in one group"). fn single_leaf_v2_btree( btree_type: u8, record_size: u16, records: &[Vec], addr: u64, + what: &str, ) -> Result, FormatError> { let os = OFFSET_SIZE as usize; let ls = LENGTH_SIZE as usize; @@ -1002,15 +969,21 @@ fn single_leaf_v2_btree( // internal nodes, which the writer does not build. let num_records = u16::try_from(records.len()).map_err(|_| { FormatError::SerializationError(format!( - "{} links in one group: a group holds at most {} links \ - (a deeper link index is not implemented)", + "{} {what}: at most {} can be written \ + (a deeper B-tree index is not implemented)", records.len(), u16::MAX )) })?; let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4; let btlf_size = 4 + 1 + 1 + (records.len() * record_size as usize) + 4; - let node_size = btlf_size.next_power_of_two().max(512) as u32; + // libhdf5 sizes a leaf's capacity from the node size, and a leaf's + // record count is a 2-byte field: a node with room for more than + // 65 535 records makes it overflow that count when it adds one (the + // group can then no longer be listed). Cap the node at a full leaf. + let max_node = btlf_size - records.len() * record_size as usize + + usize::from(u16::MAX) * record_size as usize; + let node_size = btlf_size.next_power_of_two().max(512).min(max_node) as u32; let btlf_addr = addr + bthd_size as u64; let mut out = Vec::with_capacity(bthd_size + node_size as usize); @@ -1089,6 +1062,7 @@ pub(crate) fn build_dense_links( 4 + heap_id_length, &name_records, name_bt_addr, + "links in one group", )?); let link_info_message = if track_order { @@ -1113,6 +1087,7 @@ pub(crate) fn build_dense_links( 8 + heap_id_length, &order_records, order_bt_addr, + "links in one group", )?); let next_order = by_order.last().map_or(0, |&(o, _)| o + 1); serialize_link_info( diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 770b835..27379bb 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -538,7 +538,23 @@ fn more_links_than_one_index_leaf_holds_is_an_error() { b.add_soft_link(&format!("s{i}"), "/x"); } let err = b.finish().unwrap_err().to_string(); - assert!(err.contains("at most 65535 links"), "{err}"); + assert!( + err.contains("70000 links in one group: at most 65535"), + "{err}" + ); + // Dense attributes have the same one-leaf index. Their count used to + // be written modulo 65 536. + let mut b = FileBuilder::new(); + let x = b.create_dataset("x"); + x.with_i32_data(&[1]); + for i in 0..70_000 { + x.set_attr(&format!("a{i}"), AttrValue::I64(i)); + } + let err = b.finish().unwrap_err().to_string(); + assert!( + err.contains("70000 attributes on one object: at most 65535"), + "{err}" + ); } #[test] @@ -712,17 +728,21 @@ fn dense_links_past_the_direct_blocks_of_the_root() { // libhdf5 can add to and delete from the heap. It could not when the // header's block allocation offset was 0: its next block overwrote the - // first ("bad version number for message"). + // first ("bad version number for message"). Adding to `deep` also + // needs its index's leaf node to have room for at most 65 535 records: + // a bigger node made libhdf5 overflow the leaf's 2-byte record count + // (a crash, or "unknown link class" when listing). let out = h5py( &path, "with h5py.File(path, 'r+') as f:\n\ \x20 f['g']['zz_new'] = np.arange(3)\n\ + \x20 f['deep']['zz_new'] = np.arange(4)\n\ \x20 del f['g/dataset_number_000005']\n\ with h5py.File(path, 'r') as f:\n\ - \x20 print(json.dumps([len(f['g']), int(f['g/zz_new'][2]),\n\ - \x20 int(f['g/dataset_number_019998'][0])]))", + \x20 print(json.dumps([len(f['g']), int(f['g/zz_new'][2]), len(f['deep']),\n\ + \x20 list(f['deep'])[-1], int(f['g/dataset_number_019998'][0])]))", ); - assert_eq!(out, r#"[20000, 2, 19998]"#); + assert_eq!(out, r#"[20000, 2, 65536, "zz_new", 19998]"#); h5dump_ok(&path); } From 400e3a9feca335f0877c88a1ada8ff9dd79a2ba1 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 09:01:13 -0500 Subject: [PATCH 08/10] fix(format): resolve each hard link once A hard link's target may go through other hard links, and each was resolved again every time a path went through it. With each link's target naming the previous link twice (g/s{i} -> /g/s{i-1}/s{i-1}) the work doubled per link: finish() took 46 s for 26 links in a debug build, and 60 would never finish. Resolved links are now remembered, so the work is linear in the links, and a hard link met again while it is being resolved is reported as a cycle by name. The depth limit (64) still bounds the recursion through links not yet resolved. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/writer_tree.rs | 62 ++++++++++++++++--- .../clawhdf5/tests/writer_groups_interop.rs | 49 +++++++++++++++ 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/crates/clawhdf5-format/src/writer_tree.rs b/crates/clawhdf5-format/src/writer_tree.rs index 724ba70..275b73d 100644 --- a/crates/clawhdf5-format/src/writer_tree.rs +++ b/crates/clawhdf5-format/src/writer_tree.rs @@ -17,8 +17,8 @@ use std::collections::BTreeMap; use crate::error::FormatError; use crate::type_builders::{AttrValue, DatasetBuilder, GroupBuilder, GroupItem}; -/// Hard links followed while resolving one hard-link target path. Guards -/// against hard links whose targets name each other. +/// Depth of the chain of unresolved hard links followed while resolving one +/// hard-link target path (a bound on recursion; cycles are found exactly). const MAX_LINK_DEPTH: usize = 64; fn err(msg: String) -> FormatError { @@ -256,10 +256,22 @@ impl Builder { } /// The object a hard link's `target` path names, from group `from`. - fn resolve(&self, from: usize, target: &str, depth: usize) -> Result { + /// + /// Hard links met on the way are resolved once and remembered in + /// `memo` (by group and link index), so a target that goes through + /// other hard links costs time linear in the links, not exponential; a + /// hard link met again while it is being resolved is a cycle. + fn resolve( + &self, + memo: &mut [Vec], + from: usize, + target: &str, + depth: usize, + ) -> Result { if depth > MAX_LINK_DEPTH { return Err(err(format!( - "hard link target {target:?}: too many hard links to follow (a cycle?)" + "hard link target {target:?}: more than {MAX_LINK_DEPTH} hard links \ + to follow" ))); } let (mut cur, rest) = match target.strip_prefix('/') { @@ -291,7 +303,22 @@ impl Builder { obj = match &grp.links[li].1 { Target::Group(child) => Obj::Group(*child), Target::Dataset(d) => Obj::Dataset(*d), - Target::Hard(p) => self.resolve(cur, p, depth + 1)?, + Target::Hard(p) => match memo[cur][li] { + Resolution::Done(o) => o, + Resolution::InProgress => { + return Err(err(format!( + "hard link target {target:?}: the hard link {:?} leads \ + back to itself (a cycle)", + join(&grp.path, c) + ))); + } + Resolution::Todo => { + memo[cur][li] = Resolution::InProgress; + let o = self.resolve(memo, cur, p, depth + 1)?; + memo[cur][li] = Resolution::Done(o); + o + } + }, Target::Soft(_) | Target::External { .. } => { return Err(err(format!( "hard link target {target:?} goes through a soft or external \ @@ -305,6 +332,14 @@ impl Builder { } } +/// Where resolving one hard link has got to. +#[derive(Clone, Copy)] +enum Resolution { + Todo, + InProgress, + Done(Obj), +} + #[derive(Clone, Copy)] enum Obj { Group(usize), @@ -325,14 +360,27 @@ pub(crate) fn build(root: GroupBuilder, default_track_order: bool) -> Result> = b + .groups + .iter() + .map(|g| vec![Resolution::Todo; g.links.len()]) + .collect(); let mut resolved: Vec>> = Vec::with_capacity(b.groups.len()); for (gi, g) in b.groups.iter().enumerate() { let mut row = Vec::with_capacity(g.links.len()); - for (_, t) in &g.links { + for (li, (_, t)) in g.links.iter().enumerate() { let obj = match t { Target::Group(i) => Some(Obj::Group(*i)), Target::Dataset(d) => Some(Obj::Dataset(*d)), - Target::Hard(p) => Some(b.resolve(gi, p, 0)?), + Target::Hard(p) => Some(match memo[gi][li] { + Resolution::Done(o) => o, + _ => { + memo[gi][li] = Resolution::InProgress; + let o = b.resolve(&mut memo, gi, p, 0)?; + memo[gi][li] = Resolution::Done(o); + o + } + }), Target::Soft(_) | Target::External { .. } => None, }; match obj { diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 27379bb..e8488c9 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -827,3 +827,52 @@ fn a_link_too_big_for_dense_storage_is_an_error() { assert_eq!(out, "[11, 65001]"); h5dump_ok(&path); } + +#[test] +fn chained_hard_links_resolve_in_linear_time() { + skip_if_no_python!(); + // Each link's target goes through the previous link twice. Resolving + // them without remembering resolved links doubled the work per link: + // 26 links took 46 s in a debug build, so 60 would never finish. + fn chain(reverse: bool) -> FileBuilder { + let mut b = FileBuilder::new(); + let mut g = b.create_group("g"); + g.create_dataset("v").with_i32_data(&[5]); + b.add_group(g.finish()); + let mut order: Vec = (0..60).collect(); + if reverse { + order.reverse(); + } + for i in order { + if i == 0 { + b.add_hard_link("g/s0", "/g"); + } else { + b.add_hard_link(&format!("g/s{i}"), &format!("/g/s{}/s{}", i - 1, i - 1)); + } + } + b + } + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let bytes = [false, true].map(|r| chain(r).finish().unwrap()); + tx.send(bytes).unwrap(); + }); + let [forward, reverse] = rx + .recv_timeout(std::time::Duration::from_secs(60)) + .expect("resolving 60 chained hard links took over a minute"); + + let dir = tempfile::tempdir().unwrap(); + for (name, bytes) in [("forward.h5", forward), ("reverse.h5", reverse)] { + let path = dir.path().join(name).display().to_string(); + std::fs::write(&path, bytes).unwrap(); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([h5py.h5o.get_info(f['g'].id).rc, len(f['g']),\n\ + \x20 int(f['g/s59/s30/s0/v'][0]), f['g/s59'] == f['g']]))", + ); + assert_eq!(out, "[61, 61, 5, true]", "{name}"); + let f = File::open(&path).unwrap(); + assert_eq!(f.dataset("g/s59/s0/v").unwrap().read_i32().unwrap(), [5]); + } +} From 8bcae3c78e14c75b7edeb5927a74041c7bfde3d8 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 09:02:09 -0500 Subject: [PATCH 09/10] fix(format): a dataset attribute set again replaces the earlier value b0a1e4f fixed this for group and root attributes only. Setting a dataset attribute twice still wrote two attribute messages with one name, and h5py read back the first value: set_attr("a", 1) then set_attr("a", 2) read as 1, and list(attrs) was ["a", "a"]. DatasetBuilder::set_attr now replaces the earlier value, compact or dense. Likewise, a hand-set attribute named like a provenance attribute (_provenance_sha256, ...) is replaced by the computed one instead of being written next to it and read first. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/file_writer.rs | 5 +- crates/clawhdf5-format/src/type_builders.rs | 7 ++- .../clawhdf5/tests/writer_groups_interop.rs | 62 +++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 0b318a9..11345a1 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -1355,7 +1355,10 @@ fn flatten_ds(db: DatasetBuilder, refcount: u32) -> Result timestamp: prov.timestamp.clone(), source: prov.source.clone(), }; - attrs.extend(p.build_attrs(&raw)); + // The provenance attributes replace any the caller set by hand. + let prov = p.build_attrs(&raw); + attrs.retain(|a| prov.iter().all(|b| b.name != a.name)); + attrs.extend(prov); } let fill_message = fill_value_message(db.fill_time, db.fill_value.as_deref(), &dt)?; Ok(DsFlat { diff --git a/crates/clawhdf5-format/src/type_builders.rs b/crates/clawhdf5-format/src/type_builders.rs index fa64465..ce43f64 100644 --- a/crates/clawhdf5-format/src/type_builders.rs +++ b/crates/clawhdf5-format/src/type_builders.rs @@ -695,8 +695,13 @@ impl DatasetBuilder { self } + /// Set attribute `name`. Setting it again replaces the earlier value, + /// as `attrs[name] = v` does in h5py. pub fn set_attr(&mut self, name: &str, value: AttrValue) -> &mut Self { - self.attrs.push((name.to_string(), value)); + match self.attrs.iter_mut().find(|(n, _)| n == name) { + Some(slot) => slot.1 = value, + None => self.attrs.push((name.to_string(), value)), + } self } diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index e8488c9..97e3544 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -876,3 +876,65 @@ fn chained_hard_links_resolve_in_linear_time() { assert_eq!(f.dataset("g/s59/s0/v").unwrap().read_i32().unwrap(), [5]); } } + +#[test] +fn a_dataset_attribute_set_again_takes_the_new_value() { + skip_if_no_python!(); + // Setting a dataset attribute twice wrote two attribute messages with + // one name, and h5py read back the first value. Also with dense + // attribute storage (more than 8). + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.create_dataset("x") + .with_f64_data(&[1.0]) + .set_attr("a", AttrValue::I64(1)) + .set_attr("a", AttrValue::I64(2)); + let d = b.create_dataset("dense"); + d.with_i32_data(&[1]); + for i in 0..12 { + d.set_attr(&format!("k{i:02}"), AttrValue::I64(i)); + } + d.set_attr("k03", AttrValue::String("three".into())); + let path = write(&dir, "ds_attrs.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 a, d = f['x'].attrs, f['dense'].attrs\n\ + \x20 print(json.dumps([list(a), int(a['a']), len(d), d['k03'].decode(), int(d['k04'])]))", + ); + assert_eq!(out, r#"[["a"], 2, 12, "three", 4]"#); + let f = File::open(&path).unwrap(); + assert!(matches!( + f.dataset("x").unwrap().attrs().unwrap()["a"], + AttrValue::I64(2) + )); +} + +#[cfg(feature = "provenance")] +#[test] +fn provenance_attributes_replace_ones_set_by_hand() { + skip_if_no_python!(); + // A hand-set attribute with a provenance attribute's name was written + // next to the computed one, and h5py read the hand-set value. + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.create_dataset("p") + .with_i32_data(&[1, 2]) + .with_provenance("me", "2026-09-26T00:00:00Z", None) + .set_attr("_provenance_sha256", AttrValue::String("forged".into())); + let path = write(&dir, "prov.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 a = f['p'].attrs\n\ + \x20 h = a['_provenance_sha256']\n\ + \x20 h = h.decode() if isinstance(h, bytes) else h\n\ + \x20 print(json.dumps([list(a).count('_provenance_sha256'), h != 'forged']))", + ); + assert_eq!(out, "[1, true]"); + let f = File::open(&path).unwrap(); + assert_eq!( + f.dataset("p").unwrap().verify_provenance().unwrap(), + clawhdf5_format::provenance::VerifyResult::Ok + ); +} From 546fdb84fab84d5d65ef3baba6bf567f1dad26c7 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 09:02:47 -0500 Subject: [PATCH 10/10] docs: dense storage fixes and the real limits of big groups The changelog, known issues and README said a group holds up to 65 535 links while a group of about 17 000 was already unreadable. Record the fixes (child indirect blocks, the next-block offset, the index leaf cap, refusing oversized dense messages, hard-link memoisation, dataset attribute overwrite) and the limits that remain true: 65 535 links or dense attributes per object, and 65 515 bytes per dense message. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 44 ++++++++++++++++++++++++++++++++++++++++---- README.md | 3 ++- docs/known-issues.md | 15 ++++++++++++--- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 080045d..d1f6b8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,19 +28,55 @@ Link Info message carries the flags, each link its order, and a dense group a creation-order B-tree (type 6). h5py then lists members in insertion order. Attribute creation order is not tracked. -- A group holds at most 65 535 links (its link index is one B-tree leaf); - more is an error. `GroupBuilder`'s fields changed (they were +- A group holds at most 65 535 links (its link index is one B-tree leaf), + and in a group of more than 8 links (dense storage) each link message + must be at most 65 515 bytes (one fractal heap block; huge heap objects + are not written); more is an error. Measured at the limit: 65 535 links + with 100-byte names (a 7 MB heap) read in h5py, h5dump and clawhdf5, and + h5py can add to the group. `GroupBuilder`'s fields changed (they were crate-private); `FinishedGroup` is unchanged for callers. - Files that use one level of groups and no new link kinds are laid out as before: byte-identical to the writer with the Group Info fix below (compared on simple, mixed dense/chunked/compact/external-link and paged files). Tests: h5py and clawhdf5 read the same tree (every path, attribute and value) from a 5-level file; soft, hard, - external and cyclic hard links; 10 000 links in one group, with and - without creation order; libhdf5 adding and deleting links in our groups; + external and cyclic hard links; 10 000, 20 000 and 65 535 links in one + group, with and without creation order; libhdf5 adding and deleting links + in our groups; `h5rs check` passes and `h5rs dump` equals h5dump (`crates/clawhdf5/tests/writer_groups_interop.rs`, `crates/clawhdf5-tools/tests/h5rs_interop.rs`). +- **Big dense groups and attribute sets were unreadable.** The fractal heap + holding dense links or attributes wrote every doubling-table row as + direct blocks, but past the 512 KiB the root's direct blocks hold, rows + are child indirect blocks, and libhdf5 and `h5rs check` read them as + such: a group with 20 000 links of 20-byte names was written without + error and h5py could not list it ("incorrect metadata checksum"); 150 + dense attributes of up to 56 KB could not be opened. This was in 2.7.0 + too. The heap writer now writes child indirect blocks, nested as deep as + needed. Found on the way: an object bigger than the next block's space + was cut off (it now goes in the first block big enough), and h5py adding + a link to a heap over 64 KiB overwrote its first block (the header's + next-block offset was 0). +- **h5py crashed adding a link to a group of more than about 47 700 + links** (35 000 with creation order tracked). The link index leaf's node size gave libhdf5 room for more than + 65 535 records, which overflows the leaf's 2-byte count. The node is now + capped at 65 535 records. Dense attributes use the same index builder: + more than 65 535 on one object used to be written with the count modulo + 65 536, and are now an error. +- **A dense link or attribute message over 65 515 bytes** (e.g. a soft link + with a long target in a group of more than 8 links) was written cut off, + and libhdf5 could not list the group ("object overruns end of direct + block"). It is now an error. +- **Chained hard links took exponential time to resolve.** A hard-link + target going through other hard links resolved them again on every path + through them: 26 links whose targets each named the previous one twice + took 46 s. Each hard link is now resolved once, and a cycle is reported + by the link's name. +- **A dataset attribute set twice read back as its first value**, as for + groups below (h5py listed the name twice). The later value now replaces + the earlier one; a hand-set attribute named like a provenance attribute + is replaced by the computed one. - **A group attribute set twice read back as its first value.** Setting a group (or root) attribute again wrote a second attribute message with the same name, and h5py returned the first value. The later value now replaces diff --git a/README.md b/README.md index 3b3fd22..20294ec 100644 --- a/README.md +++ b/README.md @@ -429,7 +429,8 @@ b.add_external_link("raw", "raw.h5", "/data"); b.write("groups.h5")?; ``` -A group holds at most 65 535 links; more is an error. +A group holds at most 65 535 links; more is an error, as is a link over +65 515 bytes (a very long soft-link target) in a group of more than 8 links. ### Agent Memory diff --git a/docs/known-issues.md b/docs/known-issues.md index 76fc86e..cd40971 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -209,11 +209,20 @@ fill-value item that did is fixed). h5py, h5dump and `h5rs check --data` read them (`crates/clawhdf5/tests/writer_groups_interop.rs`, `crates/clawhdf5-tools/tests/h5rs_interop.rs`). Still missing: a group - with more than 65 535 links (its link index is one B-tree leaf) is an - error, and attribute creation order is not tracked. + with more than 65 535 links, or an object with more than 65 535 dense + attributes, is an error (the index is one B-tree leaf), and attribute + creation order is not tracked. + - ~~Dense link or attribute storage past 512 KiB of messages was written + unreadable (child indirect blocks of the fractal heap written as direct + blocks).~~ **Fixed 2026-09-26** (it affected 2.7.0 too): tested with + 20 000 and 65 535 links and with 8 MB of dense attributes, read by + h5py, h5dump, `h5rs check` and clawhdf5, and h5py can add links to + such groups. - ~~libhdf5 could not add a link to a group we wrote (no Group Info message).~~ **Fixed 2026-09-26.** - - Dense attribute storage for attributes over 64 KiB. + - Huge fractal heap objects: in dense storage (more than 8 attributes on + an object, or more than 8 links in a group) one attribute or link + message over 65 515 bytes is an error. - Output that HDF5 1.8 can read. - A B-tree v2 chunk index larger than one leaf, so datasets with several unlimited dimensions are limited to 65 535 chunks.