fix(tools): h5rs diff compares every name of a hard-linked object

The path walk skipped the second hard link to an object, so a file that
shares one dataset between /x and /y differed from a file holding two
identical copies: "</y> exists only in <B>", exit 1, where h5diff exits 0.
For a hard-linked group every member was reported the same way.

diff now enumerates every path below the start object (a hard link back
to an ancestor is recorded but not descended into), so each name is
compared. A group whose links cannot be read is now an error instead of
an empty group.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-26 01:14:53 -05:00
co-authored by Claude Opus 5.5
parent 845a9d0125
commit 699ee9c447
5 changed files with 156 additions and 26 deletions
+4 -2
View File
@@ -130,8 +130,10 @@
files); metadata space is one figure, not broken down. files); metadata space is one figure, not broken down.
- `h5rs diff [-r] [-q] [-d D] [-p R] A B [OBJ1 [OBJ2]]` compares objects, - `h5rs diff [-r] [-q] [-d D] [-p R] A B [OBJ1 [OBJ2]]` compares objects,
kinds, datatypes, shapes, attributes, values and link targets; exit kinds, datatypes, shapes, attributes, values and link targets; exit
status 0/1/2 as h5diff's. Objects that cannot be compared count as a status 0/1/2 as h5diff's. Every path is compared, including every name
difference (h5diff exits 0 for them), and NaN equals NaN. of a hard-linked object and the members of a hard-linked group. Objects
that cannot be compared count as a difference (h5diff exits 0 for them),
and NaN equals NaN.
- `h5rs check [--data] FILE` is a structural validator: it walks every - `h5rs check [--data] FILE` is a structural validator: it walks every
object, parses every header message, verifies the checksums of every object, parses every header message, verifies the checksums of every
version 2+ structure it meets (superblock, object headers and version 2+ structure it meets (superblock, object headers and
+8 -4
View File
@@ -145,10 +145,14 @@ $ h5rs diff -p 0.01 a.h5 b.h5 # |a - b| / |a| > 1% is a difference
Exit status: 0 no differences, 1 differences, 2 error — the same as h5diff's Exit status: 0 no differences, 1 differences, 2 error — the same as h5diff's
on the cases `diff_exit_codes_match_h5diff` runs. Compared: which objects on the cases `diff_exit_codes_match_h5diff` runs. Compared: which objects
exist, their kinds, datatypes and shapes, attribute sets and values, dataset exist, their kinds, datatypes and shapes, attribute sets and values, dataset
values, and soft/external link targets. Two differences from h5diff, on values, and soft/external link targets. Every path is compared: an object
purpose: objects that cannot be compared (different shapes or datatype hard-linked under two names is compared under both, with everything below
classes) count as a difference (h5diff warns and exits 0), and two NaNs are it, so a file that shares one object between two names equals a file that
equal. stores two identical copies. Differences from h5diff, on purpose: objects
that cannot be compared (different shapes or datatype classes) count as a
difference (h5diff warns and exits 0); two NaNs are equal; and the members
of a group reached by a second hard link are compared (h5diff lists them in
one file only and exits 1).
## `h5rs check` ## `h5rs check`
+91 -20
View File
@@ -2,6 +2,7 @@
use std::cell::OnceCell; use std::cell::OnceCell;
use std::collections::{BTreeMap, HashMap}; use std::collections::{BTreeMap, HashMap};
use std::rc::Rc;
use clawhdf5_format::attribute::AttributeMessage; use clawhdf5_format::attribute::AttributeMessage;
use clawhdf5_format::dataspace::{Dataspace, DataspaceType}; use clawhdf5_format::dataspace::{Dataspace, DataspaceType};
@@ -9,7 +10,7 @@ use clawhdf5_format::datatype::Datatype;
use clawhdf5_format::object_header::ObjectHeader; use clawhdf5_format::object_header::ObjectHeader;
use crate::cli::{Args, Out}; use crate::cli::{Args, Out};
use crate::h5::{H5, Kind, LinkKind}; use crate::h5::{H5, Kind, Link, LinkKind};
use crate::value::{self, Decoder, Value}; use crate::value::{self, Decoder, Value};
pub const USAGE: &str = "\ pub const USAGE: &str = "\
@@ -177,25 +178,13 @@ pub fn run(args: &mut Args, out: &mut Out) -> std::io::Result<i32> {
} else { } else {
format!("/{base}") format!("/{base}")
}; };
let mut entries = BTreeMap::new(); let entries = match collect(h5, addr) {
let walk = h5.walk_from(addr, "", &mut |it| { Ok(e) => e,
let e = match (it.link.map(|l| &l.kind), it.header) { Err(e) => {
(Some(LinkKind::Soft(t)), _) => Entry::Soft(t.clone()), writeln!(out.e, "h5rs diff: {f}: {e}")?;
(Some(LinkKind::External { file, path }), _) => { return Ok(2);
Entry::External(file.clone(), path.clone()) }
} };
(Some(LinkKind::UserDefined(t)), _) => Entry::UserDefined(*t),
(_, Some(Err(e))) => Entry::Broken(e.to_string()),
(_, Some(Ok(h))) => Entry::Obj(it.addr.unwrap_or(0), Kind::of(h)),
// A second hard link to an object already compared.
(_, None) => return,
};
entries.insert(it.path.to_string(), e);
});
if let Err(e) = walk {
writeln!(out.e, "h5rs diff: {f}: {e}")?;
return Ok(2);
}
sides.push(Side { sides.push(Side {
h5, h5,
label: f.clone(), label: f.clone(),
@@ -248,6 +237,88 @@ pub fn run(args: &mut Args, out: &mut Out) -> std::io::Result<i32> {
}) })
} }
/// One object as the path walk sees it: what its paths compare as, and
/// (for a group) its links.
struct Node {
entry: Entry,
links: Vec<Link>,
}
/// Every path below the object at `start`, relative to it (`""` is `start`
/// itself). Each hard link is its own path, so an object linked under two
/// names is compared under both, with everything below it: two files that
/// hold the same values are equal whether one shares an object between
/// names and the other stores copies. A hard link back to an ancestor (a
/// cycle) is recorded, not descended into.
fn collect(h5: &H5, start: u64) -> crate::h5::Result<BTreeMap<String, Entry>> {
let mut cache: HashMap<u64, Rc<Node>> = HashMap::new();
let mut node = |addr: u64| -> Rc<Node> {
cache
.entry(addr)
.or_insert_with(|| {
Rc::new(match h5.header(addr) {
Err(e) => Node {
entry: Entry::Broken(e.to_string()),
links: Vec::new(),
},
Ok(h) => match Kind::of(&h) {
Kind::Group => match h5.links(&h) {
Ok(links) => Node {
entry: Entry::Obj(addr, Kind::Group),
links,
},
Err(e) => Node {
entry: Entry::Broken(format!("links: {e}")),
links: Vec::new(),
},
},
k => Node {
entry: Entry::Obj(addr, k),
links: Vec::new(),
},
},
})
})
.clone()
};
let mut entries = BTreeMap::new();
// (address, relative path, addresses of the groups above it)
let mut stack: Vec<(u64, String, Rc<Vec<u64>>)> =
vec![(start, String::new(), Rc::new(Vec::new()))];
while let Some((addr, path, above)) = stack.pop() {
if entries.len() >= crate::h5::MAX_OBJECTS {
return Err(crate::h5::Error::new(format!(
"more than {} paths; stopped walking",
crate::h5::MAX_OBJECTS
)));
}
let n = node(addr);
entries.insert(path.clone(), n.entry.clone());
if n.links.is_empty() || above.contains(&addr) {
continue;
}
let mut chain = (*above).clone();
chain.push(addr);
let chain = Rc::new(chain);
for l in &n.links {
let child = format!("{path}/{}", l.name);
match &l.kind {
LinkKind::Hard(a) => stack.push((*a, child, chain.clone())),
LinkKind::Soft(t) => {
entries.insert(child, Entry::Soft(t.clone()));
}
LinkKind::External { file, path } => {
entries.insert(child, Entry::External(file.clone(), path.clone()));
}
LinkKind::UserDefined(t) => {
entries.insert(child, Entry::UserDefined(*t));
}
}
}
}
Ok(entries)
}
fn kind_word(k: Kind) -> &'static str { fn kind_word(k: Kind) -> &'static str {
match k { match k {
Kind::Group => "group", Kind::Group => "group",
+17
View File
@@ -109,6 +109,23 @@ small("attr.h5", attr=True)
small("reshaped.h5", shape=(4, 3)) small("reshaped.h5", shape=(4, 3))
small("int.h5", dtype="i4") small("int.h5", dtype="i4")
# One object under two names (a hard link) against two separate copies.
with h5py.File(os.path.join(out, "hardlinked.h5"), "w") as f:
f["x"] = np.arange(5)
f["y"] = f["x"]
g = f.create_group("g")
g["d"] = np.arange(3)
g.create_group("s")["e"] = np.arange(2)
f["h"] = g
for name, last in (("copied.h5", 1), ("copied_changed.h5", 9)):
with h5py.File(os.path.join(out, name), "w") as f:
f["x"] = np.arange(5)
f["y"] = np.arange(5)
for gname in ("g", "h"):
g = f.create_group(gname)
g["d"] = np.arange(3)
g.create_group("s")["e"] = np.array([0, last if gname == "h" else 1])
with h5py.File(os.path.join(out, "userblock.h5"), "w", userblock_size=1024) as f: with h5py.File(os.path.join(out, "userblock.h5"), "w", userblock_size=1024) as f:
f["d"] = np.arange(10) f["d"] = np.arange(10)
@@ -380,6 +380,42 @@ fn diff_exit_codes_match_h5diff() {
assert!(s.contains("2 difference(s) found"), "{s}"); assert!(s.contains("2 difference(s) found"), "{s}");
} }
/// An object hard-linked under two names is compared under both, with
/// everything below it, so a file that shares one object between two names
/// equals a file that stores two identical copies.
#[test]
fn diff_compares_every_hard_link_path() {
let Some(f) = generate() else { return };
let (linked, copied, changed) = (
f.p("hardlinked.h5"),
f.p("copied.h5"),
f.p("copied_changed.h5"),
);
// A hard-linked dataset: h5diff agrees (exit 0).
for (a, b) in [(&linked, &copied), (&copied, &linked)] {
let o = h5rs(&["diff", a, b, "/y"]);
assert_eq!(code(&o), 0, "{a} {b} /y: {}", stdout(&o));
if tool_available("h5diff") {
assert_eq!(code(&run("h5diff", &[a, b, "/y"])), 0, "h5diff /y");
}
// The whole file, including a hard-linked group's members (h5diff
// does not descend a group's second name, so it reports those
// members as present in one file only and exits 1).
let o = h5rs(&["diff", a, b]);
assert_eq!(code(&o), 0, "{a} {b}: {}", stdout(&o));
assert!(!stdout(&o).contains("exists only"), "{}", stdout(&o));
}
// A value under the second name is compared, not skipped.
let o = h5rs(&["diff", "-r", &linked, &changed]);
assert_eq!(code(&o), 1, "{}", stdout(&o));
assert!(
stdout(&o).contains("dataset: </h/s/e> and </h/s/e>"),
"{}",
stdout(&o)
);
assert!(!stdout(&o).contains("</g/s/e>"), "{}", stdout(&o));
}
/// Where h5rs deliberately differs from h5diff: objects that cannot be /// Where h5rs deliberately differs from h5diff: objects that cannot be
/// compared (another shape, another datatype class) are a difference. /// compared (another shape, another datatype class) are a difference.
#[test] #[test]