Skip to content

feat(xmp): exiv2-parity schema registry and XMP sidecar files - #507

Open
justin13888 wants to merge 10 commits into
feat/449-xmp-dcterms-provenancefrom
feat/421-xmp-schemas-sidecars
Open

justin13888 wants to merge 10 commits into
feat/449-xmp-dcterms-provenancefrom
feat/421-xmp-schemas-sidecars

Conversation

@justin13888

@justin13888 justin13888 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #503 (feat/449-xmp-dcterms-provenance); this PR's base is that branch, and its head d6fd0a0 is merged in by merge commit 6ffe02b. Issue #421. Commits: 1c57b2e schemas, bc1102a sidecars, 40db95e self-review repair, fd6ba22 + fce19d6 review-round repairs, b3375af documentation round.

Summary

  • Schema breadth (feat(xmp), commit 1). WellKnownNs grows from 18 to 30 entries — exifEX, aux, plus, mwg-rs, mwg-kw, GPano, lr, MicrosoftPhoto, digiKam, acdsee, crss, dwc — every schema exiv2 documents (https://exiv2.org/metadata.html) that the registry lacked. Each URI is cited on its variant (schema owner's specification where published; the vendored reference for all twelve is exiv2's own registry, third_party/exiv2/src/properties.cpp xmpNsInfo, which is also the oracle's). Additions are minor: WellKnownNs is #[non_exhaustive] from feat(metadata): report remote provenance from dcterms:provenance #503. The registry stays a registry — it fixes the prefix a schema serializes under and interprets no value.

  • Sidecars (feat(xmp), commit 2). New gamut_xmp::XmpSidecar with read(&[u8]) -> Result<XmpMeta> and write(&XmpMeta) -> Vec<u8>: the bytes of a standalone .xmp file (XMP Part 3, "External storage of metadata"). read accepts everything XmpMeta::from_packet does (XML declaration, BOM, packet wrapper or bare) but requires the x:xmpmeta document element, resolved by namespace, rejecting a document whose element is not x:xmpmeta with XmpError::MissingXmpMeta naming the element found (it was Prohibited until fd6ba22; see decision 14/R1). write emits the XML declaration Part 3 asks for, then a read-only (end="r"), unpadded canonical packet inside x:xmpmeta — byte-stable per graph. No filesystem API; the photo.xmp-beside-photo.dng convention is documented, not enforced.

  • Oracle. tests/oracle.rs gains one test per added schema, reading a documented property of that schema back from Adobe XMPCore by its Xmp.<prefix>.<name> key (a lookup that fails for a wrong URI or a wrong prefix; the four struct-only schemas use their documented structure/bag shape), and two sidecar tests: the file gamut writes is accepted by XMPCore and read back by key, and XMPCore's own <?xpacket?>/<x:xmpmeta> serialization — what exiv2 stores in a .xmp file — is read as a sidecar.

  • One oracle normalization pinned, not hidden. exiv2 appends / to any namespace URI ending in neither / nor # before registering it with XMPCore (XmpProperties::registerNs, properties.cpp:4966), so the engine re-serializes Darwin Core as http://rs.tdwg.org/dwc/index.htm/ while keying it correctly on read. xmpcore_output_uri in tests/oracle.rs states the rule with the citation; the dwc test additionally asserts gamut's bytes declare the unslashed URI exiv2 documents. Recorded in STATUS.md (decision 9 below).

  • Review round (fd6ba22, fce19d6). A read-only review verified all twelve URI/prefix pairs character-for-character against exiv2 0.28.8 and returned three Lows, all taken. (1) XmpSidecar::read reported a missing wrapper as XmpError::Prohibited, whose documented meaning is a construct the specification prohibits — but Part 1 §7.3.3 permits the wrapper-less form and XmpWriter::wrap_xmpmeta(false) emits it, so the message asserted a prohibition that does not exist; there is now an additive XmpError::MissingXmpMeta(String). (2) from_uri did not recognise http://rs.tdwg.org/dwc/index.htm/, the form XMPCore emits, so a graph parsed from an exiv2-written packet re-serialized under ns1 instead of dwc; the slashed form is now a read-only alias (DWC_URI_TRAILING_SLASH) while uri() still emits the unslashed URI, leaving written bytes unchanged. (3) The nested oracle helper's re-namespacing was a verified no-op — both callers passed a URI the engine leaves alone — so it is now driven once with Darwin Core, where source and target actually differ. Plus a reachability note on the sidecar root check and a registry-count drift guard named for what it guards.

This is an unattended run. No human approved the plan; the decision record below is what a human reads afterwards.

Validation

Run from the lane worktree. Heavy gates inside systemd-run --user --scope -q --slice=agents.slice -p MemoryMax=16G -p MemorySwapMax=0 -- env CARGO_BUILD_JOBS=2 CMAKE_BUILD_PARALLEL_LEVEL=2 TMPDIR=/var/mnt/scratch/golem/tmp prlimit --as=12000000000 … (same 12 GB address-space limit as ulimit -v 12000000; see decision 10).

CI is green. All eight checks pass at the head b3375af (gh pr checks 507: Format & Metadata, Clippy & Doctests, Coverage (test gate), the four Incremental (PR diff) shards; Full workspace is skipping by design), in runs 34436828105 / 34436828094 — as they did at fce19d6 in runs 34385470688 / 34385470677. An earlier revision of this body reported that run as red for issue #517 — an apt/Chrome-repository failure in the runner image — which was true when written and is not true now; the run completed successfully. Nothing was weakened, skipped or bypassed. The local gates below are the primary evidence.

  • CARGO_BUILD_JOBS=2 cargo test -p gamut-xmp --all-featurespass on the review-round tree (fce19d6): 106 unit, 4 golden, 22 oracle, 5 roundtrip, 4 doctests. Earlier on 40db95e: 104 unit (9 new in sidecar.rs, 1 new in namespace.rs), 4 golden, 21 oracle (14 new), 5 roundtrip, 4 doctests (1 new). Also pass on each commit tree alone: schemas-only 1c57b2e 95/4/19/5/3; sidecars bc1102a 104/4/21/5/4.
  • __CARGO_TEST_ROOT=<worktree> mise run fmt then mise run fmt-checkpass on every commit tree (the prefix is the documented workaround for cargo walking past a nested worktree to the primary checkout's manifests).
  • mise run check-testspass.
  • mise run check-commitspass, "no errors in 7 commits" (merge commits skipped); convco check origin/feat/449-xmp-dcterms-provenance..HEADpass on the two lane commits before the merge.
  • CARGO_BUILD_JOBS=2 cargo clippy -p gamut-xmp --all-targets --all-features -- -D warningspass (exit 0) on every commit tree.
  • mise run lint (scope above, timeout 3000) — pass twice: on 6ffe02b and again on the final tree 40db95e (exit 0; the only output beyond clippy is cargo's future-incompat note for proc-macro-error2, a transitive dependency this PR does not touch).
  • mise run test (scope above, timeout 3000) — pass on the final tree: 203 suites, 3,803 tests, 0 failed, exit 0.
  • mise run mutants-crate gamut-xmp --file crates/gamut-xmp/src/namespace.rs --file crates/gamut-xmp/src/sidecar.rs --file crates/gamut-xmp/src/error.rs --shard 0/1 (scope above, timeout 3600) on the review-round tree — pass: 21 mutants tested in 3m, 18 caught, 3 unviable, 0 missed, 0 timeouts.
  • CARGO_BUILD_JOBS=2 cargo clippy -p gamut-xmp --all-targets --all-features -- -D warnings, __CARGO_TEST_ROOT=<worktree> mise run fmt-check, mise run check-tests, mise run check-commits ("no errors in 9 commits") — all pass on the review-round tree.
  • Whole-workspace mise run lint / mise run test were not re-run for the review round, deliberately: the box is under heavy load from sibling lanes, and the round's only cross-crate-visible change is an added variant on an #[non_exhaustive] enum plus one new pub const, neither of which can change another crate's compilation or behaviour. Both passed on 40db95e (lint exit 0; test 203 suites / 3,803 tests / 0 failed).
  • Earlier run: mise run mutants-crate gamut-xmp --file crates/gamut-xmp/src/namespace.rs --file crates/gamut-xmp/src/sidecar.rs --shard 0/1 (the two source files that PR state changed — cargo mutants -p gamut-xmp --list counts 296 mutants for the crate, all of which link the exiv2 oracle, so the full crate is hours for files this PR does not touch, and CI's blocking gate is --in-diff) — pass: 18 mutants tested in 3m, 16 caught, 2 unviable (Default::default() on types without Default), 0 missed, 0 timeouts.
  • Documentation round (b3375af). CARGO_BUILD_JOBS=2 cargo test -p gamut-xmp --all-featurespass: 107 unit (+1), 4 golden, 22 oracle, 5 roundtrip, 4 doctests. CARGO_BUILD_JOBS=2 cargo clippy -p gamut-xmp --all-targets --all-features -- -D warningspass (exit 0). RUSTDOCFLAGS="-D warnings" cargo doc -p gamut-xmp --no-deps --all-featurespass (exit 0), for the new intra-doc link. __CARGO_TEST_ROOT=<worktree> mise run fmt then mise run fmt-checkpass. mise run check-testspass. mise run check-commitspass, "no errors in 10 commits"; convco check origin/feat/449-xmp-dcterms-provenance..HEADpass, 6 commits. mise run lint (scope above, timeout 3600) — pass, exit 0 (only output beyond clippy is cargo's future-incompat note for the transitive proc-macro-error2). mise run mutants-diff (scope above, timeout 3600) — pass: 26 mutants tested in 3m, 24 caught, 2 unviable, 0 missed.
  • Whole-workspace mise run test was not re-run for the documentation round, deliberately: the round changes no executable library code — the source edits are doc comments only — and its three code-shaped changes (one added inline test, one oracle-helper assertion, one dropped redundant assertion) are confined to gamut-xmp's own suite, which passed in full above. The doc comments are covered by cargo test -p gamut-xmp (4 doctests) and by the rustdoc run. It passed on 40db95e (203 suites / 3,803 tests / 0 failed) and mise run lint, which compiles every crate and target in the workspace, passed on this tree.
  • Findings verified by execution rather than by reading, before the docs were written: an XMPCore-shaped packet declaring http://rs.tdwg.org/dwc/index.htm/ yields get_text(WellKnownNs::DarwinCore.uri(), "Record") == None and get_text(DWC_URI_TRAILING_SLASH, "Record") == Some("gamut"), and gamut writes both the dwc prefix and the slashed URI back; XmpSidecar::read on an xpacket-wrapped bare rdf:RDF returns MissingXmpMeta("RDF") while exiv2's isXmpType accepts it (third_party/exiv2/src/xmpsidecar.cpp:190). The throwaway test used for this was deleted before the commit.
  • Not run, by the lane rules: check-release-deps / check-ffi-features (no Cargo.toml touched), check-ffi-header (no C-surface type), coverage (the new module is fully reached by its inline tests and the oracle tests; CI's Coverage job is the gate).
  • Closing round (9909bb3). Every command below completed in this run, under the memory-capped scope. __CARGO_TEST_ROOT=<worktree> mise run fmt then mise run fmt-checkpass (exit 0). mise run check-testspass, "module docs, pinned proptest seeds and oracle filenames all conform". convco check origin/feat/449-xmp-dcterms-provenance..HEADpass, "no errors in 9 commits". RUSTDOCFLAGS="-D warnings" cargo doc -p gamut-xmp --no-deps --all-featurespass (exit 0). CARGO_BUILD_JOBS=2 cargo test -p gamut-xmp --all-featurespass: 107 unit, 4 golden, 22 oracle, 5 roundtrip, 4 doctests, 0 failed. CARGO_BUILD_JOBS=2 cargo clippy -p gamut-xmp --all-targets --all-features -- -D warningspass (exit 0). GAMUT_MUTANTS_BASE=origin/feat/449-xmp-dcterms-provenance mise run mutants-diff (scope above) — pass: 18 mutants tested in 10m, 17 caught, 1 unviable, 0 missed.
  • The strengthened from_uri test was verified against the mutation it is named for, both ways round. With the_trailing_slash_alias_is_the_only_uri_from_uri_does_not_hand_back present, adding a second read alias to from_uri fails that test and no other — for a slashed alias (http://ns.adobe.com/lightroom/1.0//Lightroom): 106 passed, 1 failed; for an unslashed one (http://ns.adobe.com/xap/1.0Xmp): 106 passed, 1 failed. With the test deleted and both aliases present, the lib suite is green: 106 passed, 0 failed. The test therefore kills something no other test in the crate kills, which its previous form did not.
  • The truncation this round documents was established by executing it, not by reading the code. Two XmpSidecar::write outputs concatenated (xmp:Rating = 5, then xmp:CreatorTool = gamut) return Ok with one property: get_text(xmp, "Rating") == Some("5"), get_text(xmp, "CreatorTool") == None. No error is raised. The throwaway test used for this was deleted before the commit.
  • Correction to an earlier claim in this section. The bullet above justifying the skipped whole-workspace mise run test for the documentation round cited "the rustdoc run" among the things covering the change, as though a documentation build guards this repository. It does not: there is no such gate anywhere in this repository. Verified in this run: grep -n "cargo doc\|RUSTDOCFLAGS" .github/workflows/*.yml mise.toml returns nothing, and grep -rn "lints.rustdoc\|rustdoc::" --include=Cargo.toml . returns nothing — no workflow step, no mise task, no rustdoc lint table. A broken intra-doc link ships green anywhere in the workspace. That the local cargo doc run above is a real check was confirmed by breaking a link in this diff on purpose: RUSTDOCFLAGS="-D warnings" cargo doc -p gamut-xmp then reports error: unresolved link to \WellKnownNs::no_such_method`` and exits 101 — but nothing in CI would run it. The gap is filed as No gate compiles a README code block or fails on a broken rustdoc link #549; it is repository-wide and deliberately not wired here.
  • The argument that skipping whole-workspace mise run test is safe stands on its own, and on stronger ground than the sentence above gave it. The workspace test gate is the Coverage job, and the doctests it cannot instrument are added back separately — both stated by the workflow itself, quoted here from .github/workflows/ci.yml and re-read in this run. Line 163: name: Coverage (test gate). Lines 196–198: "This is the workspace's test gate as well as its coverage gate: llvm-cov runs the same suite cargo test would and fails the job on any test failure." Lines 137–139, on the Clippy & Doctests job: "The full test suite is NOT run here — the coverage job runs it instrumented and fails on any test failure, so it is the green-test gate; test-doc adds back only the doctests, which cargo llvm-cov cannot instrument on stable." The union of those two required checks is therefore a superset of the workspace test gate, and both are required on this PR.
  • Not re-run this round, and why: whole-workspace mise run lint and mise run test — the round changes no executable library code outside one inline test body in gamut-xmp, no public signature, and no Cargo.toml; the rest is doc comments and Markdown. Nothing outside gamut-xmp can compile or behave differently, and gamut-xmp's own suite (including the exiv2/XMPCore oracle) plus crate-scoped clippy at -D warnings passed in full above; the two workflow-quoted required checks re-run both on the pushed head. check-release-deps / check-ffi-features / check-ffi-header — no Cargo.toml and no C-surface type touched. coverage — no new module; the round adds documentation and assertions to an existing test, which cannot lower the floor.

Risks and rollout

  • Additive, minor for gamut-xmp: twelve new #[non_exhaustive] enum variants, one new module and type, one new lib.rs re-export; no existing signature, URI, prefix or canonical byte changes. WellKnownNs::ALL.len() changes from 18 to 30 — a consumer that hard-codes the count would notice; none in the workspace does.
  • Serialization impact: a graph that already used one of the twelve URIs previously serialized under a synthesized nsN prefix and now serializes under the documented prefix. The parsed graph is identical (prefixes are non-semantic, Part 1 §6.2); only the bytes of such packets change. None of the existing golden vectors uses these namespaces.
  • Sidecar strictness: XmpSidecar::read rejects a bare rdf:RDF document that XmpMeta::from_packet accepts. That is the intended boundary (decision 3); a caller with such input uses from_packet.
  • Oracle: the dwc normalization is exiv2's, and pinned as such; if a future exiv2 stops appending /, xmpcore_output_uri is the one place to change.

Issue

Closes #421

Filed by this lane, linked rather than done here: #547gamut-xmp: consider canonicalizing a well-known namespace URI on read (Darwin Core trailing slash). Reading does not rewrite the URI a packet declares, so a graph parsed from an XMPCore-written Darwin Core packet re-serializes under the dwc prefix but stays keyed by DWC_URI_TRAILING_SLASH; the issue records the option, the asymmetry it would trade away (byte fidelity for the one schema affected), and what either decision would have to pin.

Also filed by this lane: #516references/exif: vendor CIPA DC-010 and re-derive the exifEX XMP namespace URI from it. WellKnownNs::ExifEx is bound to http://cipa.jp/exif/1.0/, which is what exiv2 and XMPCore bind but is not derived from any specification vendored under references/; the only vendored text binding the prefix is Exif 3.0 Annex J.2/J.3, where http://cipa.jp/exif/2.32/ appears as an exifEX:ExifAN annotation example rather than as the schema URI.

Also filed by this lane: #562gamut-xmp: XmpSidecar::read silently truncates a catenated multi-packet sidecar to its first packet. Part 3 asks that external metadata be written "as though it were embedded and then had the XMP packets extracted and catenated by a postprocessor", so a conforming producer may hand over a .xmp file holding several <?xpacket?> packets end to end; read takes the first and discards the rest with no error, where Adobe XMPCore rejects the same bytes. This PR documents the behaviour and does not change it — rejecting, merging every packet (which needs a conflict rule Part 3 does not supply) and keeping the truncation are three different products, and that is not this PR's scope.

Not filed, because it already exists: the missing repository-wide documentation gate is #549No gate compiles a README code block or fails on a broken rustdoc link — which names exactly this gap ("RUSTDOCFLAGS=\"-D warnings\" cargo doc is not run by any mise task or workflow"). Nothing was added to it; it is linked here as the home of the fix.

Decisions taken

Issue 421 - gamut-xmp: schema breadth (17 -> 29) + XMP sidecars
Plan:     v1
Branch:   feat/421-xmp-schemas-sidecars
Base:     head of PR #503 (origin/feat/449-xmp-dcterms-provenance, f809390) - stacked; PR opened against that branch
Cause:    -
Touches:  gamut-xmp namespace.rs (+12 variants), new sidecar.rs, lib.rs, tests, STATUS/README
Will not: validate per-schema values (the registry stays a registry); add a dependency; change canonical serialisation of existing namespaces; touch gamut-metadata
Lane:     serialised behind E-449 (stacked)
Settled:  #503's decision 2 (WellKnownNs is #[non_exhaustive]; additions are minor); S3 docs/testing.md

Decisions taken.
1. Deliverable boundary
   Taken:    the whole issue - the twelve missing namespaces plus sidecar read/write; no split
   Filed:    -
2. Registry additions
   Taken:    twelve new WellKnownNs variants (ExifEx, Aux, Plus, MwgRegions, MwgKeywords, GPano, Lightroom, MicrosoftPhoto, DigiKam, Acdsee, CameraRawSavedSettings, DarwinCore), each with prefix, URI and a doc citing the URI's source; added to ALL so from_uri and canonical prefix assignment see them; minor (non_exhaustive)
   Rejected: a runtime-extensible registry - the crate's convention is a const table; interpreting any value - the registry is not a validator
   Reverses: remove the variants
3. Sidecar
   Taken:    `XmpSidecar` in sidecar.rs: `read(&[u8]) -> Result<XmpMeta>` accepts a UTF-8 file that is a packet with or without the <?xpacket?> wrapper and REQUIRES the x:xmpmeta wrapper (a sidecar without it is InvalidInput naming the missing element - XMP Part 3 §1.1.1 / exiv2 xmpsidecar behaviour); `write(&XmpMeta) -> Vec<u8>` emits a UTF-8 file with a BOM-less xpacket wrapper and x:xmpmeta, canonical serialisation, no padding; the file-name convention (.xmp beside the image) is documented, not enforced
   Rejected: a filesystem API - the crate is bytes-in/bytes-out and no_std-friendly in spirit; accepting a bare rdf:RDF as a sidecar - exiv2/XMPCore reject it
   Reverses: relax read to accept bare bodies
4. Oracle
   Taken:    for each added namespace, a packet gamut serialises with a property in it must parse in XMPCore (tooling/exiv2-oracle) to the same property set, and a sidecar gamut writes must be read by XMPCore; one oracle test per namespace family is acceptable if a single test names one namespace per assertion group is not possible - prefer one test per namespace

Appended by the lane, in the record's shape:

5. exifEX URI
   Taken:    `http://cipa.jp/exif/1.0/` (the record's URI; CIPA DC-010-2012, referenced by XMP Part 2 §3.4; the URI exiv2's registry and deployed writers bind to `exifEX`) - the vendored Exif 3.0 text (references/exif/exif-3.0-dc-008-translation-2023.pdf, Annex J.2-J.3) binds the same prefix to `http://cipa.jp/exif/2.32/` in its ExifAN annotation examples, and the vendored Exif 2.32 text names no XMP namespace at all; the discrepancy is documented on the variant, README and STATUS
   Rejected: registering `.../exif/2.32/` instead (the oracle would key it under a synthesized prefix, and no deployed writer uses it); registering both (a thirteenth variant is outside the record; additive later if a consumer needs it)
   Reverses: add a second variant for `.../exif/2.32/`
6. Sidecar citation and the exiv2 claim
   Taken:    the x:xmpmeta requirement stands as the record decides, but its citations are corrected: the vendored Part 3 (2020) has no §1.1.1 on sidecars (§1.1.1 is DNG) - the sidecar text is "External storage of metadata" in the Introduction, and Part 1 §7.3.3 is what gives x:xmpmeta its identifying purpose; exiv2's sidecar sniffer (`third_party/exiv2/src/xmpsidecar.cpp`, `isXmpType`) accepts a file starting with `<?xpacket` OR `<x:xmpmeta` after an optional declaration/BOM, so "exiv2 rejects a bare rdf:RDF sidecar" holds for a bare document but not for an xpacket-wrapped one; gamut is the stricter of the two, deliberately
   Rejected: weakening `read` to exiv2's sniff (an xpacket header alone identifies a packet, not XMP-in-general-XML)
   Reverses: accept `<?xpacket` without x:xmpmeta
7. XML declaration in `write`
   Taken:    `XmpSidecar::write` leads with `<?xml version="1.0" encoding="UTF-8"?>` - Part 3 asks that a sidecar be "a complete, well-formed XML document, including the leading XML declaration"; XMPCore accepts the file (pinned in tests/oracle.rs) and `XmpPacket::scan` locates the packet after it
   Rejected: omitting it as exiv2's writer does - the specification's recommendation wins over the oracle's habit
   Reverses: drop the constant
8. Error variant for a missing wrapper
   Taken:    `XmpError::Prohibited(String)` naming the document element found - the existing variant that maps to gamut_core InvalidInput (the record's classification) and reads as "a construct the spec does not allow here"; error.rs is outside the manifest, so no new variant
   Rejected: `MissingRdf` (wrong: rdf:RDF is present), `UnsupportedForm` (maps to Unsupported, not InvalidInput), a manifest revision for a dedicated variant (cosmetic; `XmpError` is non_exhaustive so it stays additive later)
   Reverses: add `XmpError::MissingXmpMeta` and switch `read` to it
9. Oracle normalization of the Darwin Core URI
   Taken:    pinned as an oracle limitation, with evidence: exiv2's `XmpProperties::registerNs`/`prefix` (third_party/exiv2/src/properties.cpp:4966, 5018) append `/` to a URI ending in neither `/` nor `#`, so XMPCore re-serializes `http://rs.tdwg.org/dwc/index.htm` as `.../index.htm/` while reading `Xmp.dwc.*` keys correctly; `xmpcore_output_uri` in tests/oracle.rs states the rule, the ALL-namespace and struct-field helpers parse back under it, and the dwc test asserts gamut's bytes carry the unslashed URI exiv2 documents (xmp.cpp:519); recorded in STATUS/README
   Rejected: registering the slashed URI in gamut (exiv2 documents the unslashed one, and the TDWG namespace has no trailing slash); dropping the parse-back for dwc silently
   Reverses: revert `xmpcore_output_uri` to the identity when the oracle stops normalizing
10. Resource-limit incantation
   Taken:    `prlimit --as=12000000000 <cmd>` under the same `systemd-run ... MemoryMax=16G` scope, in place of `sh -c 'ulimit -v 12000000; exec <cmd>'` - the worktree-isolation guard refuses `sh -c`; the address-space limit is identical (as #503's decision 9)
   Reverses: n/a
11. Per-schema oracle tests use documented property names
   Taken:    each per-namespace test writes a property from exiv2's table for that schema (exifEX:LensModel, aux:Lens, plus:Version, GPano:ProjectionType, MicrosoftPhoto:Rating, digiKam:ColorLabel, acdsee:caption, lr:hierarchicalSubject as a Bag) and the struct-only schemas their documented shape (mwg-rs:Regions/RegionList[1]/Name, mwg-kw:Keywords/Hierarchy[1]/Keyword, crss:SavedSettings/Name, dwc:Record/institutionID) - so the key lookup checks the prefix exiv2 binds, not only the URI; the existing ALL-loop test keeps its synthetic `GamutCheck` name; tests live in tests/oracle.rs beside the crate's other differential tests (docs/testing.md rule 3: either placement is legal; the assertions read only pub items)
   Rejected: `GamutCheck` in every per-schema test (would duplicate the ALL-loop test without adding the prefix check)
   Reverses: n/a
12. Commit series and base advance
   Taken:    two commits, each verified alone (schemas 1c57b2e: 95/4/19/5/3 tests, fmt-check, clippy; sidecars bc1102a: 104/4/21/5/4), then #503's advance (f809390 -> d6fd0a0, gamut-metadata only) merged by merge commit 6ffe02b as instructed, then the review repair 40db95e (decision 13); `convco check` passes on the mise range (7 commits) and the stacked range - merge commits are skipped
   Reverses: n/a
13. Findings of the lane's own review of its diff (all taken, commit 40db95e)
   Taken:    (a) `document_element_unless_xmpmeta` tested `Event::Eof` as a match arm - with that arm deleted the loop spins forever, so the mutant would time out (exit 3) instead of being caught; the check is now a guard before the match, as `reader.rs::build_tree` does, and the mutant no longer exists (18 mutants after, 20 before); (b) `struct_bag_item_field_survives_xmpcore` compared the re-parsed MWG structure under `ns.uri()` rather than `xmpcore_output_uri(ns)` like its sibling - identical for the two MWG URIs, wrong for a `dwc`-style one; now compares under the engine's output URI via a namespace-rename of the expected graph; (c) the `ALL.len() == 30` comment claimed "29 + two structure types", which does not add up - now "18 + 12"; (d) README implied exiv2 also requires x:xmpmeta - now states its sniffer accepts `<?xpacket` or `<x:xmpmeta` and that gamut is the stricter
   Rejected: none of the findings
   Reverses: n/a
14. Findings of the orchestrator's read-only review (all taken; commits fd6ba22, fce19d6)
   Taken:    (R1) `XmpError::MissingXmpMeta(String)` naming the element found, raised by `XmpSidecar::read` in place of `Prohibited` - the wrapper-less form is permitted by Part 1 §7.3.3 and emitted by `XmpWriter::wrap_xmpmeta(false)`, so `Prohibited` asserted a prohibition that does not exist; the enum is `#[non_exhaustive]`, so additive/minor. `crates/gamut-xmp/src/error.rs` was granted into the manifest for this, superseding decision 8. (R2) `from_uri` recognises `http://rs.tdwg.org/dwc/index.htm/` as a READ alias (`DWC_URI_TRAILING_SLASH`, a new `pub const`) while `uri()` keeps emitting the unslashed URI, so written bytes are unchanged and the alias is not an `ALL` entry (uniqueness unaffected, pinned); the nested oracle helper is driven once with `DarwinCore` so its re-namespacing runs with `from != to`, which no existing caller did. (R3) documented that via `read` only `rdf:RDF` is reachable at the sidecar root check, keeping the helper total and unit-tested. (R4) the `ALL.len() == 30` assertion is its own drift guard, named for registry-count drift rather than for the exiv2-parity twelve
   Rejected: widening `Prohibited`'s doc to cover the sidecar case - it would blur a variant with a precise, spec-anchored meaning; comparing under `ns.uri()` and keeping the alias out; threading the root element out of `find_rdf` - a wider blast radius in `reader.rs` for a bounded one-off cost
   Reverses: revert to `Prohibited` and widen its doc; drop the alias and compare under `ns.uri()`; thread the root out of `find_rdf`; fold the count assertion back
15. Design questions answered on review (decided by the orchestrator on review; no code change beyond docs)
   Taken:    (Q2) `XmpSidecar::read` stays STRICT - it requires `x:xmpmeta`, per Part 1 §7.3.3 and Part 3's sidecar clause and this record's decision 3 - and the docs now state precisely that exiv2's `isXmpType` accepts `<?xpacket` OR `<x:xmpmeta`, so a user whose file exiv2 reads and gamut rejects can see why; R1's variant is what makes that message honest. (Q3) `ExifEx` keeps `http://cipa.jp/exif/1.0/` (what exiv2 and XMPCore bind, verified in source), and issue #516 is filed to vendor CIPA DC-010 into `references/` and re-derive the URI from it, so the workspace's "specs in references/" rule stays visible rather than a registry entry silently resting on the oracle. (Q6) `XmpSidecar::read` keeps its unbounded `&[u8]` and parse-first order: `XmpMeta::from_packet` carries identical exposure on the same bytes, so the sidecar API adds no new hostile-input class
   Rejected: (Q2) matching exiv2's sniffer - being lenient would accept as a sidecar a packet the sidecar clause does not define; (Q3) switching to `.../exif/2.32/` on the strength of an annotation example, or registering both now; (Q6) treating a decode-limit convention as something this PR introduced
   Reverses: (Q2) accept `<?xpacket` without `x:xmpmeta`; (Q3) re-derive from DC-010 under #516; (Q6) add a crate-wide decode limit under a separate issue
16. Findings of the second read-only review - a documentation round (all taken; commit b3375af)
   Taken:    (F2/decision 5) the x:xmpmeta requirement STANDS, but stops citing the specification as its authority. The vendored Part 1 SS7.3.3 reads "An optional x:xmpmeta element may be placed around the rdf:RDF element" and "An XMP processor should tolerate an x:xmpmeta element in any input" - permission and tolerance, never a requirement - and the vendored Part 3's "External storage of metadata" bullets (complete well-formed XML document with the leading XML declaration, .xmp extension, application/rdf+xml, written "as though it were embedded and then had the XMP packets extracted") never mention the element; `xmpmeta` occurs in Part 3 exactly once, inside an SVG example. So `read` rejects a spec-conformant sidecar, and README/STATUS/sidecar.rs now say exactly that: it is gamut's own rule, stricter than the specification, made because SS7.3.3 gives the element its identifying purpose and a standalone .xmp file is the general XML text it identifies XMP within. (F1/decision 6) the parity sentence at sidecar.rs is deleted; exiv2's `isXmpType` accepts `<?xpacket` OR `<x:xmpmeta`, so an xpacket-wrapped bare rdf:RDF is a sidecar to exiv2 and MissingXmpMeta("RDF") here - verified by executing both sides. (F3/decision 1) the Darwin Core alias is documented as what it is: it maps the slashed URI to the `dwc` PREFIX, and read does not canonicalize, so `get_text(WellKnownNs::DarwinCore.uri(), ...)` returns None for such a graph and a caller resolves by the URI its packet carries; the qualification is carried to `from_uri`'s own doc, which is where a caller meets the trap. (decision 3) the law is pinned: `from_uri(u).map(uri) == Some(u)` asserted over every ALL entry plus the one alias exception, so the exception is documented rather than silent. (decision 4) the unbounded-read residual moved from this body into STATUS.md's "Intentional skips", where it survives the merge. (F4/decision 7) `text_property_survives_xmpcore` reads back under `xmpcore_output_uri(ns)` like its sibling. (F6) the `assert_ne!` implied by the `assert_eq!` above it is dropped. (F5) both stale statements in this body corrected against what was re-verified here: the error variant has been `MissingXmpMeta` since fd6ba22, and CI is green at fce19d6, not red for #517.
   Rejected: canonicalizing the slashed URI on read (it would rewrite bytes exiv2 wrote; identity preservation is this crate's posture) - filed as #547 instead, naming the asymmetry it leaves; splitting the alias out of `from_uri` into a separate entry point (it would push the burden onto every caller); relaxing `read` to exiv2's sniff or to the specification's optionality (the requirement is settled by decisions 3 and 15/Q2; only its justification was wrong)
   Reverses: canonicalize on read under #547; drop the alias; accept `<?xpacket` without `x:xmpmeta`
17. Closing-round repairs (three findings, all taken; commits 825353d, 490dc57, 9909bb3)
   Taken:    (L1) the `from_uri` test is made to kill something. Its previous form looped over ALL asserting `from_uri(u).map(uri) == Some(u)`, which `uri_and_prefix_are_exact_and_round_trip` already asserts more strongly, so it killed nothing and its NAME went unpinned - a second, undocumented read alias added to `from_uri` passed the whole suite. It now closes `from_uri` over the alias family exiv2 actually generates (`XmpProperties::registerNs` appends `/` to a URI ending in neither `/` nor `#`), asserting that no schema but Darwin Core answers to either the slashed or the unslashed form of its own URI; verified in both directions, and verified to be the ONLY test that fails on either mutation. (L2) the Part 3 quotation is restored in full - the dropped phrase was "and catenated by a postprocessor", the one phrase that raises the question - and the truncation it exposes is documented where a caller meets it (`XmpSidecar::read`, the module docs, README, STATUS), established by executing it rather than by reading the code, and FILED as #562 rather than decided here. (L3) the `WellKnownNs::DarwinCore` variant doc, the likeliest rustdoc landing point for a caller about to call `.uri()`, now carries the alias caveat. Plus: the README line-wrap artefact this branch introduced is fixed (and the same artefact in STATUS.md), and the false premise in this body's Validation section - that a warnings-as-errors documentation build guards this repository - is retracted there against evidence, and replaced with the workflow's own wording, re-read in this run
   Rejected: adding a test that pins the catenation truncation - the same standard L1 was raised under applies to it: `split_packet`'s two trailer branches are already covered by `scans_writable_wrapper_with_exact_padding` and `header_without_trailer_is_read_only`, a catenation fixture kills no mutant either of those does not, and a test that kills nothing is what L1 objected to. The behaviour is pinned by documentation in four places and by #562, not by a redundant test. Also rejected: renaming the strengthened test (issue #547 cites it by name as the pin, and this lane may not edit an existing issue, so a rename would strand that citation); wiring the documentation gate here (repository-wide, unrelated to this subject, and already filed as #549); changing the truncation behaviour in this PR
   Reverses: revert the test to the ALL round-trip loop; drop the alias caveat from the variant doc; decide #562 one way and delete the truncation paragraphs

Unresolved review notes

  • Decode limits for gamut-xmp are a crate-wide question, not one this PR opens. XmpSidecar::read takes an unbounded &[u8] and parses before checking the wrapper, but XmpMeta::from_packet has identical exposure on the same bytes, so the sidecar API adds no new hostile-input class. Whether the crate should grow a decode-limit convention like gamut-png's is for a human to decide, for the crate as a whole (decision 15, Q6). Recorded in crates/gamut-xmp/STATUS.md's "Intentional skips" (decision 16) so it survives this PR's merge rather than living only here.

  • exifEX's URI rests on the oracle, not on a vendored specification — tracked in the filed issue references/exif: vendor CIPA DC-010 and re-derive the exifEX XMP namespace URI from it #516 (decision 15, Q3). The registry entry is what exiv2 and XMPCore bind, and the oracle test pins it; what is missing is CIPA DC-010 under references/.

  • The Darwin Core read alias resolves a prefix, not a URI, and this PR deliberately leaves it that way (decision 16). A consumer that needs WellKnownNs::DarwinCore.uri() to resolve a graph parsed from an XMPCore-written packet needs issue gamut-xmp: consider canonicalizing a well-known namespace URI on read (Darwin Core trailing slash) #547 decided first; the trap is documented on from_uri, in README and in STATUS, and the law and its one exception are pinned by a test.

  • A read alias outside the slash family would still be unpinned. The strengthened test closes from_uri over the family exiv2's registerNs generates — a registry URI with a trailing slash added or removed — because that is the family the one real alias comes from and the only one a mutation of this code plausibly produces. An alias of some entirely different shape (a different host, a version bump in the path) would pass it. Pinning that would need from_uri to be a closed lookup rather than a chain of or_else, which is a refactor of shipped public behaviour and not a closing-round repair.

  • The catenated-sidecar truncation is documented, not fixed (gamut-xmp: XmpSidecar::read silently truncates a catenated multi-packet sidecar to its first packet #562). A caller merging a sidecar into an image can still lose properties with no signal; the documentation now says so at XmpSidecar::read, but until gamut-xmp: XmpSidecar::read silently truncates a catenated multi-packet sidecar to its first packet #562 is decided the behaviour is what it is. XmpMeta::from_packet has the same single-packet behaviour for embedded payloads, where a catenation is not something a container hands over — gamut-xmp: XmpSidecar::read silently truncates a catenated multi-packet sidecar to its first packet #562 notes that any fix has to decide about both.

  • A broken intra-doc link anywhere in this workspace still ships green (No gate compiles a README code block or fails on a broken rustdoc link #549). The RUSTDOCFLAGS="-D warnings" cargo doc run recorded in Validation is this lane's own, not a gate: it protects this diff and nothing else, and it will not protect the next one.

`WellKnownNs` grows from 18 to 30 entries - exifEX, aux, plus, mwg-rs,
mwg-kw, GPano, lr, MicrosoftPhoto, digiKam, acdsee, crss and dwc - the
schemas exiv2 documents (https://exiv2.org/metadata.html) that the
registry lacked. Each URI is cited on its variant; the vendored
reference for all twelve is exiv2's own registry
(third_party/exiv2/src/properties.cpp). The registry stays a registry:
it fixes the prefix a schema serializes under and interprets no value.

exifEX is registered under http://cipa.jp/exif/1.0/ (CIPA DC-010, the
URI the reference engine and deployed writers use); the vendored Exif
3.0 text binds the same prefix to .../exif/2.32/ in its annotation
examples, which is documented and not registered.

tests/oracle.rs gains one test per schema, reading a documented
property back from Adobe XMPCore by its `Xmp.<prefix>.<name>` key -
a lookup that fails for a wrong URI or a wrong prefix. One oracle
normalization is pinned as such rather than hidden: exiv2 appends `/`
to a namespace URI ending in neither `/` nor `#` when registering it
with XMPCore (XmpProperties::registerNs), so the engine re-serializes
Darwin Core as .../index.htm/; gamut writes the URI exiv2 documents.

Refs #421
`XmpSidecar::read(&[u8])` and `XmpSidecar::write(&XmpMeta)` are the
bytes of a standalone `.xmp` file, the interchange RAW workflows keep
beside a non-extensible raw image (Adobe XMP Part 3, "External storage
of metadata"). `read` accepts everything `XmpMeta::from_packet` does -
XML declaration, BOM, packet wrapper or bare - but requires the
`x:xmpmeta` document element, resolved by namespace rather than
prefix: Part 1 s7.3.3 gives that element exactly one purpose,
identifying XMP inside general XML text, which a standalone file is;
exiv2's sidecar sniffer keys on the same `<?xpacket`/`<x:xmpmeta`.
A bare `rdf:RDF` document is rejected with `XmpError::Prohibited`
naming the element found. `write` emits the XML declaration Part 3
asks for, then a read-only, unpadded packet in canonical form inside
`x:xmpmeta` - byte-stable per graph. No filesystem API; the
`photo.xmp`-beside-`photo.dng` convention is documented, not enforced.

tests/oracle.rs pins both directions against Adobe XMPCore: the file
gamut writes is accepted and its properties read back by key, and the
engine's own `<?xpacket?>`/`<x:xmpmeta>` serialization - what exiv2
stores in a .xmp file - is read as a sidecar.

Closes #421
Review findings on the lane's own diff. The sidecar document-element
scan tested `Event::Eof` as a match arm; with that arm removed the loop
spins forever, so the mutant would time out instead of being caught.
The check is now a guard before the match, as `reader.rs` does, and end
of input is terminal regardless of the arms. The MWG oracle helper
compares the re-parsed structure under the engine's output URI like its
sibling, so it stays correct for a URI exiv2 normalizes; a count comment
in the registry test now adds up; README states that exiv2's sidecar
sniffer accepts `<?xpacket` or `<x:xmpmeta` and that gamut is stricter.

Refs #421
Review finding on the lane's own diff. `XmpSidecar::read` reported a
document element other than `x:xmpmeta` as `XmpError::Prohibited`, whose
documented meaning is a construct the *specification* prohibits (Part 1
§7.8/§7.9.3: `rdf:_n` items, `rdf:value` carrying `xml:lang`). A bare
`rdf:RDF` document is not such a construct - §7.3.3 makes the wrapper
optional, `XmpWriter::wrap_xmpmeta(false)` emits exactly that form, and
`XmpMeta::from_packet` reads it - so the message asserted a prohibition
that does not exist.

`XmpError::MissingXmpMeta` now names the element found. The enum is
`#[non_exhaustive]`, so this is additive. The docs state precisely where
gamut is stricter than its oracle: exiv2's sidecar sniffer (`isXmpType`)
accepts a `.xmp` file starting with `<?xpacket` *or* `<x:xmpmeta`, so a
file exiv2 reads as a sidecar and gamut rejects is expected, and the
caller reads those bytes with `XmpMeta::from_packet`.

Also records why the sidecar root check is a second lex: reached through
`read` it can only ever see `rdf:RDF`, because the reader admits exactly
one document element over a tree with a single root, so the namespace
comparison, the `Empty` arm and the end-of-input error are unreachable
from that path. The helper stays total over any XML and unit-tested for
the cases `read` cannot produce, rather than threading the root element
out of `find_rdf` and widening the blast radius in `reader.rs`.

Refs #421
Review finding on the lane's own diff. exiv2 appends `/` to any
namespace URI ending in neither `/` nor `#` before registering it with
XMPCore (`third_party/exiv2/src/properties.cpp`), and Darwin Core is the
only registered schema whose URI ends in neither. A packet the reference
engine wrote - including a sidecar - therefore declares
`http://rs.tdwg.org/dwc/index.htm/`, which `from_uri` did not recognise,
so such a graph re-serialized under a synthesized `ns1` prefix instead
of `dwc`. Prefixes are non-semantic (Part 1 §6.2), so this was a
round-trip fidelity gap rather than a correctness defect - but one the
oracle test's own normalization helper hid from the suite.

`from_uri` now recognises the slashed form as a read-only alias,
`DWC_URI_TRAILING_SLASH`. `uri()` still emits the unslashed URI exiv2
documents, so gamut's bytes are unchanged, and the alias is not an `ALL`
entry, so iteration and the registry's URI/prefix uniqueness are
untouched.

The nested oracle helper is now driven once with Darwin Core, so its
expected-graph re-namespacing runs with source and target actually
differing; both existing callers pass a URI the engine leaves alone,
which left that whole recursion a verified no-op.

Also splits the registry-count assertion into its own drift guard named
for what it guards, since every future addition to the registry edits
it, and records in STATUS that `exifEX`'s URI rests on the oracle rather
than on a vendored specification (issue #516).

Refs #421
`XmpSidecar::read` requires `x:xmpmeta`, and the docs argued that follows
from Part 1 §7.3.3. It does not: §7.3.3 reads "An optional x:xmpmeta
element may be placed around the rdf:RDF element" and asks a processor to
"tolerate" one, and Part 3's external-storage bullets never mention the
element — `xmpmeta` occurs in Part 3 once, in an SVG example. The
requirement stands, but as gamut's own rule, stricter than the
specification and stated as such.

The same passages claimed parity with exiv2 where there is none: exiv2's
`isXmpType` keys on `<?xpacket` *or* `<x:xmpmeta`, so an xpacket-wrapped
bare `rdf:RDF` is a sidecar to exiv2 and `MissingXmpMeta("RDF")` here.

The Darwin Core read alias was likewise oversold. It maps the slashed URI
XMPCore emits to the `dwc` prefix; it does not rewrite the URI, so a graph
parsed from such a packet keeps the slashed URI and
`get_text(WellKnownNs::DarwinCore.uri(), …)` returns `None` for it. Say
that where a caller meets it — `from_uri`, README and STATUS — and pin the
law it breaks: `from_uri(u).map(uri) == Some(u)` holds for every registry
entry and for no other URI but the one documented alias. Canonicalizing on
read is filed as #547.

Also: move the unbounded-read residual into STATUS's intentional skips so
it survives the merge, read back the per-schema oracle property under
`xmpcore_output_uri` as its sibling helper already does, and drop an
`assert_ne!` implied by the `assert_eq!` above it.

Refs #421
…I differently

The variant's own rustdoc is where a caller about to call `.uri()` lands, and
it said nothing about the trailing-slash read alias. A caller reading a packet
XMPCore wrote would find `get_text(DarwinCore.uri(), ..)` returning None with
no clue why, because reading does not canonicalize and the graph keeps the
slashed URI the packet declared.

State it where they meet it, and point at `from_uri` for the whole rule.

Refs #421
`the_trailing_slash_alias_is_the_only_uri_from_uri_does_not_hand_back` looped
over ALL asserting `from_uri(u).map(uri) == Some(u)` — which
`uri_and_prefix_are_exact_and_round_trip` already asserts more strongly, so the
test killed nothing, and its name's claim (that the Darwin Core alias is the
*only* exception) went unpinned: a second, undocumented read alias added to
`from_uri` passed the whole suite.

Pin the exclusivity instead. exiv2's `XmpProperties::registerNs` appends `/` to
a URI ending in neither `/` nor `#`, so an alias is a registry URI with a
trailing slash added or removed; assert that no schema but Darwin Core answers
to either form of its own URI. Verified against both directions of that family:
with the test present each fails it and nothing else (106 passed, 1 failed);
with the test deleted both pass the suite (106 passed, 0 failed).

Refs #421
The Part 3 bullet these docs quote was cut short. It reads "as though it were
embedded and then had the XMP packets extracted **and catenated by a
postprocessor**" — and the dropped phrase is the one that matters, because it
says a conforming producer may hand over a .xmp file holding several
<?xpacket?> packets end to end.

XmpSidecar::read does not read one. XmpPacket::scan ends the body at the next
<?xpacket instruction, which in a catenated file is the first packet's own
trailer, so everything after it is discarded with no error: two
XmpSidecar::write outputs concatenated read back as the properties of the first
alone. Adobe XMPCore rejects those same bytes.

Restore the quotation and state the truncation on XmpSidecar::read, in the
module docs, in STATUS.md and in README.md. The behaviour predates this branch
and is not changed here: rejecting, merging every packet (which needs a
conflict rule Part 3 does not supply) and keeping the truncation are three
different products, and that choice is #562.

Also re-wrap the sidecar prose in README.md and STATUS.md, which an earlier
edit left short of the column the rest of both files keep.

Refs #421, #562
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant