Merge main into fix/issue-101-torn-append

# Conflicts:
#	docs/whats-new.md
This commit is contained in:
Brandon Thomas 2026-10-05 18:52:48 -04:00
commit 0f49ebf8f0
40 changed files with 2530 additions and 240 deletions

View File

@ -533,11 +533,17 @@ fn render(args: &[String]) -> Result<(), String> {
.ok_or_else(|| format!("{output}: unknown extension (use .jpg .png .tif .webp .avif .dng or --opt format=…)"))?;
}
let e = export_photo(&mut s, lightcraft_engine::catalog::PhotoId(id), &o, 1)?;
std::fs::write(&output, &e.bytes).map_err(|err| format!("{output}: {err}"))?;
for (ext, bytes) in &e.sidecars {
let sc = Path::new(&output).with_extension(ext);
std::fs::write(&sc, bytes).map_err(|err| format!("{}: {err}", sc.display()))?;
eprintln!("lightcraft-cli: wrote {}", sc.display());
// never over the input (or its sidecar), however it is spelled: `render IMG.jpg -o IMG.jpg`
let sidecars: Vec<(String, &Vec<u8>)> =
e.sidecars.iter().map(|(ext, bytes)| (Path::new(&output).with_extension(ext).to_string_lossy().to_string(), bytes)).collect();
let guard = s.original_guard();
for p in std::iter::once(&output).chain(sidecars.iter().map(|(p, _)| p)) {
guard.check(Path::new(p)).map_err(|err| format!("render: {err}"))?;
}
lightcraft_engine::export::write_file(&output, &e.bytes)?;
for (sc, bytes) in &sidecars {
lightcraft_engine::export::write_file(sc, bytes)?;
eprintln!("lightcraft-cli: wrote {sc}");
}
eprintln!("lightcraft-cli: wrote {output} ({}×{})", e.width, e.height);
Ok(())
@ -593,18 +599,8 @@ fn snapshot(args: &[String]) -> Result<(), String> {
eprintln!("lightcraft-cli snapshot: imported {n} files in {:.0} ms", ti.elapsed().as_secs_f64() * 1e3);
}
let services = lightcraft_ui_egui::Services {
write_shared: Some(std::sync::Arc::new(|p: &str, b: &[u8]| {
if let Some(dir) = Path::new(p).parent().filter(|d| !d.as_os_str().is_empty()) {
std::fs::create_dir_all(dir).map_err(|e| e.to_string())?;
}
std::fs::write(p, b).map_err(|e| format!("{p}: {e}"))
})),
write: Some(Box::new(|p: &str, b: &[u8]| {
if let Some(dir) = Path::new(p).parent().filter(|d| !d.as_os_str().is_empty()) {
std::fs::create_dir_all(dir).map_err(|e| e.to_string())?;
}
std::fs::write(p, b).map_err(|e| format!("{p}: {e}"))
})),
write_shared: Some(std::sync::Arc::new(lightcraft_engine::export::write_file)),
write: Some(Box::new(lightcraft_engine::export::write_file)),
png: Some(Box::new(|img: &lightcraft_raster::Rgba8| {
lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(img), &lightcraft_codecs::EncodeMeta::default()).unwrap_or_default()
})),

View File

@ -86,6 +86,24 @@ fn render_subcommand() {
assert!(String::from_utf8_lossy(&o.stderr).contains("unknown control"));
}
/// Issue #93: `render IMG -o IMG` replaced its own input with the render.
#[test]
fn render_refuses_to_overwrite_its_input() {
let input = tmp("self-in.png");
gradient_png(&input);
let before = std::fs::read(&input).unwrap();
let dir = input.parent().unwrap();
std::fs::create_dir_all(dir.join("sub")).unwrap();
let other_spelling = dir.join("sub/../self-in.png");
for out in [&input, &other_spelling] {
let o =
Command::new(BIN).args(["render", input.to_str().unwrap(), "-o", out.to_str().unwrap(), "--set", "light.exposure=1"]).output().unwrap();
assert!(!o.status.success());
assert!(String::from_utf8_lossy(&o.stderr).contains("never writes over an original"), "{}", String::from_utf8_lossy(&o.stderr));
}
assert_eq!(std::fs::read(&input).unwrap(), before, "the input is untouched");
}
#[test]
fn render_export_options() {
let input = tmp("o-in.png");

View File

@ -209,18 +209,9 @@ fn services() -> Services {
.save_file()
.map(|p| p.to_string_lossy().to_string())
})),
write_shared: Some(std::sync::Arc::new(|p: &str, b: &[u8]| {
if let Some(dir) = std::path::Path::new(p).parent().filter(|d| !d.as_os_str().is_empty()) {
std::fs::create_dir_all(dir).map_err(|e| e.to_string())?;
}
std::fs::write(p, b).map_err(|e| e.to_string())
})),
write: Some(Box::new(|p: &str, b: &[u8]| {
if let Some(dir) = std::path::Path::new(p).parent().filter(|d| !d.as_os_str().is_empty()) {
std::fs::create_dir_all(dir).map_err(|e| e.to_string())?;
}
std::fs::write(p, b).map_err(|e| e.to_string())
})),
// atomic (temp file + sync + rename): a failed write never leaves a truncated file
write_shared: Some(std::sync::Arc::new(lightcraft_engine::export::write_file)),
write: Some(Box::new(lightcraft_engine::export::write_file)),
png: Some(Box::new(|img: &lightcraft_raster::Rgba8| {
lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(img), &lightcraft_codecs::EncodeMeta::default()).unwrap_or_default()
})),

View File

@ -15,6 +15,7 @@ pub mod local;
pub mod model;
pub mod query;
pub mod rules;
pub mod safe_file;
pub mod stacks;
pub mod store;

View File

@ -0,0 +1,317 @@
//! Crash-safe writes of user-visible files: exports, renders, DNG conversions, smart previews and
//! XMP sidecars all go through here (the catalog's own files use [`crate::store::FsStore`]).
//!
//! - [`write_atomic`] replaces a file so that after a crash, a full disk or an unplugged drive
//! either the old or the complete new content is there — never a truncated mix. The content
//! goes to a new temp file in the same folder (unique name, `create_new`), is synced to disk and
//! renamed over the target; then the folder is synced (POSIX). Any failure removes the temp
//! file and leaves the target as it was.
//! - [`write_new`] / [`write_new_unique`] write a file that must not exist yet: the same temp file
//! and sync, plus a read-back check, and the file then appears under its final name **without
//! replacing anything** (a hard link, which fails on a taken name; a check-then-rename where the
//! volume has no hard links).
//! - [`same_file`] tells whether two paths name the same file on disk (device + inode on Unix,
//! resolved paths elsewhere), for "is this target an original?" checks.
use std::cell::Cell;
use std::fs::{self, File, OpenOptions};
use std::io::{self, Write};
use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicU64, Ordering};
thread_local! {
static FAIL_AFTER: Cell<Option<u64>> = const { Cell::new(None) };
}
/// Fault injection for tests (in any crate): while the returned guard lives, every write made by
/// this module **on the calling thread** fails once `bytes` bytes of a file were written — as a
/// full disk or an unplugged drive would.
#[doc(hidden)]
pub fn fail_writes_after(bytes: u64) -> FaultGuard {
FAIL_AFTER.with(|c| c.set(Some(bytes)));
FaultGuard(())
}
/// Ends a [`fail_writes_after`] when dropped.
#[doc(hidden)]
pub struct FaultGuard(());
impl Drop for FaultGuard {
fn drop(&mut self) {
FAIL_AFTER.with(|c| c.set(None));
}
}
/// Counts what is written; fails past an injected limit.
struct Limited<W> {
inner: W,
n: u64,
limit: Option<u64>,
}
impl<W: Write> Write for Limited<W> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
let buf = match self.limit {
Some(l) => {
let room = usize::try_from(l.saturating_sub(self.n)).unwrap_or(usize::MAX);
if room == 0 && !buf.is_empty() {
return Err(io::Error::other("write failed (injected fault)"));
}
buf.get(..room.min(buf.len())).unwrap_or(buf)
}
None => buf,
};
let k = self.inner.write(buf)?;
self.n += k as u64;
Ok(k)
}
fn flush(&mut self) -> io::Result<()> {
self.inner.flush()
}
}
fn parent_of(path: &Path) -> &Path {
path.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new("."))
}
/// Make a rename or a new name in `dir` durable (POSIX). Opening a directory fails on Windows,
/// where NTFS journals the rename itself, so errors are ignored.
pub fn sync_dir(dir: &Path) {
if let Ok(d) = File::open(dir) {
let _ = d.sync_all();
}
}
/// A new, empty temp file next to `path` (hidden, unique in this process and folder).
fn create_temp(path: &Path) -> io::Result<(PathBuf, File)> {
static N: AtomicU64 = AtomicU64::new(0);
if path.file_name().is_none() {
return Err(io::Error::new(io::ErrorKind::InvalidInput, format!("{} is not a file path", path.display())));
}
let dir = parent_of(path);
for _ in 0..1000 {
let tmp = dir.join(format!(".lightcraft-{}-{}.tmp", std::process::id(), N.fetch_add(1, Ordering::Relaxed)));
match OpenOptions::new().write(true).create_new(true).open(&tmp) {
Ok(f) => return Ok((tmp, f)),
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => continue,
Err(e) => return Err(e),
}
}
Err(io::Error::new(io::ErrorKind::AlreadyExists, format!("no free temp file name in {}", dir.display())))
}
/// Write a temp file next to `path` with what `fill` writes, synced to disk. On failure the temp
/// file is removed. Returns its path and length.
fn write_temp(path: &Path, fill: &mut dyn FnMut(&mut dyn Write) -> io::Result<()>) -> io::Result<(PathBuf, u64)> {
let (tmp, file) = create_temp(path)?;
let r = (|| {
let limit = FAIL_AFTER.with(Cell::get);
let mut w = Limited { inner: io::BufWriter::with_capacity(1 << 20, file), n: 0, limit };
fill(&mut w)?;
w.flush()?;
let n = w.n;
let f = w.inner.into_inner().map_err(|e| e.into_error())?;
f.sync_all()?;
Ok(n)
})();
match r {
Ok(n) => Ok((tmp, n)),
Err(e) => {
let _ = fs::remove_file(&tmp);
Err(e)
}
}
}
/// Replace (or create) `path` with `data` atomically and durably; see the module docs. The
/// folder must exist.
pub fn write_atomic(path: &Path, data: &[u8]) -> io::Result<()> {
write_atomic_with(path, &mut |w| w.write_all(data)).map(|_| ())
}
/// [`write_atomic`] with streamed content: `fill` writes it (an error aborts and leaves the old
/// file). Returns the length written.
pub fn write_atomic_with(path: &Path, fill: &mut dyn FnMut(&mut dyn Write) -> io::Result<()>) -> io::Result<u64> {
let (tmp, n) = write_temp(path, fill)?;
if let Err(e) = fs::rename(&tmp, path) {
let _ = fs::remove_file(&tmp);
return Err(e);
}
sync_dir(parent_of(path));
Ok(n)
}
/// Write `data` as the new file `path`: never replaces an existing file (`AlreadyExists`), and
/// the file only appears once it is complete, synced and read back identical.
pub fn write_new(path: &Path, data: &[u8]) -> io::Result<()> {
write_new_unique(&mut std::iter::once(path.to_path_buf()), data).map(|_| ())
}
/// [`write_new`] under the first free name of `candidates` (all in the same folder, e.g.
/// `IMG_1.dng`, `IMG_1-2.dng`, …): the file is written once and then given the first name that
/// is not taken. Returns that name; `AlreadyExists` when every candidate is taken.
pub fn write_new_unique(candidates: &mut dyn Iterator<Item = PathBuf>, data: &[u8]) -> io::Result<PathBuf> {
let mut candidates = candidates.peekable();
let Some(first) = candidates.peek().cloned() else {
return Err(io::Error::new(io::ErrorKind::InvalidInput, "no file name to write"));
};
let (tmp, _) = write_temp(&first, &mut |w| w.write_all(data))?;
let r = (|| {
if !same_content(&tmp, data)? {
return Err(io::Error::other("the written file reads back different (failing drive or connection?)"));
}
for path in candidates.by_ref().take(100_000) {
match publish_new(&tmp, &path) {
Ok(()) => {
sync_dir(parent_of(&path));
return Ok(path);
}
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => continue,
Err(e) => return Err(e),
}
}
Err(io::Error::new(io::ErrorKind::AlreadyExists, format!("{}: no free file name", first.display())))
})();
// after a hard link the temp name is a second name of the new file; after a rename it's gone
let _ = fs::remove_file(&tmp);
r
}
/// Give the complete temp file `tmp` the name `path`, unless that is taken.
fn publish_new(tmp: &Path, path: &Path) -> io::Result<()> {
match fs::hard_link(tmp, path) {
Ok(()) => Ok(()),
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => Err(e),
// no hard links on this volume (FAT/exFAT, some shares): check, then rename
Err(_) => {
if fs::symlink_metadata(path).is_ok() {
return Err(io::Error::new(io::ErrorKind::AlreadyExists, format!("{} exists", path.display())));
}
fs::rename(tmp, path)
}
}
}
/// Does the file at `path` hold exactly `data`?
fn same_content(path: &Path, data: &[u8]) -> io::Result<bool> {
use std::io::Read;
let mut f = File::open(path)?;
if f.metadata()?.len() != data.len() as u64 {
return Ok(false);
}
let mut buf = vec![0u8; 1 << 20];
let mut at = 0usize;
loop {
let k = match f.read(&mut buf) {
Ok(0) => return Ok(at == data.len()),
Ok(k) => k,
Err(e) if e.kind() == io::ErrorKind::Interrupted => continue,
Err(e) => return Err(e),
};
if data.get(at..at + k) != buf.get(..k) {
return Ok(false);
}
at += k;
}
}
/// Do `a` and `b` name the same existing file (through links, `..`, case-insensitive names)?
pub fn same_file(a: &Path, b: &Path) -> bool {
let (Ok(ma), Ok(mb)) = (fs::metadata(a), fs::metadata(b)) else { return false };
#[cfg(unix)]
{
use std::os::unix::fs::MetadataExt;
ma.dev() == mb.dev() && ma.ino() == mb.ino()
}
#[cfg(not(unix))]
{
if ma.len() != mb.len() {
return false;
}
match (fs::canonicalize(a), fs::canonicalize(b)) {
(Ok(x), Ok(y)) => x == y || x.to_string_lossy().to_lowercase() == y.to_string_lossy().to_lowercase(),
_ => false,
}
}
}
#[cfg(test)]
mod tests {
use super::*;
fn temp(tag: &str) -> PathBuf {
let d = std::env::temp_dir().join(format!("lc-safe-file-{tag}-{}", std::process::id()));
let _ = fs::remove_dir_all(&d);
fs::create_dir_all(&d).unwrap();
d
}
fn names(dir: &Path) -> Vec<String> {
let mut v: Vec<String> = fs::read_dir(dir).unwrap().flatten().map(|e| e.file_name().to_string_lossy().to_string()).collect();
v.sort();
v
}
#[test]
fn atomic_write_replaces_and_leaves_no_temp() {
let d = temp("replace");
let p = d.join("out.jpg");
write_atomic(&p, b"one").unwrap();
write_atomic(&p, b"two two").unwrap();
assert_eq!(fs::read(&p).unwrap(), b"two two");
assert_eq!(names(&d), vec!["out.jpg"]);
let _ = fs::remove_dir_all(&d);
}
#[test]
fn a_failure_mid_write_keeps_the_old_file() {
let d = temp("midwrite");
let p = d.join("photo.jpg");
fs::write(&p, b"the original bytes").unwrap();
{
let _f = fail_writes_after(4);
let e = write_atomic(&p, &[7u8; 10_000]).unwrap_err();
assert!(e.to_string().contains("injected"), "{e}");
assert!(write_new(&d.join("new.dng"), &[1u8; 100]).is_err());
}
assert_eq!(fs::read(&p).unwrap(), b"the original bytes", "the target is untouched");
assert_eq!(names(&d), vec!["photo.jpg"], "no temp file and no partial new file");
// a fill error aborts the same way
let r = write_atomic_with(&p, &mut |w| {
w.write_all(b"half")?;
Err(io::Error::other("encoder failed"))
});
assert!(r.is_err());
assert_eq!(fs::read(&p).unwrap(), b"the original bytes");
assert_eq!(names(&d), vec!["photo.jpg"]);
let _ = fs::remove_dir_all(&d);
}
#[test]
fn new_files_never_replace_anything() {
let d = temp("new");
let p = d.join("a.dng");
fs::write(&p, b"keep").unwrap();
assert_eq!(write_new(&p, b"other").unwrap_err().kind(), io::ErrorKind::AlreadyExists);
assert_eq!(fs::read(&p).unwrap(), b"keep");
let mut c = (1..).map(|k| if k == 1 { d.join("a.dng") } else { d.join(format!("a-{k}.dng")) });
let got = write_new_unique(&mut c, b"fresh").unwrap();
assert_eq!(got, d.join("a-2.dng"));
assert_eq!(fs::read(&got).unwrap(), b"fresh");
assert_eq!(names(&d), vec!["a-2.dng", "a.dng"], "the temp file is gone");
let _ = fs::remove_dir_all(&d);
}
#[test]
fn same_file_sees_through_other_spellings() {
let d = temp("same");
let p = d.join("x.jpg");
fs::write(&p, b"x").unwrap();
fs::write(d.join("y.jpg"), b"x").unwrap();
fs::create_dir_all(d.join("sub")).unwrap();
assert!(same_file(&p, &d.join("sub/../x.jpg")));
assert!(!same_file(&p, &d.join("y.jpg")));
assert!(!same_file(&p, &d.join("missing.jpg")));
let _ = fs::remove_dir_all(&d);
}
}

View File

