You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
image-codecs: delegate metadata edits to craft-metadata - #1261
Make craft-image-codecs delegate EXIF and XMP resolution parsing/export editing to craft-metadata::edit.
Preserve the existing codec-facing API with thin wrappers.
Remove the duplicate local resolution/editor implementation (543 lines).
Keep format-specific resolution precedence in the codec callers.
Correct the workspace SVG depth test to set the dedicated reference-expanded tree_depth limit it asserts.
Evidence
The exact #1249 macOS and Windows workspace gates failed resolution::exports_drop_the_source_exif_thumbnail after 861 integration tests passed. The original reproduction on #1247's base (e2dc681, now 313 commits behind current main) also failed on all three platforms, but that stale-base result alone does not establish the current-main state.
An exact current-main run on b77af389 using cargo test --locked -p craft-image-codecs --test it resolution::exports_drop_the_source_exif_thumbnail -- --exact --nocapture fails (0/1): the exported EXIF still contains the stale thumbnail bytes. The identical focused integration test on this PR's metadata-cutover candidate passed (1/1). craft-image-codecs previously cleared the EXIF IFD1 pointer but left old JPEG thumbnail bytes; the shared craft-metadata::edit::export_exif implementation also clears those bytes, and these wrappers now route codec export through it.
The first full workspace gate on the candidate also exposed an existing test configuration mismatch in craft-svg::tree::tests::every_limit_refuses_the_file_past_it: XML nesting is controlled by Limits.depth, while resolved <use> nesting is controlled by Limits.tree_depth. The assertion expected TooDeep(24) for reference expansion but the test had only set the XML limit. The test now sets tree_depth: 24 and retains the expected error assertion.
No actionable comments were generated in the recent review.
Walkthrough
The image codecs crate adds craft-metadata as a dependency. Its resolution module delegates metadata reading, editing, and export operations to craft_metadata::edit, and adds a public resolution wrapper.
The crate adds craft-metadata as a workspace dependency. The resolution module delegates reading, editing, and export operations to craft_metadata::edit, and adds the public resolution wrapper. Existing wrapper APIs retain their signatures.
Image codecs now use the shared metadata crate to read and edit resolution metadata. This removes duplicate code and fixes stale EXIF thumbnails left in exported images. Existing APIs keep their signatures, and no concrete regression was found. Merge readiness depends on the integration tests passing in CI.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title "image-codecs: delegate metadata edits to craft-metadata" matches the main change. The PR moves EXIF, Photoshop ResolutionInfo, and XMP resolution reading, editing, and export from local cod…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Full details: Title check
Explanation
The title "image-codecs: delegate metadata edits to craft-metadata" matches the main change. The PR moves EXIF, Photoshop ResolutionInfo, and XMP resolution reading, editing, and export from local code in crates/image-codecs/src/resolution.rs to craft_metadata::edit. The summary also shows the crate dependency was added to Cargo.toml, and about 543 lines of local implementation were removed. The title is short, specific, and does not describe unrelated work.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between b77af38 and 9481cec.
Files ignored due to path filters: 1
Cargo.lock is excluded by !**/*.lock
Files selected for processing: 2
crates/image-codecs/Cargo.toml
crates/image-codecs/src/resolution.rs
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
craft-image-codecsdelegate EXIF and XMP resolution parsing/export editing tocraft-metadata::edit.tree_depthlimit it asserts.Evidence
The exact #1249 macOS and Windows workspace gates failed
resolution::exports_drop_the_source_exif_thumbnailafter 861 integration tests passed. The original reproduction on #1247's base (e2dc681, now 313 commits behind current main) also failed on all three platforms, but that stale-base result alone does not establish the current-main state.An exact current-main run on
b77af389usingcargo test --locked -p craft-image-codecs --test it resolution::exports_drop_the_source_exif_thumbnail -- --exact --nocapturefails (0/1): the exported EXIF still contains thestalethumbnail bytes. The identical focused integration test on this PR's metadata-cutover candidate passed (1/1).craft-image-codecspreviously cleared the EXIF IFD1 pointer but left old JPEG thumbnail bytes; the sharedcraft-metadata::edit::export_exifimplementation also clears those bytes, and these wrappers now route codec export through it.The first full workspace gate on the candidate also exposed an existing test configuration mismatch in
craft-svg::tree::tests::every_limit_refuses_the_file_past_it: XML nesting is controlled byLimits.depth, while resolved<use>nesting is controlled byLimits.tree_depth. The assertion expectedTooDeep(24)for reference expansion but the test had only set the XML limit. The test now setstree_depth: 24and retains the expected error assertion.Validation
cargo metadata --locked --format-version 1passed.rustup run stable cargo fmt --all -- --checkpassed.git diff --checkpassed.cargo test --locked -p craft-image-codecs --test it resolution::exports_drop_the_source_exif_thumbnail -- --exact --nocapture(1/1).