aleleba/lightcraft
1
0
Fork 0
mirror of https://github.com/storytold/lightcraft.git synced 2026-11-03 06:59:53 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge main into #1919 (main already dropped the drop(write) line)

Browse Source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-18 09:21:51 -04:00
co-authored by Claude Opus 5.5
parent 6938be39e4 4f0b110bcb
commit cbdbd50f84
35 changed files with 937 additions and 752 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/cull.rs
+1 -1
View File
@@ -16,7 +16,7 @@ const SAME: f32 = 0.93;
fn secs(iso: &str) -> Option<i64> {
let d = lightcraft_meta::DateTime::parse_iso(iso)?;
Some(((d.year as i64 * 372 + d.month as i64 * 31 + d.day as i64) * 24 + d.hour as i64) * 3600 + d.minute as i64 * 60 + d.second as i64)
Some(d.unix_seconds())
}
fn analyze(s: &mut Session, p: &Value) -> Result<Value> {
crates/engine/src/cmd/library.rs
+1 -1
View File
@@ -1488,7 +1488,7 @@ pub fn specs() -> Vec<CommandSpec> {
always, |_, p| {
let path = str_param(p,"path").ok_or_else(|| bad("library.inspectLightroom","missing path"))?;
let data = crate::lightroom_catalog::read(std::path::Path::new(path)).map_err(|e| bad("library.inspectLightroom",e))?;
Ok(json!({"photos":data.photos.len(),"collections":data.collections.iter().filter(|r|r.get("systemOnly").and_then(Value::as_f64).unwrap_or(0.0)==0.0).count(),"missing":data.photos.iter().filter(|p| !std::path::Path::new(&p.path).is_file()).map(|p|&p.path).collect::<Vec<_>>(),"warnings":data.warnings}))
Ok(json!({"photos":data.photos.len(),"collections":data.collections.iter().filter(|r|r.get("systemOnly").and_then(Value::as_f64).unwrap_or(0.0)==0.0).count(),"missing":data.photos.iter().filter(|p| !p.original_is_file()).map(|p|&p.path).collect::<Vec<_>>(),"warnings":data.warnings}))
}),
cmd!(
"library.importLightroom",
crates/engine/src/cmd/library/tests_relative_ratings.rs
+1
View File
@@ -77,6 +77,7 @@ fn relative_ratings_and_history_use_auto_write() {
std::fs::create_dir_all(&dir).unwrap();
// Metadata-only fixtures: no import, image decoding or model/service dependency.
for id in 1..=4 {
std::fs::write(dir.join(format!("{id}.jpg")), b"original").unwrap();
s.catalog
.apply(Op::SetFile {
id: PhotoId(id),
crates/engine/src/doc_path.rs
+63
View File
@@ -0,0 +1,63 @@
//! Paths stored in a file LightCraft reads but the user did not choose path by path, such as the
//! originals a Lightroom catalog lists.
//!
//! Such a path reaches the file system only through [`resolve`]: a relative path is taken from the
//! document's folder, and only local files, or files on the network share the document itself is
//! on, are looked at (see `craft_docpath`). A path that is not followed is handled like a missing
//! file.
use std::fmt;
use std::path::{Path, PathBuf};
pub use craft_docpath::Refused;
/// Why a stored path is not followed.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum NotFollowed {
/// The path is on another computer (other than the document's share), names a device, is a
/// URL, or is not a usable path.
Refused(Refused),
/// The path is not an absolute path on this computer, such as a Windows drive path read on
/// macOS or Linux.
NotHere,
}
impl fmt::Display for NotFollowed {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Refused(r) => r.fmt(f),
Self::NotHere => f.write_str("the path is not a path on this computer"),
}
}
}
impl std::error::Error for NotFollowed {}
/// The file to look at for `stored`, a path or `file:` URL stored in the document at `document`,
/// or why it is not followed. A relative `document` is taken from the working directory, where it
/// was opened from.
pub fn resolve(stored: &str, document: Option<&Path>) -> Result<PathBuf, NotFollowed> {
let document = document.map(|d| if d.is_relative() { std::path::absolute(d).unwrap_or_else(|_| d.to_path_buf()) } else { d.to_path_buf() });
let path = craft_docpath::resolve(stored, document.as_deref()).map_err(NotFollowed::Refused)?;
// A path that is still relative would be taken from the working directory, which the
// document does not choose.
if path.is_relative() {
return Err(NotFollowed::NotHere);
}
Ok(path)
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn relative_paths_are_taken_from_the_document_folder_and_other_computers_are_refused() {
let doc = std::env::temp_dir().join("lrcat").join("catalog.lrcat");
assert_eq!(resolve("Photos/a.jpg", Some(&doc)), Ok(std::env::temp_dir().join("lrcat").join("Photos").join("a.jpg")));
assert_eq!(resolve("a.jpg", None), Err(NotFollowed::NotHere));
assert_eq!(resolve(r"\\192.0.2.1\share\x.jpg", Some(&doc)), Err(NotFollowed::Refused(Refused::Network)));
assert_eq!(resolve("file://192.0.2.1/s/x.jpg", Some(&doc)), Err(NotFollowed::Refused(Refused::Network)));
assert_eq!(resolve(r"\\.\COM1", Some(&doc)), Err(NotFollowed::Refused(Refused::Device)));
}
}
crates/engine/src/lib.rs
+2
View File
@@ -29,6 +29,7 @@ pub mod demo;
pub mod denoise;
pub mod devices;
pub mod display;
pub mod doc_path;
mod errors;
pub mod export;
pub mod face_download;
@@ -56,6 +57,7 @@ pub mod media;
pub mod memory;
pub mod merge;
mod model_download;
pub mod open_path;
pub mod originals;
mod pdf;
pub mod preset_import;
crates/engine/src/lightroom_archive.rs
+2
View File
@@ -538,6 +538,7 @@ mod tests {
keywords: vec!["Trips|Goa".into()],
history: vec![BTreeMap::from([("text".into(), serde_json::json!("step")), ("curve".into(), serde_json::json!([0, 128, 255]))])],
snapshots: vec![BTreeMap::from([("name".into(), serde_json::json!("before"))])],
original: crate::lightroom_catalog::Original::Local,
};
CatalogImport {
source: "source.lrcat".into(),
@@ -621,6 +622,7 @@ mod tests {
keywords: Vec::new(),
history: vec![BTreeMap::from([("text".into(), serde_json::json!(format!("history-{index}-{}", "x".repeat(8 * 1024))))])],
snapshots: Vec::new(),
original: crate::lightroom_catalog::Original::Local,
})
.collect();
assert_eq!(store_best_effort(&path, &input).unwrap(), None);
crates/engine/src/lightroom_catalog.rs
+213 -23
View File
@@ -2,7 +2,7 @@
//! Lightroom, Python and a C SQLite runtime are not needed. Original paths stay in place.
//! Raw settings are archived before mutation; mapped edits are approximations, not Adobe renders.
use std::collections::{BTreeMap, HashMap, HashSet};
use std::path::{Component, Path, PathBuf};
use std::path::{Path, PathBuf};
use std::sync::{
Arc,
atomic::{AtomicBool, AtomicUsize, Ordering},
@@ -146,6 +146,31 @@ pub struct CatalogPhoto {
pub keywords: Vec<String>,
pub history: Vec<Row>,
pub snapshots: Vec<Row>,
/// Whether `path` may be looked at (derived when the catalog is read; not archived).
#[serde(skip)]
pub original: Original,
}
/// Whether the original a catalog lists may be looked at. Its path is resolved with the catalog
/// as the document ([`crate::doc_path::resolve`]).
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub enum Original {
/// `path` is a local file path, or one on the network share the catalog is on.
Local,
/// `path` is not a path on this computer (a catalog made on another system): the photo is
/// kept as an offline record for later relinking, and the path is not looked at.
#[default]
NotHere,
/// `path` is refused (why): it is on another computer or a device, or its folder or file name
/// is an absolute path. The photo is reported missing and not added to the library.
Refused(String),
}
impl CatalogPhoto {
/// Whether the original is a file here; only a [`Original::Local`] path is looked at.
pub fn original_is_file(&self) -> bool {
self.original == Original::Local && Path::new(&self.path).is_file()
}
}
#[derive(Clone, Debug, Serialize, Deserialize)]
@@ -343,25 +368,36 @@ fn committed_wal_len(main: &[u8], wal: &[u8]) -> Result<usize, String> {
/// Where a root folder is when its `absolutePath` isn't a folder here (the catalog came from
/// another computer): `relativePathFromCatalog`, which Lightroom records for roots on the
/// catalog's volume, resolved against the catalog's folder, if that is a folder.
/// catalog's volume, resolved against the catalog's folder, if that is a folder. Both paths are
/// looked at only when [`crate::doc_path::resolve`] follows them.
fn relocated_root(absolute: &str, relative: &str, catalog: &Path) -> Option<PathBuf> {
if relative.is_empty() || !Path::new(relative).is_relative() || Path::new(absolute).is_dir() {
if relative.is_empty() || !Path::new(relative).is_relative() || crate::doc_path::resolve(absolute, Some(catalog)).is_ok_and(|p| p.is_dir()) {
return None;
}
let dir = std::path::absolute(catalog).ok()?.parent()?.join(relative);
let mut out = PathBuf::new();
for part in dir.components() {
match part {
Component::ParentDir => {
if !out.pop() {
return None;
}
}
Component::CurDir => {}
part => out.push(part),
}
crate::doc_path::resolve(relative, Some(catalog)).ok().filter(|dir| dir.is_dir())
}
/// Whether a folder path or file name from the catalog is absolute: `Path::join` would put it in
/// place of the root folder.
fn is_absolute_part(part: &str) -> bool {
let bytes = part.as_bytes();
part.starts_with(['/', '\\'])
|| (bytes.first().is_some_and(u8::is_ascii_alphabetic) && bytes.get(1) == Some(&b':'))
|| Path::new(part).is_absolute()
}
/// The path of an original under `root` and whether it may be looked at, resolved with the
/// catalog at `catalog` as the document.
fn original_path(root: &str, folder: &str, name: &str, catalog: &Path) -> (String, Original) {
let stored = Path::new(root).join(folder).join(name).to_string_lossy().to_string();
if is_absolute_part(folder) || is_absolute_part(name) {
return (stored, Original::Refused("its folder or file name is an absolute path".into()));
}
match crate::doc_path::resolve(&stored, Some(catalog)) {
Ok(found) => (found.to_string_lossy().to_string(), Original::Local),
Err(crate::doc_path::NotFollowed::NotHere) => (stored, Original::NotHere),
Err(crate::doc_path::NotFollowed::Refused(why)) => (stored, Original::Refused(why.to_string())),
}
out.is_dir().then_some(out)
}
/// Inspect `.lrcat` plus committed WAL pages without modifying either source file.
@@ -457,6 +493,7 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize,
out
};
let mut root_paths = HashMap::new();
let mut refused_roots = HashSet::new();
let mut root_ids: Vec<_> = roots.keys().copied().collect();
root_ids.sort_unstable();
for (id, root) in root_ids.iter().filter_map(|id| Some((*id, roots.get(id)?))) {
@@ -469,6 +506,11 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize,
}
None => absolute.to_string(),
};
if let Err(crate::doc_path::NotFollowed::Refused(why)) = crate::doc_path::resolve(&resolved, Some(path)) {
warnings
.push(format!("root folder {}: {resolved} is not opened: {why}; its photos are listed as missing and not added", text(root, "name")));
refused_roots.insert(id);
}
root_paths.insert(id, resolved);
}
let mut history = group(optional_table("Adobe_libraryImageDevelopHistoryStep", &mut warnings)?);
@@ -487,13 +529,19 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize,
let file = files.get(&number(&image, "rootFile")).ok_or_else(|| format!("image {id}: missing file record"))?;
let folder = folders.get(&number(file, "folder")).ok_or_else(|| format!("image {id}: missing folder record"))?;
let root = roots.get(&number(folder, "rootFolder")).ok_or_else(|| format!("image {id}: missing root record"))?;
let root_path = root_paths.get(&number(root, "id_local")).map_or("", String::as_str);
let root_id = number(root, "id_local");
let root_path = root_paths.get(&root_id).map_or("", String::as_str);
let folder_path = text(folder, "pathFromRoot");
let name = text(file, "idx_filename");
if root_path.is_empty() || name.is_empty() {
return Err(format!("image {id}: incomplete original path"));
}
let path = Path::new(root_path).join(folder_path).join(name).to_string_lossy().to_string();
let (photo_path, original) = original_path(root_path, folder_path, name, path);
if let Original::Refused(why) = &original
&& !refused_roots.contains(&root_id)
{
out.warnings.push(format!("image {id}: {photo_path} is not opened: {why}; listed as missing and not added"));
}
let packet = match xmp(metadata.get(&id).and_then(|r| r.get("xmp"))) {
Ok(s) => s,
Err(e) => {
@@ -504,19 +552,64 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize,
out.photos.push(CatalogPhoto {
source_id: id,
uuid: text(&image, "id_global").into(),
path,
path: photo_path,
image,
settings: develops.get(&id).cloned().unwrap_or_default(),
xmp: packet,
keywords: keywords.remove(&id).unwrap_or_default(),
history: history.remove(&id).unwrap_or_default(),
snapshots: snapshots.remove(&id).unwrap_or_default(),
original,
});
}
refuse_non_media(&mut out);
ensure_read_active(cancel)?;
Ok(out)
}
/// Drop the catalog's images whose original is not a file type LightCraft reads, saying so in the
/// import's warnings.
///
/// The catalog decides these paths, and a record it leaves behind is one click from the system
/// opener (Photo ▸ Edit In ▸ Open Original in Other Editor). A record for `payload.exe`,
/// `start.lnk` or `Evil.app` would make a `.lrcat` someone was sent a way to have a program run
/// (issue #2135), so such an image never becomes a photo: not imported, not relinkable, not
/// openable. A virtual copy of a refused original goes with it — it has no original of its own.
///
/// Called where the catalog is read and again where its photos become records, so neither route
/// can keep one; the second pass finds nothing left to say.
pub(crate) fn refuse_non_media(data: &mut CatalogImport) {
// masters first: a virtual copy may come before its master in the table
let refused: HashSet<i64> = data
.photos
.iter()
.filter(|p| number(&p.image, "masterImage") == 0 && !crate::open_path::is_media(Path::new(&p.path)))
.map(|p| p.source_id)
.collect();
if refused.is_empty() {
return;
}
let mut warnings = Vec::new();
data.photos.retain(|p| {
let master = number(&p.image, "masterImage");
// `masterImage` is 0 for a master, so a refused id of 0 (a malformed catalog) must not
// take every master of the catalog with it
if master != 0 {
if !refused.contains(&master) {
return true;
}
warnings.push(format!("image {}: not imported; its original was not imported", p.source_id));
return false;
}
if !refused.contains(&p.source_id) {
return true;
}
warnings.push(format!("image {}: not imported; {} isn't one of the file types LightCraft reads", p.source_id, p.path));
false
});
data.warnings.extend(warnings);
}
fn ensure_read_active(cancel: &AtomicBool) -> Result<(), String> {
if cancel.load(Ordering::Relaxed) { Err("Lightroom import cancelled".into()) } else { Ok(()) }
}
@@ -665,11 +758,11 @@ fn apply(s: &mut crate::Session, data: CatalogImport, update_existing: bool) ->
let paths: Vec<_> = data
.photos
.iter()
.filter(|p| number(&p.image, "masterImage") == 0 && !existing.contains_key(&key(&p.path)) && Path::new(&p.path).is_file())
.filter(|p| number(&p.image, "masterImage") == 0 && !existing.contains_key(&key(&p.path)) && p.original_is_file())
.map(|p| p.path.clone())
.collect();
let missing_sources: HashSet<String> =
data.photos.iter().filter(|p| number(&p.image, "masterImage") == 0).filter(|p| !Path::new(&p.path).is_file()).map(|p| key(&p.path)).collect();
data.photos.iter().filter(|p| number(&p.image, "masterImage") == 0).filter(|p| !p.original_is_file()).map(|p| key(&p.path)).collect();
let files = crate::import::import(s, &paths, crate::import::ImportMode::Add)?;
let applied =
apply_prepared(s, data, ApplyPrepared { update_existing, files, index: &mut index, archive_path, missing_sources: &missing_sources, undo0 })?;
@@ -692,8 +785,10 @@ pub(crate) struct ApplyPrepared<'a> {
pub(crate) undo0: usize,
}
pub(crate) fn apply_prepared(s: &mut crate::Session, data: CatalogImport, context: ApplyPrepared<'_>) -> crate::Result<AppliedImport> {
pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, context: ApplyPrepared<'_>) -> crate::Result<AppliedImport> {
let ApplyPrepared { update_existing, files, index, archive_path, missing_sources, undo0 } = context;
// nothing but a file type LightCraft reads becomes a record, whichever route got here (#2135)
refuse_non_media(&mut data);
validate(&data).map_err(error)?;
let mut warnings = data.warnings.clone();
let duplicates: HashMap<_, _> = files.duplicates.iter().filter_map(|d| d.existing.map(|id| (key(&d.path), PhotoId(id)))).collect();
@@ -713,12 +808,18 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, data: CatalogImport, contex
let mut photos: Vec<_> = data.photos.iter().collect();
photos.sort_by_key(|p| number(&p.image, "masterImage") != 0);
let copy_masters: HashSet<_> = data.photos.iter().map(|p| number(&p.image, "masterImage")).filter(|id| *id != 0).collect();
let refused: HashSet<_> = data.photos.iter().filter(|p| matches!(p.original, Original::Refused(_))).map(|p| p.source_id).collect();
let mut planned: HashMap<PhotoId, Photo> = HashMap::new();
for src in photos {
let master = number(&src.image, "masterImage");
if master == 0 && missing_sources.contains(&key(&src.path)) {
missing.push(src.path.clone());
}
// A refused path must not become a library record the app would look at later; its
// virtual copies go with it.
if refused.contains(&src.source_id) || refused.contains(&master) {
continue;
}
let indexed = index
.photos
.get(&identity(&data, src))
@@ -940,6 +1041,7 @@ mod tests {
keywords: vec!["Travel|Goa".into()],
history: vec![row(json!({"text":"s = { Exposure2012 = 0.5 }"}))],
snapshots: Vec::new(),
original: Original::Local,
};
let mut copy = photo.clone();
copy.source_id = 2;
@@ -962,6 +1064,48 @@ mod tests {
warnings: Vec::new(),
}
}
/// A crafted `.lrcat` that names a program as a photo's original gets no record for it, so
/// Photo ▸ Edit In ▸ Open Original has nothing to hand the system opener (#2135). Its virtual
/// copies go with it; the catalog's real photos still import.
#[test]
fn a_catalog_original_that_is_not_a_photo_file_is_not_imported() {
let mut data = sample();
let mut exe = data.photos[1].clone();
exe.source_id = 3;
exe.uuid = "payload".into();
exe.path = "/Users/sky/Downloads/payload.exe".into();
exe.image = row(json!({"id_local":3,"fileFormat":"ARW","fileWidth":20,"fileHeight":10}));
let mut exe_copy = exe.clone();
exe_copy.source_id = 4;
exe_copy.uuid = "payload-copy".into();
exe_copy.path = "/Users/sky/Downloads/payload.exe".into();
exe_copy.image = row(json!({"id_local":4,"masterImage":3,"copyName":"Mono"}));
// the copy comes before its master, as a catalog's table order may have it
data.photos.insert(0, exe_copy);
data.photos.push(exe);
for bad in [r"C:\Users\sky\start.lnk", "/Applications/Evil.app", "/Users/sky/go.command", "/Users/sky/run.bat"] {
let mut one = data.clone();
one.photos.retain(|p| p.source_id < 3);
let mut planted = one.photos[1].clone();
planted.source_id = 5;
planted.uuid = "planted".into();
planted.path = bad.into();
planted.image = row(json!({"id_local":5,"fileFormat":"ARW"}));
one.photos.push(planted);
let mut s = crate::Session::new();
let r = apply(&mut s, one, false).unwrap();
assert_eq!(r["photos"], 2, "{bad} is not catalogued: {r}");
assert!(s.catalog.photos().all(|p| !matches!(&p.source, Source::File { path } if path == bad)), "{bad} became a library record");
}
let mut s = crate::Session::new();
let r = apply(&mut s, data, false).unwrap();
assert_eq!(r["photos"], 2, "only the real master and its copy: {r}");
assert!(s.catalog.photos().all(|p| !matches!(&p.source, Source::File { path } if path.ends_with(".exe"))), "the program was catalogued");
let warnings = r["warnings"].to_string();
assert!(warnings.contains("payload.exe") && warnings.contains("file types LightCraft reads"), "{warnings}");
assert!(warnings.contains("image 4: not imported"), "the virtual copy of the refused original goes with it: {warnings}");
}
#[test]
fn offline_local_master_and_copy_join_library_with_one_undo_step() {
let mut s = crate::Session::new();
@@ -1508,11 +1652,57 @@ mod tests {
// a relative path that leads nowhere keeps the recorded one
std::fs::write(&path, native_catalog(&photo_tables(("C:/Users/me/Project/RAW/", "../Elsewhere/")))).unwrap();
let catalog = read(&path).unwrap();
assert!(catalog.photos[0].path.starts_with("C:/Users/me/Project/RAW/"), "{}", catalog.photos[0].path);
assert!(Path::new(&catalog.photos[0].path).starts_with("C:/Users/me/Project/RAW/"), "{}", catalog.photos[0].path);
assert!(catalog.warnings.is_empty(), "{:?}", catalog.warnings);
std::fs::remove_dir_all(dir).unwrap();
}
#[test]
fn originals_on_other_computers_are_never_looked_at_or_added() {
let dir = std::env::temp_dir().join(format!("lightcraft-lrcat-remote-roots-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(dir.join("Photos")).unwrap();
std::fs::write(dir.join("Photos/test.ARW"), b"original").unwrap();
let source = dir.join("source.lrcat");
let mut roots = vec![r"\\192.0.2.1\share\".to_string(), "file://192.0.2.1/s/".to_string()];
if cfg!(unix) {
// `//<local folder>/` is a local folder to Linux, but a network path to the resolver.
roots.push(format!("/{}/", dir.display()));
}
for root in &roots {
std::fs::write(&source, native_catalog(&photo_tables((root, "")))).unwrap();
let catalog = read(&source).unwrap();
assert!(matches!(catalog.photos[0].original, Original::Refused(_)), "{root}: {:?}", catalog.photos[0]);
assert!(catalog.warnings.iter().any(|w| w.contains("not opened")), "{root}: {:?}", catalog.warnings);
let mut s = crate::Session::new();
let probes = Arc::new(AtomicUsize::new(0));
let calls = probes.clone();
s.media.file_probe = Some(Arc::new(move |_| {
calls.fetch_add(1, Ordering::Relaxed);
Err("a refused original must not be probed".into())
}));
let report = import(&mut s, &source, false).unwrap();
assert_eq!(probes.load(Ordering::Relaxed), 0, "{root}");
assert_eq!(report["missing"].as_array().map(Vec::len), Some(1), "{root}: {report}");
assert!(s.catalog.is_empty(), "{root}");
}
// An absolute folder path would replace the root.
let mut tables = photo_tables((dir.to_str().unwrap(), ""));
tables[1].2[2] = SqlValue::Text(format!("{}/Photos/", dir.display()));
std::fs::write(&source, native_catalog(&tables)).unwrap();
assert!(matches!(read(&source).unwrap().photos[0].original, Original::Refused(_)));
// A root beside the catalog is followed.
std::fs::write(&source, native_catalog(&photo_tables((dir.to_str().unwrap(), "")))).unwrap();
let catalog = read(&source).unwrap();
assert_eq!(catalog.photos[0].original, Original::Local);
assert!(catalog.photos[0].original_is_file());
// A root that moved is found through its path relative to the catalog.
std::fs::write(&source, native_catalog(&photo_tables((r"\\192.0.2.1\share\", "./")))).unwrap();
let catalog = read(&source).unwrap();
assert!(catalog.photos[0].original_is_file(), "{:?}", catalog.photos[0]);
std::fs::remove_dir_all(dir).unwrap();
}
#[test]
fn oversized_optional_table_warns_and_is_skipped() {
let mut main = vec![0u8; 2 * 512];
crates/engine/src/lightroom_job.rs
+3 -1
View File
@@ -329,7 +329,7 @@ fn original_paths(data: &CatalogImport, cancel: &AtomicBool) -> crate::Result<(V
if !seen.insert(key.clone()) {
continue;
}
if Path::new(&photo.path).is_file() {
if photo.original_is_file() {
paths.push(photo.path.clone());
} else {
missing.insert(key);
@@ -393,6 +393,7 @@ mod tests {
keywords: Vec::new(),
history: Vec::new(),
snapshots: Vec::new(),
original: lightroom_catalog::Original::Local,
}],
collections: Vec::new(),
members: Vec::new(),
@@ -531,6 +532,7 @@ mod tests {
serde_json::json!(format!("history-{source_id}-{}", "x".repeat(8 * 1024))),
)])],
snapshots: Vec::new(),
original: lightroom_catalog::Original::Local,
}
})
.collect();
crates/engine/src/open_path.rs
+117
View File
@@ -0,0 +1,117 @@
//! Whether a path the library holds may be handed to the operating system.
//!
//! A photo's `Source::File` path is not always something a user typed: it can come from an
//! imported Lightroom catalog, an `.lrcat` someone was sent, a relink or a synchronised folder.
//! Photo ▸ Edit In ▸ *Open Original in Other Editor* and the system default opener run whatever
//! the path names, so a record pointing at `payload.exe`, `thing.lnk`, `run.bat`, `page.hta`
//! (Windows) or `Evil.app`, `go.command` (macOS) would be executed on one click (issue #2135).
//!
//! [`openable`] is the one gate every such hand-over goes through: the path must be a regular
//! file whose extension is one LightCraft itself reads ([`crate::import::EXTENSIONS`], which
//! already covers the Edit In copy formats `tif`/`tiff`/`psd`), and what a link resolves to must
//! be one too, so a `photo.jpg` pointing at `/bin/sh` is refused as well. What comes back is the
//! absolute path to hand over; what comes back on refusal is the sentence to show the user.
//!
//! Nothing here opens, stats-for-content or reads the file: it only decides whether the path may
//! leave LightCraft. Catalog import applies the same test ([`crate::lightroom_catalog`]), so a
//! refused original never becomes a library record in the first place.
use std::path::Path;
/// The absolute path to hand to an opener or external editor, or the reason it is refused.
///
/// `label` names the thing in the error (a file name, a photo's name); an empty `label` uses
/// the path's own file name.
pub fn openable(path: &str, label: &str) -> Result<String, String> {
let p = Path::new(path);
let name = match label.trim() {
"" => p.file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().into_owned()),
l => l.to_string(),
};
if !is_media(p) {
return Err(format!("{name} isn't one of the file types LightCraft reads, so it isn't handed to another application"));
}
match std::fs::metadata(p) {
Ok(m) if m.is_file() => {}
Ok(_) => return Err(format!("{name} isn't a file, so it isn't handed to another application")),
Err(e) => return Err(format!("the original is missing: {path} ({e})")),
}
// a link may name a photo and point at a program; what runs is what it points at
if let Ok(target) = p.canonicalize()
&& !is_media(&target)
{
return Err(format!("{name} points at {}, which isn't a file type LightCraft reads", target.display()));
}
let absolute = std::path::absolute(p).map_err(|e| format!("{name}: {e}"))?;
Ok(absolute.to_string_lossy().into_owned())
}
/// Whether `path`'s extension is one LightCraft reads (the Edit In copy formats included).
pub fn is_media(path: &Path) -> bool {
crate::import::is_supported(path)
}
#[cfg(test)]
mod tests {
use super::*;
fn write(dir: &Path, name: &str) -> String {
let p = dir.join(name);
std::fs::write(&p, b"x").unwrap();
p.to_string_lossy().into_owned()
}
#[test]
fn only_file_types_lightcraft_reads_are_handed_over() {
let dir = std::env::temp_dir().join(format!("lc-openable-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
for good in ["DSC_1.NEF", "a.jpg", "copy-Edit.tif", "copy-Edit.psd"] {
let path = write(&dir, good);
assert_eq!(openable(&path, "").unwrap(), path, "{good} is a photo LightCraft reads");
}
for bad in ["payload.exe", "shortcut.lnk", "run.bat", "page.hta", "go.command", "no-extension"] {
let path = write(&dir, bad);
let e = openable(&path, "").unwrap_err();
assert!(e.contains(bad) && e.contains("file types LightCraft reads"), "{bad}: {e}");
}
// a bundle (macOS `.app`) and any other folder are not files
let bundle = dir.join("Evil.app");
std::fs::create_dir_all(&bundle).unwrap();
let e = openable(&bundle.to_string_lossy(), "").unwrap_err();
assert!(e.contains("isn't one of the file types") || e.contains("isn't a file"), "{e}");
let e = openable(&dir.join("gone.jpg").to_string_lossy(), "").unwrap_err();
assert!(e.contains("missing"), "{e}");
let _ = std::fs::remove_dir_all(&dir);
}
/// A relative path is made absolute: the opener runs with its own working directory.
#[test]
fn the_path_handed_over_is_absolute() {
let dir = std::env::temp_dir().join(format!("lc-openable-abs-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let path = write(&dir, "a.jpg");
let dotted = Path::new(&path).parent().unwrap().join(".").join("a.jpg");
let out = openable(&dotted.to_string_lossy(), "").unwrap();
assert!(Path::new(&out).is_absolute(), "{out}");
assert!(out.ends_with("a.jpg"), "{out}");
let _ = std::fs::remove_dir_all(&dir);
}
/// A link named like a photo that points at a program is refused: what the opener runs is
/// the target, not the name.
#[cfg(unix)]
#[test]
fn a_link_to_a_program_is_refused() {
let dir = std::env::temp_dir().join(format!("lc-openable-link-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let target = write(&dir, "payload.command");
let link = dir.join("holiday.jpg");
std::os::unix::fs::symlink(&target, &link).unwrap();
let e = openable(&link.to_string_lossy(), "").unwrap_err();
assert!(e.contains("points at") && e.contains("payload.command"), "{e}");
let _ = std::fs::remove_dir_all(&dir);
}
}
crates/engine/src/sidecar.rs
+30
View File
@@ -4,6 +4,8 @@
//! change when [`XmpPrefs::auto_write`] is on): rating, colour label, title, caption, copyright,
//! 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`).
//! A sidecar is written only beside an original that is an existing file at an absolute path;
//! otherwise the save fails.
//!
//! 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
@@ -632,6 +634,10 @@ impl Session {
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)))?;
// A relative path would be taken from the working directory.
if Path::new(orig).is_relative() || !Path::new(orig).is_file() {
return Err(EngineError::Other(format!("{}: the original {orig} is missing; no XMP sidecar is written", p.file_name)));
}
let naming = owners.naming(p, self.xmp.naming);
let preferred = sidecar_path(orig, naming);
let other = if naming == SidecarNaming::Stem { SidecarNaming::Full } else { SidecarNaming::Stem };
@@ -786,6 +792,30 @@ impl Session {
#[cfg(test)]
mod tests {
#[test]
fn sidecars_are_written_only_beside_an_existing_original() {
let dir = std::env::temp_dir().join(format!("lightcraft-sidecar-local-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(dir.join("IMG_1.jpg"), b"original").unwrap();
let originals = [
(r"\\192.0.2.1\share\IMG_1.jpg".to_string(), false),
(dir.join("IMG_2.jpg").to_string_lossy().to_string(), false),
(dir.join("IMG_1.jpg").to_string_lossy().to_string(), true),
];
for (original, written) in originals {
let _ = std::fs::remove_file(dir.join("IMG_1.xmp"));
let mut s = Session::new();
let p = Photo::new(PhotoId(1), Source::File { path: original.clone() }, "IMG_1.jpg", "JPEG", 4, 3, "2026-01-01T00:00:00");
s.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
assert_eq!(s.save_sidecar(PhotoId(1)).is_ok(), written, "{original}");
assert_eq!(dir.join("IMG_1.xmp").is_file(), written, "{original}");
assert!(!dir.join("IMG_2.xmp").exists(), "{original}");
assert!(!Path::new(r"\\192.0.2.1\share\IMG_1.xmp").exists(), "{original}");
}
std::fs::remove_dir_all(dir).unwrap();
}
/// A sidecar written by Lightroom Classic (names flat, paths in `lr:hierarchicalSubject`)
/// gives the photo its keywords as paths.
#[test]
crates/engine/src/slide_overlay.rs
+1 -1
View File
@@ -903,7 +903,7 @@ fn shadow(img: &mut Rgba8, tl: &crate::text::TextLayout, x: f64, y: f64, sigma:
let my0 = (y - pad).floor().max(-pad);
let mx1 = (x + tl.width as f64 + pad).ceil().min(iw + pad);
let my1 = (y + tl.height as f64 + pad).ceil().min(ih + pad);
if !(mx1 > mx0 && my1 > my0) || !mx0.is_finite() || !my0.is_finite() {
if !(mx1 > mx0 && my1 > my0 && mx0.is_finite() && my0.is_finite()) {
return;
}
let (w, h) = ((mx1 - mx0) as usize, (my1 - my0) as usize);
crates/engine/src/tests_export.rs
+12 -9
View File
@@ -1024,21 +1024,24 @@ fn same_folder_as_original_refuses_photos_without_a_folder() {
let mut s = Session::with_demo();
let ids: Vec<_> = s.catalog.photos().take(2).map(|p| p.id).collect();
let mut written = Vec::new();
let mut write = |p: &str, _: &[u8]| -> Result<(), String> {
written.push(p.to_string());
Ok(())
// the writer lives in a block of its own, so its borrow of `written` has ended by the time
// the assertion below reads it
let files = {
let mut write = |p: &str, _: &[u8]| -> Result<(), String> {
written.push(p.to_string());
Ok(())
};
let err = crate::export::export_batch(&mut s, &ids, &o, &to, &mut write, &|_| false).unwrap_err();
assert!(err.contains("no folder on disk"), "{err}");
// without stopping: one error per photo, nothing written
let items = crate::export::prepare_batch(&mut s, &ids, &o).unwrap();
crate::export::run_batch(items, &o, &to, &mut write, &|_| false, false, &mut |_, _| true).unwrap()
};
let err = crate::export::export_batch(&mut s, &ids, &o, &to, &mut write, &|_| false).unwrap_err();
assert!(err.contains("no folder on disk"), "{err}");
// without stopping: one error per photo, nothing written
let items = crate::export::prepare_batch(&mut s, &ids, &o).unwrap();
let files = crate::export::run_batch(items, &o, &to, &mut write, &|_| false, false, &mut |_, _| true).unwrap();
assert_eq!(files.len(), 2);
for (f, id) in files.iter().zip(&ids) {
assert_eq!(f["photo"], id.0, "{f}");
assert!(f["error"].as_str().unwrap().contains("no folder on disk"), "{f}");
}
// (`write` borrows `written` until its last use above; a closure has no `Drop` to wait for)
assert!(written.is_empty(), "{written:?}");
// a photo on disk whose folder is gone (moved, an unplugged drive)
crates/engine/src/tests_import.rs
+44
View File
@@ -778,6 +778,50 @@ fn assisted_culling_groups_bursts_and_scores_focus() {
let _ = std::fs::remove_dir_all(&dir);
}
/// Assisted culling: two look-alike shots one second apart across midnight form a burst at the end of
/// every month, including those with fewer than 31 days.
#[test]
fn assisted_culling_groups_bursts_across_month_end() {
let dir = temp_dir("cull-month-end");
for (name, (w, h)) in [("a.png", (480, 320)), ("b.png", (482, 321))] {
let img = lightcraft_scenes::demo_library()[0].render(w, h);
let data: Vec<[u8; 4]> = img
.data
.iter()
.map(|c| {
let e = |v: f32| (lightcraft_color::transfer::linear_to_srgb(v.clamp(0.0, 1.0)) * 255.0).round() as u8;
[e(c[0]), e(c[1]), e(c[2]), 255]
})
.collect();
let png = lightcraft_codecs::encode_png(
&lightcraft_codecs::EncodeImage::rgba8(&lightcraft_raster::Rgba8 { width: img.width, height: img.height, data }),
&Default::default(),
)
.unwrap();
std::fs::write(dir.join(name), png).unwrap();
}
let mut s = Session::new().with_fs();
let r = s.execute("library.import", &json!({"paths": [dir.to_string_lossy()]})).unwrap();
let ids: Vec<u64> = r["imported"].as_array().unwrap().iter().filter_map(Value::as_u64).collect();
let by_name = |s: &Session, n: &str| s.catalog.photos().find(|p| p.file_name == n).unwrap().id;
let (a, b) = (by_name(&s, "a.png"), by_name(&s, "b.png"));
for (before, after) in [
("2026-01-31T23:59:59", "2026-02-01T00:00:00"),
("2026-02-28T23:59:59", "2026-03-01T00:00:00"),
("2026-04-30T23:59:59", "2026-05-01T00:00:00"),
("2026-06-30T23:59:59", "2026-07-01T00:00:00"),
("2026-09-30T23:59:59", "2026-10-01T00:00:00"),
("2026-11-30T23:59:59", "2026-12-01T00:00:00"),
("2026-12-31T23:59:59", "2027-01-01T00:00:00"),
] {
s.commit("t", lightcraft_catalog::Op::SetCaptured { id: a, captured: Some(before.into()) }).unwrap();
s.commit("t", lightcraft_catalog::Op::SetCaptured { id: b, captured: Some(after.into()) }).unwrap();
let r = s.execute("photo.analyze", &json!({"ids": ids})).unwrap();
assert_eq!(r["groups"], 1, "shots 1 s apart ({before} / {after}) must form a burst: {r}");
}
let _ = std::fs::remove_dir_all(&dir);
}
/// Find Dust Spots: soft dark spots on a smooth sky are found and healed (one undo step).
#[test]
fn dust_spots_are_found_and_healed() {