Skip to content

Write XMP sidecars atomically and keep virtual copies off the master XMP - #1967

Open
apweisskopf wants to merge 2 commits into
CyberTimon:mainfrom
apweisskopf:xmp/1-safety
Open

apweisskopf wants to merge 2 commits into
CyberTimon:mainfrom
apweisskopf:xmp/1-safety

Conversation

@apweisskopf

Copy link
Copy Markdown
Contributor

Description

Two fixes that stop RapidRAW from damaging XMP sidecars. Part of #1945.

  • A crash or full disk during a save could leave a broken or empty .xmp. The XMP is now written to a temp file and renamed, as .rrcache already does with the tempfile crate. No new dependency.
  • A virtual copy shares the master's IMG.xmp. Rating or tagging the copy overwrote the master's values. Copies now leave the XMP alone.

The content of the written XMP does not change.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Performance improvement
  • Code refactoring
  • Documentation update
  • UI/UX improvement
  • Build/CI or Dependency update

Changes Made

  • write_atomic: temp file in the same folder, fsync, rename
    • an existing file keeps its mode, symlink and name case
    • unchanged content is not written, so the mtime only moves on real changes
    • write errors are logged with the path instead of dropped
  • new sidecars are built in memory and written once
  • virtual copies never write the XMP and skip the XMP read on load

Testing

  • These changes were tested locally by a human and confirmed to work.
  • I haven't added any automated tests to the code because the codebase currently lacks a test suite.

Checked by hand on this branch:

  • rating an image with an existing Lightroom-style XMP updates the rating, keeps unknown attributes and leaves no temp file
  • rating a virtual copy leaves the master's XMP byte for byte unchanged

Test Configuration:

  • OS: macOS 27.0.1
  • Hardware: Apple M4 Pro

Checklist

  • My code follows the project's code style
  • I haven't added unnecessary AI-generated code comments
  • My changes generate no new warnings or errors

Additional Notes

One behaviour change: a writable XMP in a read-only folder is no longer written, because the temp file cannot be created there. It is logged. An in-place fallback would bring back the broken write.

Other known limits are in the commit message: hard links break, xattrs are lost, and a crash before the rename leaves a .rapidraw-*.xmp.tmp file.

AI Disclaimer:

Please state the involvement of AI in this PR:

  • This PR is created by an AI agent
  • This PR is mostly AI-generated but edited/merged together by a human
  • This PR was handwritten with AI assistance (spell check, logic suggestions, error resolving)
  • This PR contains only blood, sweat, and coffee (AI-free)

fs::write truncates in place. A crash or full disk mid-write left a
broken sidecar, and the error was dropped.

Write to a temp file in the same folder, then rename, as .rrcache
already does with the tempfile crate. The rename keeps what fs::write
kept:
- mode, symlink and name case of an existing file
- the permission checks, through a write probe

Unchanged content is not written, so the mtime only moves on real
changes.

Known limits:
- a writable XMP in a read-only folder is no longer written. An
  in-place fallback would bring back the broken write
- hard links break, xattrs are lost
- a crash before the rename leaves a .rapidraw-*.xmp.tmp file
- a dangling symlink is skipped

Part of CyberTimon#1945.
A virtual copy shares the master's IMG.xmp. Every rating, label or tag
change on a copy overwrote the master's values.

- only the primary sidecar writes the XMP, the same rule as
  write_sidecar
- copies also skip the XMP read on load. Otherwise a value removed on
  the copy comes back from the master's XMP

Known limit: a copy of a master without .rrdata never sees the
master's XMP values.

Part of CyberTimon#1945.
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