diff --git a/crates/catalog/src/safe_file.rs b/crates/catalog/src/safe_file.rs index 1e482b81..df239a17 100644 --- a/crates/catalog/src/safe_file.rs +++ b/crates/catalog/src/safe_file.rs @@ -1,11 +1,23 @@ //! 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 policy** (issue #134): what can't be recreated — originals, XMP sidecars, the catalog, +//! settings, DNGs that replace raws, smart previews (the only stand-in while an original is +//! offline), an edit copy that becomes a library photo — is written **durably**: synced to disk +//! before it counts as written. What can always be made again from the originals — exports, +//! renders, screenshots, merge previews — is written **atomically** only: a half-written file +//! never replaces a good one, but there is no per-file sync (on a USB drive or a NAS that sync +//! dominates a large export). +//! //! - [`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_atomic_nosync`] is the same temp file + rename without the two syncs, for +//! recreatable outputs only: a failed write still removes the temp file and leaves the target as +//! it was, but after a power cut the new file may be lost (on some file systems left empty) — +//! it is then simply exported again. //! - [`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 @@ -21,6 +33,19 @@ use std::sync::atomic::{AtomicU64, Ordering}; thread_local! { static FAIL_AFTER: Cell> = const { Cell::new(None) }; + static SYNCS: Cell = const { Cell::new(0) }; +} + +/// How many file and folder syncs this module made **on the calling thread** (for tests: a +/// durable write path syncs, a recreatable output doesn't). +#[doc(hidden)] +pub fn syncs_on_this_thread() -> u64 { + SYNCS.with(Cell::get) +} + +fn sync_file(f: &File) -> io::Result<()> { + SYNCS.with(|c| c.set(c.get() + 1)); + f.sync_all() } /// Fault injection for tests (in any crate): while the returned guard lives, every write made by @@ -78,7 +103,7 @@ fn parent_of(path: &Path) -> &Path { /// 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(); + let _ = sync_file(&d); } } @@ -100,9 +125,9 @@ fn create_temp(path: &Path) -> io::Result<(PathBuf, File)> { 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)> { +/// Write a temp file next to `path` with what `fill` writes, synced to disk when `sync`. On +/// failure the temp file is removed. Returns its path and length. +fn write_temp(path: &Path, sync: bool, 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); @@ -111,7 +136,9 @@ fn write_temp(path: &Path, fill: &mut dyn FnMut(&mut dyn Write) -> io::Result<() w.flush()?; let n = w.n; let f = w.inner.into_inner().map_err(|e| e.into_error())?; - f.sync_all()?; + if sync { + sync_file(&f)?; + } Ok(n) })(); match r { @@ -132,12 +159,26 @@ pub fn write_atomic(path: &Path, data: &[u8]) -> io::Result<()> { /// [`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 { - let (tmp, n) = write_temp(path, fill)?; + replace_with(path, true, fill) +} + +/// Replace (or create) `path` with `data` atomically but **without syncing** it to disk — only for +/// outputs that can always be recreated (exports, renders, screenshots); see the module docs. +/// Anything that can't be made again goes through [`write_atomic`]. +pub fn write_atomic_nosync(path: &Path, data: &[u8]) -> io::Result<()> { + replace_with(path, false, &mut |w| w.write_all(data)).map(|_| ()) +} + +/// Temp file (synced when `sync`) renamed over `path`, then the folder synced when `sync`. +fn replace_with(path: &Path, sync: bool, fill: &mut dyn FnMut(&mut dyn Write) -> io::Result<()>) -> io::Result { + let (tmp, n) = write_temp(path, sync, fill)?; if let Err(e) = fs::rename(&tmp, path) { let _ = fs::remove_file(&tmp); return Err(e); } - sync_dir(parent_of(path)); + if sync { + sync_dir(parent_of(path)); + } Ok(n) } @@ -155,8 +196,11 @@ pub fn write_new_unique(candidates: &mut dyn Iterator, data: &[u 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 (tmp, _) = write_temp(&first, true, &mut |w| w.write_all(data))?; let r = (|| { + // Kept (issue #134 point 4): these files replace raws (Convert / Copy as DNG) or become + // library photos (merges). On a local disk the read usually comes from the cache, but on a + // network volume it can go back to the server, and next to encoding a DNG it costs little. if !same_content(&tmp, data)? { return Err(io::Error::other("the written file reads back different (failing drive or connection?)")); } @@ -287,6 +331,33 @@ mod tests { let _ = fs::remove_dir_all(&d); } + #[test] + fn recreatable_outputs_are_atomic_without_a_sync() { + let d = temp("nosync"); + let p = d.join("export.jpg"); + let before = syncs_on_this_thread(); + write_atomic_nosync(&p, b"one").unwrap(); + write_atomic_nosync(&p, b"two two").unwrap(); + assert_eq!(syncs_on_this_thread(), before, "no file or folder sync"); + assert_eq!(fs::read(&p).unwrap(), b"two two"); + assert_eq!(names(&d), vec!["export.jpg"], "no temp file left"); + // still atomic: a failure part-way leaves the previous file and no temp file + { + let _f = fail_writes_after(3); + assert!(write_atomic_nosync(&p, &[9u8; 10_000]).is_err()); + } + assert_eq!(fs::read(&p).unwrap(), b"two two"); + assert_eq!(names(&d), vec!["export.jpg"]); + // the durable writers do sync (the file; the folder too where it can be opened) + let before = syncs_on_this_thread(); + write_atomic(&p, b"three").unwrap(); + let mid = syncs_on_this_thread(); + assert!(mid > before); + write_new(&d.join("new.dng"), b"dng").unwrap(); + assert!(syncs_on_this_thread() > mid); + let _ = fs::remove_dir_all(&d); + } + #[test] fn new_files_never_replace_anything() { let d = temp("new"); diff --git a/crates/engine/src/cmd/convert.rs b/crates/engine/src/cmd/convert.rs index c81267bf..1641d63f 100644 --- a/crates/engine/src/cmd/convert.rs +++ b/crates/engine/src/cmd/convert.rs @@ -149,7 +149,8 @@ fn edit_external(s: &mut Session, p: &Value) -> Result { }; let mut written = Vec::new(); let mut write = |path: &str, bytes: &[u8]| -> std::result::Result<(), String> { - crate::export::write_file(path, bytes)?; + // durable: the TIFF becomes a library photo the external editor then changes + crate::export::write_file_durable(path, bytes)?; written.push(path.to_string()); Ok(()) }; diff --git a/crates/engine/src/cmd/previews.rs b/crates/engine/src/cmd/previews.rs index d3b63389..3dacc31f 100644 --- a/crates/engine/src/cmd/previews.rs +++ b/crates/engine/src/cmd/previews.rs @@ -160,7 +160,9 @@ fn smart_run( if damaged && crate::smart::is_valid(&path) { n.built += 1; } else { - // atomic: a failed write leaves no partial proxy that would pass for a built one + // atomic: a failed write leaves no partial proxy that would pass for a built one; + // and synced (issue #134): built so the photo can be edited while its original + // is offline, when it is the only copy — the sync is small next to the decode let r = source .ok_or_else(|| "nothing to build from".to_string()) .and_then(|s| s.load_source()) diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index 6c30b1a0..d40ab577 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -1131,15 +1131,27 @@ pub fn export_batch( } /// 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). +/// replaced atomically ([`lightcraft_catalog::safe_file::write_atomic_nosync`]: a temp file +/// renamed into place), so a failure part-way leaves any previous file intact and no truncated +/// one. Not synced to disk (issue #134): an export can always be made again from the original. +/// The native writer behind exports, renders and screenshots (app, CLI, MCP). pub fn write_file(path: &str, bytes: &[u8]) -> Result<(), String> { + write_with(path, bytes, false) +} + +/// [`write_file`], synced to disk: for a written file that becomes a library photo (Edit a Copy) +/// and so can't simply be exported again once the catalog points at it. +pub fn write_file_durable(path: &str, bytes: &[u8]) -> Result<(), String> { + write_with(path, bytes, true) +} + +fn write_with(path: &str, bytes: &[u8], durable: bool) -> Result<(), String> { + use lightcraft_catalog::safe_file::{write_atomic, write_atomic_nosync}; 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}")) + if durable { write_atomic(p, bytes) } else { write_atomic_nosync(p, bytes) }.map_err(|e| format!("{path}: {e}")) } /// The path of the sidecar with extension `ext` of the exported file `main`. diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index a85d6287..6f35b670 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -375,13 +375,16 @@ 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. 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 { +/// synced and checked against `probe_hash`, the content hash the probe computed — or compared +/// byte for byte with the source when there is none); a bad one is removed and the photo +/// reported as failed. +fn copy_into(root: &Path, src: &str, folders: &[String], name: Option<&str>, probe_hash: Option<&str>) -> Result { let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f)); 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())); - crate::import_move::copy_new(Path::new(src), &dir, &name).map(|p| p.to_string_lossy().to_string()) + let expect = probe_hash.and_then(lightcraft_preview::Hash128::parse); + crate::import_move::copy_new(Path::new(src), &dir, &name, expect).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 @@ -784,7 +787,7 @@ impl ImportJob { let reason = "a symbolic link: the file it points to was copied, the link and its target stay"; out.items.push(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() })); } - match copy_into(root, &path, &folders, name.as_deref()) { + match copy_into(root, &path, &folders, name.as_deref(), info.content_hash.as_deref()) { Ok(p) => p, Err(e) => { out.items.push(PreparedItem::Failed(path, e)); diff --git a/crates/engine/src/import_move.rs b/crates/engine/src/import_move.rs index 7221c595..3465eaa7 100644 --- a/crates/engine/src/import_move.rs +++ b/crates/engine/src/import_move.rs @@ -171,9 +171,15 @@ pub(crate) fn place(src: &Path, dir: &Path, name: &str) -> Result Result { +/// disk and verified; a bad copy is removed and reported, so a card is never wiped on the strength +/// of it. Returns the new path. +/// +/// `expect` is the content hash the import's probe computed from its full read of the source +/// ([`lightcraft_preview::hash_bytes`]): the copy is read back and checked against it, which spares +/// a third read of the card (issue #134). It also catches a source that changed since the probe +/// (whose recorded metadata and hash would be stale). Without one the copy is compared byte for +/// byte with the source. +pub(crate) fn copy_new(src: &Path, dir: &Path, name: &str, expect: Option) -> Result { 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}")), @@ -181,7 +187,7 @@ pub(crate) fn copy_new(src: &Path, dir: &Path, name: &str) -> Result 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())), @@ -262,12 +268,14 @@ fn link_or_copy(src: &Path, dst: &Path) -> io::Result<()> { Err(_) => {} } } - copy_verified(src, dst) + // Move keeps the byte-for-byte comparison: the source is deleted on the strength of it + copy_verified(src, dst, None) } -/// Copy into a new file, sync it to disk and compare it with the original; on any failure the -/// partial copy is removed. -fn copy_verified(src: &Path, dst: &Path) -> io::Result<()> { +/// Copy into a new file, sync it to disk and verify it: against `expect` (the probe's hash of the +/// source) when given, else byte for byte against the source. On any failure the partial copy is +/// removed. +pub(crate) fn copy_verified(src: &Path, dst: &Path, expect: Option) -> io::Result<()> { let mut out = OpenOptions::new().write(true).create_new(true).open(dst)?; let r = (|| { let mut inp = File::open(src)?; @@ -283,10 +291,14 @@ fn copy_verified(src: &Path, dst: &Path) -> io::Result<()> { if let Ok(m) = fs::metadata(src).and_then(|m| m.modified()) { let _ = out.set_modified(m); } - if !same_bytes(src, dst)? { - return Err(io::Error::other("the copy differs from the original")); + match expect { + Some(h) if hash_file(dst)? != h => { + Err(io::Error::other("the copy differs from the original as it was read for the import (a bad copy, or the file changed meanwhile)")) + } + Some(_) => Ok(()), + None if !same_bytes(src, dst)? => Err(io::Error::other("the copy differs from the original")), + None => Ok(()), } - Ok(()) })(); drop(out); if r.is_err() { @@ -295,6 +307,20 @@ fn copy_verified(src: &Path, dst: &Path) -> io::Result<()> { r } +/// [`lightcraft_preview::hash_bytes`] of a file's content, read in chunks. +fn hash_file(path: &Path) -> io::Result { + let mut f = File::open(path)?; + let mut h = lightcraft_preview::Hasher128::new(); + let mut buf = vec![0u8; 1 << 20]; + loop { + let n = fill(&mut f, &mut buf)?; + if n == 0 { + return Ok(h.finish()); + } + h.update(buf.get(..n).unwrap_or(&[])); + } +} + /// Do two files hold the same bytes? pub(crate) fn same_bytes(a: &Path, b: &Path) -> io::Result { if fs::metadata(a)?.len() != fs::metadata(b)?.len() { diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 7fd8a596..91f72e02 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -830,7 +830,9 @@ pub struct ProbeInfo { pub captured: Option, pub meta: lightcraft_catalog::Meta, pub as_shot_wb: Option<(f64, f64)>, - /// Hash of the file's bytes (hex), for duplicate detection. + /// Hash of the file's bytes (hex), for duplicate detection. When it is a 32-digit + /// [`lightcraft_preview::hash_bytes`] of the whole file (as the native probe computes it), + /// Import → Copy verifies each copy against it instead of reading the source again. pub content_hash: Option, /// Lens corrections embedded in the file (DNG opcodes). pub embedded_lens: Option, diff --git a/crates/engine/src/originals.rs b/crates/engine/src/originals.rs index 59a5f6a2..86128108 100644 --- a/crates/engine/src/originals.rs +++ b/crates/engine/src/originals.rs @@ -68,9 +68,14 @@ impl crate::Session { OriginalGuard::new(&self.catalog) } - /// `Err` when writing `path` would replace a catalogued photo's original or sidecar. + /// `Err` when writing `path` would replace a catalogued photo's original or sidecar. A free + /// path (the usual case) is answered without building the guard over the whole library. pub fn check_write_target(&self, path: &str) -> Result<(), String> { - self.original_guard().check(Path::new(path)) + let path = Path::new(path); + if std::fs::symlink_metadata(path).is_err() { + return Ok(()); // nothing there to replace + } + self.original_guard().check(path) } } diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index dcd74cb4..1515d7bf 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -323,3 +323,24 @@ fn a_failed_export_write_keeps_the_previous_file() { assert_ne!(std::fs::read(&prev).unwrap(), b"last week's export"); let _ = std::fs::remove_dir_all(&dir); } + +/// Issue #134: exports can always be made again, so they are written atomically but not synced to +/// disk file by file; an edit copy that becomes a library photo still is. +#[test] +fn exports_are_atomic_without_a_sync() { + use crate::export::{Conflict, Destination}; + use lightcraft_catalog::safe_file::syncs_on_this_thread; + let (mut s, id, dir, _src, _) = library_with_jpeg("export-nosync"); + let out = dir.join("out"); + let to = Destination { dir: out.to_string_lossy().to_string(), exact: None }; + let o = ExportOptions { conflict: Conflict::Unique, ..Default::default() }; + let before = syncs_on_this_thread(); + for _ in 0..3 { + disk_batch(&mut s, id, &o, &to).unwrap(); + } + assert_eq!(syncs_on_this_thread(), before, "no per-file sync for exports"); + assert_eq!(std::fs::read_dir(&out).unwrap().count(), 3, "three exports, no temp files"); + crate::export::write_file_durable(&out.join("edit.tif").to_string_lossy(), b"tiff").unwrap(); + assert!(syncs_on_this_thread() > before, "the durable writer syncs"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/engine/src/tests_import_move.rs b/crates/engine/src/tests_import_move.rs index 5eb278fe..76ea0a3d 100644 --- a/crates/engine/src/tests_import_move.rs +++ b/crates/engine/src/tests_import_move.rs @@ -207,6 +207,67 @@ fn copy_import_verifies_each_copy() { let _ = std::fs::remove_dir_all(&base); } +/// Copy verifies against the probe's content hash (issue #134): the hash, not a re-read of the +/// source, is the reference — a copy that matches the source but not the hash is refused, a +/// corrupted copy is caught, and the file hash agrees with `hash_bytes` across read chunks. +#[test] +fn copy_is_verified_against_the_probe_hash() { + use crate::import_move::copy_verified; + let base = temp_dir("copyhash"); + let src = base.join("src.bin"); + // larger than one 1 MB read, not a multiple of it + let bytes: Vec = (0..(3 << 20) + 12_345u32).map(|i| (i.wrapping_mul(2_654_435_761) >> 13) as u8).collect(); + std::fs::write(&src, &bytes).unwrap(); + let good = lightcraft_preview::hash_bytes(&bytes); + copy_verified(&src, &base.join("ok.bin"), Some(good)).unwrap(); + assert_eq!(std::fs::read(base.join("ok.bin")).unwrap(), bytes); + // a hash that doesn't match: refused (a byte compare with the source would have passed) + let stale = lightcraft_preview::hash_bytes(b"what the probe read earlier"); + let e = copy_verified(&src, &base.join("stale.bin"), Some(stale)).unwrap_err(); + assert!(e.to_string().contains("differs"), "{e}"); + assert!(!base.join("stale.bin").exists(), "the refused copy is removed"); + // a copy that comes out different: caught by the hash + inject(Fault::Corrupt); + let r = copy_verified(&src, &base.join("bad.bin"), Some(good)); + inject(Fault::None); + assert!(r.is_err()); + assert!(!base.join("bad.bin").exists()); + assert_eq!(std::fs::read(&src).unwrap(), bytes, "the source is untouched"); + let _ = std::fs::remove_dir_all(&base); +} + +/// A file replaced after Review Import probed it is not imported on the strength of the old probe: +/// its copy no longer matches the probe's hash, so it's reported as failed and nothing is left. +#[test] +fn copy_import_refuses_a_file_changed_since_the_review() { + let base = temp_dir("copychanged"); + let (card, dest) = (base.join("card"), base.join("out")); + write_png(&card.join("a.png"), 3); + let mut s = session(); + let r = s.execute("library.importPreview", &json!({"paths": [card.to_string_lossy()]})).unwrap(); + assert_eq!(r["scanned"], 1, "{r}"); + write_png(&card.join("a.png"), 9); // other pixels + let r = s + .execute( + "library.import", + &json!({"paths": [card.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": "flat"}), + ) + .unwrap(); + assert_eq!(len(&r, "imported"), 0, "{r}"); + assert_eq!(len(&r, "failed"), 1, "{r}"); + assert!(r.to_string().contains("changed meanwhile"), "{r}"); + assert_eq!(files_under(&dest), Vec::::new()); + // importing again probes afresh and succeeds + let r = s + .execute( + "library.import", + &json!({"paths": [card.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": "flat"}), + ) + .unwrap(); + assert_eq!(len(&r, "imported"), 1, "{r}"); + 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] @@ -320,3 +381,55 @@ fn move_into_the_library_is_saved_before_sources_go() { assert_eq!(photo_paths(&again), [want.to_string_lossy()]); let _ = std::fs::remove_dir_all(&base); } + +/// Timing for issue #134 (not a pass/fail test): `cargo test -p lightcraft-engine --release +/// write_policy_timing -- --ignored --nocapture`. Writes N export-sized files durably (temp + +/// sync + rename) and atomically only, and copies N files verified byte for byte and against the +/// probe hash, alternating so a loaded machine affects both alike. `LIGHTCRAFT_BENCH_DIR` puts +/// the files on another volume (a USB drive or a NAS is where the difference shows). +#[test] +#[ignore] +fn write_policy_timing() { + use crate::import_move::copy_verified; + use lightcraft_catalog::safe_file::{write_atomic, write_atomic_nosync}; + use std::time::{Duration, Instant}; + let root = std::env::var_os("LIGHTCRAFT_BENCH_DIR").map(PathBuf::from).unwrap_or_else(std::env::temp_dir); + let base = root.join(format!("lc-write-policy-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + std::fs::create_dir_all(&base).unwrap(); + let data = |n: usize, seed: u32| -> Vec { (0..n as u32).map(|i| (i.wrapping_add(seed).wrapping_mul(2_654_435_761) >> 11) as u8).collect() }; + let (n, size) = (200usize, 300 << 10); + let (mut synced, mut unsynced) = (Duration::ZERO, Duration::ZERO); + for i in 0..n { + let b = data(size, i as u32); + let t = Instant::now(); + write_atomic(&base.join(format!("sync-{i}.jpg")), &b).unwrap(); + synced += t.elapsed(); + let t = Instant::now(); + write_atomic_nosync(&base.join(format!("nosync-{i}.jpg")), &b).unwrap(); + unsynced += t.elapsed(); + } + println!("export {n} × {} KB: write_atomic (synced) {synced:?}, write_atomic_nosync {unsynced:?}", size >> 10); + let (n, size) = (40usize, 8 << 20); + let card = base.join("card"); + std::fs::create_dir_all(&card).unwrap(); + let hashes: Vec<_> = (0..n) + .map(|i| { + let b = data(size, 7 * i as u32); + std::fs::write(card.join(format!("{i}.raw")), &b).unwrap(); + lightcraft_preview::hash_bytes(&b) + }) + .collect(); + let (mut bytewise, mut hashed) = (Duration::ZERO, Duration::ZERO); + for (i, h) in hashes.iter().enumerate() { + let src = card.join(format!("{i}.raw")); + let t = Instant::now(); + copy_verified(&src, &base.join(format!("bytes-{i}.raw")), None).unwrap(); + bytewise += t.elapsed(); + let t = Instant::now(); + copy_verified(&src, &base.join(format!("hash-{i}.raw")), Some(*h)).unwrap(); + hashed += t.elapsed(); + } + println!("copy {n} × {} MB: byte compare with the source {bytewise:?}, probe hash {hashed:?}", size >> 20); + let _ = std::fs::remove_dir_all(&base); +} diff --git a/docs/parity.md b/docs/parity.md index cfc61c96..4eb48c4e 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -114,7 +114,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); the import itself (probing, sidecars, copying / moving files) runs on a worker thread and its batches join the library between frames, with a progress window and Cancel; one undo step. Drag-and-drop goes through the same background import | | 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); 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-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 (a new file, synced, checked against the content hash the scan computed — byte compare with the source when there is none, issue #134; 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 | @@ -434,7 +434,7 @@ 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), `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-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 and renamed into place (issue #93; not synced file by file — exports can be made again, issue #134) | | 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`), `crates/engine/src/tests_export.rs` | file copied byte for byte, sidecar named after the output and subject to the conflict policy | diff --git a/docs/whats-new.md b/docs/whats-new.md index 701600ef..5ae3fd53 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -14,18 +14,25 @@ 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. + Exported files are written to a temp file 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. + checked against the content read from 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. +- Faster exports and card imports, with the same protection for what can't be recreated (issue #134): exports, + renders and screenshots are still written to a temp file and renamed into place, but no longer forced to disk + one by one — they can always be exported again, and on a USB drive or a NAS that per-file sync dominated a large + export. The catalog, XMP sidecars, settings, DNGs, merges, smart previews and the copies an import makes are still + synced. Import ▸ Copy now checks each copy against the content hash taken while scanning the card instead of + reading the card a third time (Move still compares byte for byte before it deletes a source); a file that changed + on the card after Review Import is reported instead of being imported with stale details. Checking an export path + against the library's originals no longer scans the whole library when nothing is at that path. - 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 `.xmp.bak-