@ -10,22 +10,21 @@ use serde_json::{Value, json};
use super::{CommandSpec, bad, cmd, has_active, has_selection};
use crate::{Result, Session};
/// A free `<stem>.dng` (then `<stem>-2.dng`…) next to `path`.
fn dng_path(path: &str) -> String {
/// `<stem>.dng`, then `<stem>-2.dng`, `<stem>-3.dng`… next to `path`.
fn dng_names(path: &str) -> impl Iterator<Item = std::path::PathBuf> {
let p = Path::new(path);
let dir = p.parent().unwrap_or(Path::new(""));
let dir = p.parent().unwrap_or(Path::new("")).to_path_buf();
let stem = p.file_stem().map(|s| s.to_string_lossy().to_string()).unwrap_or_else(|| "photo".into());
let mut out = dir.join(format!("{stem}.dng"));
let mut n = 2;
while out.exists() {
out = dir.join(format!("{stem}-{n}.dng"));
n += 1;
}
out.to_string_lossy().to_string()
(1u64..).map(move |n| if n == 1 { dir.join(format!("{stem}.dng")) } else { dir.join(format!("{stem}-{n}.dng")) })
}
/// Write the DNG for raw file `path` (develop settings in `packet`); returns the new path.
/// An empty `packet` writes no XMP.
/// Write the DNG for raw file `path` (develop settings in `packet`) next to it under a free name;
/// returns the new path. An empty `packet` writes no XMP.
///
/// The DNG is checked before anyone relies on it (Convert to DNG relinks the photo to it, Copy as
/// DNG deletes the raw copy): the encoded file must decode to the same raw data, and it is
/// written to a temp file, synced, read back identical and only then given its name — never
/// replacing an existing file. On any failure no DNG is left behind and the raw stays in use.
pub(crate) fn write_dng_for(s: &Session, path: &str, packet: String) -> std::result::Result<String, String> {
let bytes = match &s.media.file_bytes {
Some(r) => r(path)?,
@ -35,9 +34,21 @@ pub(crate) fn write_dng_for(s: &Session, path: &str, packet: String) -> std::res
drop(bytes);
let dng = lightcraft_raw::write_dng(&raw, &lightcraft_raw::DngWriteOptions { xmp: (!packet.is_empty()).then_some(packet), ..Default::default() })
.map_err(|e| e.to_string())?;
let out = dng_path(path);
std::fs::write(&out, dng).map_err(|e| format!("{out}: {e}"))?;
Ok(out)
let back = lightcraft_raw::decode(&dng).map_err(|e| format!("{path}: the DNG written for it doesn't decode ({e}); the raw is kept"))?;
let same = match (&back.data, &raw.data) {
(lightcraft_raw::RawData::U16(a), lightcraft_raw::RawData::U16(b)) => a == b,
(lightcraft_raw::RawData::F32(a), lightcraft_raw::RawData::F32(b)) => {
a.len() == b.len() && a.iter().zip(b).all(|(x, y)| x.to_bits() == y.to_bits())
}
_ => false,
};
if (back.width, back.height, back.cpp) != (raw.width, raw.height, raw.cpp) || !same {
return Err(format!("{path}: the DNG written for it doesn't hold the same raw data; the raw is kept"));
}
drop((raw, back));
let out = lightcraft_catalog::safe_file::write_new_unique(&mut dng_names(path), &dng)
.map_err(|e| format!("{path}: could not write its DNG ({e}); the raw is kept"))?;
Ok(out.to_string_lossy().to_string())
}
fn convert(s: &mut Session, p: &Value) -> Result<Value> {
@ -133,7 +144,7 @@ fn edit_external(s: &mut Session, p: &Value) -> Result<Value> {
};
let mut written = Vec::new();
let mut write = |path: &str, bytes: &[u8]| -> std::result::Result<(), String> {
std::fs::write(path, bytes).map_err(|e| format!("{path}: {e}"))?;
crate::export::write_file(path, bytes)?;
written.push(path.to_string());
Ok(())
};

View File

@ -96,5 +96,18 @@ pub fn specs() -> Vec<CommandSpec> {
save
),
cmd!("export.deletePreset", "Delete Export Preset", [], None, "{name} — removes a user preset → presets", always, delete),
cmd!(
query "export.checkTarget",
"Check Output Path",
[],
None,
"{path} — fails when writing `path` would replace a photo's original (or its XMP sidecar) in the library; front ends call it before saving a render or screenshot to a user-given path → {path}",
always,
|s, p| {
let path = p.get("path").and_then(Value::as_str).ok_or_else(|| bad("export.checkTarget", "missing `path`"))?;
s.check_write_target(path).map_err(|e| bad("export.checkTarget", e))?;
Ok(json!({"path": path}))
}
),
]
}

View File

@ -30,7 +30,8 @@ fn run(s: &mut Session, id: &str, p: &Value) -> Result<Value> {
if let (Some(path), Some(img)) = (str_param(p, "previewPath"), &out.preview) {
let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(img), &lightcraft_codecs::EncodeMeta::default())
.map_err(|e| bad(id, e.to_string()))?;
std::fs::write(path, png).map_err(|e| bad(id, format!("{path}: {e}")))?;
s.check_write_target(path).map_err(|e| bad(id, e))?;
crate::export::write_file(path, &png).map_err(|e| bad(id, e))?;
info["previewPath"] = json!(path);
}
if let Some(img) = &out.preview {

View File

@ -1,8 +1,9 @@
//! Missing originals: photos whose file is no longer where the library expects it (moved,
//! renamed, on an unplugged drive), and relinking them — one at a time (`photo.relink`) or by
//! searching a folder for files with the same name and size (`library.findMissing`).
//! searching a folder for files with the same name and size, or the same content
//! (`library.findMissing`).
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::path::Path;
use lightcraft_catalog::{Catalog, Op, Photo, PhotoId, Source};
@ -83,8 +84,29 @@ fn relink(s: &mut Session, p: &Value) -> Result<Value> {
Ok(json!({"id": id.0, "path": abs}))
}
/// Search `folder` (recursively) for each missing photo's file name — and its size when the
/// library knows it — and relink every match in one undoable step.
/// The stored content hash of a photo, without the `:dupN` suffix duplicates get.
fn stored_hash(p: &Photo) -> Option<&str> {
p.content_hash.as_deref().map(|h| h.split(':').next().unwrap_or(h)).filter(|h| !h.is_empty())
}
/// The content hash of the file at `path` (as import computes it), if it can be read.
fn file_hash(path: &str) -> Option<String> {
std::fs::read(path).ok().map(|b| lightcraft_preview::hash_bytes(&b).to_string())
}
/// Search `folder` (recursively) for each missing photo's file and relink every match in one
/// undoable step.
///
/// Matching, per photo:
/// 1. a file with the same name (any letter case), the same size and the same content hash;
/// 2. renamed: a file of the same size with the same content hash (only same-size files are
/// read, so this stays cheap);
/// 3. without a hash to confirm (or the file was edited in place): the one file with the same
/// name and size. Several such files are ambiguous: the photo is skipped and reported in
/// `ambiguous`, never guessed (two cameras' `IMG_0001.CR3` of one size are different photos).
///
/// Files that are already some photo's original, or that another photo was just matched with,
/// are not used.
fn find_missing(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "library.findMissing";
let folder = str_param(p, "folder").ok_or_else(|| bad(C, "missing `folder`"))?;
@ -93,27 +115,72 @@ fn find_missing(s: &mut Session, p: &Value) -> Result<Value> {
}
let lost = missing(s);
if lost.is_empty() {
return Ok(json!({"found": [], "missing": 0}));
return Ok(json!({"found": [], "missing": 0, "ambiguous": []}));
}
// file name (lower case) → candidate paths
let in_use: HashSet<String> = s.catalog.photos().filter_map(|p| checked_path(p).map(str::to_string)).collect();
// file name (lower case) → candidate paths; size → paths (filled lazily)
let mut by_name: HashMap<String, Vec<String>> = HashMap::new();
for f in crate::import::expand(&[folder.to_string()], None) {
if let Some(n) = Path::new(&f).file_name() {
by_name.entry(n.to_string_lossy().to_lowercase()).or_default().push(f);
let files: Vec<String> = crate::import::expand(&[folder.to_string()], None).into_iter().filter(|f| !in_use.contains(f)).collect();
for f in &files {
if let Some(n) = Path::new(f).file_name() {
by_name.entry(n.to_string_lossy().to_lowercase()).or_default().push(f.clone());
}
}
let mut by_size: Option<HashMap<u64, Vec<String>>> = None;
let mut hashes: HashMap<String, Option<String>> = HashMap::new();
let mut hash_of = |f: &str| hashes.entry(f.to_string()).or_insert_with(|| file_hash(f)).clone();
let size_of = |f: &str| std::fs::metadata(f).map(|m| m.len()).ok();
let mut used: HashSet<String> = HashSet::new();
let mut ops = Vec::new();
let mut found = Vec::new();
let mut ambiguous = Vec::new();
for (id, old) in &lost {
let Some(ph) = s.catalog.photo(*id) else { continue };
let name = Path::new(old).file_name().map(|n| n.to_string_lossy().to_lowercase()).unwrap_or_default();
let size = s.catalog.photo(*id).map(|p| p.file_size).unwrap_or(0);
let hit = by_name.get(&name).and_then(|c| {
c.iter().find(|f| size == 0 || std::fs::metadata(f).is_ok_and(|m| m.len() == size)).or_else(|| (c.len() == 1 && size == 0).then(|| &c[0]))
});
if let Some(path) = hit {
ops.push(relink_op(*id, path));
found.push(json!({"id": id.0, "from": old, "to": path}));
let size = ph.file_size;
let hash = stored_hash(ph).map(str::to_string);
let named: Vec<&String> = by_name
.get(&name)
.map(|c| c.iter().filter(|f| !used.contains(*f) && (size == 0 || size_of(f) == Some(size))).collect())
.unwrap_or_default();
// 1. same name and size, confirmed by the content hash
let mut hit: Option<(String, &str)> =
hash.as_deref().and_then(|h| named.iter().find(|f| hash_of(f).as_deref() == Some(h))).map(|f| (f.to_string(), "name"));
// 2. renamed: same size and content
if hit.is_none()
&& let Some(h) = &hash
&& size > 0
{
let sizes = by_size.get_or_insert_with(|| {
let mut m: HashMap<u64, Vec<String>> = HashMap::new();
for f in &files {
if let Some(n) = size_of(f) {
m.entry(n).or_default().push(f.clone());
}
}
m
});
hit = sizes
.get(&size)
.and_then(|c| c.iter().find(|f| !used.contains(*f) && hash_of(f).as_deref() == Some(h.as_str())))
.map(|f| (f.clone(), "content"));
}
// 3. same name and size only (no hash, or the file was edited in place): one candidate
// is taken, several are ambiguous and skipped rather than guessed
if hit.is_none() {
match named.len() {
1 => hit = Some((named[0].clone(), "name")),
0 => {}
_ => {
ambiguous.push(json!({"id": id.0, "from": old, "candidates": named}));
continue;
}
}
}
let Some((path, by)) = hit else { continue };
ops.push(relink_op(*id, &path));
found.push(json!({"id": id.0, "from": old, "to": path, "by": by}));
used.insert(path);
}
let still = lost.len() - found.len();
if !ops.is_empty() {
@ -124,7 +191,7 @@ fn find_missing(s: &mut Session, p: &Value) -> Result<Value> {
}
}
}
Ok(json!({"found": found, "missing": still}))
Ok(json!({"found": found, "missing": still, "ambiguous": ambiguous}))
}
pub fn specs() -> Vec<CommandSpec> {
@ -146,7 +213,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Find Missing Photos",
[],
None,
"{folder} — relink every missing photo whose file (same name and size) is somewhere in the folder → {found: [{id, from, to}], missing}",
"{folder} — relink every missing photo whose file is somewhere in the folder: same name and size (and content, when the library knows its hash), or — renamed — same size and content → {found: [{id, from, to, by: name|content}], missing, ambiguous: [{id, from, candidates}] (several same-name candidates and no hash to tell them apart: skipped)}",
always,
find_missing
),

View File

@ -102,7 +102,7 @@ fn smart(s: &mut Session, p: &Value) -> Result<Value> {
let dir = s.media.smart_dir.clone().ok_or_else(|| bad(C, "smart previews need a library on disk"))?;
let discard = p.get("discard").and_then(Value::as_bool).unwrap_or(false);
let ids = if p.get("ids").is_some() || !s.selection.ids.is_empty() { s.targets(p) } else { s.visible_cloned() };
let (mut built, mut removed, mut failed) = (0usize, 0usize, Vec::new());
let (mut built, mut repaired, mut removed, mut failed) = (0usize, 0usize, 0usize, Vec::new());
if !discard {
// a chosen folder on another drive must be there: never recreate it on this one
if s.smart_previews_dir.is_some() {
@ -123,20 +123,27 @@ fn smart(s: &mut Session, p: &Value) -> Result<Value> {
}
continue;
}
if path.exists() {
// a complete proxy is kept; a missing or damaged one (cut short by a crash or a full
// drive) is (re)built
let damaged = path.exists();
if damaged && crate::smart::is_valid(&path) {
built += 1;
continue;
}
// atomic: a failed write leaves no partial proxy that would pass for a built one
let r = s
.source_now(id, crate::media::SourceLevel::Preview)
.and_then(|src| crate::smart::encode(&src))
.and_then(|b| std::fs::write(&path, b).map_err(|e| e.to_string()));
.and_then(|b| lightcraft_catalog::safe_file::write_atomic(&path, &b).map_err(|e| format!("{}: {e}", path.display())));
match r {
Ok(()) => built += 1,
Ok(()) => {
built += 1;
repaired += usize::from(damaged);
}
Err(e) => failed.push(json!([id.0, e])),
}
}
Ok(json!({"built": built, "removed": removed, "failed": failed}))
Ok(json!({"built": built, "repaired": repaired, "removed": removed, "failed": failed}))
}
/// The smart previews folder: where it is, whether it is custom, what it holds, whether it can be
@ -205,7 +212,7 @@ fn smart_location(s: &mut Session, p: &Value) -> Result<Value> {
/// Whether photo `id` has a smart preview.
pub fn has_smart_preview(s: &Session, id: lightcraft_catalog::PhotoId) -> bool {
match (&s.media.smart_dir, s.catalog.photo(id)) {
(Some(dir), Some(p)) => dir.join(crate::smart::file_name(p)).exists(),
(Some(dir), Some(p)) => crate::smart::is_valid(&dir.join(crate::smart::file_name(p))),
_ => false,
}
}
@ -218,7 +225,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Build Smart Previews",
[],
None,
"{ids?, discard?: bool} — build (or discard) the smart previews of the selected photos (else all in view): compact proxies in the library that keep photos editable and exportable (at proxy size) while their originals are offline → {built, removed, failed}",
"{ids?, discard?: bool} — build (or discard) the smart previews of the selected photos (else all in view): compact proxies in the library that keep photos editable and exportable (at proxy size) while their originals are offline; a damaged proxy (cut short) is rebuilt → {built, repaired, removed, failed}",
always,
smart
),

View File

@ -113,7 +113,7 @@ pub fn specs() -> Vec<CommandSpec> {
.map(|r| json!({"group": r.group, "tag": r.tag, "name": r.name, "value": r.value}))
.collect();
// XMP: the sidecar if there is one, else the file's own packet
let packet = crate::sidecar::read_packet(path, ph.kind, s.xmp.naming).map(|(x, _)| x).or_else(|| lightcraft_meta::embedded(&bytes).xmp);
let packet = crate::sidecar::read_packet(path, ph.kind, s.sidecar_naming(ph.id)).map(|(x, _)| x).or_else(|| lightcraft_meta::embedded(&bytes).xmp);
let xmp: Vec<Value> = packet
.and_then(|x| lightcraft_meta::parse_xmp(&x).ok())
.map(|d| d.properties.into_iter().filter(|(k, _)| !k.starts_with("lc:")).map(|(k, v)| json!({"name": k, "value": v.join("; ")})).collect())

View File

@ -9,14 +9,26 @@ use crate::{Result, Session};
fn save(s: &mut Session, p: &Value) -> Result<Value> {
let mut written = Vec::new();
let mut merged = Vec::new();
let mut backups = Vec::new();
let mut failed = Vec::new();
let owners = crate::sidecar::StemOwners::of(&s.catalog);
for id in s.targets(p) {
match s.save_sidecar(id) {
Ok(path) => written.push(path.display().to_string()),
match s.save_sidecar_with(id, &owners) {
Ok(r) => {
let path = r.path.display().to_string();
if r.merged {
merged.push(path.clone());
}
if let Some(b) = r.backup {
backups.push(json!({"path": path, "backup": b.display().to_string()}));
}
written.push(path);
}
Err(e) => failed.push(json!({"id": id.0, "error": e.to_string()})),
}
}
Ok(json!({"written": written, "failed": failed}))
Ok(json!({"written": written, "merged": merged, "backups": backups, "failed": failed}))
}
fn read(s: &mut Session, p: &Value) -> Result<Value> {
@ -63,7 +75,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Save Metadata to File",
["Photo"],
Some("Cmd+S"),
"{ids?} — writes each photo's XMP sidecar (metadata + develop settings) next to the original → {written: [path], failed}",
"{ids?} — writes each photo's XMP sidecar (metadata + develop settings) next to the original, merged into an existing one (other apps' data kept; an unreadable one is backed up first) → {written: [path], merged: [path], backups: [{path, backup}], failed}",
has_selection,
save
),

View File

@ -987,6 +987,8 @@ pub struct PreparedExport {
pub photo: lightcraft_catalog::PhotoId,
pub file_name: String,
work: Work,
/// The library's originals, which [`run_batch`] never writes over (shared by a batch).
guard: std::sync::Arc<crate::originals::OriginalGuard>,
}
struct RenderWork {
@ -1009,12 +1011,30 @@ enum Work {
},
}
/// Set up the export of photo `id` at 1-based position `seq` of a batch.
/// Set up the export of photo `id` at 1-based position `seq` of a batch. (For a whole batch use
/// [`prepare_batch`]: it looks at the library's originals once.)
pub fn prepare_export(
session: &mut crate::Session,
id: lightcraft_catalog::PhotoId,
o: &ExportOptions,
seq: usize,
) -> Result<PreparedExport, String> {
let guard = std::sync::Arc::new(session.original_guard());
prepare_guarded(session, id, o, seq, guard)
}
/// [`prepare_export`] for each of `ids` in order (`{seq}` = position, from 1).
pub fn prepare_batch(session: &mut crate::Session, ids: &[lightcraft_catalog::PhotoId], o: &ExportOptions) -> Result<Vec<PreparedExport>, String> {
let guard = std::sync::Arc::new(session.original_guard());
ids.iter().enumerate().map(|(i, id)| prepare_guarded(session, *id, o, i + 1, guard.clone())).collect()
}
fn prepare_guarded(
session: &mut crate::Session,
id: lightcraft_catalog::PhotoId,
o: &ExportOptions,
seq: usize,
guard: std::sync::Arc<crate::originals::OriginalGuard>,
) -> Result<PreparedExport, String> {
let p = session.catalog.photo(id).ok_or("no such photo")?;
let file_name = o.file_name_for(p, seq);
@ -1036,7 +1056,7 @@ pub fn prepare_export(
size: (p.width as usize, p.height as usize),
}
};
Ok(PreparedExport { photo: id, file_name, work })
Ok(PreparedExport { photo: id, file_name, work, guard })
}
impl PreparedExport {
@ -1088,14 +1108,16 @@ pub enum DngCompression {
}
/// The destination of a batch: a folder (with `ExportOptions::subfolder` and the conflict policy
/// applied), or one exact file path (single photo; overwritten).
/// applied to each file together with its sidecars), or one exact file path (single photo; an
/// ordinary file already there is replaced). Either way a catalogued original (or its sidecar)
/// is never written over: see [`crate::originals`].
#[derive(Clone, Debug, Default)]
pub struct Destination {
pub dir: String,
pub exact: Option<String>,
}
/// Export `ids` in order ([`prepare_export`] + [`run_batch`]), stopping at the first error.
/// Export `ids` in order ([`prepare_batch`] + [`run_batch`]), stopping at the first error.
pub fn export_batch(
session: &mut crate::Session,
ids: &[lightcraft_catalog::PhotoId],
@ -1104,15 +1126,34 @@ pub fn export_batch(
write: &mut dyn FnMut(&str, &[u8]) -> Result<(), String>,
exists: &dyn Fn(&str) -> bool,
) -> Result<Vec<serde_json::Value>, String> {
let items = ids.iter().enumerate().map(|(i, id)| prepare_export(session, *id, o, i + 1)).collect::<Result<Vec<_>, _>>()?;
let items = prepare_batch(session, ids, o)?;
run_batch(items, o, to, write, exists, true, &mut |_, _| true)
}
/// Write an exported or rendered file on disk: its folder is created if needed, and the file is
/// replaced atomically ([`lightcraft_catalog::safe_file::write_atomic`]: a temp file, synced, then
/// renamed), so a failure part-way leaves any previous file intact and no truncated one. The
/// native writer behind exports, renders and screenshots (app, CLI, MCP).
pub fn write_file(path: &str, bytes: &[u8]) -> Result<(), String> {
let p = std::path::Path::new(path);
if let Some(dir) = p.parent().filter(|d| !d.as_os_str().is_empty()) {
std::fs::create_dir_all(dir).map_err(|e| format!("{}: {e}", dir.display()))?;
}
lightcraft_catalog::safe_file::write_atomic(p, bytes).map_err(|e| format!("{path}: {e}"))
}
/// The path of the sidecar with extension `ext` of the exported file `main`.
fn sidecar_path(main: &str, ext: &str) -> String {
std::path::Path::new(main).with_extension(ext).to_string_lossy().to_string()
}
/// Run prepared exports in order: pick each one's path, and hand the bytes (and sidecars) to
/// `write`. `exists` tells whether a path is taken (conflict policy). `progress(done, next file)`
/// is called before each photo; returning false cancels the rest. Returns one JSON object per
/// photo: `{path, width, height, bytes, sidecars}`, `{skipped: path}` or (unless
/// `stop_on_error`) `{photo, file, error}`.
/// `write`. `exists` tells whether a path is taken. The conflict policy applies to a file and its
/// sidecars as one: with Unique both get the same free name, with Skip the photo is skipped when
/// either is taken. A path that is a catalogued original (or its sidecar) is refused whatever the
/// policy. `progress(done, next file)` is called before each photo; returning false cancels the
/// rest. Returns one JSON object per photo: `{path, width, height, bytes, sidecars}`,
/// `{skipped: path}` or (unless `stop_on_error`) `{photo, file, error}`.
pub fn run_batch(
items: Vec<PreparedExport>,
o: &ExportOptions,
@ -1132,7 +1173,7 @@ pub fn run_batch(
if !progress(i, &item.file_name) {
break;
}
let (photo, name) = (item.photo, item.file_name.clone());
let (photo, name, guard) = (item.photo, item.file_name.clone(), item.guard.clone());
let e = match item.run() {
Ok(e) => e,
Err(err) if stop_on_error => return Err(err),
@ -1141,14 +1182,18 @@ pub fn run_batch(
continue;
}
};
// the exported file and its sidecars
let group = |main: &str| std::iter::once(main.to_string()).chain(e.sidecars.iter().map(|(x, _)| sidecar_path(main, x))).collect::<Vec<_>>();
let path = match to.exact.as_deref().filter(|_| single) {
Some(p) => p.to_string(),
Some(p) => Ok(p.to_string()),
None => {
let mut path = join(&dir, &e.file_name);
let busy = |p: &str, taken: &std::collections::HashSet<String>| taken.contains(p) || exists(p);
if busy(&path, &taken) {
let path = join(&dir, &e.file_name);
let busy = |main: &str| group(main).iter().any(|p| taken.contains(p) || exists(p));
if !busy(&path) {
Ok(path)
} else {
match o.conflict {
Conflict::Overwrite => {}
Conflict::Overwrite => Ok(path),
Conflict::Skip => {
out.push(json!({"skipped": path}));
continue;
@ -1156,29 +1201,34 @@ pub fn run_batch(
Conflict::Unique => {
let (stem, ext) = e.file_name.rsplit_once('.').map_or((e.file_name.as_str(), None), |(a, b)| (a, Some(b)));
let name = |n: usize| join(&dir, &ext.map_or(format!("{stem}-{n}"), |x| format!("{stem}-{n}.{x}")));
path = (2..).map(name).find(|p| !busy(p, &taken)).expect("a free name");
(2..1_000_000).map(name).find(|p| !busy(p)).ok_or_else(|| format!("{path}: no free file name"))
}
}
}
path
}
};
let written = write(&path, &e.bytes).and_then(|()| {
let mut sidecars = Vec::new();
for (ext, bytes) in &e.sidecars {
let sc = std::path::Path::new(&path).with_extension(ext).to_string_lossy().to_string();
write(&sc, bytes)?;
sidecars.push(sc);
let file = path.clone().unwrap_or_else(|_| name.clone());
let written = path.and_then(|path| {
let files = group(&path);
// never over an original, whatever the conflict policy or the exact path said
for f in &files {
guard.check(std::path::Path::new(f))?;
}
Ok(sidecars)
write(&path, &e.bytes)?;
let mut sidecars = Vec::new();
for ((_, bytes), sc) in e.sidecars.iter().zip(files.iter().skip(1)) {
write(sc, bytes)?;
sidecars.push(sc.clone());
}
Ok((path, files, sidecars))
});
match written {
Ok(sidecars) => {
taken.insert(path.clone());
Ok((path, files, sidecars)) => {
taken.extend(files);
out.push(json!({"path": path, "width": e.width, "height": e.height, "bytes": e.bytes.len(), "sidecars": sidecars}));
}
Err(err) if stop_on_error => return Err(err),
Err(err) => out.push(json!({"photo": photo.0, "file": path, "error": err})),
Err(err) => out.push(json!({"photo": photo.0, "file": file, "error": err})),
}
}
Ok(out)

View File

@ -374,25 +374,14 @@ fn probe_all(probe: Option<&crate::media::FileProbe>, paths: &[String], progress
}
/// Copy `src` into `root`/`folders` as `name` (default: its own name), made unique with -1, -2…;
/// returns the new path.
/// returns the new path. The copy is verified ([`crate::import_move::copy_new`]: a new file,
/// synced and compared byte for byte); a bad one is removed and the photo reported as failed.
fn copy_into(root: &Path, src: &str, folders: &[String], name: Option<&str>) -> Result<String, String> {
let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f));
std::fs::create_dir_all(&dir).map_err(|e| format!("{}: {e}", dir.display()))?;
let name = name
.map(str::to_string)
.unwrap_or_else(|| Path::new(src).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "photo".into()));
let (stem, ext) = match name.rsplit_once('.') {
Some((s, e)) => (s.to_string(), format!(".{e}")),
None => (name.clone(), String::new()),
};
let mut dst = dir.join(&name);
let mut i = 1;
while dst.exists() {
dst = dir.join(format!("{stem}-{i}{ext}"));
i += 1;
}
std::fs::copy(src, &dst).map_err(|e| format!("copy {src}: {e}"))?;
Ok(dst.to_string_lossy().to_string())
crate::import_move::copy_new(Path::new(src), &dir, &name).map(|p| p.to_string_lossy().to_string())
}
/// What a [`scan`] needs from the session, so it can run on another thread (a folder on a network

View File

@ -169,6 +169,27 @@ pub(crate) fn place(src: &Path, dir: &Path, name: &str) -> Result<Placed, String
Ok(placed)
}
/// Import → **Copy**: copy `src` into `dir` as `name` (-1, -2… when taken) with the same checks as
/// a move's copy — a new file (never overwriting one, even one that appears meanwhile), synced to
/// disk and compared byte for byte with the source; a bad copy is removed and reported, so a card
/// is never wiped on the strength of it. Returns the new path.
pub(crate) fn copy_new(src: &Path, dir: &Path, name: &str) -> Result<PathBuf, String> {
fs::create_dir_all(dir).map_err(|e| format!("{}: {e}", dir.display()))?;
let (stem, ext) = match name.rsplit_once('.') {
Some((s, e)) if !s.is_empty() => (s.to_string(), format!(".{e}")),
_ => (name.to_string(), String::new()),
};
for i in 0..100_000u32 {
let d = if i == 0 { dir.join(name) } else { dir.join(format!("{stem}-{i}{ext}")) };
match copy_verified(src, &d) {
Ok(()) => return Ok(d),
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => continue,
Err(e) => return Err(format!("copy {}: {e} (nothing was imported from it; the original is untouched)", src.display())),
}
}
Err(format!("copy {}: no free name in {}", src.display(), dir.display()))
}
/// Undo a [`place`]: remove what it created at the destination (the sources were never touched).
pub(crate) fn rollback(p: &Placed) {
for sc in p.sidecars.iter().filter(|s| s.created) {
@ -213,7 +234,7 @@ pub(crate) fn finish(p: &Placed) -> Result<Vec<(String, String)>, String> {
}
/// A photo (by extension) still beside a stem-named sidecar, sharing its stem.
fn sibling_photo(sidecar: &Path) -> Option<String> {
pub(crate) fn sibling_photo(sidecar: &Path) -> Option<String> {
let dir = sidecar.parent()?;
let stem = sidecar.file_stem()?.to_string_lossy().to_string();
fs::read_dir(dir).ok()?.flatten().map(|e| e.path()).find_map(|f| {

View File

@ -24,6 +24,7 @@ pub mod library;
pub mod media;
pub mod memory;
pub mod merge;
pub mod originals;
pub mod preset_import;
pub mod preset_luminar;
pub mod presets;
@ -378,15 +379,22 @@ impl Session {
Ok(e.label)
}
/// Apply an undo/redo op, first moving the files its renames imply (all or nothing).
/// Apply an undo/redo op, first moving the files its renames imply (all or nothing). Files
/// that can't be moved back after a failure are reported and the library follows them (a
/// logged change outside the undo history), so none goes missing.
fn apply_with_files(&mut self, op: &Op) -> Result<Op> {
let fs = rename::RealFs;
let moves = self.file_moves(op);
Session::move_files(&moves)?;
if let Err(e) = rename::move_all(&fs, &moves) {
return Err(EngineError::Other(format!("can't move the files back: {}", self.follow_stuck(op, e, false))));
}
match self.catalog.apply(op.clone()) {
Ok(inv) => Ok(inv),
Err(e) => {
let back: Vec<(String, String)> = moves.iter().rev().map(|(a, b)| (b.clone(), a.clone())).collect();
let _ = Session::move_files(&back);
if let Err(be) = rename::move_all(&fs, &back) {
return Err(EngineError::Other(format!("{e}; the files could not all be moved back: {}", be.message)));
}
Err(e.into())
}
}

View File

@ -290,7 +290,7 @@ impl MediaCache {
&& !std::path::Path::new(path).exists()
{
let sp = dir.join(crate::smart::file_name(p));
if sp.exists() {
if crate::smart::is_valid(&sp) {
return SourceRef::Smart { path: sp };
}
}

View File

@ -304,15 +304,13 @@ fn apply_auto(s: &mut DevelopSettings, src: &Rgb32f, info: &SourceInfo) {
s.color.saturation = a.saturation;
}
/// A free file name `dir/<stem>-<suffix>.dng` (`-2`, `-3`… when taken).
fn output_path(first: &str, suffix: &str) -> String {
/// `dir/<stem>-<suffix>.dng`, then `-2`, `-3`… (the candidates for a merge's output).
fn output_names(first: &str, suffix: &str) -> impl Iterator<Item = std::path::PathBuf> {
let p = std::path::Path::new(first);
let dir = p.parent().map(|d| d.to_path_buf()).unwrap_or_default();
let stem = p.file_stem().map(|s| s.to_string_lossy().to_string()).unwrap_or_else(|| "Merge".into());
let cand = |k: u64| dir.join(if k == 1 { format!("{stem}-{suffix}.dng") } else { format!("{stem}-{suffix}-{k}.dng") });
// (`u64::MAX` taken names can't exist; the last candidate is the fallback)
let free = (1..u64::MAX).map(cand).find(|c| !c.exists()).unwrap_or_else(|| cand(u64::MAX));
free.to_string_lossy().to_string()
let suffix = suffix.to_string();
(1u64..).map(move |k| dir.join(if k == 1 { format!("{stem}-{suffix}.dng") } else { format!("{stem}-{suffix}-{k}.dng") }))
}
/// Parse `merge.*` command parameters into a kind and finishing options.
@ -367,8 +365,11 @@ impl Session {
/// apply Auto Settings / the auto crop. Returns `{id, path, …info}`.
pub fn finish_merge(&mut self, job: &MergeJob, out: MergeOutput) -> Result<Value> {
let first = &job.sources.first().ok_or_else(|| EngineError::Other("nothing merged".into()))?.1;
let path = output_path(first, job.kind.suffix());
std::fs::write(&path, &out.dng).map_err(|e| EngineError::Other(format!("{path}: {e}")))?;
// a new file under a free name (never replacing one), complete and synced before it appears
let path = lightcraft_catalog::safe_file::write_new_unique(&mut output_names(first, job.kind.suffix()), &out.dng)
.map_err(|e| EngineError::Other(format!("could not write the merged DNG next to {first}: {e}")))?
.to_string_lossy()
.to_string();
let report = crate::import::import(self, std::slice::from_ref(&path), crate::import::ImportMode::Add)?;
let id = report
.imported

View File

@ -0,0 +1,102 @@
//! Never write over an original: exports, renders and other files LightCraft writes for the user
//! must not land on a catalogued photo's own file (or its XMP sidecar), whatever the conflict
//! setting or exact path asked for. [`OriginalGuard`] is a snapshot of the library's originals
//! that answers "would writing here replace one?" by file identity (the same file under another
//! spelling, through `..`, a link or a case-insensitive volume counts), not by comparing strings.
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use lightcraft_catalog::safe_file::same_file;
use lightcraft_catalog::{Catalog, Source};
/// The library's originals and their sidecars, looked up by file name.
#[derive(Debug, Default)]
pub struct OriginalGuard {
/// lower-case file name → (path on disk, what it is: "the original of IMG_1.CR3", …)
by_name: HashMap<String, Vec<(PathBuf, String)>>,
}
impl OriginalGuard {
/// The originals of every photo in `catalog`.
pub fn new(catalog: &Catalog) -> Self {
let mut g = Self::default();
for p in catalog.photos() {
if let Source::File { path } = &p.source {
g.add(Path::new(path), &p.file_name);
}
}
g
}
/// Protect `path` (an original named `label`) and its sidecars (`IMG_1.xmp`, `IMG_1.CR3.xmp`).
pub fn add(&mut self, path: &Path, label: &str) {
let Some(name) = path.file_name().map(|n| n.to_string_lossy().to_string()) else { return };
let mut put = |file: String, p: PathBuf, what: String| self.by_name.entry(file.to_lowercase()).or_default().push((p, what));
put(name.clone(), path.to_path_buf(), format!("the original of {label}"));
let stem = path.file_stem().map(|s| s.to_string_lossy().to_string()).unwrap_or_default();
put(format!("{stem}.xmp"), path.with_extension("xmp"), format!("the XMP sidecar of {label}"));
put(format!("{name}.xmp"), PathBuf::from(format!("{}.xmp", path.to_string_lossy())), format!("the XMP sidecar of {label}"));
}
/// `Err` (a message for the user) when writing `target` would replace a protected file.
pub fn check(&self, target: &Path) -> Result<(), String> {
if std::fs::symlink_metadata(target).is_err() {
return Ok(()); // nothing there to replace
}
let mut names: Vec<String> = target.file_name().map(|n| n.to_string_lossy().to_lowercase()).into_iter().collect();
if let Some(n) = std::fs::canonicalize(target).ok().and_then(|c| c.file_name().map(|n| n.to_string_lossy().to_lowercase())) {
names.push(n);
}
for n in &names {
for (p, what) in self.by_name.get(n).into_iter().flatten() {
if same_file(p, target) {
return Err(format!(
"{} is {what} in the library: LightCraft never writes over an original (choose another folder or file name)",
target.display()
));
}
}
}
Ok(())
}
}
impl crate::Session {
/// A guard over this library's originals (see [`OriginalGuard`]).
pub fn original_guard(&self) -> OriginalGuard {
OriginalGuard::new(&self.catalog)
}
/// `Err` when writing `path` would replace a catalogued photo's original or sidecar.
pub fn check_write_target(&self, path: &str) -> Result<(), String> {
self.original_guard().check(Path::new(path))
}
}
#[cfg(test)]
mod tests {
use super::*;
use lightcraft_catalog::{Op, Photo, PhotoId};
#[test]
fn originals_and_their_sidecars_are_protected_by_identity() {
let d = std::env::temp_dir().join(format!("lc-originals-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&d);
std::fs::create_dir_all(d.join("sub")).unwrap();
let orig = d.join("IMG_1.jpg");
std::fs::write(&orig, b"original").unwrap();
std::fs::write(d.join("IMG_1.xmp"), b"<x/>").unwrap();
std::fs::write(d.join("other.jpg"), b"other").unwrap();
let mut c = Catalog::default();
let p = Photo::new(PhotoId(1), Source::File { path: orig.to_string_lossy().to_string() }, "IMG_1.jpg", "JPEG", 10, 10, "2026-01-01");
c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
let g = OriginalGuard::new(&c);
assert!(g.check(&orig).unwrap_err().contains("original of IMG_1.jpg"));
assert!(g.check(&d.join("sub/../IMG_1.jpg")).is_err(), "another spelling of the same file");
assert!(g.check(&d.join("IMG_1.xmp")).unwrap_err().contains("sidecar"));
assert!(g.check(&d.join("other.jpg")).is_ok(), "an ordinary existing file");
assert!(g.check(&d.join("new.jpg")).is_ok(), "a free name");
let _ = std::fs::remove_dir_all(&d);
}
}

View File

@ -11,7 +11,11 @@
//!
//! Safety:
//! - a target that exists on disk, or that another photo in the batch gets, receives a `-1`, `-2`…
//! suffix — no file is ever overwritten;
//! suffix — no file is ever overwritten. Whether an existing target is the photo itself (a
//! change of letter case on a case-insensitive volume) is decided by file identity, not by
//! comparing names, so on a case-sensitive volume `img_1.jpg` next to `IMG_1.JPG` is a collision
//! (issue #95); moves never replace a file even if one appears after the check (hard link +
//! unlink where the volume supports it);
//! - the moves happen before the catalog changes; if one fails, the ones already done are moved
//! back and nothing is committed;
//! - the catalog op ([`Op::SetFile`]) is undoable: undo/redo move the files back and forth (see
@ -297,58 +301,251 @@ fn sidecars(path: &str) -> Vec<(PathBuf, SidecarNaming, bool)> {
v
}
/// Move `from` to `to` (never over an existing file) together with its sidecars. On error nothing
/// is left moved.
pub fn move_file(from: &str, to: &str) -> std::result::Result<(), String> {
/// The file-system primitives renaming uses. [`RealFs`] is the disk; tests substitute a model of
/// a case-insensitive or case-sensitive volume (and inject failures).
pub(crate) trait MoveFs {
/// Something (file, folder, link) answers to `p`.
fn exists(&self, p: &Path) -> bool;
fn is_file(&self, p: &Path) -> bool;
/// `a` and `b` name the same file (e.g. `IMG.JPG` and `img.jpg` on a case-insensitive
/// volume). Decided by file identity, never by comparing names: on a case-sensitive volume
/// they are two different photos. When in doubt: `false`.
fn same_file(&self, a: &Path, b: &Path) -> bool;
/// Rename `a` to `b`, failing (never replacing) when `b` exists.
fn rename_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()>;
/// Copy `a` to a new file `b` (an independent copy, never a link), failing when `b` exists.
fn copy_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()>;
fn remove_file(&self, p: &Path) -> std::io::Result<()>;
/// Another photo file beside the stem-named sidecar `sidecar` that shares its stem (e.g.
/// `IMG_1.JPG` for `IMG_1.xmp`), which still uses that sidecar.
fn stem_sibling(&self, sidecar: &Path) -> Option<String>;
}
/// The real file system.
pub(crate) struct RealFs;
impl MoveFs for RealFs {
fn exists(&self, p: &Path) -> bool {
std::fs::symlink_metadata(p).is_ok()
}
fn is_file(&self, p: &Path) -> bool {
p.is_file()
}
fn same_file(&self, a: &Path, b: &Path) -> bool {
if a == b {
return true;
}
let (Ok(ma), Ok(mb)) = (std::fs::metadata(a), std::fs::metadata(b)) else { return false };
#[cfg(unix)]
{
use std::os::unix::fs::MetadataExt;
ma.dev() == mb.dev() && ma.ino() == mb.ino()
}
#[cfg(not(unix))]
{
// no stable file id in std here: `b` is `a` under another spelling when they look
// alike and the folder has no entry spelled exactly like `b` (on a case-sensitive
// folder holding both `IMG.JPG` and `img.jpg`, both entries are listed)
let (Some(dir), Some(name)) = (b.parent(), b.file_name()) else { return false };
if a.parent() != Some(dir) || ma.len() != mb.len() || ma.modified().ok() != mb.modified().ok() {
return false;
}
let Ok(entries) = std::fs::read_dir(dir) else { return false };
!entries.flatten().any(|e| e.file_name() == name)
}
}
fn rename_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()> {
if self.exists(b) {
return Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, format!("{} already exists", b.display())));
}
// A hard link fails when `b` exists — even if it appeared since the check above — so
// nothing is ever replaced. Volumes without hard links (FAT, some network shares) fall back
// to a plain rename after the check.
match std::fs::hard_link(a, b) {
Ok(()) => std::fs::remove_file(a).inspect_err(|_| {
// both names are the same file: dropping the new one loses nothing
let _ = std::fs::remove_file(b);
}),
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => Err(e),
Err(_) => std::fs::rename(a, b),
}
}
fn copy_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()> {
use std::io::Write;
let bytes = std::fs::read(a)?;
let mut f = std::fs::OpenOptions::new().write(true).create_new(true).open(b)?;
f.write_all(&bytes).and_then(|_| f.sync_all()).inspect_err(|_| {
let _ = std::fs::remove_file(b);
})
}
fn remove_file(&self, p: &Path) -> std::io::Result<()> {
std::fs::remove_file(p)
}
fn stem_sibling(&self, sidecar: &Path) -> Option<String> {
crate::import_move::sibling_photo(sidecar)
}
}
/// `from` → `to` differs only in letter case and `to` is that very file (a case-insensitive
/// volume): the rename must go through a temporary name.
fn is_respelling(fs: &dyn MoveFs, from: &Path, to: &Path) -> bool {
from != to && from.to_string_lossy().to_lowercase() == to.to_string_lossy().to_lowercase() && fs.exists(to) && fs.same_file(from, to)
}
/// Rename `a` to `b` without ever replacing a file; a change of letter case only goes through a
/// unique temporary name so case-insensitive volumes see a real change.
fn move_one(fs: &dyn MoveFs, a: &Path, b: &Path) -> std::io::Result<()> {
if !is_respelling(fs, a, b) {
return fs.rename_no_replace(a, b);
}
let tmp = (0..1000u32)
.map(|k| a.with_file_name(format!(".lc-rename-{}-{k}", std::process::id())))
.find(|t| !fs.exists(t))
.ok_or_else(|| std::io::Error::other("no free temporary name"))?;
fs.rename_no_replace(a, &tmp)?;
fs.rename_no_replace(&tmp, b).map_err(|e| match fs.rename_no_replace(&tmp, a) {
Ok(()) => e,
Err(e2) => std::io::Error::other(format!("{e}; it could not be given its old name back either ({e2}): it is now {}", tmp.display())),
})
}
/// Why [`move_file`] failed, and what it left behind.
#[derive(Clone, Debug, PartialEq)]
pub struct MoveError {
/// What went wrong, including every step that could not be undone and where that file is now.
pub message: String,
/// The file itself ended up at the new path: it moved, a sidecar didn't, and moving the file
/// back failed too. Its sidecars that did move stay with it.
pub moved: bool,
}
impl std::fmt::Display for MoveError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(&self.message)
}
}
/// Move `from` to `to` (never over an existing file) together with its sidecars. On error the
/// moves are undone; whatever can't be undone is named in the error (see [`MoveError`]).
pub fn move_file(from: &str, to: &str) -> std::result::Result<(), MoveError> {
move_file_with(&RealFs, from, to)
}
/// [`move_file`] on the file system `fs`.
pub(crate) fn move_file_with(fs: &dyn MoveFs, from: &str, to: &str) -> std::result::Result<(), MoveError> {
if from == to {
return Ok(());
}
let fail = |message: String| MoveError { message, moved: false };
let (f, t) = (Path::new(from), Path::new(to));
// a case-only change on a case-insensitive file system "exists" already: that is this file
let case_only = from.to_lowercase() == to.to_lowercase();
if t.exists() && !case_only {
return Err(format!("{to} already exists"));
// `to` may be this very file spelled differently (case-insensitive volume); any other file
// there — e.g. `img.jpg` next to `IMG.JPG` on a case-sensitive volume — is never replaced
if fs.exists(t) && !is_respelling(fs, f, t) {
return Err(fail(format!("{to} already exists")));
}
let mut done: Vec<(PathBuf, PathBuf)> = Vec::new();
let mv = |a: &Path, b: &Path| -> std::io::Result<()> {
if case_only {
// via a temporary name, so case-insensitive file systems see a real change
let tmp = a.with_file_name(format!(".lc-rename-{}", std::process::id()));
std::fs::rename(a, &tmp)?;
std::fs::rename(&tmp, b).inspect_err(|_| {
let _ = std::fs::rename(&tmp, a);
})
} else {
std::fs::rename(a, b)
}
};
mv(f, t).map_err(|e| format!("rename {from}: {e}"))?;
done.push((f.to_path_buf(), t.to_path_buf()));
move_one(fs, f, t).map_err(|e| fail(format!("rename {from}: {e}")))?;
// sidecars moved (or copied: `true`)
let mut done: Vec<(PathBuf, PathBuf, bool)> = Vec::new();
for (sc, naming, upper) in sidecars(from) {
if !sc.is_file() {
if !fs.is_file(&sc) {
continue;
}
let mut dst = crate::sidecar::sidecar_path(to, naming);
if upper {
dst = dst.with_extension("XMP");
}
if dst.exists() && !case_only {
// a stem sidecar another file still uses (`IMG_1.xmp` of `IMG_1.CR3` and `IMG_1.JPG`)
// is copied, not taken away from it (issue #92)
let shared = naming == SidecarNaming::Stem && fs.stem_sibling(&sc).is_some();
if fs.exists(&dst) && (shared || !is_respelling(fs, &sc, &dst)) {
continue; // never overwrite: the old sidecar stays where it was
}
if let Err(e) = mv(&sc, &dst) {
for (a, b) in done.iter().rev() {
let _ = std::fs::rename(b, a);
let r = if shared { fs.copy_no_replace(&sc, &dst) } else { move_one(fs, &sc, &dst) };
if let Err(e) = r {
let mut message = format!("rename {}: {e}", sc.display());
// the file first: if it can't go back, the sidecars that moved stay with it
if let Err(e) = move_one(fs, t, f) {
message.push_str(&format!("; {from} could not be moved back ({e}): it is now {to}"));
if !done.is_empty() {
let names: Vec<String> = done.iter().map(|(_, b, _)| b.display().to_string()).collect();
message.push_str(&format!(" with its sidecar {}", names.join(", ")));
}
return Err(MoveError { message, moved: true });
}
return Err(format!("rename {}: {e}", sc.display()));
for (a, b, copied) in done.iter().rev() {
if *copied {
// the original is still there: the copy just goes (a leftover copy loses nothing)
let _ = fs.remove_file(b);
} else if let Err(e) = move_one(fs, b, a) {
message.push_str(&format!("; sidecar {} could not be moved back ({e}): it is now {}", a.display(), b.display()));
}
}
return Err(fail(message));
}
done.push((sc, dst));
done.push((sc, dst, shared));
}
Ok(())
}
/// What a failed batch of moves ([`move_all`]) left behind.
#[derive(Debug)]
pub(crate) struct BatchMoveError {
/// The first failure and every move that could not be undone.
pub message: String,
/// Moves that stayed done, (from, to): those files are at `to` now.
pub stuck: Vec<(String, String)>,
}
/// Move the files `moves` names, all or nothing: after a failure the earlier moves are undone.
/// Moves that can't be undone (e.g. the network share went away) are reported, not ignored.
pub(crate) fn move_all(fs: &dyn MoveFs, moves: &[(String, String)]) -> std::result::Result<(), BatchMoveError> {
let mut done: Vec<&(String, String)> = Vec::new();
for m in moves {
let Err(e) = move_file_with(fs, &m.0, &m.1) else {
done.push(m);
continue;
};
let mut message = e.message;
let mut stuck = Vec::new();
if e.moved {
stuck.push(m.clone());
}
for d in done.iter().rev() {
if let Err(e) = move_file_with(fs, &d.1, &d.0) {
message.push_str(&format!("; {} could not be moved back to {}: {}", d.1, d.0, e.message));
if !e.moved {
stuck.push((*d).clone());
}
}
}
return Err(BatchMoveError { message, stuck });
}
Ok(())
}
/// The `SetFile` ops in `op` (nested batches included) that point a photo at one of `to`.
fn set_file_ops_to(op: &Op, to: &HashSet<&str>, out: &mut Vec<Op>) {
match op {
Op::Batch { ops } => ops.iter().for_each(|o| set_file_ops_to(o, to, out)),
Op::SetFile { source: Source::File { path }, .. } if to.contains(path.as_str()) => out.push(op.clone()),
_ => {}
}
}
impl Session {
/// Plan renaming `ids` with `template` (sequence numbers from `start`), resolving collisions.
pub fn plan_rename(&self, ids: &[PhotoId], template: &str, start: usize) -> Vec<RenamePlan> {
self.plan_rename_with(&RealFs, ids, template, start)
}
/// [`Session::plan_rename`] on the file system `fs`.
pub(crate) fn plan_rename_with(&self, fs: &dyn MoveFs, ids: &[PhotoId], template: &str, start: usize) -> Vec<RenamePlan> {
// photos sharing one file (virtual copies) rename once
let mut seen_paths: HashSet<String> = HashSet::new();
let mut taken: HashSet<String> = HashSet::new(); // lower-case target paths/names claimed in this batch
@ -382,10 +579,12 @@ impl Session {
let name = candidate(k);
let tp = dir.join(&name).to_string_lossy().to_string();
let key = tp.to_lowercase();
let same = key == path.to_lowercase();
// this very file, maybe spelled differently (case-insensitive volume)? By
// identity: on a case-sensitive volume `img_1.jpg` may be another photo
let same = tp == *path || is_respelling(fs, Path::new(path), Path::new(&tp));
// free: not claimed in this batch and not on disk (unless it is this very file).
// A file this batch moves away still counts as taken: simple and safe.
let on_disk = Path::new(&tp).exists() && !same;
let on_disk = fs.exists(Path::new(&tp)) && !same;
if !taken.contains(&key) && !on_disk {
taken.insert(key);
break (name, Some(tp));
@ -415,21 +614,16 @@ impl Session {
/// Carry out `plans`: move the files (rolled back on failure), then commit one undoable op
/// that updates the photos (and their virtual copies).
///
/// When a failure can't be fully rolled back, the files that kept their new names are
/// committed as a smaller undoable rename, so the library points at them (nothing goes
/// missing), and the error lists them (old → new).
pub fn apply_rename(&mut self, plans: &[RenamePlan]) -> Result<usize> {
let mut moved: Vec<(String, String)> = Vec::new();
for pl in plans {
if let (Some(a), Some(b)) = (&pl.from_path, &pl.to_path)
&& a != b
{
if let Err(e) = move_file(a, b) {
for (a, b) in moved.iter().rev() {
let _ = move_file(b, a);
}
return Err(EngineError::Other(e));
}
moved.push((a.clone(), b.clone()));
}
}
self.apply_rename_with(&RealFs, plans)
}
/// [`Session::apply_rename`] on the file system `fs`.
pub(crate) fn apply_rename_with(&mut self, fs: &dyn MoveFs, plans: &[RenamePlan]) -> Result<usize> {
let mut ops = Vec::new();
let by_path: HashMap<&str, &RenamePlan> = plans.iter().filter_map(|p| p.from_path.as_deref().map(|f| (f, p))).collect();
for p in self.catalog.photos() {
@ -447,17 +641,66 @@ impl Session {
}
}
let n = ops.len();
let op = Op::Batch { ops };
let moves: Vec<(String, String)> = plans
.iter()
.filter_map(|pl| match (&pl.from_path, &pl.to_path) {
(Some(a), Some(b)) if a != b => Some((a.clone(), b.clone())),
_ => None,
})
.collect();
if let Err(e) = move_all(fs, &moves) {
return Err(EngineError::Other(self.follow_stuck(&op, e, true)));
}
if n > 0
&& let Err(e) = self.commit(&format!("Rename {n} Photo{}", if n == 1 { "" } else { "s" }), Op::Batch { ops })
&& let Err(err) = self.commit(&format!("Rename {n} Photo{}", if n == 1 { "" } else { "s" }), op)
{
for (a, b) in moved.iter().rev() {
let _ = move_file(b, a);
let back: Vec<(String, String)> = moves.iter().rev().map(|(a, b)| (b.clone(), a.clone())).collect();
if let Err(e) = move_all(fs, &back) {
return Err(EngineError::Other(format!("{err}; the renamed files could not all be moved back: {}", e.message)));
}
return Err(e);
return Err(err);
}
Ok(n)
}
/// After a failed batch of moves for `op`: point the photos whose files stayed moved at
/// their new paths (an undoable "partly" rename when `undoable`, otherwise a plain logged
/// change), so the library matches the disk. Returns the message for the user.
pub(crate) fn follow_stuck(&mut self, op: &Op, e: BatchMoveError, undoable: bool) -> String {
let mut msg = e.message;
if e.stuck.is_empty() {
return msg;
}
let to: HashSet<&str> = e.stuck.iter().map(|m| m.1.as_str()).collect();
let mut keep = Vec::new();
set_file_ops_to(op, &to, &mut keep);
let list: Vec<String> = e.stuck.iter().map(|(a, b)| format!("{a} → {b}")).collect();
let n = e.stuck.len();
let s = if n == 1 { "" } else { "s" };
let fix = Op::Batch { ops: keep };
let applied = if undoable {
self.commit(&format!("Rename {n} Photo{s} (partly)"), fix)
} else {
match self.catalog.apply(fix.clone()) {
Ok(_) => {
self.pending_log.push(fix);
Ok(())
}
Err(e) => Err(e.into()),
}
};
match applied {
Ok(()) => msg
.push_str(&format!(". {n} file{s} could not be moved back and kept the new name; the library now points there: {}", list.join(", "))),
Err(err) => msg.push_str(&format!(
". {n} file{s} could not be moved back and the library could not follow ({err}); relink with Find Missing Photos: {}",
list.join(", ")
)),
}
msg
}
/// File moves an undo/redo op implies: `SetFile` ops whose source path differs from the photo's
/// current one.
pub(crate) fn file_moves(&self, op: &Op) -> Vec<(String, String)> {
@ -479,26 +722,240 @@ impl Session {
rec(self, op, &mut v);
v
}
/// Move the files `moves` names; all or nothing.
pub(crate) fn move_files(moves: &[(String, String)]) -> Result<()> {
let mut done: Vec<&(String, String)> = Vec::new();
for m in moves {
if let Err(e) = move_file(&m.0, &m.1) {
for (a, b) in done.iter().rev().map(|m| (&m.0, &m.1)) {
let _ = move_file(b, a);
}
return Err(EngineError::Other(format!("can't undo the rename: {e}")));
}
done.push(m);
}
Ok(())
}
}
#[cfg(test)]
mod tests {
use super::*;
use std::cell::RefCell;
/// A model volume: files by path with contents; case-insensitive (default APFS/NTFS) or
/// case-sensitive (Linux, case-sensitive APFS). `fail` makes renames from a matching source fail.
pub(crate) struct FakeFs {
pub ci: bool,
pub files: RefCell<Vec<(String, Vec<u8>)>>,
pub fail: RefCell<Vec<String>>,
}
impl FakeFs {
pub fn new(ci: bool, files: &[(&str, &str)]) -> FakeFs {
FakeFs {
ci,
files: RefCell::new(files.iter().map(|(p, c)| (p.to_string(), c.as_bytes().to_vec())).collect()),
fail: RefCell::new(Vec::new()),
}
}
fn idx(&self, p: &Path) -> Option<usize> {
let p = p.to_string_lossy();
self.files.borrow().iter().position(|(f, _)| if self.ci { f.to_lowercase() == p.to_lowercase() } else { *f == p })
}
/// The listing: (exact path, contents), sorted.
pub fn listing(&self) -> Vec<(String, String)> {
let mut v: Vec<_> = self.files.borrow().iter().map(|(p, c)| (p.clone(), String::from_utf8_lossy(c).to_string())).collect();
v.sort();
v
}
}
impl MoveFs for FakeFs {
fn exists(&self, p: &Path) -> bool {
self.idx(p).is_some()
}
fn is_file(&self, p: &Path) -> bool {
self.exists(p)
}
fn same_file(&self, a: &Path, b: &Path) -> bool {
self.idx(a).is_some() && self.idx(a) == self.idx(b)
}
fn rename_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()> {
if self.fail.borrow().iter().any(|f| a.to_string_lossy().contains(f.as_str())) {
return Err(std::io::Error::other("injected failure"));
}
if self.exists(b) {
return Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "exists"));
}
let i = self.idx(a).ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "no such file"))?;
self.files.borrow_mut()[i].0 = b.to_string_lossy().to_string();
Ok(())
}
fn copy_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()> {
if self.exists(b) {
return Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "exists"));
}
let i = self.idx(a).ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "no such file"))?;
let c = self.files.borrow()[i].1.clone();
self.files.borrow_mut().push((b.to_string_lossy().to_string(), c));
Ok(())
}
fn remove_file(&self, p: &Path) -> std::io::Result<()> {
let i = self.idx(p).ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "no such file"))?;
self.files.borrow_mut().remove(i);
Ok(())
}
fn stem_sibling(&self, sidecar: &Path) -> Option<String> {
let stem = sidecar.with_extension("").to_string_lossy().to_string();
self.files.borrow().iter().map(|(f, _)| f.clone()).find(|f| {
let p = Path::new(f);
p.with_extension("").to_string_lossy() == stem && !p.extension().is_some_and(|e| e.eq_ignore_ascii_case("xmp"))
})
}
}
fn file_photo(id: u64, path: &str) -> Photo {
let name = Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default();
Photo::new(PhotoId(id), Source::File { path: path.into() }, &name, "JPG", 1, 1, "2026-01-01T00:00:00")
}
/// Issue #95: a case-only rename is a real change on a case-insensitive volume and never
/// replaces a different file that differs only by case on a case-sensitive one.
#[test]
fn case_only_renames_never_replace_another_file() {
// case-sensitive: IMG_1.JPG and img_1.JPG are two photos
let fs = FakeFs::new(false, &[("/p/IMG_1.JPG", "first"), ("/p/img_1.JPG", "second"), ("/p/IMG_1.xmp", "sc1"), ("/p/img_1.xmp", "sc2")]);
assert!(move_file_with(&fs, "/p/IMG_1.JPG", "/p/img_1.JPG").unwrap_err().message.contains("already exists"));
assert_eq!(fs.listing().len(), 4);
assert!(fs.listing().contains(&("/p/img_1.JPG".into(), "second".into())));
// the planner picks a free name instead
let mut s = Session::new();
s.catalog.apply(Op::AddPhoto { photo: Box::new(file_photo(1, "/p/IMG_1.JPG")) }).unwrap();
let plans = s.plan_rename_with(&fs, &[PhotoId(1)], "img_1", 1);
assert_eq!(plans[0].to, "img_1-1.JPG");
move_file_with(&fs, "/p/IMG_1.JPG", plans[0].to_path.as_deref().unwrap()).unwrap();
assert_eq!(
fs.listing(),
vec![
("/p/img_1-1.JPG".into(), "first".into()),
("/p/img_1-1.xmp".into(), "sc1".into()),
("/p/img_1.JPG".into(), "second".into()),
("/p/img_1.xmp".into(), "sc2".into())
]
);
// case-sensitive, target free: a plain rename
let fs = FakeFs::new(false, &[("/p/IMG_2.JPG", "x")]);
move_file_with(&fs, "/p/IMG_2.JPG", "/p/img_2.JPG").unwrap();
assert_eq!(fs.listing(), vec![("/p/img_2.JPG".into(), "x".into())]);
// case-insensitive: the "existing" target is the photo itself → renamed via a temp name
let fs = FakeFs::new(true, &[("/p/IMG_1.JPG", "first"), ("/p/IMG_1.xmp", "sc1")]);
let mut s = Session::new();
s.catalog.apply(Op::AddPhoto { photo: Box::new(file_photo(1, "/p/IMG_1.JPG")) }).unwrap();
let plans = s.plan_rename_with(&fs, &[PhotoId(1)], "img_1", 1);
assert_eq!(plans[0].to, "img_1.JPG", "not a collision with itself");
move_file_with(&fs, "/p/IMG_1.JPG", "/p/img_1.JPG").unwrap();
assert_eq!(fs.listing(), vec![("/p/img_1.JPG".into(), "first".into()), ("/p/img_1.xmp".into(), "sc1".into())]);
// case-insensitive, another file under another name: still a collision
let fs = FakeFs::new(true, &[("/p/A.JPG", "a"), ("/p/b.JPG", "b")]);
assert!(move_file_with(&fs, "/p/A.JPG", "/p/B.JPG").is_err());
}
/// Issue #105: a rollback that fails is reported, and the library follows the files that kept
/// their new names (an undoable partial rename), so nothing goes missing.
#[test]
fn failed_rollbacks_are_reported_and_followed() {
let fs = FakeFs::new(false, &[("/p/a.jpg", "A"), ("/p/a.xmp", "As"), ("/p/b.jpg", "B"), ("/p/c.jpg", "C")]);
let mut s = Session::new();
for (i, f) in ["/p/a.jpg", "/p/b.jpg", "/p/c.jpg"].iter().enumerate() {
s.catalog.apply(Op::AddPhoto { photo: Box::new(file_photo(i as u64 + 1, f)) }).unwrap();
}
let ids = [PhotoId(1), PhotoId(2), PhotoId(3)];
let plans = s.plan_rename_with(&fs, &ids, "Trip-{seq}", 1);
// c can't be renamed (the share went away), and neither can Trip-1 be moved back
fs.fail.borrow_mut().extend(["c.jpg".to_string(), "Trip-1".to_string()]);
let err = s.apply_rename_with(&fs, &plans).unwrap_err().to_string();
assert!(err.contains("rename /p/c.jpg"), "{err}");
assert!(err.contains("/p/Trip-1.jpg could not be moved back"), "{err}");
assert!(err.contains("/p/a.jpg → /p/Trip-1.jpg"), "lists what stayed renamed: {err}");
assert_eq!(
fs.listing(),
vec![
("/p/Trip-1.jpg".into(), "A".into()),
("/p/Trip-1.xmp".into(), "As".into()),
("/p/b.jpg".into(), "B".into()),
("/p/c.jpg".into(), "C".into())
]
);
// the catalog matches the disk
let path = |s: &Session, id: u64| match &s.catalog.photo(PhotoId(id)).unwrap().source {
Source::File { path } => path.clone(),
Source::Demo { .. } => String::new(),
};
assert_eq!(path(&s, 1), "/p/Trip-1.jpg");
assert_eq!(s.catalog.photo(PhotoId(1)).unwrap().file_name, "Trip-1.jpg");
assert_eq!((path(&s, 2), path(&s, 3)), ("/p/b.jpg".into(), "/p/c.jpg".into()));
assert_eq!(s.undo.last().map(|e| e.label.as_str()), Some("Rename 1 Photo (partly)"));
// a sidecar that won't move, and a file that won't go back: the file keeps its new name
// with the sidecars that did move, and says so
let fs = FakeFs::new(false, &[("/p/d.jpg", "D"), ("/p/d.xmp", "Ds"), ("/p/d.jpg.xmp", "Df")]);
fs.fail.borrow_mut().extend(["d.jpg.xmp".to_string(), "/p/e.jpg".to_string()]);
let e = move_file_with(&fs, "/p/d.jpg", "/p/e.jpg").unwrap_err();
assert!(e.moved, "{e}");
assert!(e.message.contains("it is now /p/e.jpg with its sidecar /p/e.xmp"), "{e}");
assert_eq!(fs.listing(), vec![("/p/d.jpg.xmp".into(), "Df".into()), ("/p/e.jpg".into(), "D".into()), ("/p/e.xmp".into(), "Ds".into())]);
// … and when the file does go back, its sidecars follow it
let fs = FakeFs::new(false, &[("/p/d.jpg", "D"), ("/p/d.xmp", "Ds"), ("/p/d.jpg.xmp", "Df")]);
fs.fail.borrow_mut().push("d.jpg.xmp".to_string());
let e = move_file_with(&fs, "/p/d.jpg", "/p/e.jpg").unwrap_err();
assert!(!e.moved);
assert_eq!(fs.listing().len(), 3);
assert!(fs.listing().iter().all(|(p, _)| p.starts_with("/p/d.")));
}
/// Issue #92 (comment): renaming one of `IMG_1.CR3` + `IMG_1.JPG` copies their shared
/// `IMG_1.xmp` instead of taking it away from the other; the last one renamed takes it.
#[test]
fn shared_stem_sidecar_is_copied_not_taken() {
let fs = FakeFs::new(false, &[("/p/IMG_1.CR3", "raw"), ("/p/IMG_1.JPG", "jpg"), ("/p/IMG_1.xmp", "shared"), ("/p/IMG_1.CR3.xmp", "own")]);
move_file_with(&fs, "/p/IMG_1.CR3", "/p/Trip_1.CR3").unwrap();
assert_eq!(
fs.listing(),
vec![
("/p/IMG_1.JPG".into(), "jpg".into()),
("/p/IMG_1.xmp".into(), "shared".into()),
("/p/Trip_1.CR3".into(), "raw".into()),
("/p/Trip_1.CR3.xmp".into(), "own".into()),
("/p/Trip_1.xmp".into(), "shared".into()),
]
);
// the JPEG is the last one using it: it moves
move_file_with(&fs, "/p/IMG_1.JPG", "/p/Trip_2.JPG").unwrap();
assert!(fs.listing().contains(&("/p/Trip_2.xmp".into(), "shared".into())));
assert!(!fs.exists(Path::new("/p/IMG_1.xmp")));
// a failure after the copy removes the copy and keeps the original
let fs = FakeFs::new(false, &[("/p/A.CR3", "raw"), ("/p/A.JPG", "jpg"), ("/p/A.xmp", "shared"), ("/p/A.CR3.xmp", "own")]);
fs.fail.borrow_mut().push("A.CR3.xmp".into());
assert!(move_file_with(&fs, "/p/A.CR3", "/p/B.CR3").is_err());
assert_eq!(fs.listing().len(), 4);
assert!(fs.listing().contains(&("/p/A.xmp".into(), "shared".into())) && !fs.exists(Path::new("/p/B.xmp")));
}
/// Issue #95 on the real disk, whichever kind of volume the temp folder is on.
#[test]
fn case_only_rename_on_disk() {
let dir = std::env::temp_dir().join(format!("lc-rename-case-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let (upper, lower) = (dir.join("IMG_1.JPG"), dir.join("img_1.JPG"));
std::fs::write(&upper, b"first").unwrap();
let case_sensitive = !lower.exists();
let p = |q: &Path| q.to_string_lossy().to_string();
if case_sensitive {
std::fs::write(&lower, b"second").unwrap();
assert!(move_file(&p(&upper), &p(&lower)).is_err());
assert_eq!(std::fs::read(&lower).unwrap(), b"second");
assert_eq!(std::fs::read(&upper).unwrap(), b"first");
} else {
move_file(&p(&upper), &p(&lower)).unwrap();
let names: Vec<String> = std::fs::read_dir(&dir).unwrap().flatten().map(|e| e.file_name().to_string_lossy().to_string()).collect();
assert_eq!(names, vec!["img_1.JPG".to_string()]);
assert_eq!(std::fs::read(&lower).unwrap(), b"first");
}
// a plain rename never replaces
std::fs::write(dir.join("other.JPG"), b"other").unwrap();
assert!(move_file(&p(&dir.join("other.JPG")), &p(&lower)).is_err());
assert_eq!(std::fs::read(dir.join("other.JPG")).unwrap(), b"other");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn folder_templates_stay_inside_the_destination() {

View File

@ -5,12 +5,25 @@
//! creator, keywords, capture time and GPS in the standard namespaces, plus our complete develop
//! settings (`lc:settings`, JSON), the pick/reject flag (`lc:flag`) and location (`lc:location`).
//!
//! An existing sidecar is never replaced wholesale (issue #92): LightCraft's properties
//! ([`OWNED`], [`OWNED_IF_STATED`]) are merged into it ([`lightcraft_meta::merge_xmp`]) and
//! everything else — another editor's `crs:` develop settings, `xmpMM` history, unknown
//! namespaces — is kept byte for byte. A sidecar that can't be read as XMP is first copied to
//! `<name>.xmp.bak-<time>` (never overwritten), then replaced. Writes are atomic (temp file,
//! fsync, rename).
//!
//! **Shared names** (Stem naming): `IMG_0001.CR3` and `IMG_0001.JPG` would share `IMG_0001.xmp`.
//! When two catalogued files map to one stem sidecar, it belongs to one of them (a raw first, then
//! the first by file name — [`Session::sidecar_naming`]); the others use `<file>.xmp`
//! (`IMG_0001.JPG.xmp`) for writing and reading, so neither overwrites the other.
//!
//! **Read** (on import, and `photo.readMetadataFromFile`): the sidecar wins for metadata, except
//! the capture time: a time embedded in the file wins, and the sidecar's (`exif:DateTimeOriginal`,
//! else `photoshop:DateCreated`, else `xmp:CreateDate`) fills in only when the file has none; develop
//! settings are restored from `lc:settings` when present, else mapped from interoperable `crs:`
//! fields ([`crate::crs`]). For raw/DNG files without a sidecar the file's embedded XMP is used.
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::sync::Arc;
@ -258,13 +271,99 @@ pub fn sidecar_packet(p: &Photo, cat: &lightcraft_catalog::Catalog) -> String {
lightcraft_meta::write_xmp_lc(&meta, &lc)
}
/// Write `data` to `path` atomically (temp file + rename).
/// The properties LightCraft writes and owns in a sidecar: replaced on every save (removed when
/// LightCraft has no value, so clearing a field clears it in the file too). All of them are read
/// back on import and by Read Metadata from File.
pub const OWNED: &[&str] = &[
"xmp:Rating",
"xmp:Label",
"dc:title",
"dc:description",
"dc:rights",
"dc:creator",
"dc:subject",
"Iptc4xmpCore:Location",
"Iptc4xmpCore:AltTextAccessibility",
"Iptc4xmpCore:ExtDescrAccessibility",
"photoshop:City",
"photoshop:State",
"photoshop:Country",
"xmpRights:Marked",
"xmpRights:UsageTerms",
"xmpRights:WebStatement",
"lc:*",
];
/// Properties replaced only when LightCraft has a value (capture time; GPS, which LightCraft
/// doesn't read from sidecars): otherwise the sidecar's own value stays. Anything else LightCraft
/// writes (`xmp:CreatorTool`) goes only into new sidecars.
pub const OWNED_IF_STATED: &[&str] = &["exif:DateTimeOriginal", "photoshop:DateCreated", "exif:GPSLatitude", "exif:GPSLongitude"];
/// Write `data` to `path` atomically: a temp file next to it, synced, then renamed over it.
fn write_atomic(path: &Path, data: &[u8]) -> std::io::Result<()> {
let tmp = path.with_extension(format!("xmp.tmp-{}", std::process::id()));
std::fs::write(&tmp, data)?;
std::fs::rename(&tmp, path).inspect_err(|_| {
let _ = std::fs::remove_file(&tmp);
})
lightcraft_catalog::safe_file::write_atomic(path, data)
}
/// Copy `path` to a new `<name>.bak-<stamp>` (`-1`, `-2`… if taken; never overwriting).
fn backup(path: &Path, stamp: &str) -> std::io::Result<PathBuf> {
use std::io::Write;
let bytes = std::fs::read(path)?;
let stamp: String = stamp.chars().filter(char::is_ascii_alphanumeric).collect();
let name = path.file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default();
for k in 0..1000 {
let b = path.with_file_name(if k == 0 { format!("{name}.bak-{stamp}") } else { format!("{name}.bak-{stamp}-{k}") });
match std::fs::OpenOptions::new().write(true).create_new(true).open(&b) {
Ok(mut f) => {
f.write_all(&bytes)?;
f.sync_all()?;
return Ok(b);
}
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
Err(e) => return Err(e),
}
}
Err(std::io::Error::other("no free backup name"))
}
/// What [`Session::save_sidecar`] did.
#[derive(Clone, Debug, PartialEq)]
pub struct SidecarSaved {
pub path: PathBuf,
/// An existing sidecar was merged into (its other contents kept).
pub merged: bool,
/// An existing sidecar that couldn't be read as XMP was copied here before being replaced.
pub backup: Option<PathBuf>,
}
/// The new contents for the sidecar at `path`: `fresh` merged into the existing file, if any.
/// Returns (contents, merged, backup).
fn sidecar_contents(path: &Path, fresh: String, stamp: &str) -> std::io::Result<(String, bool, Option<PathBuf>)> {
let bytes = match std::fs::read(path) {
Ok(b) => b,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok((fresh, false, None)),
// can't see what is there: never overwrite it blindly
Err(e) => return Err(e),
};
if bytes.iter().all(u8::is_ascii_whitespace) {
return Ok((fresh, false, None));
}
let rules = lightcraft_meta::MergeRules { owned: OWNED, owned_if_present: OWNED_IF_STATED };
match std::str::from_utf8(&bytes)
.map_err(|e| e.to_string())
.and_then(|old| lightcraft_meta::merge_xmp(old, &fresh, rules).map_err(|e| e.to_string()))
{
Ok(merged) => Ok((merged, true, None)),
Err(why) => {
let b = backup(path, stamp)?;
log::warn!("{}: not readable as XMP ({why}); kept a copy as {} before replacing it", path.display(), b.display());
Ok((fresh, false, Some(b)))
}
}
}
/// Key of the stem sidecar a file maps to (case-insensitive).
fn stem_key(original: &str) -> String {
sidecar_path(original, SidecarNaming::Stem).to_string_lossy().to_lowercase()
}
fn file_path(p: &Photo) -> Option<&str> {
@ -302,24 +401,70 @@ pub(crate) fn op_photos(op: &Op, out: &mut Vec<PhotoId>) {
}
}
/// Which photo owns each stem sidecar that several catalogued files map to (see
/// [`Session::sidecar_naming`]).
pub(crate) struct StemOwners(HashMap<String, PhotoId>);
impl StemOwners {
pub(crate) fn of(cat: &lightcraft_catalog::Catalog) -> StemOwners {
// per stem: (raw first, then file name, then id) of the best candidate; and how many
let mut best: HashMap<String, ((bool, String, u64), PhotoId, usize)> = HashMap::new();
for p in cat.photos().filter(|p| p.copy_of.is_none()) {
let Some(path) = file_path(p) else { continue };
let rank = (p.kind != MediaKind::Raw, path.to_lowercase(), p.id.0);
let e = best.entry(stem_key(path)).or_insert_with(|| (rank.clone(), p.id, 0));
e.2 += 1;
if rank < e.0 {
(e.0, e.1) = (rank, p.id);
}
}
StemOwners(best.into_iter().filter(|(_, v)| v.2 > 1).map(|(k, v)| (k, v.1)).collect())
}
/// `naming` for photo `p`, except that a stem sidecar another file owns becomes Full.
fn naming(&self, p: &Photo, naming: SidecarNaming) -> SidecarNaming {
match (naming, file_path(p)) {
(SidecarNaming::Stem, Some(path)) if self.0.get(&stem_key(path)).is_some_and(|owner| *owner != p.id) => SidecarNaming::Full,
_ => naming,
}
}
}
impl Session {
/// Write the photo's XMP sidecar; returns its path.
pub fn save_sidecar(&self, id: PhotoId) -> Result<PathBuf> {
/// The sidecar naming for photo `id`: the library's, except under Stem naming when another
/// catalogued file shares the stem (`IMG_0001.CR3` + `IMG_0001.JPG`) and owns the stem
/// sidecar — a raw first, else the first by file name. The others use Full naming
/// (`IMG_0001.JPG.xmp`), so their metadata never overwrites each other.
pub fn sidecar_naming(&self, id: PhotoId) -> SidecarNaming {
match self.catalog.photo(id) {
Some(p) if self.xmp.naming == SidecarNaming::Stem => StemOwners::of(&self.catalog).naming(p, self.xmp.naming),
_ => self.xmp.naming,
}
}
/// Write the photo's XMP sidecar, merged into an existing one (see the module docs).
pub fn save_sidecar(&self, id: PhotoId) -> Result<SidecarSaved> {
self.save_sidecar_with(id, &StemOwners::of(&self.catalog))
}
pub(crate) fn save_sidecar_with(&self, id: PhotoId, owners: &StemOwners) -> Result<SidecarSaved> {
let p = self.catalog.photo(id).ok_or(lightcraft_catalog::CatalogError::NoPhoto(id))?;
if p.copy_of.is_some() {
return Err(EngineError::Other(format!("{} is a virtual copy: its settings live only in the library", p.file_name)));
}
let orig = file_path(p).ok_or_else(|| EngineError::Other(format!("{} is not a file on disk", p.file_name)))?;
let path = sidecar_path(orig, self.xmp.naming);
write_atomic(&path, sidecar_packet(p, &self.catalog).as_bytes()).map_err(|e| EngineError::Other(format!("{}: {e}", path.display())))?;
Ok(path)
let path = sidecar_path(orig, owners.naming(p, self.xmp.naming));
let err = |e: std::io::Error| EngineError::Other(format!("{}: {e}", path.display()));
let (data, merged, backup) = sidecar_contents(&path, sidecar_packet(p, &self.catalog), &(self.clock)()).map_err(err)?;
write_atomic(&path, data.as_bytes()).map_err(err)?;
Ok(SidecarSaved { path, merged, backup })
}
/// The op that applies a photo's sidecar (or embedded XMP) to the catalog, if there is one.
pub fn read_sidecar_op(&self, id: PhotoId) -> Result<Option<(Op, PathBuf)>> {
let p = self.catalog.photo(id).ok_or(lightcraft_catalog::CatalogError::NoPhoto(id))?;
let Some(orig) = file_path(p) else { return Ok(None) };
let Some((packet, from)) = read_packet(orig, p.kind, self.xmp.naming) else { return Ok(None) };
let Some((packet, from)) = read_packet(orig, p.kind, self.sidecar_naming(id)) else { return Ok(None) };
let sc = parse_sidecar(&packet, p.kind == MediaKind::Raw)
.map_err(|e| EngineError::Other(format!("{}: {e}", from.display())))?
.resolve_label(&self.catalog);
@ -344,9 +489,13 @@ impl Session {
pub(crate) fn auto_write_sidecars(&self, ops: &[Op]) {
let mut ids = Vec::new();
ops.iter().for_each(|o| op_photos(o, &mut ids));
if ids.is_empty() {
return;
}
let owners = StemOwners::of(&self.catalog);
for id in ids {
if self.catalog.photo(id).is_some_and(|p| file_path(p).is_some() && p.copy_of.is_none())
&& let Err(e) = self.save_sidecar(id)
&& let Err(e) = self.save_sidecar_with(id, &owners)
{
log::warn!("auto-write XMP: {e}");
}

View File

@ -130,11 +130,16 @@ pub fn migrate(from: &Path, to: &Path, what: Existing) -> (usize, Vec<String>) {
Existing::Discard => std::fs::remove_file(&src),
_ => {
let dst = to.join(e.file_name());
if dst.exists() {
// named by content: the copy already there is the same preview
if is_valid(&dst) {
// named by content: the complete copy already there is the same preview
std::fs::remove_file(&src)
} else {
std::fs::rename(&src, &dst).or_else(|_| std::fs::copy(&src, &dst).and_then(|_| std::fs::remove_file(&src)))
// across drives: copied atomically (never a partial proxy at the new place)
std::fs::rename(&src, &dst).or_else(|_| {
std::fs::read(&src)
.and_then(|b| lightcraft_catalog::safe_file::write_atomic(&dst, &b))
.and_then(|()| std::fs::remove_file(&src))
})
}
}
};
@ -146,6 +151,29 @@ pub fn migrate(from: &Path, to: &Path, what: Existing) -> (usize, Vec<String>) {
(done, failed)
}
/// A cheap check that the smart preview at `path` is complete: our header, then a JPEG from its
/// start marker to its end marker. A file cut short by a crash, a full drive or an unplugged one
/// fails it (and is then rebuilt by Build Smart Previews, not counted as there).
pub fn is_valid(path: &Path) -> bool {
use std::io::{Read, Seek, SeekFrom};
let Ok(mut f) = std::fs::File::open(path) else { return false };
let mut head = Vec::with_capacity(256);
if (&mut f).take(256).read_to_end(&mut head).is_err() {
return false;
}
let Some(rest) = head.strip_prefix(MAGIC) else { return false };
let Some(nl) = rest.iter().position(|b| *b == b'\n') else { return false };
let size_ok = rest
.get(..nl)
.and_then(|h| serde_json::from_slice::<serde_json::Value>(h).ok())
.is_some_and(|h| h["w"].as_u64().is_some_and(|w| w > 0) && h["h"].as_u64().is_some_and(|h| h > 0));
if !size_ok || rest.get(nl + 1..nl + 3) != Some(&[0xFF, 0xD8][..]) {
return false;
}
let mut end = [0u8; 2];
f.seek(SeekFrom::End(-2)).is_ok() && f.read_exact(&mut end).is_ok() && end == [0xFF, 0xD9]
}
/// Load the proxy at `path`.
pub fn load(path: &Path) -> Result<Arc<Rgb32f>, String> {
let b = std::fs::read(path).map_err(|e| format!("{}: {e}", path.display()))?;
@ -193,6 +221,27 @@ mod tests {
let _ = std::fs::remove_dir_all(&base);
}
#[test]
fn damaged_proxies_are_invalid_and_never_kept_over_good_ones() {
let base = temp("valid");
let (a, b) = (base.join("a"), base.join("b"));
std::fs::create_dir_all(&a).unwrap();
std::fs::create_dir_all(&b).unwrap();
let img = lightcraft_scenes::demo_library()[0].render(64, 40);
let good = encode(&img).unwrap();
std::fs::write(a.join("1.lcsp"), &good).unwrap();
assert!(is_valid(&a.join("1.lcsp")));
for cut in [good.len() - 1, good.len() / 2, 10, 0] {
std::fs::write(b.join("1.lcsp"), &good[..cut]).unwrap();
assert!(!is_valid(&b.join("1.lcsp")), "cut at {cut}");
}
assert!(!is_valid(&b.join("missing.lcsp")));
// moving onto a damaged copy replaces it instead of deleting the good one
assert_eq!(migrate(&a, &b, Existing::Move).0, 1);
assert_eq!(std::fs::read(b.join("1.lcsp")).unwrap(), good);
let _ = std::fs::remove_dir_all(&base);
}
#[test]
fn roundtrip_keeps_the_picture() {
let img = lightcraft_scenes::demo_library()[0].render(160, 100);

View File

@ -207,3 +207,119 @@ fn export_falls_back_to_the_cpu_when_gpu_work_is_lost() {
assert!(m > 0.02, "not black: mean {m}");
assert!((m - healthy).abs() < 0.01, "GPU {healthy} vs CPU {m}");
}
/// A JPEG original `dir/IMG_1.jpg` imported into a library on disk.
fn library_with_jpeg(tag: &str) -> (Session, lightcraft_catalog::PhotoId, std::path::PathBuf, std::path::PathBuf, Vec<u8>) {
let dir = temp_dir(tag);
let src = dir.join("IMG_1.jpg");
let img = lightcraft_raster::Rgba8::from_fn(48, 32, |x, y| [(x * 5) as u8, (y * 7) as u8, 120, 255]);
let bytes = crate::export::encode_image(&img, &ExportOptions::default()).unwrap();
std::fs::write(&src, &bytes).unwrap();
let mut s = Session::new().with_fs();
s.execute("library.import", &json!({"paths": [src.to_string_lossy()]})).unwrap();
let id = s.catalog.photos().next().unwrap().id;
s.execute("develop.set", &json!({"ids": [id.0], "control": "light.exposure", "value": 1.0})).unwrap();
(s, id, dir, src, bytes)
}
fn disk_batch(
s: &mut Session,
id: lightcraft_catalog::PhotoId,
o: &ExportOptions,
to: &crate::export::Destination,
) -> Result<Vec<serde_json::Value>, String> {
crate::export::export_batch(s, &[id], o, to, &mut crate::export::write_file, &|p| std::path::Path::new(p).exists())
}
/// Issue #93: exporting into the photo's own folder as `{name}` with "Overwrite" replaced the
/// original with the re-encoded render. It is refused now, whatever the policy or exact path.
#[test]
fn export_never_overwrites_an_original() {
use crate::export::{Conflict, Destination};
let (mut s, id, dir, src, original) = library_with_jpeg("export-guard");
let folder = Destination { dir: dir.to_string_lossy().to_string(), exact: None };
let o = ExportOptions { naming: "{name}".into(), conflict: Conflict::Overwrite, ..Default::default() };
let err = disk_batch(&mut s, id, &o, &folder).unwrap_err();
assert!(err.contains("original of IMG_1.jpg"), "{err}");
assert_eq!(std::fs::read(&src).unwrap(), original, "the original is byte-identical");
// the exact path of the control channel / MCP, also spelled another way
std::fs::create_dir_all(dir.join("sub")).unwrap();
for exact in [src.to_string_lossy().to_string(), dir.join("sub/../IMG_1.jpg").to_string_lossy().to_string()] {
let to = Destination { dir: String::new(), exact: Some(exact) };
assert!(disk_batch(&mut s, id, &o, &to).unwrap_err().contains("never writes over an original"));
}
// the "Original" format re-writing the file onto itself (and its sidecar)
let orig = ExportOptions { format: ExportFormat::Original, ..o.clone() };
assert!(disk_batch(&mut s, id, &orig, &folder).is_err());
assert_eq!(std::fs::read(&src).unwrap(), original);
assert!(s.execute("export.checkTarget", &json!({"path": src.to_string_lossy()})).is_err());
assert!(s.execute("export.checkTarget", &json!({"path": dir.join("free.jpg").to_string_lossy()})).is_ok());
// Unique moves past it; Overwrite still replaces an ordinary earlier export
let unique = ExportOptions { conflict: Conflict::Unique, ..o.clone() };
let files = disk_batch(&mut s, id, &unique, &folder).unwrap();
assert!(files[0]["path"].as_str().unwrap().ends_with("IMG_1-2.jpg"), "{files:?}");
let earlier = dir.join("IMG_1-2.jpg");
std::fs::write(&earlier, b"an earlier export").unwrap();
let named = ExportOptions { naming: "{name}-2".into(), ..o };
disk_batch(&mut s, id, &named, &folder).unwrap();
assert_ne!(std::fs::read(&earlier).unwrap(), b"an earlier export", "an ordinary file is overwritten");
assert_eq!(std::fs::read(&src).unwrap(), original);
let _ = std::fs::remove_dir_all(&dir);
}
/// Issue #93: the XMP sidecar of an "Original" export ignored the conflict policy.
#[test]
fn export_sidecars_follow_the_conflict_policy() {
use crate::export::{Conflict, Destination};
let (mut s, id, dir, _src, original) = library_with_jpeg("export-sidecar");
let out = dir.join("out");
std::fs::create_dir_all(&out).unwrap();
let theirs = out.join("IMG_1.xmp");
std::fs::write(&theirs, b"someone else's sidecar").unwrap();
let to = Destination { dir: out.to_string_lossy().to_string(), exact: None };
let o = ExportOptions { format: ExportFormat::Original, naming: "{name}".into(), ..Default::default() };
// Unique: the photo and its sidecar both move to the next free name
let files = disk_batch(&mut s, id, &ExportOptions { conflict: Conflict::Unique, ..o.clone() }, &to).unwrap();
assert!(files[0]["path"].as_str().unwrap().ends_with("IMG_1-2.jpg"), "{files:?}");
assert!(files[0]["sidecars"][0].as_str().unwrap().ends_with("IMG_1-2.xmp"), "{files:?}");
assert_eq!(std::fs::read(&theirs).unwrap(), b"someone else's sidecar");
assert_eq!(std::fs::read(out.join("IMG_1-2.jpg")).unwrap(), original);
// Skip: a taken sidecar name skips the photo
std::fs::remove_file(out.join("IMG_1-2.jpg")).unwrap();
std::fs::remove_file(out.join("IMG_1-2.xmp")).unwrap();
let files = disk_batch(&mut s, id, &ExportOptions { conflict: Conflict::Skip, ..o.clone() }, &to).unwrap();
assert!(files[0]["skipped"].as_str().is_some(), "{files:?}");
assert!(!out.join("IMG_1.jpg").exists());
assert_eq!(std::fs::read(&theirs).unwrap(), b"someone else's sidecar");
// Overwrite: replaced, as asked
disk_batch(&mut s, id, &ExportOptions { conflict: Conflict::Overwrite, ..o }, &to).unwrap();
assert!(std::fs::read_to_string(&theirs).unwrap().contains("xmpmeta"));
let _ = std::fs::remove_dir_all(&dir);
}
/// Issue #93: the native writer truncated the target first; a failure mid-write (full disk,
/// unplugged drive) now leaves the previous file intact and no partial one.
#[test]
fn a_failed_export_write_keeps_the_previous_file() {
use crate::export::{Conflict, Destination};
let (mut s, id, dir, _src, _) = library_with_jpeg("export-midwrite");
let out = dir.join("out");
std::fs::create_dir_all(&out).unwrap();
let prev = out.join("IMG_1.jpg");
std::fs::write(&prev, b"last week's export").unwrap();
let to = Destination { dir: out.to_string_lossy().to_string(), exact: None };
let o = ExportOptions { conflict: Conflict::Overwrite, ..Default::default() };
{
let _fault = lightcraft_catalog::safe_file::fail_writes_after(100);
let err = disk_batch(&mut s, id, &o, &to).unwrap_err();
assert!(err.contains("injected"), "{err}");
}
assert_eq!(std::fs::read(&prev).unwrap(), b"last week's export");
assert_eq!(std::fs::read_dir(&out).unwrap().count(), 1, "no temp file left behind");
disk_batch(&mut s, id, &o, &to).unwrap();
assert_ne!(std::fs::read(&prev).unwrap(), b"last week's export");
let _ = std::fs::remove_dir_all(&dir);
}

View File

@ -144,6 +144,45 @@ fn browsing_a_folder_lists_its_photos_without_adding_them() {
let _ = std::fs::remove_dir_all(&dir);
}
/// Issue #105: Find Missing finds renamed files by content, prefers the content match over a
/// same-name same-size impostor, and skips (reports) photos it can't tell apart.
#[test]
fn find_missing_matches_renamed_files_by_content() {
let dir = temp_dir("missing-content");
std::fs::create_dir_all(dir.join("old")).unwrap();
std::fs::create_dir_all(dir.join("moved/real")).unwrap();
for (n, seed) in [("a.png", 1), ("b.png", 2), ("c.png", 3)] {
write_png(&dir.join("old").join(n), seed);
}
let mut s = Session::new().with_fs();
s.execute("library.import", &json!({"paths": [dir.join("old").to_string_lossy()]})).unwrap();
// an impostor: same name and size, other bytes
let impostor = |src: &Path, dst: &Path| {
let mut b = std::fs::read(src).unwrap();
let k = b.len() - 20;
b[k] ^= 0xff;
std::fs::write(dst, b).unwrap();
};
// a: renamed; b: renamed, with an impostor under its old name; c: gone, two impostors
std::fs::rename(dir.join("old/a.png"), dir.join("moved/Trip-001.png")).unwrap();
impostor(&dir.join("old/b.png"), &dir.join("moved/b.png"));
std::fs::rename(dir.join("old/b.png"), dir.join("moved/real/Trip-002.png")).unwrap();
impostor(&dir.join("old/c.png"), &dir.join("moved/c.png"));
impostor(&dir.join("old/c.png"), &dir.join("moved/real/c.png"));
std::fs::remove_file(dir.join("old/c.png")).unwrap();
let r = s.execute("library.findMissing", &json!({"folder": dir.join("moved").to_string_lossy()})).unwrap();
let found = r["found"].as_array().unwrap();
assert_eq!(found.len(), 2, "{r}");
let to = |name: &str| {
found.iter().find(|f| f["from"].as_str().unwrap().ends_with(name)).map(|f| (f["to"].as_str().unwrap().to_string(), f["by"].clone()))
};
assert_eq!(to("a.png"), Some((dir.join("moved/Trip-001.png").to_string_lossy().to_string(), json!("content"))));
assert_eq!(to("b.png"), Some((dir.join("moved/real/Trip-002.png").to_string_lossy().to_string(), json!("content"))), "not the impostor");
assert_eq!(r["missing"], 1);
assert_eq!(r["ambiguous"][0]["candidates"].as_array().map(Vec::len), Some(2), "{r}");
let _ = std::fs::remove_dir_all(&dir);
}
#[test]
fn missing_files_are_found_and_relinked() {
let dir = std::env::temp_dir().join(format!("lc-missing-{}", std::process::id()));
@ -352,6 +391,44 @@ fn smart_previews_stand_in_for_offline_originals() {
let _ = std::fs::remove_dir_all(&lib);
}
/// Issue #106: a smart preview cut short (crash, full drive) counted as built forever and failed
/// exactly when the original went offline. Writes are atomic now, and a damaged proxy is not
/// reported as present and is rebuilt.
#[test]
fn damaged_smart_previews_are_rebuilt_and_failed_writes_leave_none() {
let src = temp_dir("smartdmg-src");
let lib = temp_dir("smartdmg-lib");
write_png(&src.join("a.png"), 5);
let mut s = Session::new().with_fs();
s.open_library(&lib, false).unwrap();
s.execute("library.import", &json!({"paths": [src.join("a.png").to_string_lossy()]})).unwrap();
let id = s.active().unwrap();
let dir = s.media.smart_dir.clone().unwrap();
let file = dir.join(crate::smart::file_name(s.catalog.photo(id).unwrap()));
{
let _fault = lightcraft_catalog::safe_file::fail_writes_after(64);
let r = s.execute("library.smartPreviews", &json!({})).unwrap();
assert_eq!((r["built"].as_u64(), r["failed"].as_array().map(Vec::len)), (Some(0), Some(1)), "{r}");
}
assert!(!file.exists(), "no partial proxy");
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 0, "no temp file left");
assert_eq!(s.execute("photo.smartPreview", &json!({})).unwrap()["smartPreview"], false);
let r = s.execute("library.smartPreviews", &json!({})).unwrap();
assert_eq!((r["built"].as_u64(), r["repaired"].as_u64()), (Some(1), Some(0)), "{r}");
assert!(crate::smart::is_valid(&file));
// cut short, as by a crash before the fix
let full = std::fs::read(&file).unwrap();
std::fs::write(&file, &full[..full.len() / 2]).unwrap();
assert!(!crate::smart::is_valid(&file));
assert_eq!(s.execute("photo.smartPreview", &json!({})).unwrap()["smartPreview"], false, "not counted as there");
let r = s.execute("library.smartPreviews", &json!({})).unwrap();
assert_eq!((r["built"].as_u64(), r["repaired"].as_u64()), (Some(1), Some(1)), "{r}");
assert_eq!(std::fs::read(&file).unwrap(), full, "rebuilt whole");
let _ = std::fs::remove_dir_all(&src);
let _ = std::fs::remove_dir_all(&lib);
}
/// LUT profiles: a .cube imports as a creative profile (into the library), renders differently
/// (Amount 0 = no change), is listed in the profile browser and comes back with the library.
#[test]

View File

@ -163,6 +163,50 @@ fn a_failed_or_corrupt_copy_keeps_the_source() {
}
}
/// Issue #96: Import → Copy used a plain unverified copy (and check-then-copy naming that could
/// replace a file). It now gets the same verified copy as Move: a copy that fails or comes out
/// different is removed and reported as a failed import, nothing is catalogued, and the card
/// is untouched; a good copy never replaces an existing file.
#[test]
fn copy_import_verifies_each_copy() {
let import_copy = |s: &mut Session, card: &Path, dest: &Path| {
s.execute(
"library.import",
&json!({"paths": [card.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": "flat"}),
)
.unwrap()
};
for fault in [Fault::FailWrite, Fault::Corrupt] {
let base = temp_dir(&format!("copyfail-{fault:?}"));
let (card, dest) = (base.join("card"), base.join("out"));
write_png(&card.join("a.png"), 3);
let bytes = std::fs::read(card.join("a.png")).unwrap();
let mut s = session();
inject(fault);
let r = import_copy(&mut s, &card, &dest);
inject(Fault::None);
assert_eq!(len(&r, "failed"), 1, "{fault:?}: {r}");
assert_eq!(len(&r, "imported"), 0, "{fault:?}: {r}");
assert!(r.to_string().contains("the original is untouched"), "{r}");
assert_eq!(s.catalog.len(), 0);
assert_eq!(std::fs::read(card.join("a.png")).unwrap(), bytes, "{fault:?}: source intact");
assert_eq!(files_under(&dest), Vec::<String>::new(), "{fault:?}: no bad copy left");
let _ = std::fs::remove_dir_all(&base);
}
// a good copy: verified, and a taken name gets -1 instead of being replaced
let base = temp_dir("copyok");
let (card, dest) = (base.join("card"), base.join("out"));
write_png(&card.join("a.png"), 4);
std::fs::create_dir_all(&dest).unwrap();
std::fs::write(dest.join("a.png"), b"someone else's file").unwrap();
let mut s = session();
let r = import_copy(&mut s, &card, &dest);
assert_eq!(len(&r, "imported"), 1, "{r}");
assert_eq!(std::fs::read(dest.join("a.png")).unwrap(), b"someone else's file");
assert_eq!(std::fs::read(dest.join("a-1.png")).unwrap(), std::fs::read(card.join("a.png")).unwrap());
let _ = std::fs::remove_dir_all(&base);
}
/// A source that can't be removed (a read-only card) stays and is reported; the photo is
/// imported from its copy.
#[test]

View File

@ -329,6 +329,55 @@ fn convert_raw_to_dng() {
let _ = std::fs::remove_dir_all(&dir);
}
/// Issue #106: Convert to DNG wrote straight to the final name (no temp file, sync or check) and
/// relinked the photo even when the write was cut short. A failed write now leaves no DNG and
/// the photo on its raw; a good one is verified and never replaces an existing file.
#[test]
fn convert_to_dng_is_verified_and_atomic() {
let dir = temp_dir("todng-safe");
// a synthetic raw (stored as DNG, catalogued as a NEF so Convert to DNG takes it)
let raw = dir.join("shot.dng");
let bytes = synthetic_dng_with(None, Default::default());
std::fs::write(&raw, &bytes).unwrap();
let mut s = Session::new().with_fs();
s.execute("library.import", &json!({"paths": [raw.to_string_lossy()]})).unwrap();
let id = s.catalog.photos().next().unwrap().id;
let path = raw.to_string_lossy().to_string();
s.catalog
.apply(lightcraft_catalog::Op::Relink {
id,
file_name: "shot.dng".into(),
source: lightcraft_catalog::Source::File { path: path.clone() },
format: Some("NEF".into()),
})
.unwrap();
s.execute("library.select", &json!({"ids": [id.0]})).unwrap();
let names = || {
let mut v: Vec<String> = std::fs::read_dir(&dir).unwrap().flatten().map(|e| e.file_name().to_string_lossy().to_string()).collect();
v.sort();
v
};
{
// the drive fills up part-way
let _fault = lightcraft_catalog::safe_file::fail_writes_after(200);
let r = s.execute("photo.convertToDng", &json!({})).unwrap();
assert_eq!(r["converted"].as_array().unwrap().len(), 0, "{r}");
assert!(r["skipped"][0][1].as_str().unwrap().contains("the raw is kept"), "{r}");
}
assert_eq!(names(), ["shot.dng"], "no partial DNG");
assert_eq!(s.catalog.photo(id).unwrap().source, lightcraft_catalog::Source::File { path: path.clone() }, "not relinked");
// a good conversion: a new name (the existing file is not replaced), decodable, relinked
let r = s.execute("photo.convertToDng", &json!({})).unwrap();
let out = r["converted"][0]["path"].as_str().unwrap().to_string();
assert!(out.ends_with("shot-2.dng"), "{r}");
assert_eq!(std::fs::read(&raw).unwrap(), bytes, "the raw is untouched");
let back = lightcraft_raw::decode(&std::fs::read(&out).unwrap()).unwrap();
assert_eq!(back.data, lightcraft_raw::decode(&bytes).unwrap().data);
assert_eq!(s.catalog.photo(id).unwrap().source, lightcraft_catalog::Source::File { path: out });
assert_eq!(names(), ["shot-2.dng", "shot.dng"], "no temp file left");
let _ = std::fs::remove_dir_all(&dir);
}
/// Edit in External Editor (engine half): a 16-bit TIFF `-Edit` copy with the edits, next to
/// the original, added and stacked on top of it; a second one doesn't overwrite the first.
#[test]
@ -576,3 +625,132 @@ fn copyright_status_usage_terms_and_url_round_trip() {
let _ = std::fs::remove_dir_all(&src);
let _ = std::fs::remove_dir_all(&lib);
}
/// A sidecar another raw developer wrote: develop settings in attributes and elements, a
/// structured edit history, an unknown namespace.
const OTHER_APP_SIDECAR: &str = r#"<?xpacket begin="" id="W5M0MpCehiHzreSzNTczkc9d"?>
<x:xmpmeta xmlns:x="adobe:ns:meta/" x:xmptk="Other Toolkit">
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
<rdf:Description rdf:about=""
xmlns:xmp="http://ns.adobe.com/xap/1.0/"
xmlns:xmpMM="http://ns.adobe.com/xap/1.0/mm/"
xmlns:stEvt="http://ns.adobe.com/xap/1.0/sType/ResourceEvent#"
xmlns:crs="http://ns.adobe.com/camera-raw-settings/1.0/"
xmlns:other="http://example.com/other/1.0/"
xmp:Rating="2"
crs:Version="99.0"
crs:Exposure2012="+0.65"
crs:HasSettings="True"
other:Secret="keep me">
<xmpMM:History>
<rdf:Seq>
<rdf:li stEvt:action="derived" stEvt:parameters="converted from image/x-raw to image/png"/>
</rdf:Seq>
</xmpMM:History>
<crs:ToneCurvePV2012>
<rdf:Seq>
<rdf:li>0, 0</rdf:li>
<rdf:li>128, 140</rdf:li>
<rdf:li>255, 255</rdf:li>
</rdf:Seq>
</crs:ToneCurvePV2012>
</rdf:Description>
</rdf:RDF>
</x:xmpmeta>
<?xpacket end="w"?>"#;
/// Issue #92: saving merges into another app's sidecar instead of replacing it.
#[test]
fn saving_merges_into_another_apps_sidecar() {
let src = temp_dir("merge");
let lib = temp_dir("merge-lib");
write_png(&src.join("shot.png"), 1);
std::fs::write(src.join("shot.xmp"), OTHER_APP_SIDECAR).unwrap();
let mut s = Session::new().with_fs();
s.open_library(&lib, false).unwrap();
s.execute("library.import", &json!({"paths": [src.join("shot.png").to_string_lossy()]})).unwrap();
let id = only(&s);
s.execute("library.select", &json!({"ids": [id.0]})).unwrap();
s.execute("photo.rate", &json!({"rating": 5})).unwrap();
s.execute("photo.setMeta", &json!({"title": "Harbour", "keywords": ["boats"]})).unwrap();
s.execute("develop.set", &json!({"control": "effects.clarity", "value": 30})).unwrap();
let r = s.execute("photo.saveMetadataToFile", &json!({})).unwrap();
assert_eq!(r["merged"][0], src.join("shot.xmp").to_string_lossy().as_ref(), "{r}");
let out = std::fs::read_to_string(src.join("shot.xmp")).unwrap();
// the other app's data, byte for byte
for part in [
"crs:Exposure2012=\"+0.65\"",
"other:Secret=\"keep me\"",
" <xmpMM:History>\n <rdf:Seq>\n <rdf:li stEvt:action=\"derived\" stEvt:parameters=\"converted from image/x-raw to image/png\"/>\n </rdf:Seq>\n </xmpMM:History>",
" <crs:ToneCurvePV2012>\n <rdf:Seq>\n <rdf:li>0, 0</rdf:li>\n <rdf:li>128, 140</rdf:li>",
] {
assert!(out.contains(part), "lost {part:?}:\n{out}");
}
let d = lightcraft_meta::parse_xmp(&out).unwrap();
assert_eq!(d.metadata.rating, Some(5), "ours replaces theirs");
assert_eq!(d.metadata.title.as_deref(), Some("Harbour"));
assert_eq!(d.properties.get("crs:Version"), Some(&vec!["99.0".to_string()]));
let saved = s.catalog.photo(id).unwrap().develop.clone();
// reading it back restores our settings (lc:settings wins over crs:)
s.execute("develop.set", &json!({"control": "effects.clarity", "value": 0})).unwrap();
s.execute("photo.readMetadataFromFile", &json!({})).unwrap();
assert_eq!(s.catalog.photo(id).unwrap().develop, saved);
// saving again keeps one LightCraft description and the other app's data
s.execute("photo.rate", &json!({"rating": 3})).unwrap();
s.execute("photo.saveMetadataToFile", &json!({})).unwrap();
let again = std::fs::read_to_string(src.join("shot.xmp")).unwrap();
assert_eq!(again.matches("<rdf:Description").count(), 2, "{again}");
assert!(again.contains("other:Secret=\"keep me\"") && again.contains("crs:Exposure2012=\"+0.65\""));
assert_eq!(lightcraft_meta::parse_xmp(&again).unwrap().metadata.rating, Some(3));
// a sidecar that isn't XMP is kept as a backup before it is replaced
std::fs::write(src.join("shot.xmp"), b"<not xmp").unwrap();
let r = s.execute("photo.saveMetadataToFile", &json!({})).unwrap();
let backup = r["backups"][0]["backup"].as_str().unwrap().to_string();
assert!(backup.contains("shot.xmp.bak-"), "{r}");
assert_eq!(std::fs::read(&backup).unwrap(), b"<not xmp");
assert_eq!(lightcraft_meta::parse_xmp(&std::fs::read_to_string(src.join("shot.xmp")).unwrap()).unwrap().metadata.rating, Some(3));
// … and another one never overwrites the first backup
std::fs::write(src.join("shot.xmp"), b"<still not xmp").unwrap();
let r = s.execute("photo.saveMetadataToFile", &json!({})).unwrap();
let second = r["backups"][0]["backup"].as_str().unwrap().to_string();
assert_ne!(second, backup);
assert_eq!(std::fs::read(&backup).unwrap(), b"<not xmp");
assert_eq!(std::fs::read(&second).unwrap(), b"<still not xmp");
let _ = std::fs::remove_dir_all(&src);
let _ = std::fs::remove_dir_all(&lib);
}
/// Issue #92: two catalogued files with one stem (`IMG_1.jpg` + `IMG_1.png`) don't share a
/// Stem-named sidecar: the first by name keeps `IMG_1.xmp`, the other writes and reads
/// `IMG_1.png.xmp`.
#[test]
fn files_sharing_a_stem_get_their_own_sidecars() {
let src = temp_dir("stem");
let lib = temp_dir("stem-lib");
write_png(&src.join("IMG_1.png"), 1);
let img = lightcraft_raster::Rgba8::from_fn(40, 24, |x, y| [(x * 6) as u8, (y * 9) as u8, 7, 255]);
let jpg =
crate::export::encode_image(&img, &crate::export::ExportOptions { format: crate::export::ExportFormat::Jpeg, ..Default::default() }).unwrap();
std::fs::write(src.join("IMG_1.jpg"), jpg).unwrap();
let mut s = Session::new().with_fs();
s.open_library(&lib, false).unwrap();
s.execute("library.import", &json!({"paths": [src.to_string_lossy()]})).unwrap();
let id = |s: &Session, name: &str| s.catalog.photos().find(|p| p.file_name == name).unwrap().id;
let (j, p) = (id(&s, "IMG_1.jpg"), id(&s, "IMG_1.png"));
assert_eq!(s.sidecar_naming(j), crate::sidecar::SidecarNaming::Stem);
assert_eq!(s.sidecar_naming(p), crate::sidecar::SidecarNaming::Full);
s.execute("photo.rate", &json!({"ids": [j.0], "rating": 2})).unwrap();
s.execute("photo.rate", &json!({"ids": [p.0], "rating": 5})).unwrap();
let r = s.execute("photo.saveMetadataToFile", &json!({"ids": [j.0, p.0]})).unwrap();
assert_eq!(r["written"].as_array().unwrap().len(), 2, "{r}");
let rating = |f: &str| lightcraft_meta::parse_xmp(&std::fs::read_to_string(src.join(f)).unwrap()).unwrap().metadata.rating;
assert_eq!(rating("IMG_1.xmp"), Some(2));
assert_eq!(rating("IMG_1.png.xmp"), Some(5));
// each reads its own back
s.execute("photo.rate", &json!({"ids": [j.0, p.0], "rating": 0})).unwrap();
s.execute("photo.readMetadataFromFile", &json!({"ids": [j.0, p.0]})).unwrap();
assert_eq!((s.catalog.photo(j).unwrap().rating, s.catalog.photo(p).unwrap().rating), (2, 5));
let _ = std::fs::remove_dir_all(&src);
let _ = std::fs::remove_dir_all(&lib);
}

View File

@ -78,12 +78,7 @@ impl Headless {
None => vec![self.photo_or_active(p)?],
};
let dir = p.get("dir").and_then(Value::as_str).unwrap_or("");
let write = &mut |path: &str, bytes: &[u8]| {
if let Some(parent) = Path::new(path).parent().filter(|d| !d.as_os_str().is_empty()) {
std::fs::create_dir_all(parent).map_err(|err| err.to_string())?;
}
std::fs::write(path, bytes).map_err(|err| format!("{path}: {err}"))
};
let write = &mut lightcraft_engine::export::write_file;
let files =
export_batch(&mut self.session, &ids, &opts, &Destination { dir: dir.to_string(), exact: exact.map(str::to_string) }, write, &|path| {
Path::new(path).exists()
@ -118,6 +113,7 @@ impl Backend for Headless {
let img = self.render(&p, 1600)?;
match p.get("path").and_then(Value::as_str) {
Some(path) => {
self.session.check_write_target(path)?;
write_image(Path::new(path), &img, 92)?;
Ok(json!({"path": path, "width": img.width, "height": img.height}))
}
@ -183,7 +179,7 @@ pub fn encode_image(ext: &str, img: &Rgba8, quality: u8) -> Result<Vec<u8>, Stri
pub fn write_image(path: &Path, img: &Rgba8, quality: u8) -> Result<(), String> {
let ext = path.extension().map(|e| e.to_string_lossy().to_string()).unwrap_or_default();
let bytes = encode_image(&ext, img, quality)?;
std::fs::write(path, bytes).map_err(|e| format!("{}: {e}", path.display()))
lightcraft_engine::export::write_file(&path.to_string_lossy(), &bytes)
}
#[cfg(test)]

View File

@ -1,6 +1,6 @@
use serde_json::{Value, json};
use crate::{Headless, PROTOCOL_VERSION, Server, call_tool, command_tool_name};
use crate::{Backend, Headless, PROTOCOL_VERSION, Server, call_tool, command_tool_name};
fn server() -> Server {
Server::new(Box::new(Headless::demo()))
@ -92,6 +92,34 @@ fn import_tool_copies_with_a_folder_template() {
let _ = std::fs::remove_dir_all(&base);
}
/// Issue #93: `render_photo`, `ui.render` and `app.export` with an exact `path` must not replace
/// a photo's original.
#[test]
fn path_writes_never_replace_an_original() {
let base = std::env::temp_dir().join(format!("lc-mcp-guard-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&base);
std::fs::create_dir_all(&base).unwrap();
let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[90, 30, 9, 255]; 64] };
let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap();
let orig = base.join("a.png");
std::fs::write(&orig, &png).unwrap();
let mut b = Headless::demo();
let r = call_tool(&mut b, "import", &json!({"paths": [orig.to_string_lossy()]}));
assert!(!r.is_error, "{r:?}");
let id = b.session.catalog.photos().find(|p| p.file_name == "a.png").unwrap().id.0;
let path = orig.to_string_lossy().to_string();
let r = call_tool(&mut b, "render_photo", &json!({"id": id, "path": path}));
assert!(r.is_error, "{r:?}");
assert!(b.call("ui.render", json!({"id": id, "path": path})).unwrap_err().contains("original"));
let e = b.call("app.export", json!({"ids": [id], "path": path})).unwrap_err();
assert!(e.contains("never writes over an original"), "{e}");
assert_eq!(std::fs::read(&orig).unwrap(), png, "the original is untouched");
// another path still works
let r = call_tool(&mut b, "render_photo", &json!({"id": id, "path": base.join("render.png").to_string_lossy()}));
assert!(!r.is_error, "{r:?}");
let _ = std::fs::remove_dir_all(&base);
}
/// The `import` helper moves: renamed into the folder template, the source removed.
#[test]
fn import_tool_moves_with_a_folder_template() {

View File

@ -352,9 +352,9 @@ fn image_result(file: &std::path::Path, max: Option<u32>, format: &str, save_to:
(bytes, w, h)
};
if let Some(p) = save_to
&& let Err(e) = std::fs::write(p, &bytes)
&& let Err(e) = lightcraft_engine::export::write_file(p, &bytes)
{
return ToolResult::error(format!("{p}: {e}"));
return ToolResult::error(e);
}
let mime = if jpeg { "image/jpeg" } else { "image/png" };
let mut info = meta;
@ -372,7 +372,18 @@ fn image_result(file: &std::path::Path, max: Option<u32>, format: &str, save_to:
}
}
/// A user-given `path` to save to must not be a photo's original (or its sidecar).
fn check_save_path(b: &mut dyn Backend, args: &Value) -> Result<(), String> {
match args.get("path").and_then(Value::as_str) {
Some(p) => exec(b, "export.checkTarget", json!({"path": p})).map(|_| ()),
None => Ok(()),
}
}
fn render_photo(b: &mut dyn Backend, args: &Value) -> ToolResult {
if let Err(e) = check_save_path(b, args) {
return ToolResult::error(e);
}
let size = args.get("size").and_then(Value::as_u64).unwrap_or(1024).clamp(16, 4096);
let id = match args.get("id").and_then(Value::as_u64) {
Some(id) => Some(id),
@ -394,6 +405,9 @@ fn render_photo(b: &mut dyn Backend, args: &Value) -> ToolResult {
}
fn screenshot(b: &mut dyn Backend, args: &Value) -> ToolResult {
if let Err(e) = check_save_path(b, args) {
return ToolResult::error(e);
}
let file = temp_path("screenshot");
let path = file.to_string_lossy().to_string();
match b.call("ui.screenshot", json!({"path": path})) {

View File

@ -17,6 +17,7 @@ mod gpx;
mod iptc;
pub mod tags;
mod xmp;
mod xmp_merge;
pub use container::{Embedded, embedded, jpeg_segments, png_chunks, webp_chunks};
pub use datetime::DateTime;
@ -26,6 +27,7 @@ pub use iptc::parse_iptc;
pub use lightcraft_geom::Orientation;
pub use tags::{TagRow, file_tag_rows, tag_rows};
pub use xmp::{CRS_NS, LC_NS, XmpData, XmpError, XmpValue, parse_xmp, write_xmp, write_xmp_lc};
pub use xmp_merge::{MergeRules, merge_xmp};
use serde::{Deserialize, Serialize};

View File

@ -145,6 +145,11 @@ impl Node {
const MAX_DEPTH: usize = 64;
const MAX_NODES: usize = 200_000;
/// `prefix:local` with the prefix canonicalised by namespace URI (see [`NAMESPACES`]).
pub(crate) fn canonical_name(qname: &str, scopes: &[Vec<(String, String)>]) -> String {
canonical(qname, scopes)
}
fn canonical(qname: &str, scopes: &[Vec<(String, String)>]) -> String {
let (prefix, local) = qname.split_once(':').unwrap_or(("", qname));
if prefix == "xml" || prefix == "xmlns" {

View File

@ -0,0 +1,403 @@
//! Merging a freshly written XMP packet into an existing one (a sidecar another application
//! wrote, e.g. with its develop settings and edit history), keeping everything the writer does not
//! own byte for byte.
//!
//! The existing packet is scanned once (namespace-resolved, with byte positions). Only top-level
//! properties of the `rdf:Description` elements directly under `rdf:RDF` are considered, in
//! attribute form (`xmp:Rating="3"`) and element form (`<dc:subject>…</dc:subject>`):
//! - properties the writer owns ([`MergeRules::owned`]) are removed, and written from the fresh
//! packet when it has them (so a cleared field is cleared in the file too);
//! - properties owned only when stated ([`MergeRules::owned_if_present`]) are replaced when the
//! fresh packet has them and kept otherwise;
//! - any other property of the fresh packet (e.g. `xmp:CreatorTool`) is not written: the file
//! keeps describing itself as its creator left it;
//! - everything else — other namespaces (e.g. `crs:`), `xmpMM:History`, unknown properties,
//! comments, the packet wrapper and padding — is copied unchanged.
//!
//! The written properties go into one new `rdf:Description` (declaring its own namespaces, so
//! the existing prefix bindings never matter) just before `</rdf:RDF>`. A description left empty
//! by the removals is dropped, so repeated saves don't accumulate. The result is checked by
//! parsing it again: every written property must read back as in the fresh packet.
use std::collections::BTreeSet;
use quick_xml::events::Event;
use crate::xmp::{XmpError, canonical_name, parse_xmp};
/// Which properties [`merge_xmp`] takes from the fresh packet. Names are canonical
/// `prefix:name` (prefixes as [`crate::parse_xmp`] reports them); `prefix:*` is a whole namespace.
#[derive(Clone, Copy, Debug, Default)]
pub struct MergeRules<'a> {
/// Always replaced: removed from the existing packet, written from the fresh one if stated.
pub owned: &'a [&'a str],
/// Replaced only when the fresh packet states them; otherwise the existing value stays.
pub owned_if_present: &'a [&'a str],
}
fn listed(list: &[&str], name: &str) -> bool {
list.iter().any(|p| match p.strip_suffix('*') {
Some(ns) => name.starts_with(ns),
None => *p == name,
})
}
#[derive(Debug)]
struct Attr {
raw: String,
name: String,
/// The value as written (still escaped).
value: String,
}
#[derive(Debug)]
struct Prop {
name: String,
start: usize,
end: usize,
}
#[derive(Debug, Default)]
struct Desc {
tag_start: usize,
tag_end: usize,
/// The element name as written (`rdf:Description`, or another prefix for the RDF namespace).
raw_name: String,
empty: bool,
attrs: Vec<Attr>,
props: Vec<Prop>,
/// The end of `</rdf:Description>` (`tag_end` when self-closing).
end: usize,
}
#[derive(Debug, Default)]
struct Scan {
descs: Vec<Desc>,
/// Where the first `</rdf:RDF>` starts.
rdf_close: Option<usize>,
}
enum Kind {
Other,
Rdf,
Desc(usize),
Prop { desc: usize, start: usize, name: String },
}
fn is_meta_attr(name: &str) -> bool {
name.starts_with("xmlns")
|| matches!(name, "rdf:about" | "rdf:ID" | "rdf:nodeID" | "xml:lang" | "rdf:parseType" | "rdf:resource" | "rdf:datatype")
}
fn pos(r: &quick_xml::Reader<&[u8]>) -> usize {
usize::try_from(r.buffer_position()).unwrap_or(usize::MAX)
}
/// Locate the descriptions, their top-level properties and `</rdf:RDF>` in a packet.
fn scan(s: &str) -> Result<Scan, XmpError> {
let mut reader = quick_xml::Reader::from_reader(s.as_bytes());
reader.config_mut().check_end_names = true;
let mut out = Scan::default();
let mut stack: Vec<Kind> = Vec::new();
let mut scopes: Vec<Vec<(String, String)>> = vec![vec![("xml".into(), "http://www.w3.org/XML/1998/namespace".into())]];
let mut count = 0usize;
loop {
let before = pos(&reader);
let ev = reader.read_event().map_err(|e| XmpError::Xml(e.to_string()))?;
let after = pos(&reader);
match ev {
Event::Start(ref e) | Event::Empty(ref e) => {
count += 1;
if count > 200_000 || stack.len() > 64 {
return Err(XmpError::TooComplex);
}
let empty = matches!(ev, Event::Empty(_));
let mut scope = Vec::new();
let mut raw_attrs = Vec::new();
for a in e.attributes() {
let a = a.map_err(|e| XmpError::Xml(e.to_string()))?;
let k = String::from_utf8_lossy(a.key.as_ref()).into_owned();
let v = String::from_utf8_lossy(&a.value).into_owned();
if k == "xmlns" {
scope.push((String::new(), v.clone()));
} else if let Some(p) = k.strip_prefix("xmlns:") {
scope.push((p.to_string(), v.clone()));
}
raw_attrs.push((k, v));
}
scopes.push(scope);
let raw_name = String::from_utf8_lossy(e.name().as_ref()).into_owned();
let name = canonical_name(&raw_name, &scopes);
let kind = match stack.last() {
Some(Kind::Rdf) if name == "rdf:Description" => {
let attrs = raw_attrs.into_iter().map(|(raw, value)| Attr { name: canonical_name(&raw, &scopes), raw, value }).collect();
out.descs.push(Desc { tag_start: before, tag_end: after, raw_name, empty, attrs, props: Vec::new(), end: after });
Kind::Desc(out.descs.len() - 1)
}
Some(Kind::Desc(i)) => Kind::Prop { desc: *i, start: before, name },
_ if name == "rdf:RDF" && out.rdf_close.is_none() => Kind::Rdf,
_ => Kind::Other,
};
if empty {
scopes.pop();
if let Kind::Prop { desc, start, name } = kind
&& let Some(d) = out.descs.get_mut(desc)
{
d.props.push(Prop { name, start, end: after });
}
} else {
stack.push(kind);
}
}
Event::End(_) => {
scopes.pop();
match stack.pop() {
Some(Kind::Prop { desc, start, name }) => {
if let Some(d) = out.descs.get_mut(desc) {
d.props.push(Prop { name, start, end: after });
}
}
Some(Kind::Desc(i)) => {
if let Some(d) = out.descs.get_mut(i) {
d.end = after;
}
}
Some(Kind::Rdf) => out.rdf_close = out.rdf_close.or(Some(before)),
Some(Kind::Other) => {}
None => return Err(XmpError::Xml("unbalanced end tag".into())),
}
}
Event::Eof => break,
_ => {}
}
}
if !stack.is_empty() {
return Err(XmpError::Xml("unclosed element".into()));
}
Ok(out)
}
/// `start` moved back over the indentation and line break before it (so a removed property
/// takes its line with it).
fn line_start(s: &str, start: usize) -> usize {
let b = s.as_bytes();
let mut i = start;
while i > 0 && matches!(b.get(i - 1), Some(b' ' | b'\t')) {
i -= 1;
}
if i > 0 && b.get(i - 1) == Some(&b'\n') {
i -= 1;
if i > 0 && b.get(i - 1) == Some(&b'\r') {
i -= 1;
}
}
i
}
/// Merge `fresh` (a packet as [`crate::write_xmp_lc`] writes it) into `existing` following
/// `rules` (see the module docs). Fails — and the caller must not overwrite `existing` blindly —
/// when `existing` isn't a well-formed XMP packet.
pub fn merge_xmp(existing: &str, fresh: &str, rules: MergeRules) -> Result<String, XmpError> {
let (bom, old) = match existing.strip_prefix('\u{feff}') {
Some(rest) => ("\u{feff}", rest),
None => ("", existing),
};
parse_xmp(old)?;
let ex = scan(old)?;
let rdf_close = ex.rdf_close.ok_or_else(|| XmpError::Xml("no rdf:RDF element".into()))?;
let new = scan(fresh)?;
let fresh_desc = new.descs.first().ok_or_else(|| XmpError::Xml("the new packet has no rdf:Description".into()))?;
let fresh_names: BTreeSet<&str> = fresh_desc.props.iter().map(|p| p.name.as_str()).collect();
let remove = |name: &str| listed(rules.owned, name) || (listed(rules.owned_if_present, name) && fresh_names.contains(name));
let write = |name: &str| listed(rules.owned, name) || listed(rules.owned_if_present, name);
// edits on `old`: (start, end, replacement), applied back to front
let mut edits: Vec<(usize, usize, String)> = Vec::new();
for d in &ex.descs {
let gone_attrs: Vec<&Attr> = d.attrs.iter().filter(|a| !is_meta_attr(&a.name) && remove(&a.name)).collect();
let gone_props: Vec<&Prop> = d.props.iter().filter(|p| remove(&p.name)).collect();
if gone_attrs.is_empty() && gone_props.is_empty() {
continue;
}
let attrs_left = d.attrs.iter().any(|a| !is_meta_attr(&a.name) && !remove(&a.name));
if !attrs_left && gone_props.len() == d.props.len() {
// nothing of its own left: drop the whole description
edits.push((line_start(old, d.tag_start), d.end, String::new()));
continue;
}
if !gone_attrs.is_empty() {
let mut tag = format!("<{}", d.raw_name);
for a in d.attrs.iter().filter(|a| is_meta_attr(&a.name) || !remove(&a.name)) {
tag.push_str(&format!("\n {}=\"{}\"", a.raw, a.value.replace('"', "&quot;")));
}
tag.push_str(if d.empty { "/>" } else { ">" });
edits.push((d.tag_start, d.tag_end, tag));
}
for p in gone_props {
edits.push((line_start(old, p.start), p.end, String::new()));
}
}
// the new description: its own namespace declarations, the properties to write
let mut block = String::from("\n <rdf:Description rdf:about=\"\"\n xmlns:rdf=\"http://www.w3.org/1999/02/22-rdf-syntax-ns#\"");
for a in fresh_desc.attrs.iter().filter(|a| a.raw.starts_with("xmlns:") && a.raw != "xmlns:rdf") {
block.push_str(&format!("\n {}=\"{}\"", a.raw, a.value));
}
block.push('>');
let mut written = 0;
for p in fresh_desc.props.iter().filter(|p| write(&p.name)) {
let text = fresh.get(p.start..p.end).ok_or_else(|| XmpError::Xml("bad property span".into()))?;
block.push_str("\n ");
block.push_str(text);
written += 1;
}
block.push_str("\n </rdf:Description>");
if written > 0 {
let at = old.get(..rdf_close).map_or(rdf_close, |h| h.trim_end().len());
edits.push((at, at, block));
}
edits.sort_by_key(|e| (e.0, e.1));
// overlapping edits can't happen (properties sit inside their description, which is either
// dropped whole or edited piecewise), but never splice blindly
let mut out = String::with_capacity(old.len() + 4096);
out.push_str(bom);
let mut at = 0;
for (s, e, r) in &edits {
if *s < at {
return Err(XmpError::Xml("overlapping edits".into()));
}
out.push_str(old.get(at..*s).ok_or_else(|| XmpError::Xml("bad span".into()))?);
out.push_str(r);
at = *e;
}
out.push_str(old.get(at..).ok_or_else(|| XmpError::Xml("bad span".into()))?);
// read back: everything written must come out as written
let merged = parse_xmp(&out)?;
let want = parse_xmp(fresh)?;
for (k, v) in &want.properties {
let top = k.split('/').next().unwrap_or(k);
if write(top) && merged.properties.get(k) != Some(v) {
return Err(XmpError::Xml(format!("{k} did not survive the merge")));
}
}
Ok(out)
}
#[cfg(test)]
mod tests {
use super::*;
const RULES: MergeRules =
MergeRules { owned: &["xmp:Rating", "xmp:Label", "dc:subject", "dc:title", "lc:*"], owned_if_present: &["exif:DateTimeOriginal"] };
/// A sidecar as another raw developer writes it: attribute-form simple properties, develop
/// settings, a structured history, an unknown namespace.
const FOREIGN: &str = "<?xpacket begin=\"\u{feff}\" id=\"W5M0MpCehiHzreSzNTczkc9d\"?>
<x:xmpmeta xmlns:x=\"adobe:ns:meta/\" x:xmptk=\"Some Toolkit 1.0\">
<rdf:RDF xmlns:rdf=\"http://www.w3.org/1999/02/22-rdf-syntax-ns#\">
<rdf:Description rdf:about=\"\"
xmlns:xmp=\"http://ns.adobe.com/xap/1.0/\"
xmlns:xmpMM=\"http://ns.adobe.com/xap/1.0/mm/\"
xmlns:stEvt=\"http://ns.adobe.com/xap/1.0/sType/ResourceEvent#\"
xmlns:crs=\"http://ns.adobe.com/camera-raw-settings/1.0/\"
xmlns:dc=\"http://purl.org/dc/elements/1.1/\"
xmlns:foo=\"http://example.com/foo/\"
xmp:Rating=\"2\"
xmp:CreatorTool=\"Other App\"
crs:Exposure2012=\"+0.50\"
crs:Contrast2012=\"-12\"
foo:Thing=\"a &amp; b\">
<xmpMM:History>
<rdf:Seq>
<rdf:li stEvt:action=\"saved\" stEvt:when=\"2026-01-02T03:04:05\"/>
</rdf:Seq>
</xmpMM:History>
<crs:ToneCurvePV2012>
<rdf:Seq>
<rdf:li>0, 0</rdf:li>
<rdf:li>255, 255</rdf:li>
</rdf:Seq>
</crs:ToneCurvePV2012>
<dc:subject>
<rdf:Bag>
<rdf:li>old</rdf:li>
</rdf:Bag>
</dc:subject>
</rdf:Description>
</rdf:RDF>
</x:xmpmeta>
";
/// With the packet's whitespace padding and trailer.
fn foreign() -> String {
format!("{FOREIGN}{}\n<?xpacket end=\"w\"?>", " ".repeat(100))
}
fn fresh(rating: i8, keywords: &[&str]) -> String {
let m = crate::Metadata {
software: Some("LightCraft".into()),
rating: Some(rating),
keywords: keywords.iter().map(|k| k.to_string()).collect(),
..Default::default()
};
crate::write_xmp_lc(&m, &[("settings", "{\"a\":1}"), ("flag", "pick")])
}
#[test]
fn keeps_foreign_properties_and_replaces_owned_ones() {
let out = merge_xmp(&foreign(), &fresh(4, &["new", "two"]), RULES).unwrap();
let d = parse_xmp(&out).unwrap();
let p = |k: &str| d.properties.get(k).cloned().unwrap_or_default();
assert_eq!(p("xmp:Rating"), vec!["4"]);
assert_eq!(p("dc:subject"), vec!["new", "two"]);
assert_eq!(p("lc:settings"), vec!["{\"a\":1}"]);
assert_eq!(p("lc:flag"), vec!["pick"]);
assert_eq!(p("xmp:CreatorTool"), vec!["Other App"], "not ours: kept");
assert_eq!(p("crs:Exposure2012"), vec!["+0.50"]);
assert_eq!(p("foo:Thing").len(), 1);
// untouched parts are byte for byte the same
for part in [
" <xmpMM:History>\n <rdf:Seq>\n <rdf:li stEvt:action=\"saved\" stEvt:when=\"2026-01-02T03:04:05\"/>\n </rdf:Seq>\n </xmpMM:History>",
" <crs:ToneCurvePV2012>\n <rdf:Seq>\n <rdf:li>0, 0</rdf:li>",
"crs:Contrast2012=\"-12\"",
"foo:Thing=\"a &amp; b\"",
"<?xpacket begin=\"\u{feff}\" id=\"W5M0MpCehiHzreSzNTczkc9d\"?>\n<x:xmpmeta xmlns:x=\"adobe:ns:meta/\" x:xmptk=\"Some Toolkit 1.0\">",
&format!("</x:xmpmeta>\n{}\n<?xpacket end=\"w\"?>", " ".repeat(100)),
] {
assert!(out.contains(part), "lost {part:?} in\n{out}");
}
assert!(!out.contains("xmp:Rating=\"2\""));
assert!(!out.contains("<rdf:li>old</rdf:li>"));
// saving again changes only our part, and doesn't pile up descriptions
let again = merge_xmp(&out, &fresh(1, &[]), RULES).unwrap();
let d = parse_xmp(&again).unwrap();
assert_eq!(d.properties.get("xmp:Rating"), Some(&vec!["1".to_string()]));
assert!(!d.properties.contains_key("dc:subject"), "a cleared field is cleared");
assert_eq!(again.matches("<rdf:Description").count(), 2, "{again}");
assert_eq!(merge_xmp(&again, &fresh(1, &[]), RULES).unwrap(), again, "idempotent");
}
#[test]
fn owned_if_present_and_prefix_independence() {
// the existing packet binds the namespaces to other prefixes, and has a capture time
let ex = "<x:xmpmeta xmlns:x=\"adobe:ns:meta/\"><r:RDF xmlns:r=\"http://www.w3.org/1999/02/22-rdf-syntax-ns#\">\
<r:Description r:about=\"\" xmlns:a=\"http://ns.adobe.com/xap/1.0/\" xmlns:e=\"http://ns.adobe.com/exif/1.0/\" \
a:Rating=\"5\" e:DateTimeOriginal=\"2020-01-01T00:00:00\"/></r:RDF></x:xmpmeta>";
let out = merge_xmp(ex, &fresh(3, &[]), RULES).unwrap();
let d = parse_xmp(&out).unwrap();
assert_eq!(d.properties.get("xmp:Rating"), Some(&vec!["3".to_string()]));
assert_eq!(d.properties.get("exif:DateTimeOriginal"), Some(&vec!["2020-01-01T00:00:00".to_string()]), "fresh has none: kept");
let mut m = crate::Metadata { rating: Some(3), ..Default::default() };
m.capture_time = crate::DateTime::parse_iso("2024-05-06T07:08:09");
let out = merge_xmp(ex, &crate::write_xmp_lc(&m, &[]), RULES).unwrap();
let d = parse_xmp(&out).unwrap();
assert_eq!(d.properties.get("exif:DateTimeOriginal"), Some(&vec!["2024-05-06T07:08:09".to_string()]));
}
#[test]
fn refuses_what_it_cannot_read() {
for bad in ["", "not xml at all", "<x:xmpmeta xmlns:x=\"adobe:ns:meta/\"><rdf:RDF", "<a><b></a>", "<x:xmpmeta xmlns:x=\"adobe:ns:meta/\"/>"] {
assert!(merge_xmp(bad, &fresh(1, &[]), RULES).is_err(), "{bad:?}");
}
}
}

View File

@ -1,4 +1,4 @@
use lightcraft_meta::{Metadata, extract, parse_iptc, parse_xmp, read_exif, write_xmp};
use lightcraft_meta::{MergeRules, Metadata, extract, merge_xmp, parse_iptc, parse_xmp, read_exif, write_xmp};
use proptest::prelude::*;
proptest! {
@ -21,6 +21,23 @@ proptest! {
let _ = parse_xmp(&s);
}
/// Merging into arbitrary (also damaged) packets never panics, and a merge that succeeds
/// reads back with the written values.
#[test]
fn merge_never_panics(cut in 0usize..900, junk in ".{0,20}", rating in 0i8..=5) {
let base = write_xmp(&Metadata { title: Some("t".into()), rating: Some(1), ..Default::default() }, Some("{}"));
let mut s: String = base.chars().take(cut).collect();
s.push_str(&junk);
s.extend(base.chars().skip(cut));
let fresh = write_xmp(&Metadata { rating: Some(rating), ..Default::default() }, Some("{\"x\":1}"));
let rules = MergeRules { owned: &["xmp:Rating", "dc:title", "lc:*"], owned_if_present: &[] };
if let Ok(out) = merge_xmp(&s, &fresh, rules) {
let d = parse_xmp(&out).unwrap();
prop_assert_eq!(d.metadata.rating, Some(rating));
prop_assert_eq!(d.lc_settings.as_deref(), Some("{\"x\":1}"));
}
}
#[test]
fn user_text_roundtrips(title in "[a-zA-Z0-9 &<>\"'éü€;|/\\\\]{1,40}", kw in proptest::collection::vec("[a-zA-Z0-9 &<>\"'|]{1,12}", 0..5), rating in -1i8..=5) {
let title = title.trim().to_string();

View File

@ -4,6 +4,11 @@
//! files) but not fsynced; unreadable files are deleted and re-rendered. When the total size
//! exceeds the budget, the least recently used files (by modification time, refreshed on read)
//! are removed down to 80 % of the budget.
//!
//! The cache only ever lists, counts or deletes files of its own name shape
//! (`<2 hex>/<32 hex>.jpg` and the `<32 hex>.tmp…` temp files of [`DiskCache::put`], see
//! [`is_cache_file`]): the directory may be shared with other files (e.g. a library opened on a
//! photo folder that already has a `thumbs/` folder), and those are never touched (issue #98).
use std::path::{Path, PathBuf};
use std::sync::Mutex;
@ -96,13 +101,25 @@ impl DiskCache {
}
}
/// All cache files: (path, size, modified).
/// All cache files (only names of the cache's own shape, see [`is_cache_file`]):
/// (path, size, modified).
fn scan(&self) -> Vec<(PathBuf, u64, std::time::SystemTime)> {
let mut out = Vec::new();
let Ok(rd) = std::fs::read_dir(&self.dir) else { return out };
for sub in rd.flatten() {
let sub_name = sub.file_name();
let Some(sub_name) = sub_name.to_str() else { continue };
if !is_shard_name(sub_name) {
continue;
}
let Ok(files) = std::fs::read_dir(sub.path()) else { continue };
for f in files.flatten() {
let name = f.file_name();
let Some(name) = name.to_str() else { continue };
if !is_cache_file(sub_name, name) {
continue;
}
// `DirEntry::metadata` does not follow symlinks: a link is never a cache file
if let Ok(m) = f.metadata()
&& m.is_file()
{
@ -144,6 +161,29 @@ impl DiskCache {
}
}
fn is_lower_hex(s: &str) -> bool {
s.bytes().all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b))
}
/// A shard directory name: two lowercase hex digits.
fn is_shard_name(s: &str) -> bool {
s.len() == 2 && is_lower_hex(s)
}
/// Whether `name` inside shard directory `shard` is a file this cache wrote: `<32 hex>.jpg`
/// (whose first two digits are the shard) or a `<32 hex>.tmp<alphanumeric>` temp file of
/// [`DiskCache::put`]. Anything else in the cache directory belongs to someone else.
pub fn is_cache_file(shard: &str, name: &str) -> bool {
if !is_shard_name(shard) {
return false;
}
let Some((stem, ext)) = name.split_once('.') else { return false };
if stem.len() != 32 || !is_lower_hex(stem) || !stem.starts_with(shard) {
return false;
}
ext == "jpg" || ext.strip_prefix("tmp").is_some_and(|t| !t.is_empty() && t.bytes().all(|b| b.is_ascii_alphanumeric()))
}
/// Encode a thumbnail as the cache stores it (JPEG, RGB, 4:2:0).
pub fn encode_jpeg(img: &Rgba8) -> Option<Vec<u8>> {
if img.width == 0 || img.height == 0 || img.width > u16::MAX as usize || img.height > u16::MAX as usize {

View File

@ -154,3 +154,41 @@ fn pool_runs_inline() {
assert_eq!(p.run_inline(10), 1);
assert_eq!(p.try_recv().map(|d| d.result), Some(1));
}
#[test]
fn disk_cache_never_touches_foreign_files() {
// issue #98: a library opened on a folder that already has a `thumbs/` folder
let dir = temp_dir("foreign");
let foreign = [
dir.join("readme.txt"),
dir.join("ab").join("holiday.jpg"),
dir.join("ab").join("0123456789abcdef0123456789abcdef.jpg"), // wrong shard
dir.join("small").join("0123456789abcdef0123456789abcdef.jpg"),
dir.join("01").join("0123456789ABCDEF0123456789ABCDEF.jpg"), // upper case
dir.join("01").join("0123456789abcdef0123456789abcdef.jpeg"),
dir.join("01").join("0123456789abcdef0123456789abcdef.tmp"),
];
for p in &foreign {
std::fs::create_dir_all(p.parent().unwrap()).unwrap();
std::fs::write(p, vec![7u8; 50_000]).unwrap();
}
// a stale temp file of our own is cleaned up
let own_tmp = dir.join("01").join("0123456789abcdef0123456789abcdef.tmpThreadId9");
std::fs::write(&own_tmp, b"partial").unwrap();
let d = DiskCache::new(&dir, 40_000);
assert_eq!(d.size(), 7, "only the cache's own temp file counts");
// fill past the budget (forces prunes), then clear
for i in 0..40u8 {
d.put(hash_bytes(&[i, 2]), &gradient(160, 100, i));
}
assert!(d.size() <= 40_000);
d.clear();
assert_eq!(d.size(), 0);
assert!(!own_tmp.exists());
for p in &foreign {
assert!(p.exists(), "foreign file deleted: {}", p.display());
}
assert!(disk::is_cache_file("01", "0123456789abcdef0123456789abcdef.jpg"));
assert!(!disk::is_cache_file("01", "0123456789abcdef0123456789abcdef.jpg.bak"));
let _ = std::fs::remove_dir_all(&dir);
}

View File

@ -290,12 +290,19 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest
ok(Value::Null)
}
"ui.screenshot" => {
// never over a photo's original
if let Some(Err(e)) = s("path").map(|path| app.session.check_write_target(path)) {
return err(e);
}
ctx.request_repaint();
Outcome::Screenshot { path: s("path").map(str::to_string), headless: p.get("headless").and_then(Value::as_bool).unwrap_or(false) }
}
"ui.render" => {
let id = p.get("id").and_then(Value::as_u64).map(lightcraft_catalog::PhotoId).or(app.session.active());
let Some(id) = id else { return err("no photo") };
if let Some(Err(e)) = s("path").map(|path| app.session.check_write_target(path)) {
return err(e);
}
let size = p.get("size").and_then(Value::as_u64).unwrap_or(1600) as usize;
match app.session.render_now(id, size, size) {
Ok(r) => match (s("path"), app.services.png.as_ref()) {
@ -356,11 +363,7 @@ pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result<Value, String
let to = Destination { dir: dir.clone(), exact: p.get("path").and_then(Value::as_str).map(str::to_string) };
let background = p.get("background").and_then(Value::as_bool).unwrap_or(false) && app.services.write_shared.is_some();
let out = if background {
let items = ids
.iter()
.enumerate()
.map(|(i, id)| lightcraft_engine::export::prepare_export(&mut app.session, *id, &opts, i + 1))
.collect::<Result<Vec<_>, _>>()?;
let items = lightcraft_engine::export::prepare_batch(&mut app.session, &ids, &opts)?;
crate::export_task::start(app, items, opts, to)?
} else {
let w = app.services.write.as_mut().ok_or("no writer")?;

View File

@ -932,7 +932,12 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option<Re
if let Ok(v) = &r {
let n = v["found"].as_array().map_or(0, Vec::len);
let left = v["missing"].as_u64().unwrap_or(0);
app.toast(&egui::Context::default(), format!("Found {n} missing photo{}; {left} still missing", if n == 1 { "" } else { "s" }));
let unsure = v["ambiguous"].as_array().map_or(0, Vec::len);
let unsure = if unsure > 0 { format!(" ({unsure} with several look-alike files: use Locate)") } else { String::new() };
app.toast(
&egui::Context::default(),
format!("Found {n} missing photo{}; {left} still missing{unsure}", if n == 1 { "" } else { "s" }),
);
}
r
}

View File

@ -116,7 +116,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/import.rs` | review dialog (Import Photos): the scanned source (a scanned folder is not added to Local), candidate grid with thumbnails and checkboxes (duplicates by path/content marked and unchecked), transfer mode (add in place / copy / move, each explained), album (existing/new), preset, keywords; the folder scan runs on a background thread with a progress window and Cancel (a NAS folder no longer freezes the window); batched import with a progress window; one undo step. Drag-and-drop still imports directly |
| LR-IMP-DRAGDROP | Drop files/folders to import | P0 | ✅ | `crates/ui-egui/src/lib.rs` (dropped files → `cmd:library.import`) | dropping onto a specific album not supported |
| LR-IMP-DUPES | Skip duplicates by content | P1 | ✅ | `crates/engine/src/import.rs`, `crates/engine/src/tests_import.rs` | |
| LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Import from Device → the import review, copying into the library by default; menus serve the last scan and rescan in the background (never blocking a frame; hot-plugs repaint); no PTP/MTP (cameras that don't mount as a disk) |
| LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Import from Device → the import review, copying into the library by default; menus serve the last scan and rescan in the background (never blocking a frame; hot-plugs repaint); copies are verified like Move's (a new file, synced, compared byte for byte; a bad copy is removed and reported as failed, never counted as imported; taken names get -1, -2…, never replaced — issue #96, `crates/engine/src/import_move.rs`); no PTP/MTP (cameras that don't mount as a disk) |
| LR-IMP-AUTO | Watched-folder auto import | P2 | ✅ | `cmd:library.autoImport`, `cmd:library.autoImportScan`, `crates/ui-egui/src/panels/settings.rs` | Settings ▸ Import ▸ Auto Import: a watched folder whose new photos are added (in place or copied) once complete, into an optional album; scanned every 3 s |
| LR-IMP-PRESET | Preset on import | P2 | ✅ | `cmd:library.import` (`preset`) | chosen in the import review; one History entry |
| LR-IMP-RAWDEFAULT | Raw defaults | P1 | ✅ | `cmd:library.preferences`, `crates/engine/src/import.rs`, `crates/ui-egui/src/panels/settings.rs` | LightCraft default / a preset / per camera (make + model); non-raw default too; the preset look counts as unedited and Reset returns to it |
@ -128,7 +128,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-IMP-CAMERA-COVERAGE | Camera coverage, verified per model | P0 | 🟡 | `crates/raw/tests/corpus.rs`, `xtask/src/main.rs` | a supported container is not the same as every camera that writes it decoding correctly: ~40 CC0 corpus files (plus ~60 NEFs checked by hand) against the >1,000 models Lightroom lists; per-model differences still surface (e.g. the CR2 colour-filter layout differed by model until it was read from the file's own tag, issue #85, checked on 35 CR2 bodies). Grow the CC0 corpus per model and test each sample decodes with plausible colour |
| LR-IMP-CULL-AT-IMPORT | Culling analysis at import | P2 | 🟡 | `cmd:photo.analyze` | run Assisted Culling on the imported photos (they're selected after an import); not automatic |
| LR-IMP-MOVE | Move on import [Classic] | P1 | ✅ | `cmd:library.import` (`mode` move), `crates/engine/src/import_move.rs`, `crates/engine/src/tests_import_move.rs`, `crates/ui-egui/src/import.rs` | Transfer ▸ Move with Copy's destination, folders (day / month / one folder / custom template), rename template and example destination; XMP sidecars (both namings) move along; each source is removed only after its destination is written (hard link on the same volume, else copied, synced and compared byte for byte) and its catalog record is saved; failed, duplicate, unchecked files keep their sources, taken names get -1, -2…, never overwritten; files already in the destination/library are added in place; sources that can't be removed (read-only card) are kept and reported (`kept`). Undo removes the photos from the library but leaves the moved files at the destination. No Copy as DNG while moving |
| LR-IMP-DNG-CONVERT | Convert to DNG on import [Classic] | P2 | ✅ | `cmd:library.import` (`dng`), `crates/ui-egui/src/import.rs` | copy imports: Raw files ▸ Copy as DNG (lossless; the card is untouched) |
| LR-IMP-DNG-CONVERT | Convert to DNG on import [Classic] | P2 | ✅ | `cmd:library.import` (`dng`), `crates/ui-egui/src/import.rs` | copy imports: Raw files ▸ Copy as DNG (lossless; the card is untouched); the raw copy is removed only once its DNG is verified and on disk (issue #106) |
## B. Library management (LIB)
@ -153,7 +153,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | no confirmation dialog, no auto-purge after N days, no "Empty" |
| LR-LIB-REMOVE-ALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos` | |
| LR-LIB-DUPLICATE | Duplicate a photo | P2 | ✅ | `cmd:photo.duplicate` | Photo ▸ Duplicate: a real `-copy` file with the same settings, metadata and albums (virtual copies share the file) |
| LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:photo.renameTokens`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` (`TOKENS`) | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming and import rename); a Tags picker beside every template field (Rename Photos, import Rename, export File name) lists each tag with meaning + live example, the date directives and the rules, and inserts the clicked tag at the text cursor; unknown tags are flagged; preview; renames files on disk with their XMP sidecars, never overwriting (-1, -2… suffixes), rolls back on failure; undo/redo move the files; virtual copies follow |
| LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:photo.renameTokens`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` (`TOKENS`) | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming and import rename); a Tags picker beside every template field (Rename Photos, import Rename, export File name) lists each tag with meaning + live example, the date directives and the rules, and inserts the clicked tag at the text cursor; unknown tags are flagged; preview; renames files on disk with their XMP sidecars (a stem sidecar another file still shares, e.g. raw + JPEG, is copied rather than taken), never overwriting (-1, -2… suffixes; a change of letter case only is checked by file identity, so on case-sensitive volumes `img_1.jpg` next to `IMG_1.JPG` is a collision), rolls back on failure (a file that can't be moved back is reported old → new and the library follows it as an undoable partial rename; the same for undo/redo); undo/redo move the files; virtual copies follow |
| LR-LIB-CAPTURETIME | Edit capture time | P1 | ✅ | `cmd:photo.setCaptureTime`, `cmd:dialog.captureTime`, `crates/catalog/src/dates.rs` | set (the other selected photos shift by the same amount, or `each`), shift by days/hours/minutes, time-zone shift; one undo step, journaled; Info panel button; no “revert to original capture time” |
| LR-LIB-SHOWFINDER | Reveal original in file manager | P0 | ✅ | `cmd:app.showInFinder` | ⌘R (see MENU-FILE-SHOWFINDER) |
| LR-LIB-COVER | Album cover | P2 | ✅ | `cmd:album.setCover` | |
@ -210,7 +210,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-META-COPYRIGHT-DEFAULT | Default copyright on import | P1 | ✅ | `cmd:library.preferences` (`import.copyright`, `import.creator`), `crates/ui-egui/src/panels/settings.rs` | Settings → Import → Metadata; fills only photos without their own |
| LR-META-LOCATION | Location editing | P2 | 🟡 | `cmd:photo.setMeta` (`location`, `city`, `state`, `country`, `gps`) | place fields and GPS (decimal or degrees / minutes / seconds) edited in Info, read/written as IPTC Core / Photoshop / EXIF XMP; Show on Map opens OpenStreetMap; no embedded map, no geocoding |
| LR-META-COPYPASTE | Copy / paste metadata | P2 | ✅ | `cmd:photo.copyMetadata`, `cmd:photo.pasteMetadata` | title, caption, alt text, extended description, copyright (notice, status, usage terms, info URL), creator, place fields, keywords; `fields` picks a subset |
| LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | sidecar wins for metadata; a sidecar capture time (`exif:DateTimeOriginal` / `photoshop:DateCreated` / `xmp:CreateDate`) fills in only when the file has none |
| LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | sidecar wins for metadata; a sidecar capture time (`exif:DateTimeOriginal` / `photoshop:DateCreated` / `xmp:CreateDate`) fills in only when the file has none; saving merges into an existing sidecar (`crates/meta/src/xmp_merge.rs`): another app's `crs:`, `xmpMM:History` and unknown namespaces are kept byte for byte, an unreadable sidecar is backed up first; with stem naming, files sharing a stem (raw + JPEG) get separate sidecars (`<file>.xmp` for all but the raw) |
| LR-META-EXIF-FULL | Full EXIF/IPTC [Classic] | P1 | ✅ | `cmd:photo.allMetadata`, `cmd:dialog.allMetadata`, `crates/meta/src/tags.rs`, `crates/meta/src/exif.rs`, `crates/meta/src/iptc.rs` | read and written on export; Info ▸ All Metadata…: every TIFF / EXIF / GPS / interop tag (named, common values spelled out) and the XMP fields, searchable; metadata presets |
## F. Edit panel — global adjustments (EDIT)
@ -436,10 +436,10 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-EXP-METADATA | Metadata policy | P1 | ✅ | `cmd:app.export` (`metadata`, `removeLocation`) | |
| LR-EXP-WATERMARK | Watermark | P1 | ✅ | `cmd:app.export` (`watermark`: text or `image` + `imageWidth`), `crates/engine/src/export.rs` (`Watermark`) | text (size, colour, shadow) or a graphic with transparency (width as % of the photo, converted to the output colour space); position, inset, opacity |
| LR-EXP-NAMING | File naming | P1 | ✅ | `cmd:app.export` (`naming`, `startNumber`), `crates/engine/src/rename.rs` (`expand_tokens`) | one template language for export, Rename Photos and import renaming: `{name}`, `{num}` (number at the end of the name), `{seq}` / `{seq:N}`, `{date}` / `{date:%Y-%m-%d}`, `{folder}`, `{camera}`, `{lens}`, `{iso}`, `{rating}`, `{title}`, `{creator}`, `{ext}`; free-form template rather than a list of named schemes |
| LR-EXP-LOCATION | Destination folder | P0 | ✅ | `cmd:app.export` (`dir`, `subfolder`, `conflict`: unique / overwrite / skip), `crates/ui-egui/src/panels/dialogs.rs` (Choose… folder picker) | folder picker on the desktop; existing files get `-2`, `-3`… by default |
| LR-EXP-LOCATION | Destination folder | P0 | ✅ | `cmd:app.export` (`dir`, `subfolder`, `conflict`: unique / overwrite / skip), `cmd:export.checkTarget`, `crates/ui-egui/src/panels/dialogs.rs` (Choose… folder picker), `crates/engine/src/originals.rs`, `crates/catalog/src/safe_file.rs` | folder picker on the desktop; existing files get `-2`, `-3`… by default; the policy covers a file and its sidecars together; a catalogued original (or its sidecar) is never written over, whatever the policy or an exact `path` (export, `ui.render`, MCP `render_photo`, merge `previewPath`, `lightcraft-cli render`); files are written to a temp file, synced and renamed into place (issue #93) |
| LR-EXP-PREVIOUS | Export with previous settings | P0 | ✅ | `cmd:app.exportPrevious`, `cmd:dialog.export` | last options persist in prefs.json; dialog prefilled; no named export presets yet |
| LR-EXP-DNGOPT | DNG options | P2 | 🟡 | `cmd:app.export` (`dngCompression`), `crates/engine/src/export.rs` | compression: lossless JPEG (default), ZIP or none; no embedded JPEG preview size, no lossy DNG output |
| LR-EXP-ORIGINAL | Original + XMP | P1 | ✅ | `cmd:app.export` (`format: original`) | file copied byte for byte, sidecar named after the output |
| LR-EXP-ORIGINAL | Original + XMP | P1 | ✅ | `cmd:app.export` (`format: original`), `crates/engine/src/tests_export.rs` | file copied byte for byte, sidecar named after the output and subject to the conflict policy |
| LR-EXP-PHOTOS | Export to the system photo library | P2 | ⬜ | | |
| LR-EXP-PSD | Round trip to an external editor | P2 | ✅ | `cmd:photo.editExternal`, `cmd:photo.editInExternal`, `crates/engine/src/cmd/convert.rs` | a 16-bit TIFF `-Edit` copy with the edits (Adobe RGB / ProPhoto / P3 / sRGB) next to the original, added stacked on top of it and opened in the editor set in Settings ▸ General (or the system default) |
@ -480,7 +480,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| Id | Feature | Tier | Status | Evidence | Missing / notes |
|---|---|---|---|---|---|
| LR-CLOUD-SYNC | Cloud sync | OOS | 🚫 | | |
| LR-CLOUD-SMARTPREVIEW | Editable proxies | P2 | ✅ | `cmd:library.smartPreviews`, `cmd:library.smartPreviewsLocation`, `cmd:photo.smartPreview`, `crates/engine/src/smart.rs`, `crates/ui-egui/src/panels/settings.rs` | File ▸ Previews ▸ Build / Discard Smart Previews: ~1 MB proxies in the library; with the original offline the photo renders, edits and exports (at proxy size) from its proxy; Info shows the status; the proxy folder is chosen per library (Settings → Performance → Smart previews, saved in the library): the effective path, count and size are shown, changing it needs a choice for the proxies already built (move / leave / delete), and an unavailable or unwritable folder is an error, never a fallback to the library drive. The thumbnail cache stays in the library |
| LR-CLOUD-SMARTPREVIEW | Editable proxies | P2 | ✅ | `cmd:library.smartPreviews`, `cmd:library.smartPreviewsLocation`, `cmd:photo.smartPreview`, `crates/engine/src/smart.rs`, `crates/ui-egui/src/panels/settings.rs` | File ▸ Previews ▸ Build / Discard Smart Previews: ~1 MB proxies in the library; with the original offline the photo renders, edits and exports (at proxy size) from its proxy; Info shows the status; the proxy folder is chosen per library (Settings → Performance → Smart previews, saved in the library): the effective path, count and size are shown, changing it needs a choice for the proxies already built (move / leave / delete), and an unavailable or unwritable folder is an error, never a fallback to the library drive; proxies are written atomically and a damaged one (cut short by a crash or a full drive) doesn't count as built, is rebuilt (`repaired`) and is never moved over a good one (issue #106). The thumbnail cache stays in the library |
| LR-AI-UPDATE-INDICATOR | AI-settings update indicator | P2 | ⬜ | | |
| LR-AI-CREDITS | Generative credits | OOS | 🚫 | | |
@ -727,8 +727,8 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06-
| LRC-LIB-PEOPLE | People view | P2 | ⬜ | | |
| LRC-LIB-COMMENTS | Comments panel | P2 | ⬜ | | |
| LRC-LIB-VISUALSEARCH | Find similar photos | P2 | ✅ | `cmd:library.findSimilar`, `crates/pipeline/src/cull.rs` (`signature`) | photo menu / Photo ▸ Find Similar Photos: look-alike photos (composition and tones), most similar first, as a filter (`only`) |
| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate`, `crates/engine/src/cmd/missing.rs` (`checked_path`) | File → Find Missing Photos… (same name and size anywhere in a folder, one undo step), photo menu → Locate Missing File…; unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. Missing Photos in the sidebar (counted on a worker thread); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike |
| LRC-LIB-CONVERT | Convert to DNG | P2 | ✅ | `cmd:photo.convertToDng`, `crates/engine/src/cmd/convert.rs` | Photo ▸ Convert to DNG: lossless DNG next to the raw with the settings embedded, photo (and its virtual copies) relinked; originals kept; undoable |
| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate`, `crates/engine/src/cmd/missing.rs` (`checked_path`) | File → Find Missing Photos… (anywhere in a folder: same name, size and content hash; renamed files by size + content hash; same name and size alone only when unambiguous — look-alikes are skipped and reported; one undo step), photo menu → Locate Missing File…; unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. Missing Photos in the sidebar (counted on a worker thread); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike |
| LRC-LIB-CONVERT | Convert to DNG | P2 | ✅ | `cmd:photo.convertToDng`, `crates/engine/src/cmd/convert.rs` | Photo ▸ Convert to DNG: lossless DNG next to the raw with the settings embedded, photo (and its virtual copies) relinked; originals kept; undoable; the DNG is decoded and compared with the raw data, written to a temp file, synced, read back and only then given a free name (never replacing a file) — a failed write leaves no DNG and the photo on its raw (issue #106) |
| LRC-LIB-PREVIEWS | Build / discard previews | P1 | ✅ | `cmd:library.buildPreviews`, `cmd:library.previewProgress`, `cmd:library.cancelPreviews`, `cmd:library.clearPreviews`, `crates/engine/src/cmd/previews.rs`, `crates/preview/src/lib.rs` | File ▸ Previews: build standard-sized (Settings → Performance size) or 1:1 previews of the selected / visible photos in the background (progress toasts, stop), discard the cache; disk thumbnail + view cache; no smart previews (offline proxies) |
| LRC-LIB-SLIDESHOW-IMPROMPTU | Impromptu slideshow | P2 | ✅ | `cmd:view.slideshow` | see LR-VIEW-SLIDESHOW |
| LRC-DEV-SNAPSHOTS | Named snapshots | P1 | ✅ | `cmd:version.create`, `cmd:version.restore` | = versions |

View File

@ -10,6 +10,27 @@
- 23 new built-in presets: Portrait, Landscape, Urban, Food, Seasons, Vintage and B&W toners.
### Reliability
- Exports and renders never write over a photo's original (issue #93): exporting into the photo's own folder with
the same name and "Overwrite" (or Export with Previous repeating it), an exact output path from the control
channel or MCP, a merge preview path or `lightcraft-cli render IMG.jpg -o IMG.jpg` is refused with a clear
message, and the original is left byte for byte. Ordinary earlier exports are still overwritten when asked.
Exported files are written to a temp file, synced and then renamed into place, so a full disk or an unplugged
drive never leaves a truncated file; the XMP sidecar of an "Original" export follows the "If file exists"
choice too.
- Convert to DNG, Copy as DNG, Photo Merge and smart previews no longer write straight to the final file (issue
#106): a DNG is checked against the raw data, written to a temp file, synced and read back before it gets its
name (never replacing a file), and only then is the photo relinked or the raw copy removed — a failed write
leaves no DNG and keeps the raw. Smart previews are written the same way; a damaged one (cut short by a crash or
a full drive) no longer counts as built and Build Smart Previews replaces it.
- Import ▸ Copy verifies every copy, like Move (issue #96): each file is written as a new file, synced to disk and
compared byte for byte with the card. A copy that fails or differs is removed and reported as a failed import —
so "import complete" means the copies are good before you format the card — and a name that is taken gets -1,
-2… instead of being replaced.
- Saving metadata to an XMP sidecar another application wrote no longer replaces it (issue #92): LightCraft merges its
fields in and keeps the rest — e.g. that application's develop settings and edit history — byte for byte. A
sidecar that isn't valid XMP is copied to `<name>.xmp.bak-<time>` first. With the default stem naming, a raw and a
JPEG with the same name (`IMG_0001.CR3` + `IMG_0001.JPG`) no longer share one sidecar: the raw keeps `IMG_0001.xmp`,
the JPEG uses `IMG_0001.JPG.xmp`.
- A save that fails part-way (a full disk, a network share that drops) no longer looks like a damaged catalog
afterwards (issue #101): the partial write is cut off before LightCraft retries, so the next launch replays every
change. Catalogs already holding such a fragment load in full. Quitting while the catalog log can't be written
@ -23,8 +44,18 @@
in short pieces so slow integrated GPUs aren't reset by their watchdog. `ui.inspect` → `perf`
(`gpuReason`, `gpuFallback`), Help ▸ System Info and Settings ▸ Performance say why the GPU isn't
used (e.g. a skipped software adapter such as llvmpipe) and why the last render fell back.
- The thumbnail cache only ever counts and deletes its own files (issue #98): a library opened on a folder that
already has a `thumbs/` folder of other pictures no longer loses them when the cache is trimmed or cleared.
### Library
- Rename Photos never overwrites another photo when only the letter case changes (issue #95): on case-sensitive
volumes (Linux, case-sensitive APFS) `img_1.JPG` next to `IMG_1.JPG` is a different photo and the renamed one gets
`img_1-1.JPG`; on case-insensitive volumes the case change still goes through.
- Rename Photos reports files it could not move back after a failure (issue #105), e.g. when a network share drops
mid-batch: the error lists them (old → new) and the library points at their new names (an undoable partial rename),
so none shows as missing. Renaming one of a raw + JPEG pair copies their shared `IMG_0001.xmp` instead of taking
it away from the other (issue #92). Find Missing Photos also finds renamed files by their content, prefers a content match
over a same-name same-size look-alike, and skips (and reports) photos it can't tell apart instead of guessing.
- Smart albums with a rule editor: match all / any / none, nested groups, 26 fields.
- Quick Collection and target album (B in the grid), keyword sets (⌥1–⌥9), colour-label sets.
- Colour-label filter with several labels at once; expandable folder tree in Local.

View File

@ -8,8 +8,8 @@ other tools), and let LightCraft pick up edits made elsewhere.
| | |
|---|---|
| Name | `<stem>.xmp` by default (`IMG_0001.CR3` → `IMG_0001.xmp`); `<file>.xmp` (`IMG_0001.CR3.xmp`) with `naming: "full"`. Reading accepts either (preferred first) and `.XMP`. |
| Write | `photo.saveMetadataToFile {ids?}` (Photo ▸ Save Metadata to File, ⌘S), or automatically after every change with `library.xmpPreferences {autoWrite: true}` (File ▸ Automatically Write Changes into XMP). Slider drags are written once, when the drag ends; undo/redo rewrite the sidecar. Written atomically (temp file + rename). |
| Name | `<stem>.xmp` by default (`IMG_0001.CR3` → `IMG_0001.xmp`); `<file>.xmp` (`IMG_0001.CR3.xmp`) with `naming: "full"`. Reading accepts either (preferred first) and `.XMP`. With stem naming, files sharing a stem don't share a sidecar: see *Shared names* below. |
| Write | `photo.saveMetadataToFile {ids?}` (Photo ▸ Save Metadata to File, ⌘S), or automatically after every change with `library.xmpPreferences {autoWrite: true}` (File ▸ Automatically Write Changes into XMP). Slider drags are written once, when the drag ends; undo/redo rewrite the sidecar. An existing sidecar is **merged into, never replaced** (see *Saving into an existing sidecar*). Written atomically (temp file, fsync, rename). The result lists `written`, `merged` and `backups`. |
| Read | On import (`library.import`, the report counts `sidecars`), and `photo.readMetadataFromFile {ids?}` (one undo step). For raw/DNG files without a sidecar, the XMP embedded in the file is used. |
| Preferences | `library.xmpPreferences {autoWrite?, naming?: stem\|full}`, stored in the library's `prefs.json`. |
@ -37,6 +37,39 @@ used — on import (it then also files a copied photo in its date folder) and by
`xmp:Rating="-1"` (the XMP convention for rejected) sets the reject flag. Develop settings come from
`lc:settings` when present (exact); otherwise from the `crs:` fields below (approximate).
## Saving into an existing sidecar
A sidecar may already hold another application's data — e.g. its `crs:` develop settings and `xmpMM:History` — often
the only copy of those edits outside that application's catalog. Saving never drops it:
- LightCraft **owns** the properties in the table above: `xmp:Rating`, `xmp:Label`, `dc:title`, `dc:description`,
`dc:rights`, `dc:creator`, `dc:subject`, `Iptc4xmpCore:Location`, `Iptc4xmpCore:AltTextAccessibility`,
`Iptc4xmpCore:ExtDescrAccessibility`, `photoshop:City`/`State`/`Country`, `xmpRights:Marked`/`UsageTerms`/
`WebStatement` and everything in `lc:`. They are replaced on every save, and removed when LightCraft has no value
(clearing a title in LightCraft clears it in the file). All of them are read back on import, so the library starts
from what the sidecar said.
- The **capture time** (`exif:DateTimeOriginal`, `photoshop:DateCreated`) and **GPS** (`exif:GPSLatitude`,
`exif:GPSLongitude`) are replaced only when LightCraft has a value; otherwise the file's stay.
- **Everything else is kept byte for byte**: other namespaces (`crs:`, `xmpMM:`, `lr:hierarchicalSubject`, unknown
ones), `xmp:CreatorTool`, comments, the packet wrapper and padding. LightCraft's properties go into one
`rdf:Description` of their own; owned properties written by another application (in attribute or element form) are
removed from its description, which otherwise stays as it was.
- A sidecar that can't be read as XMP (not well-formed, not UTF-8, no `rdf:RDF`) is first copied to
`<name>.xmp.bak-<date><time>` (`-1`, `-2`… if taken — a backup is never overwritten), then replaced. A sidecar that
can't be read at all (permissions) is left alone and the save fails.
Limits: LightCraft writes keywords to `dc:subject` only, so another application's `lr:hierarchicalSubject` is kept as
it was and may still list keywords removed in LightCraft. LightCraft doesn't write `crs:`: the other application's
develop settings stay as that application left them, next to LightCraft's own (`lc:settings`, which LightCraft reads
first).
## Shared names
With stem naming, `IMG_0001.CR3` and `IMG_0001.JPG` map to the same `IMG_0001.xmp`. When two or more files in the
library share a stem, the stem sidecar belongs to one of them — a raw first, otherwise the first by file name — and
the others write and read `<file>.xmp` (`IMG_0001.JPG.xmp`), falling back to the stem sidecar for reading when they
have none. So saving one photo never overwrites the other's metadata (`Session::sidecar_naming`).
## Reading `crs:` develop fields
Many raw developers store edits as `crs:` properties (`http://ns.adobe.com/camera-raw-settings/1.0/`) in sidecars, in