collective
Browse / github.com/storytold/lightcraft / patch
Issue #1588: one bad Lightroom record costs that record, not the import
Issue #1588: one bad Lightroom record costs that record, not the import Refs #1588 (its specific "missing file record" abort was fixed in 9c812b88; this covers the rest of that class) Stacked on #3143 (Issue #3114). This PR's own commits: 3e98bce6 (`Issue #1588: one bad Lightroom record costs that record, not the import`) and a61a46d0 (`Issue #1588: duplicate ids and bad capture times cost only that record`). Refs #2532, #2589. ## Problem One contradictory record in a Lightroom catalog failed the whole import: a second image with an id already used, a virtual copy whose master is gone or is itself a copy, a keyword or collection whose parents loop, two collecti
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- THEtheChad (imported from pull request #3152, open)
- Base
662df66f1c08bdb1b1f1d0c1c638c4cef936d0b0- Size
- +1089 −193 in 28 files
- Review
- pending
- Shared
- 2026-10-28
- Id
731a90df4e1cd4bf3db3a89f
How it went
No other operator has reported using it yet.
Diff
raw · crates/catalog/src/safe_file.rs crates/engine/src/cmd/library/specs.rs crates/engine/src/lightroom_archive.rs crates/engine/src/lightroom_catalog.rs crates/engine/src/lightroom_catalog/tests.rs crates/engine/src/lightroom_job.rs crates/engine/src/lightroom_sqlite.rs crates/ui-egui/locales/cs-formats.json crates/ui-egui/locales/de-formats.json crates/ui-egui/locales/es-formats.json crates/ui-egui/locales/fi-formats.json crates/ui-egui/locales/fr-formats.json crates/ui-egui/locales/id-formats.json crates/ui-egui/locales/it-formats.json crates/ui-egui/locales/ja-formats.json crates/ui-egui/locales/ka-formats.json crates/ui-egui/locales/pl-formats.json crates/ui-egui/locales/pt-br-formats.json crates/ui-egui/locales/ru-formats.json crates/ui-egui/locales/sv-formats.json crates/ui-egui/locales/uk-formats.json crates/ui-egui/locales/vi-formats.json crates/ui-egui/locales/zh-hans-formats.json crates/ui-egui/locales/zh-hant-formats.json crates/ui-egui/src/lightroom_import.rs docs/lightroom-catalog-import.md docs/parity-checklist.md docs/whats-new.md
@@ -22,6 +22,8 @@ //! 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; where the volume has no hard //! links, an exclusive create that claims the name, then a rename over that placeholder).+//! [`publish_new`] is that last step on its own, for a writer with its own temp file (the+//! Lightroom recovery archive). //! - [`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. @@ -34,6 +36,7 @@ use std::sync::atomic::{AtomicU64, Ordering}; thread_local! { static FAIL_AFTER: Cell<Option<u64>> = const { Cell::new(None) }; static SYNCS: Cell<u64> = const { Cell::new(0) };+ static NO_HARD_LINKS: Cell<bool> = const { Cell::new(false) }; } /// How many file and folder syncs this module made **on the calling thread** (for tests: a@@ -67,6 +70,28 @@ impl Drop for FaultGuard { } } +/// Fault injection for tests (in any crate): while the returned guard lives, [`publish_new`]+/// **on the calling thread** acts as on a volume without hard links (FAT/exFAT, some shares), so+/// it takes the exclusive-create + rename fallback. Product code never calls it.+#[doc(hidden)]+pub fn without_hard_links() -> NoHardLinksGuard {+ NoHardLinksGuard { previous: NO_HARD_LINKS.with(|c| c.replace(true)), _same_thread: std::marker::PhantomData }+}++/// Ends a [`without_hard_links`] when dropped, putting back what was set before (so nested guards+/// work). It stays on the thread whose flag it set (not `Send`).+#[doc(hidden)]+pub struct NoHardLinksGuard {+ previous: bool,+ _same_thread: std::marker::PhantomData<*const ()>,+}++impl Drop for NoHardLinksGuard {+ fn drop(&mut self) {+ NO_HARD_LINKS.with(|c| c.set(self.previous));+ }+}+ /// Counts what is written; fails past an injected limit. struct Limited<W> { inner: W,@@ -221,9 +246,12 @@ pub fn write_new_unique(candidates: &mut dyn Iterator<Item = PathBuf>, data: &[u 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) {+/// Give the complete temp file `tmp` (in the same folder) the name `path`, unless that is taken+/// (`AlreadyExists`): a hard link, or where the volume has none, [`publish_new_by_rename`]. After+/// a hard link `tmp` is a second name of the file; after the fallback it is gone.+pub fn publish_new(tmp: &Path, path: &Path) -> io::Result<()> {+ let linked = if NO_HARD_LINKS.with(Cell::get) { Err(io::Error::from(io::ErrorKind::Unsupported)) } else { fs::hard_link(tmp, path) };+ match linked { Ok(()) => Ok(()), Err(e) if e.kind() == io::ErrorKind::AlreadyExists => Err(e), // no hard links on this volume (FAT/exFAT, some shares)@@ -234,6 +262,10 @@ fn publish_new(tmp: &Path, path: &Path) -> io::Result<()> { /// Fallback of [`publish_new`]: reserve the name with an exclusive create (atomic, so a file that /// appears at the same time is never replaced), then rename the complete temp file over our own /// empty placeholder. `before_claim` runs just before the name is claimed (a test hook).+///+/// That guarantee holds against other writers that also create names exclusively (as everything in+/// a library folder does). A process that deletes or replaces our placeholder between the claim and+/// the rename would have its file replaced, and a failed rename removes whatever then has the name. fn publish_new_by_rename(tmp: &Path, path: &Path, before_claim: &dyn Fn()) -> io::Result<()> { before_claim(); OpenOptions::new().write(true).create_new(true).open(path)?;@@ -1249,7 +1249,7 @@ pub fn specs() -> Vec<CommandSpec> { "{path: .lrcat} → {photos, collections, missing, warnings}; read-only, no Lightroom required", 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))?;+ let data = crate::lightroom_catalog::read(std::path::Path::new(path))?; 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,"skipped":data.skipped})) }), cmd!(@@ -36,6 +36,22 @@ const NAME_SUFFIX: &str = ".lca"; const MAX_SCAN: usize = 1024; const IO_BUFFER: usize = 16 * 1024; +/// A saved archive. Once it is published it is complete, so cleanup that fails afterwards (a+/// temporary left behind, an old archive that could not be retired) is reported beside it, not+/// as a failure to save it.+#[derive(Debug, PartialEq, Eq)]+pub struct Stored {+ pub path: PathBuf,+ /// What could not be cleaned up after the archive was published, if anything.+ pub cleanup_failed: Option<String>,+}++impl Stored {+ fn new(path: PathBuf, problems: Vec<String>) -> Self {+ Self { path, cleanup_failed: (!problems.is_empty()).then(|| problems.join("; ")) }+ }+}+ /// Store compact source data below `directory` and return its managed archive. /// /// The operation is safe to call before catalog mutation. It rejects a@@ -44,7 +60,7 @@ const IO_BUFFER: usize = 16 * 1024; /// most [`MAX_MANAGED_ARCHIVES`] and [`MAX_TOTAL_ARCHIVE_BYTES`]. Legacy /// `lightroom-import-*.json` files are deliberately ignored and preserved for /// an explicit caller warning.-pub fn store(directory: &Path, data: &CatalogImport) -> Result<PathBuf, String> {+pub fn store(directory: &Path, data: &CatalogImport) -> Result<Stored, String> { prepare_directory(directory)?; let mut managed = managed_files(directory)?; managed.sort_by_key(|entry| entry.index);@@ -64,11 +80,17 @@ pub fn store(directory: &Path, data: &CatalogImport) -> Result<PathBuf, String> } }; if same {- remove_managed_file(&temporary).map_err(|e| e.to_string())?;+ // the matching archive already exists: what is left is cleanup+ let mut problems = Vec::new();+ if let Err(error) = remove_managed_file(&temporary) {+ problems.push(format!("could not remove the temporary {}: {error}", temporary.display()));+ } let matched = entry.path.clone(); let mut all = managed.clone();- prune_existing(directory, &mut all, Some(&matched))?;- return Ok(matched);+ if let Err(error) = prune_existing(directory, &mut all, Some(&matched)) {+ problems.push(error);+ }+ return Ok(Stored::new(matched, problems)); } } @@ -115,22 +137,33 @@ pub fn store(directory: &Path, data: &CatalogImport) -> Result<PathBuf, String> let _ = remove_managed_file(&temporary); return Err(error.to_string()); }- if let Err(error) = remove_managed_file(&temporary) {- return Err(format!("archive published but temporary cleanup failed: {error}"));+ Ok(Stored::new(destination, clean_up_after_publish(&temporary, &quarantined)))+}++/// Remove the temporary and the quarantined old archives once the new archive is published, and+/// say what could not be removed. Every removal is tried; none of them can unpublish the archive.+fn clean_up_after_publish(temporary: &Path, quarantined: &[(PathBuf, PathBuf)]) -> Vec<String> {+ let mut problems = Vec::new();+ // after a hard link the temporary is a second name of the archive; after the rename fallback it is gone+ match remove_managed_file(temporary) {+ Err(error) if error.kind() != io::ErrorKind::NotFound => {+ problems.push(format!("could not remove the temporary {}: {error}", temporary.display()));+ }+ _ => {} } for (old, held) in quarantined {- if let Err(error) = remove_managed_file(&held) {- return Err(format!("archive published but could not retire {}: {error}", old.display()));+ if let Err(error) = remove_managed_file(held) {+ problems.push(format!("could not retire {} (left as {}): {error}", old.display(), held.display())); } }- Ok(destination)+ problems } /// Store source data when possible. An archive that reaches its bounded size is /// omitted with a warning so catalog import can still complete.-pub fn store_best_effort(directory: &Path, data: &CatalogImport) -> Result<Option<PathBuf>, String> {+pub fn store_best_effort(directory: &Path, data: &CatalogImport) -> Result<Option<Stored>, String> { match store(directory, data) {- Ok(path) => Ok(Some(path)),+ Ok(stored) => Ok(Some(stored)), Err(error) if error.contains("Lightroom archive exceeds 32 MiB") => Ok(None), Err(error) => Err(error), }@@ -145,19 +178,18 @@ struct Managed { fn prepare_directory(directory: &Path) -> Result<(), String> { match fs::symlink_metadata(directory) {- Ok(meta) => {- if meta.file_type().is_symlink() || !meta.is_dir() {- return Err(format!("archive directory is not a real directory: {}", directory.display()));- }- }- Err(error) if error.kind() == io::ErrorKind::NotFound => {- fs::create_dir_all(directory).map_err(|e| e.to_string())?;- let meta = fs::symlink_metadata(directory).map_err(|e| e.to_string())?;- if meta.file_type().is_symlink() || !meta.is_dir() {- return Err(format!("archive directory is not a real directory: {}", directory.display()));- }- }- Err(error) => return Err(error.to_string()),+ Err(error) if error.kind() == io::ErrorKind::NotFound => fs::create_dir_all(directory).map_err(|e| e.to_string())?,+ _ => {}+ }+ require_real_directory(directory)+}++/// Refuse `directory` when it is a symlink or not a folder, so nothing written below it lands+/// outside the library. The archive and the import index (`lightroom_job`) both check it.+pub(crate) fn require_real_directory(directory: &Path) -> Result<(), String> {+ let meta = fs::symlink_metadata(directory).map_err(|e| e.to_string())?;+ if meta.file_type().is_symlink() || !meta.is_dir() {+ return Err(format!("{} is a link or a file, not a folder", directory.display())); } Ok(()) }@@ -420,11 +452,10 @@ fn rollback_quarantine(moved: &[(PathBuf, PathBuf)]) { } } +/// Never replaces an existing archive (`AlreadyExists`), also on a volume without hard links. fn publish_new(temporary: &Path, destination: &Path) -> io::Result<()> {- match fs::hard_link(temporary, destination) {- Ok(()) => Ok(()),- Err(error) => Err(io::Error::new(error.kind(), format!("cannot atomically publish archive: {error}"))),- }+ lightcraft_catalog::safe_file::publish_new(temporary, destination)+ .map_err(|error| io::Error::new(error.kind(), format!("cannot publish archive {}: {error}", destination.display()))) } fn remove_managed_file(path: &Path) -> io::Result<()> {@@ -560,8 +591,8 @@ mod tests { fn deduplicates_identical_deterministic_archives() { let path = tempdir(); let input = data("catalog.lrcat".into());- let first = store(&path, &input).unwrap();- let second = store(&path, &input).unwrap();+ let first = store(&path, &input).unwrap().path;+ let second = store(&path, &input).unwrap().path; assert_eq!(first, second); assert_eq!(managed_count(&path), 1); fs::remove_dir_all(path).unwrap();@@ -571,12 +602,12 @@ mod tests { fn dedup_returns_archive_that_retention_keeps() { let path = tempdir(); let input = data("catalog.lrcat".into());- let first = store(&path, &input).unwrap();+ let first = store(&path, &input).unwrap().path; for index in 1..MAX_MANAGED_ARCHIVES { let filler = path.join(format!("lightroom-import-{index}.lca")); File::create(filler).unwrap().set_len(19 * 1024 * 1024).unwrap(); }- let reused = store(&path, &input).unwrap();+ let reused = store(&path, &input).unwrap().path; assert_eq!(reused, first); assert!(reused.is_file()); assert!(managed_count(&path) <= MAX_MANAGED_ARCHIVES);@@ -635,7 +666,7 @@ mod tests { #[test] fn archive_roundtrips_zlib_payload_and_preserves_numeric_arrays() { let path = tempdir();- let archive = store(&path, &rich_data()).unwrap();+ let archive = store(&path, &rich_data()).unwrap().path; let bytes = fs::read(archive).unwrap(); assert!(bytes.starts_with(HEADER)); let json = miniz_oxide::inflate::decompress_to_vec_zlib_with_limit(&bytes[HEADER.len()..], MAX_ARCHIVE_BYTES as usize).unwrap();@@ -686,4 +717,63 @@ mod tests { assert_eq!(fs::read(&old).unwrap(), b"old archive"); fs::remove_dir_all(path).unwrap(); }++ /// Cleanup after publishing tries every removal and reports each failure; it never fails the+ /// archive, which is already published.+ #[test]+ fn cleanup_after_publish_reports_what_it_could_not_remove_and_removes_the_rest() {+ let path = tempdir();+ let temporary = path.join(".lightroom-import-new.tmp");+ fs::write(&temporary, b"second name").unwrap();+ // a held name that is no longer a regular file can't be removed; the next one still is+ let (stuck, held_stuck) = (path.join("lightroom-import-0.lca"), path.join(".lightroom-retire-test-0.tmp"));+ let (gone, held_gone) = (path.join("lightroom-import-1.lca"), path.join(".lightroom-retire-test-1.tmp"));+ fs::create_dir(&held_stuck).unwrap();+ fs::write(&held_gone, b"old").unwrap();+ let problems = clean_up_after_publish(&temporary, &[(stuck.clone(), held_stuck.clone()), (gone, held_gone.clone())]);+ assert_eq!(problems.len(), 1, "{problems:?}");+ assert!(problems[0].starts_with(&format!("could not retire {} (left as {})", stuck.display(), held_stuck.display())), "{problems:?}");+ assert!(!temporary.exists() && !held_gone.exists());+ let stored = Stored::new(path.join("lightroom-import-2.lca"), problems);+ assert!(stored.cleanup_failed.is_some_and(|note| note.contains("could not retire")));+ fs::remove_dir_all(path).unwrap();+ }++ /// Issue #3114: a library on exFAT (no hard links) aborted every Lightroom import here.+ #[test]+ fn publishes_on_a_volume_without_hard_links() {+ let path = tempdir();+ let _no_links = lightcraft_catalog::safe_file::without_hard_links();+ let first = store(&path, &rich_data()).unwrap().path;+ assert!(fs::read(&first).unwrap().starts_with(HEADER));+ // retention and dedup still work: a new archive gets the next name, the same data reuses it+ let second = store(&path, &data("other.lrcat".into())).unwrap().path;+ assert_ne!(first, second);+ assert_eq!(store(&path, &rich_data()).unwrap().path, first);+ let mut names: Vec<_> = fs::read_dir(&path).unwrap().map(|entry| entry.unwrap().file_name()).collect();+ names.sort();+ assert_eq!(names, ["lightroom-import-0.lca", "lightroom-import-1.lca"], "no temporary left behind");+ fs::remove_dir_all(path).unwrap();+ }++ #[test]+ fn publishing_without_hard_links_never_replaces_an_existing_archive() {+ let path = tempdir();+ let existing = path.join("lightroom-import-0.lca");+ let temporary = path.join(".lightroom-import-new.tmp");+ fs::write(&existing, b"old archive").unwrap();+ fs::write(&temporary, b"new archive").unwrap();+ let _no_links = lightcraft_catalog::safe_file::without_hard_links();+ let error = publish_new(&temporary, &existing).unwrap_err();+ assert_eq!(error.kind(), io::ErrorKind::AlreadyExists);+ assert!(error.to_string().contains("lightroom-import-0.lca"), "{error}");+ assert_eq!(fs::read(&existing).unwrap(), b"old archive");+ assert_eq!(fs::read(&temporary).unwrap(), b"new archive", "the temporary is kept for the caller to remove");+ // a free name takes the fallback: the temporary is renamed (a hard link would leave it in place)+ let fresh = path.join("lightroom-import-1.lca");+ publish_new(&temporary, &fresh).unwrap();+ assert_eq!(fs::read(&fresh).unwrap(), b"new archive");+ assert!(!temporary.exists(), "published by rename, not by a hard link");+ fs::remove_dir_all(path).unwrap();+ } }@@ -15,12 +15,21 @@ use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; type Row = BTreeMap<String, Value>;-const COMMAND: &str = "library.importLightroom"; /// Largest catalog database or WAL read into memory (4 GiB). The whole file is held in RAM, so the cap stays finite. const MAX_FILE: u64 = 1 << 32;+/// Deepest keyword or collection hierarchy followed; a deeper chain is cut there.+const MAX_DEPTH: usize = 64; -fn error(why: impl Into<String>) -> crate::EngineError {- crate::EngineError::BadParams { cmd: COMMAND.into(), msg: why.into() }+/// The catalog at `path` can't be read (it is missing, not allowed, too large or changing): an+/// error naming it, not "invalid parameters".+fn unreadable(path: &Path, why: impl std::fmt::Display) -> crate::EngineError {+ crate::EngineError::Other(format!("the Lightroom catalog {} can't be read: {why}", path.display()))+}++/// The catalog at `path` is read but isn't a whole catalog (its database or a table it needs is+/// broken): an error naming it.+fn damaged(path: &Path, why: impl std::fmt::Display) -> crate::EngineError {+ crate::EngineError::Other(format!("the Lightroom catalog {} is damaged: {why}", path.display())) } fn text<'a>(r: &'a Row, k: &str) -> &'a str {@@ -49,14 +58,18 @@ struct Reader { tables: Vec<LiveTable>, } impl Reader {- fn table(&self, name: &str, required: bool) -> Result<Vec<Row>, String> {- let Some(t) = self.tables.iter().find(|t| t.name == name) else {- return if required { Err(format!("unsupported Lightroom catalog: missing {name}")) } else { Ok(Vec::new()) };- };- let columns = t.column_names.as_ref().ok_or_else(|| format!("cannot read {name} columns"))?;- let rows = self.db.read_table(t.rootpage, columns.len()).map_err(|e| format!("{name}: {e:?}"))?;+ fn has(&self, name: &str) -> bool {+ self.tables.iter().any(|t| t.name == name)+ }++ /// The rows of table `name` (none when it is not in the catalog); an error says what is wrong+ /// with the table, the caller names it.+ fn table(&self, name: &str) -> Result<Vec<Row>, String> {+ let Some(t) = self.tables.iter().find(|t| t.name == name) else { return Ok(Vec::new()) };+ let columns = t.column_names.as_ref().ok_or("cannot read its columns")?;+ let rows = self.db.read_table(t.rootpage, columns.len())?; if rows.len() > 1_000_000 {- return Err(format!("{name}: more than one million records"));+ return Err("more than one million records".into()); } Ok(rows .into_iter()@@ -78,22 +91,19 @@ impl Reader { .collect()) } - fn optional_table(&self, name: &str, warnings: &mut Vec<String>) -> Result<Vec<Row>, String> {- match self.table(name, false) {- Ok(rows) => Ok(rows),- Err(error) => {- warnings.push(format!("skipping optional Lightroom table {name}: {error}"));- Ok(Vec::new())- }- }+ fn optional_table(&self, name: &str, warnings: &mut Vec<String>) -> Vec<Row> {+ self.table(name).unwrap_or_else(|error| {+ warnings.push(format!("skipping optional Lightroom table {name}: {error}"));+ Vec::new()+ }) } fn xmp_table(&self) -> Result<(Vec<Row>, Vec<String>), String> { let Some(t) = self.tables.iter().find(|t| t.name == "Adobe_AdditionalMetadata") else { return Ok((Vec::new(), Vec::new())) };- let columns = t.column_names.as_ref().ok_or_else(|| "cannot read Adobe_AdditionalMetadata columns".to_string())?;- let rows = self.db.read_table(t.rootpage, columns.len()).map_err(|e| format!("Adobe_AdditionalMetadata: {e:?}"))?;+ let columns = t.column_names.as_ref().ok_or("cannot read its columns")?;+ let rows = self.db.read_table(t.rootpage, columns.len())?; if rows.len() > 1_000_000 {- return Err("Adobe_AdditionalMetadata: more than one million records".into());+ return Err("more than one million records".into()); } let mut warnings = Vec::new(); let mut out = Vec::with_capacity(rows.len());@@ -195,7 +205,11 @@ pub struct SkippedImage { pub reason: SkipReason, } -/// Why a catalog image was not imported. Lightroom leaves such records behind and ignores them.+/// Why a catalog image was not imported. Incomplete records, which Lightroom leaves behind and+/// ignores, refer to a record that is gone or empty: every reason up to [`SkipReason::MissingMaster`]+/// (a virtual copy whose master is gone). Contradictory ones, [`SkipReason::MasterIsCopy`] and+/// [`SkipReason::DuplicateId`], conflict with another record of a damaged catalog; the desktop+/// result window counts them on their own line. Either costs that image, not the import. #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum SkipReason {@@ -209,6 +223,12 @@ pub enum SkipReason { IncompletePath, /// A virtual copy whose master was skipped. MasterSkipped,+ /// A virtual copy whose master is not in the catalog.+ MissingMaster,+ /// A virtual copy whose master is itself a virtual copy (or the copy itself).+ MasterIsCopy,+ /// An image whose id an earlier image of the catalog has: the first one is imported.+ DuplicateId, } #[derive(Clone, Default, Serialize, Deserialize)]@@ -217,24 +237,27 @@ pub(crate) struct ImportIndex { pub(crate) collections: BTreeMap<String, u64>, } +/// The library's record of earlier imports (`Interop/lightroom-index.json`; none yet: empty). An+/// error names the file: it is the library's, not the catalog's. pub(crate) fn load_index(path: Option<&Path>) -> crate::Result<ImportIndex> { match path { Some(path) => {+ let failed = |why: String| crate::EngineError::Other(format!("can't read the Lightroom import index {}: {why}", path.display())); let file = match std::fs::File::open(path) { Ok(file) => file, Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(ImportIndex::default()),- Err(e) => return Err(error(e.to_string())),+ Err(e) => return Err(failed(e.to_string())), };- if file.metadata().map_err(|e| error(e.to_string()))?.len() > 16 * 1024 * 1024 {- return Err(error("Lightroom import index exceeds 16 MiB"));+ if file.metadata().map_err(|e| failed(e.to_string()))?.len() > 16 * 1024 * 1024 {+ return Err(failed("it exceeds 16 MiB".into())); } let mut bytes = Vec::new(); use std::io::Read;- file.take(16 * 1024 * 1024 + 1).read_to_end(&mut bytes).map_err(|e| error(e.to_string()))?;+ file.take(16 * 1024 * 1024 + 1).read_to_end(&mut bytes).map_err(|e| failed(e.to_string()))?; if bytes.len() > 16 * 1024 * 1024 {- return Err(error("Lightroom import index exceeds 16 MiB"));+ return Err(failed("it exceeds 16 MiB".into())); }- serde_json::from_slice(&bytes).map_err(|e| error(format!("invalid Lightroom import index: {e}")))+ serde_json::from_slice(&bytes).map_err(|e| failed(format!("it is damaged: {e}"))) } None => Ok(ImportIndex::default()), }@@ -249,45 +272,138 @@ fn identity(data: &CatalogImport, photo: &CatalogPhoto) -> String { if photo.uuid.is_empty() { format!("{}:{}", key(&data.source), photo.source_id) } else { photo.uuid.clone() } } -fn validate(data: &CatalogImport) -> Result<(), String> {- let photos: HashMap<_, _> = data.photos.iter().map(|p| (p.source_id, p)).collect();- if photos.len() != data.photos.len() {- return Err("duplicate source photo ids".into());+/// Leave out the catalog's contradictory records, so one bad record costs that record and not the+/// import (#1588). An image with an id an earlier image has, and a virtual copy whose master is not+/// in the catalog or is itself a virtual copy, are skipped and listed in `skipped`. A collection+/// hierarchy that loops or is deeper than [`MAX_DEPTH`] is cut, the collection where it is cut+/// going to the top level with a warning. A second collection with an id an earlier one has is+/// left out with a warning.+///+/// Called where the catalog is read and again where its photos become records, so neither route+/// can keep one; the second pass finds nothing.+pub(crate) fn skip_inconsistent_records(data: &mut CatalogImport) {+ let mut skipped = Vec::new();+ let mut seen = HashSet::new();+ data.photos.retain(|p| {+ if seen.insert(p.source_id) {+ return true;+ }+ skipped.push(SkippedImage { image: p.source_id, reason: SkipReason::DuplicateId });+ false+ });+ // whether each image is a virtual copy (`masterImage` is 0 for a master)+ let is_copy: HashMap<i64, bool> = data.photos.iter().map(|p| (p.source_id, number(&p.image, "masterImage") != 0)).collect();+ data.photos.retain(|p| {+ let master = number(&p.image, "masterImage");+ let reason = match is_copy.get(&master) {+ _ if master == 0 => return true,+ Some(false) => return true,+ Some(true) => SkipReason::MasterIsCopy,+ None => SkipReason::MissingMaster,+ };+ skipped.push(SkippedImage { image: p.source_id, reason });+ false+ });+ data.skipped.extend(skipped);+ skip_duplicate_collections(data);+ let cut = collection_cuts(&data.collections);+ if cut.is_empty() {+ return; }- for photo in &data.photos {- let master = number(&photo.image, "masterImage");- if master != 0- && (master == photo.source_id- || !photos.contains_key(&master)- || photos.get(&master).is_some_and(|p| number(&p.image, "masterImage") != 0))- {- return Err(format!("invalid virtual copy master for {}", photo.source_id));+ for collection in &mut data.collections {+ if let Some(why) = cut.get(&number(collection, "id_local")) {+ collection.insert("parent".into(), json!(0));+ data.warnings.push(format!("collection “{}”: {why}; imported at the top level", text(collection, "name"))); } }- let collections: HashMap<_, _> = data.collections.iter().map(|r| (number(r, "id_local"), r)).collect();- for id in collections.keys() {- let mut seen = HashSet::new();- let mut at = *id;- while let Some(r) = collections.get(&at) {- if seen.len() >= 64 || !seen.insert(at) {- return Err("cyclic or overly deep collection hierarchy".into());- }- at = number(r, "parent");- if at == 0 {+}++/// Keep the first collection of each id and leave out the rest with a warning naming both: two+/// albums can't have one id. The photos and collections the catalog lists under that id stay+/// there, so they land in the collection kept.+fn skip_duplicate_collections(data: &mut CatalogImport) {+ let mut kept: HashMap<i64, String> = HashMap::new();+ let mut warnings = Vec::new();+ data.collections.retain(|c| {+ let id = number(c, "id_local");+ let Some(first) = kept.get(&id) else {+ kept.insert(id, text(c, "name").to_string());+ return true;+ };+ let photos = if data.members.iter().any(|m| number(m, "collection") == id) {+ format!("; the photos listed under that id go to “{first}”")+ } else {+ String::new()+ };+ warnings.push(format!("collection “{}”: not imported, it has the id of collection “{first}”{photos}", text(c, "name")));+ false+ });+ data.warnings.extend(warnings);+}++/// The collections whose parent link must go for the hierarchy to be a tree no deeper than+/// [`MAX_DEPTH`], and why. A loop is cut at its oldest collection (the lowest id: Lightroom numbers+/// records as they are made, so it is most likely the loop's real top); a chain too deep is cut+/// every [`MAX_DEPTH`] levels from its top, the collection there going to the top level. Each+/// chain is followed once.+fn collection_cuts(collections: &[Row]) -> HashMap<i64, &'static str> {+ let mut parents: HashMap<i64, i64> = collections.iter().map(|r| (number(r, "id_local"), number(r, "parent"))).collect();+ let ids: Vec<i64> = collections.iter().map(|r| number(r, "id_local")).filter(|id| *id != 0).collect();+ let mut cut = HashMap::new();+ // loops: a collection met again on the chain being followed closes one+ let mut done = HashSet::new();+ for &start in &ids {+ let mut chain = Vec::new();+ let mut on_chain = HashMap::new();+ let mut at = start;+ while at != 0 && !done.contains(&at) {+ let Some(&parent) = parents.get(&at) else { break };+ if let Some(&i) = on_chain.get(&at) {+ if let Some(&top) = chain.get(i..).and_then(|looped: &[i64]| looped.iter().min()) {+ parents.insert(top, 0);+ cut.insert(top, "its parent collections form a loop");+ } break; }+ on_chain.insert(at, chain.len());+ chain.push(at);+ at = parent; }+ done.extend(chain); }- Ok(())-}--pub(crate) fn validate_for_job(data: &CatalogImport) -> Result<(), String> {- validate(data)+ // depth from the top, now that no chain loops (bounded all the same)+ let mut depth: HashMap<i64, usize> = HashMap::new();+ for &start in &ids {+ let mut chain = Vec::new();+ let mut at = start;+ let top = loop {+ if let Some(&known) = depth.get(&at) {+ break known + 1;+ }+ match parents.get(&at) {+ Some(&parent) if at != 0 && chain.len() <= ids.len() => {+ chain.push(at);+ at = parent;+ }+ // `chain`'s last collection is at the top level+ _ => break 0,+ }+ };+ for (i, id) in chain.into_iter().rev().enumerate() {+ depth.insert(id, top + i);+ }+ }+ for (id, d) in depth {+ if d > 0 && d.is_multiple_of(MAX_DEPTH) {+ cut.insert(id, "it is nested more than 64 collections deep");+ }+ }+ cut } fn bytes(path: &Path) -> Result<Vec<u8>, String> { use std::io::Read;- let f = std::fs::File::open(path).map_err(|e| format!("{}: {e}", path.display()))?;+ let f = std::fs::File::open(path).map_err(|e| e.to_string())?; let len = f.metadata().map_err(|e| e.to_string())?.len(); if len > MAX_FILE { return Err("catalog or WAL exceeds 4 GiB import limit".into());@@ -429,7 +545,7 @@ fn original_path(root: &str, folder: &str, name: &str, catalog: &Path) -> (Strin } /// Inspect `.lrcat` plus committed WAL pages without modifying either source file.-pub fn read(path: &Path) -> Result<CatalogImport, String> {+pub fn read(path: &Path) -> crate::Result<CatalogImport> { let cancel = AtomicBool::new(false); let total = AtomicUsize::new(0); let done = AtomicUsize::new(0);@@ -437,47 +553,58 @@ pub fn read(path: &Path) -> Result<CatalogImport, String> { } /// Read a Lightroom Classic catalog for import, counting the tables read in `total` / `done` and-/// stopping when `cancel` is set. Images whose catalog records are incomplete are left out and-/// listed in [`CatalogImport::skipped`] instead of failing the read.-pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize, done: &AtomicUsize) -> Result<CatalogImport, String> {+/// stopping when `cancel` is set ([`crate::EngineError::Cancelled`]). A bad record costs that+/// record, not the read: images whose records are incomplete or contradictory are left out and+/// listed in [`CatalogImport::skipped`], a looping keyword or collection hierarchy is cut with a+/// warning. Only a catalog that can't be read, or whose database or required tables are broken,+/// is an error, naming the file.+pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize, done: &AtomicUsize) -> crate::Result<CatalogImport> { // 13 catalog tables plus one validation stage; ImportJob adds one unit per original path. total.store(14, Ordering::Relaxed); done.store(0, Ordering::Relaxed); ensure_read_active(cancel)?;- let before = std::fs::metadata(path).map_err(|e| e.to_string())?;+ let before = std::fs::metadata(path).map_err(|e| unreadable(path, e))?; let wal_path = PathBuf::from(format!("{}-wal", path.display())); let wal_before = std::fs::metadata(&wal_path).ok().map(|m| (m.len(), m.modified().ok()));- let main = FileSource::open(path)?;+ let main = FileSource::open(path).map_err(|e| unreadable(path, e))?;+ let wal_error = |e: String| format!("its write-ahead log {}: {e}", wal_path.display()); let wal = match std::fs::metadata(&wal_path) {- Ok(_) => bytes(&wal_path)?,+ Ok(_) => bytes(&wal_path).map_err(|e| unreadable(path, wal_error(e)))?, Err(e) if e.kind() == std::io::ErrorKind::NotFound => Vec::new(),- Err(e) => return Err(e.to_string()),+ Err(e) => return Err(unreadable(path, wal_error(e.to_string()))), };- let wal_len = committed_wal_len(main.header(), &wal)?;+ let wal_len = committed_wal_len(main.header(), &wal).map_err(|e| damaged(path, wal_error(e)))?; ensure_read_active(cancel)?;- let db = Database::open_file_with_wal(main, &wal[..wal_len]).map_err(|e| format!("invalid SQLite catalog: {e:?}"))?;- let after = std::fs::metadata(path).map_err(|e| e.to_string())?;+ let db = Database::open_file_with_wal(main, &wal[..wal_len]).map_err(|e| damaged(path, format!("not a readable SQLite database: {e}")))?;+ let after = std::fs::metadata(path).map_err(|e| unreadable(path, e))?; let wal_after = std::fs::metadata(&wal_path).ok().map(|m| (m.len(), m.modified().ok())); if before.len() != after.len() || before.modified().ok() != after.modified().ok() || wal_before != wal_after {- return Err("catalog changed during reading; close Lightroom and retry".into());+ return Err(unreadable(path, "it changed during reading; close Lightroom and retry")); }- let reader = Reader { tables: db.live_tables()?, db };+ let reader = Reader { tables: db.live_tables().map_err(|e| damaged(path, e))?, db }; let mut warnings = Vec::new();- let table = |name: &str, required: bool| -> Result<Vec<Row>, String> {+ // a table every catalog has: without it, or with it broken, there is nothing to import+ let table = |name: &str| -> crate::Result<Vec<Row>> { ensure_read_active(cancel)?;- let rows = reader.table(name, required)?;+ if !reader.has(name) {+ return Err(crate::EngineError::Other(format!(+ "{} is not a Lightroom Classic catalog (or is damaged): it has no {name} table",+ path.display()+ )));+ }+ let rows = reader.table(name).map_err(|e| damaged(path, format!("table {name} can't be read: {e}")))?; done.fetch_add(1, Ordering::Relaxed); Ok(rows) };- let optional_table = |name: &str, warnings: &mut Vec<String>| -> Result<Vec<Row>, String> {+ let optional_table = |name: &str, warnings: &mut Vec<String>| -> crate::Result<Vec<Row>> { ensure_read_active(cancel)?;- let rows = reader.optional_table(name, warnings)?;+ let rows = reader.optional_table(name, warnings); done.fetch_add(1, Ordering::Relaxed); Ok(rows) };- let roots: HashMap<_, _> = table("AgLibraryRootFolder", true)?.into_iter().map(|r| (number(&r, "id_local"), r)).collect();- let folders: HashMap<_, _> = table("AgLibraryFolder", true)?.into_iter().map(|r| (number(&r, "id_local"), r)).collect();- let files: HashMap<_, _> = table("AgLibraryFile", true)?.into_iter().map(|r| (number(&r, "id_local"), r)).collect();+ let roots: HashMap<_, _> = table("AgLibraryRootFolder")?.into_iter().map(|r| (number(&r, "id_local"), r)).collect();+ let folders: HashMap<_, _> = table("AgLibraryFolder")?.into_iter().map(|r| (number(&r, "id_local"), r)).collect();+ let files: HashMap<_, _> = table("AgLibraryFile")?.into_iter().map(|r| (number(&r, "id_local"), r)).collect(); let develops: HashMap<_, _> = optional_table("Adobe_imageDevelopSettings", &mut warnings)? .into_iter() .map(|r| (number(&r, "image"), text(&r, "text").to_string()))@@ -495,23 +622,19 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize, let metadata: HashMap<_, _> = metadata_rows.into_iter().map(|r| (number(&r, "image"), r)).collect(); let keyword_rows: HashMap<_, _> = optional_table("AgLibraryKeyword", &mut warnings)?.into_iter().map(|r| (number(&r, "id_local"), r)).collect(); let mut keywords: HashMap<i64, Vec<String>> = HashMap::new();+ // keywords whose looping hierarchy was reported, so the warning comes once per keyword+ let mut looped = HashSet::new(); for r in optional_table("AgLibraryKeywordImage", &mut warnings)? { ensure_read_active(cancel)?;- let mut parts = Vec::new();- let mut at = number(&r, "tag");- let mut seen = HashSet::new();- while at != 0 {- if parts.len() >= 64 || !seen.insert(at) {- return Err("cyclic or overly deep keyword hierarchy".into());- }- let Some(k) = keyword_rows.get(&at) else { break };- let name = text(k, "name");- if !name.is_empty() {- parts.push(name.to_string());- }- at = number(k, "parent");+ let tag = number(&r, "tag");+ let (parts, cut) = keyword_path(tag, &keyword_rows);+ if cut && looped.insert(tag) {+ let name = keyword_rows.get(&tag).map_or("", |k| text(k, "name"));+ warnings.push(format!(+ "keyword “{name}”: its parent keywords form a loop or are nested more than 64 deep; imported as “{}”",+ parts.join("|")+ )); }- parts.reverse(); if !parts.is_empty() { keywords.entry(number(&r, "image")).or_default().push(parts.join("|")); }@@ -559,9 +682,16 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize, // these behind and ignores them); they and their virtual copies are skipped and reported // instead of failing the import let mut orphans = HashSet::new();- for image in table("Adobe_images", true)? {+ // a second image with an id already read is skipped before anything is keyed by its id, so it+ // can't take the first one's records or its virtual copies with it (#1588)+ let mut ids = HashSet::new();+ for image in table("Adobe_images")? { ensure_read_active(cancel)?; let id = number(&image, "id_local");+ if !ids.insert(id) {+ out.skipped.push(SkippedImage { image: id, reason: SkipReason::DuplicateId });+ continue;+ } let record = files.get(&number(&image, "rootFile")).ok_or(SkipReason::MissingFile).and_then(|file| { let folder = folders.get(&number(file, "folder")).ok_or(SkipReason::MissingFolder)?; let root = roots.get(&number(folder, "rootFolder")).ok_or(SkipReason::MissingRoot)?;@@ -611,16 +741,41 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize, }); } drop_copies_of(&mut out, &orphans);+ skip_inconsistent_records(&mut out); refuse_non_media(&mut out); ensure_read_active(cancel)?;- reader.db.ensure_unchanged()?;+ reader.db.ensure_unchanged().map_err(|e| unreadable(path, e))?; let wal_after = std::fs::metadata(&wal_path).ok().map(|m| (m.len(), m.modified().ok())); if wal_before != wal_after {- return Err("catalog changed during reading; close Lightroom and retry".into());+ return Err(unreadable(path, "it changed during reading; close Lightroom and retry")); } Ok(out) } +/// A keyword's path from the top of its hierarchy (`Travel|Goa`), and whether its parent chain had+/// to be cut: when it loops, the path stops before the first keyword that comes again; when it is+/// deeper than [`MAX_DEPTH`], at that depth.+fn keyword_path(tag: i64, keywords: &HashMap<i64, Row>) -> (Vec<String>, bool) {+ let mut parts = Vec::new();+ let mut seen = HashSet::new();+ let mut at = tag;+ let mut cut = false;+ while at != 0 {+ if seen.len() >= MAX_DEPTH || !seen.insert(at) {+ cut = true;+ break;+ }+ let Some(k) = keywords.get(&at) else { break };+ let name = text(k, "name");+ if !name.is_empty() {+ parts.push(name.to_string());+ }+ at = number(k, "parent");+ }+ parts.reverse();+ (parts, cut)+}+ /// Drop the virtual copies whose master was skipped, adding them to the skipped images: a copy has /// no original of its own. fn drop_copies_of(data: &mut CatalogImport, masters: &HashSet<i64>) {@@ -683,8 +838,8 @@ pub(crate) fn refuse_non_media(data: &mut CatalogImport) { data.warnings.extend(warnings); } -fn ensure_read_active(cancel: &AtomicBool) -> Result<(), String> {- if cancel.load(Ordering::Relaxed) { Err("Lightroom import cancelled".into()) } else { Ok(()) }+fn ensure_read_active(cancel: &AtomicBool) -> crate::Result<()> {+ if cancel.load(Ordering::Relaxed) { Err(crate::EngineError::Cancelled("Lightroom import cancelled".into())) } else { Ok(()) } } fn mapped_settings(@@ -824,7 +979,10 @@ fn apply(s: &mut crate::Session, data: CatalogImport, update_existing: bool) -> let archive_dir = s.library.as_ref().filter(|l| l.on_disk).map(|l| l.dir.join("Interop")); let index_path = archive_dir.as_ref().map(|p| p.join("lightroom-index.json")); let mut index = load_index(index_path.as_deref())?;- let archive_path = archive_dir.as_ref().map(|dir| crate::lightroom_archive::store(dir, &data).map_err(error)).transpose()?;+ let archive_path = archive_dir+ .as_ref()+ .map(|dir| crate::lightroom_archive::store(dir, &data).map(|stored| stored.path).map_err(crate::EngineError::Other))+ .transpose()?; let undo0 = s.undo.len(); let existing: HashMap<_, _> = s.catalog.photos().filter(|p| p.copy_of.is_none()).filter_map(|p| source_path(p).map(|path| (key(path), p.id))).collect();@@ -841,8 +999,8 @@ fn apply(s: &mut crate::Session, data: CatalogImport, update_existing: bool) -> apply_prepared(s, data, ApplyPrepared { update_existing, files, index: &mut index, archive_path, missing_sources: &missing_sources, undo0 })?; if let Some(path) = index_path { s.persist()?;- let bytes = serde_json::to_vec(&applied.index).map_err(|e| error(e.to_string()))?;- lightcraft_catalog::safe_file::write_atomic(&path, &bytes).map_err(|e| error(e.to_string()))?;+ let bytes = serde_json::to_vec(&applied.index).map_err(|e| crate::EngineError::Other(e.to_string()))?;+ lightcraft_catalog::safe_file::write_atomic(&path, &bytes).map_err(|e| crate::EngineError::Other(e.to_string()))?; } Ok(applied.report) }@@ -862,9 +1020,11 @@ pub(crate) struct ApplyPrepared<'a> { /// (added, missing, failed, warnings and skipped images). 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)+ // nothing but a file type LightCraft reads becomes a record, whichever route got here (#2135),+ // nor a contradictory one (#1588); duplicate ids go first, so refusing a duplicate's original+ // can't take the first image with it+ skip_inconsistent_records(&mut data); 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(); let mut by_path: HashMap<_, _> =@@ -885,6 +1045,8 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co 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();+ // virtual copies whose master didn't become a record (`skip_inconsistent_records` leaves none)+ let mut masterless = Vec::new(); for src in photos { let master = number(&src.image, "masterImage"); if master == 0 && missing_sources.contains(&key(&src.path)) {@@ -905,12 +1067,13 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co let mut p = if let Some(id) = existing_id.and_then(|id| s.catalog.photo(id)) { (**id).clone() } else if master != 0 {- let master_id = ids.get(&master).copied().ok_or_else(|| error(format!("virtual copy {}: missing master", src.source_id)))?;- let mut p = planned- .get(&master_id)- .cloned()- .or_else(|| s.catalog.photo(master_id).map(|p| (**p).clone()))- .ok_or_else(|| error("virtual copy master is unavailable"))?;+ let master_record =+ ids.get(&master).copied().and_then(|id| planned.get(&id).cloned().or_else(|| s.catalog.photo(id).map(|p| (**p).clone())));+ let Some(mut p) = master_record else {+ masterless.push(SkippedImage { image: src.source_id, reason: SkipReason::MissingMaster });+ continue;+ };+ let master_id = p.id; p.id = s.catalog.alloc_photo_id(); p.copy_of = Some(master_id); p.copy_name = Some(text(&src.image, "copyName").to_string());@@ -964,8 +1127,12 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co _ => Flag::None, }; p.label = s.catalog.label_from_name(text(&src.image, "colorLabels"));- if !text(&src.image, "captureTime").is_empty() {- p.captured = Some(text(&src.image, "captureTime").into());+ // a capture time the catalog can't hold would fail the whole batch at `SetCaptured`+ let captured = text(&src.image, "captureTime");+ if lightcraft_catalog::stacks::iso_seconds(captured).is_some() {+ p.captured = Some(captured.into());+ } else if !captured.is_empty() {+ warnings.push(format!("image {}: capture time “{captured}” is not a date; imported without it", src.source_id)); } if !src.keywords.is_empty() { p.meta.keywords = src.keywords.clone();@@ -1013,8 +1180,11 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co Op::SetFlag { id: p.id, flag: p.flag }, Op::SetLabel { id: p.id, label: p.label }, Op::SetMeta { id: p.id, meta: Box::new(p.meta.clone()) },- Op::SetCaptured { id: p.id, captured: p.captured.clone() }, ]);+ // a record an earlier import gave a time that isn't a date keeps it rather than fail the batch+ if p.captured.as_deref().is_none_or(|c| lightcraft_catalog::stacks::iso_seconds(c).is_some()) {+ ops.push(Op::SetCaptured { id: p.id, captured: p.captured.clone() });+ } ops.push(Op::SetDevelop { id: p.id, settings: p.develop.clone(), label: "Lightroom catalog".into(), edited: p.edited.clone() }); } else { imported += 1;@@ -1022,6 +1192,7 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co } by_path.entry(key(&src.path)).or_insert(photo_id); }+ data.skipped.extend(masterless); let mut albums = HashMap::new(); for collection in &data.collections { if number(collection, "systemOnly") != 0 {@@ -158,25 +158,204 @@ fn imports_copies_collections_ratings_and_mapped_edits_in_one_undo_step() { assert!(data.photos[1].settings.contains("CameraProfile")); // unmapped source stays intact } #[test]-fn preserves_existing_edits_and_rejects_invalid_structure_before_mutation() {+fn preserves_existing_edits_and_rejects_unbounded_settings() { let mut s = crate::Session::new(); let data = sample(); apply(&mut s, data.clone(), false).unwrap(); let id = s.catalog.photos().find(|p| p.copy_of.is_none()).unwrap().id; s.set_develop(id, lightcraft_develop::DevelopSettings::default(), "Personal edit").unwrap();- let r = apply(&mut s, data.clone(), false).unwrap();+ let r = apply(&mut s, data, false).unwrap(); assert!(r["preservedExistingEdits"].as_u64().unwrap() > 0); assert_eq!(s.catalog.photo(id).unwrap().develop.light.exposure, 0.0);- let before = s.catalog.to_snapshot();- let mut cyclic = data;- cyclic.collections[0].insert("parent".into(), json!(10));- assert!(apply(&mut s, cyclic, false).is_err());- assert_eq!(s.catalog.to_snapshot(), before); assert!( mapped_settings(&format!("{}0{}", "{".repeat(1000), "}".repeat(1000)), crate::crs::Target::RawAbsolute, 1.0, Orientation::Normal, 1.5) .is_err() ); }+/// A damaged catalog used to fail the whole import (#1588): a repeated image id, or a virtual copy+/// whose master is gone or is itself a copy, now costs that image, and the rest is imported.+#[test]+fn contradictory_images_are_skipped_and_the_rest_imports() {+ let mut data = sample();+ let mut twin = data.photos[1].clone();+ twin.uuid = "twin".into();+ twin.path = "/missing/twin.ARW".into();+ data.photos.push(twin);+ for (id, master) in [(5, 99), (6, 2), (7, 7)] {+ let mut copy = data.photos[0].clone();+ copy.source_id = id;+ copy.uuid = format!("copy-{id}");+ copy.image = row(json!({"id_local": id, "masterImage": master, "copyName": "Bad"}));+ data.photos.push(copy);+ }+ let mut s = crate::Session::new();+ let r = apply(&mut s, data, false).unwrap();+ assert_eq!(r["photos"], 2, "the master and its copy: {r}");+ assert_eq!(+ r["skipped"],+ json!([+ {"image": 1, "reason": "duplicateId"},+ {"image": 5, "reason": "missingMaster"},+ {"image": 6, "reason": "masterIsCopy"},+ {"image": 7, "reason": "masterIsCopy"},+ ])+ );+ assert_eq!(s.catalog.len(), 2);+ assert!(s.catalog.photos().all(|p| source_path(p) == Some("/missing/test.ARW")), "the first image with id 1 is the one imported");+ assert!(s.catalog.albums().any(|a| a.name == "Keepers"));+ assert_eq!(s.undo.len(), 1);+}++/// A second image with an id already used must not take the first one, or its virtual copy, with+/// it: refusing its original, or finding its file record gone, used to drop every image with that id.+#[test]+fn a_duplicate_image_id_costs_only_the_duplicate() {+ let mut data = sample();+ let mut planted = data.photos[1].clone();+ planted.uuid = "planted".into();+ planted.path = "/Users/sky/Downloads/payload.exe".into();+ data.photos.push(planted);+ let mut s = crate::Session::new();+ let r = apply(&mut s, data, false).unwrap();+ assert_eq!(r["photos"], 2, "the first image with id 1 and its copy: {r}");+ assert_eq!(r["skipped"], json!([{"image": 1, "reason": "duplicateId"}]));+ assert!(!r["warnings"].to_string().contains("payload.exe"), "the duplicate is skipped before its original is looked at: {r}");+ assert!(s.catalog.photos().all(|p| source_path(p) == Some("/missing/test.ARW")));+ assert_eq!(s.catalog.photos().filter(|p| p.copy_of.is_some()).count(), 1);++ use SqlValue::{Integer as I, Null as N, Text as T};+ for (duplicate_file, case) in [(2, "a refused file type"), (9, "no file record")] {+ let mut tables = one_row(photo_tables(("/catalog/", "")));+ tables[2].2.push(vec![N, I(1), T("payload.exe".into())]);+ tables[3] = (+ "Adobe_images",+ "id_local INTEGER PRIMARY KEY, id_global, rootFile, masterImage, fileFormat, fileWidth, fileHeight, rating, pick",+ vec![+ vec![N, T("master".into()), I(1), N, T("ARW".into()), I(20), I(10), I(5), I(1)],+ vec![N, T("copy".into()), I(1), I(1), T("ARW".into()), I(20), I(10), I(3), I(0)],+ vec![I(1), T("duplicate".into()), I(duplicate_file), N, T("ARW".into()), I(20), I(10), I(0), I(0)],+ ],+ );+ let catalog = read_tables(&tables, &format!("duplicate-{duplicate_file}"));+ let read: Vec<_> = catalog.photos.iter().map(|p| (p.source_id, p.uuid.as_str())).collect();+ assert_eq!(read, [(1, "master"), (2, "copy")], "{case}");+ assert_eq!(catalog.skipped, [SkippedImage { image: 1, reason: SkipReason::DuplicateId }], "{case}");+ assert!(catalog.warnings.is_empty(), "{case}: {:?}", catalog.warnings);+ let mut s = crate::Session::new();+ let r = apply(&mut s, catalog, false).unwrap();+ assert_eq!(r["photos"], 2, "{case}: {r}");+ assert_eq!(s.catalog.photos().filter(|p| p.copy_of.is_some()).count(), 1, "{case}");+ }+}++/// A capture time that isn't a date used to fail the whole metadata batch at `SetCaptured` on the+/// first import that met the photo as a record (a reimport); it is left out with a warning.+#[test]+fn a_capture_time_that_is_not_a_date_is_left_out() {+ let mut data = sample();+ let mut other = data.photos[1].clone();+ other.source_id = 3;+ other.uuid = "other".into();+ other.path = "/missing/other.ARW".into();+ other.image = row(json!({"id_local": 3, "fileFormat": "ARW", "captureTime": "broken"}));+ data.photos.push(other);+ let mut s = crate::Session::new();+ let mut index = ImportIndex::default();+ let missing_sources = HashSet::new();+ let mut import = |s: &mut crate::Session| {+ let undo0 = s.undo.len();+ let context = ApplyPrepared {+ update_existing: true,+ files: Default::default(),+ index: &mut index,+ archive_path: None,+ missing_sources: &missing_sources,+ undo0,+ };+ apply_prepared(s, data.clone(), context).unwrap().report+ };+ let first = import(&mut s);+ assert_eq!(first["photos"], 3, "{first}");+ let warning = "image 3: capture time “broken” is not a date; imported without it";+ assert!(first["warnings"].as_array().unwrap().iter().any(|w| w == warning), "{first}");+ let captured = |s: &crate::Session, image: &str| s.catalog.photo(PhotoId(first["mapping"][image].as_u64().unwrap())).unwrap().captured.clone();+ assert_eq!(captured(&s, "3"), None);+ assert_eq!(captured(&s, "1").as_deref(), Some("2024-01-16T12:34:56"), "the rest of the batch stands");+ assert_eq!(s.catalog.albums().count(), 2);+ let second = import(&mut s);+ assert_eq!(second["mapping"], first["mapping"], "the reimport updates the same records: {second}");+ assert_eq!(second["imported"], 0);++ // a record an earlier version stored with that time keeps it, and the import goes on+ let mut s = crate::Session::new();+ let id = s.catalog.alloc_photo_id();+ let mut old = Photo::new(id, Source::File { path: "/missing/other.ARW".into() }, "other.ARW", "ARW", 20, 10, "before");+ old.captured = Some("broken".into());+ s.commit("Earlier import", Op::AddPhoto { photo: Box::new(old) }).unwrap();+ let r = apply(&mut s, data, false).unwrap();+ assert_eq!(r["mapping"]["3"], id.0, "{r}");+ assert_eq!(s.catalog.photo(id).unwrap().captured.as_deref(), Some("broken"));+ assert_eq!(s.catalog.photo(PhotoId(r["mapping"]["1"].as_u64().unwrap())).unwrap().captured.as_deref(), Some("2024-01-16T12:34:56"));+}++/// Two collections with one id used to stage two albums with one id, failing the import: the first+/// is kept, and the photos listed under the id go to it.+#[test]+fn a_duplicate_collection_id_costs_only_the_duplicate() {+ let mut data = sample();+ data.collections.push(row(json!({"id_local": 10, "parent": 0, "name": "Rejects", "creationId": "collection"})));+ let mut s = crate::Session::new();+ let r = apply(&mut s, data, false).unwrap();+ assert_eq!(r["photos"], 2);+ assert_eq!(r["collections"], 2, "{r}");+ let names: Vec<_> = s.catalog.albums().map(|a| a.name.clone()).collect();+ assert!(names.contains(&"Keepers".to_string()) && !names.contains(&"Rejects".to_string()), "{names:?}");+ let keepers = s.catalog.albums().find(|a| a.name == "Keepers").unwrap();+ assert_eq!(keepers.photos.len(), 2);+ assert!(keepers.parent.is_some(), "the first row's place is kept");+ assert_eq!(+ r["warnings"],+ json!(["collection “Rejects”: not imported, it has the id of collection “Keepers”; the photos listed under that id go to “Keepers”"])+ );+ assert_eq!(s.undo.len(), 1);+}++/// A looping (or too deep) collection hierarchy used to fail the whole import: it is cut at the+/// loop's oldest collection, which goes to the top level with a warning.+#[test]+fn a_looping_collection_hierarchy_is_cut_not_fatal() {+ let mut data = sample();+ data.collections[0].insert("parent".into(), json!(10));+ data.collections.push(row(json!({"id_local": 11, "parent": 11, "name": "Self", "creationId": "collection"})));+ // a chain of 70 sets, each in the one before+ data.collections.extend((100..170).map(|id| {+ row(json!({"id_local": id, "parent": if id == 100 { 0 } else { id - 1 }, "name": format!("Set {id}"), "creationId": "collection.group"}))+ }));+ let mut s = crate::Session::new();+ let r = apply(&mut s, data, false).unwrap();+ assert_eq!(r["photos"], 2);+ assert_eq!(r["collections"], 73);+ let album = |name: &str| s.catalog.albums().find(|a| a.name == name).unwrap();+ assert_eq!(album("Travel").parent, None, "the loop's oldest collection is at the top level");+ assert_eq!(album("Keepers").parent, Some(album("Travel").id));+ assert_eq!(album("Keepers").photos.len(), 2);+ assert_eq!(album("Self").parent, None);+ assert_eq!(album("Set 164").parent, None, "cut once, 64 below the top");+ assert_eq!(album("Set 165").parent, Some(album("Set 164").id));+ assert_eq!(album("Set 163").parent, Some(album("Set 162").id));+ assert_eq!(album("Set 101").parent, Some(album("Set 100").id));+ let warnings: Vec<_> = r["warnings"].as_array().unwrap().iter().filter_map(Value::as_str).collect();+ assert_eq!(+ warnings,+ [+ "collection “Travel”: its parent collections form a loop; imported at the top level",+ "collection “Self”: its parent collections form a loop; imported at the top level",+ "collection “Set 164”: it is nested more than 64 collections deep; imported at the top level",+ ]+ );+ assert_eq!(s.undo.len(), 1);+}+ #[test] fn compressed_xmp_is_bounded_and_settings_are_data_only() { let packet = b"<x:xmpmeta>example</x:xmpmeta>";@@ -442,6 +621,121 @@ fn persistent_reimport_keeps_copy_and_collection_ids_and_capture_time() { std::fs::remove_dir_all(dir).unwrap(); } +/// Issue #3114: a library on exFAT (no hard links) aborted every import when saving the archive.+#[test]+fn imports_into_a_library_on_a_volume_without_hard_links() {+ let dir = std::env::temp_dir().join(format!("lightcraft-lrcat-no-hard-links-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ std::fs::create_dir_all(&dir).unwrap();+ let source = dir.join("source.lrcat");+ std::fs::write(&source, native_catalog(&photo_tables((dir.join("offline").to_str().unwrap(), "")))).unwrap();+ let library = dir.join("library");+ let mut s = crate::Session::new();+ s.open_library(&library, false).unwrap();+ let _no_links = lightcraft_catalog::safe_file::without_hard_links();+ let report = import(&mut s, &source, false).unwrap();+ assert_eq!(report["imported"], 1);+ assert_eq!(report["warnings"], json!([]));+ let archive = Path::new(report["archive"].as_str().unwrap());+ assert_eq!(archive, library.join("Interop").join("lightroom-import-0.lca"));+ assert!(std::fs::read(archive).unwrap().starts_with(b"LC-LRARCH"));+ assert!(library.join("Interop").join("lightroom-index.json").is_file());+ let mut names: Vec<_> = std::fs::read_dir(library.join("Interop")).unwrap().map(|entry| entry.unwrap().file_name()).collect();+ names.sort();+ assert_eq!(names, ["lightroom-import-0.lca", "lightroom-index.json"], "the writability probe leaves nothing behind");+ s.close_library().unwrap();+ std::fs::remove_dir_all(dir).unwrap();+}++/// The recovery archive is best-effort: when it can't be saved (here `Interop` holds more entries+/// than the archive scans) the import still goes on, with a warning naming the folder.+#[test]+fn an_archive_that_cannot_be_saved_warns_and_the_import_proceeds() {+ let dir = std::env::temp_dir().join(format!("lightcraft-lrcat-archive-refused-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ std::fs::create_dir_all(&dir).unwrap();+ let source = dir.join("source.lrcat");+ std::fs::write(&source, native_catalog(&photo_tables((dir.join("offline").to_str().unwrap(), "")))).unwrap();+ let library = dir.join("library");+ let mut s = crate::Session::new();+ s.open_library(&library, false).unwrap();+ let interop = library.join("Interop");+ std::fs::create_dir_all(&interop).unwrap();+ // one more entry than the archive scans (`MAX_SCAN`)+ for i in 0..=1024 {+ std::fs::write(interop.join(format!("other-{i}")), b"").unwrap();+ }+ let report = import(&mut s, &source, false).unwrap();+ assert_eq!(report["imported"], 1);+ assert_eq!(report["archive"], Value::Null);+ let warnings = report["warnings"].as_array().unwrap();+ assert!(+ warnings.iter().any(|w| w.as_str().unwrap().starts_with(&format!("Lightroom source archive not saved in {}", interop.display()))),+ "{warnings:?}"+ );+ assert!(interop.join("lightroom-index.json").is_file(), "re-import mapping is still saved");+ s.close_library().unwrap();+ std::fs::remove_dir_all(dir).unwrap();+}++/// A linked `Interop` folder gets neither the archive nor the import index: both would land+/// outside the library. The import is refused before any photo is imported.+#[cfg(unix)]+#[test]+fn a_linked_interop_folder_gets_neither_the_archive_nor_the_index() {+ let dir = std::env::temp_dir().join(format!("lightcraft-lrcat-interop-linked-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ std::fs::create_dir_all(dir.join("elsewhere")).unwrap();+ let source = dir.join("source.lrcat");+ std::fs::write(&source, native_catalog(&photo_tables((dir.join("offline").to_str().unwrap(), "")))).unwrap();+ let library = dir.join("library");+ let mut s = crate::Session::new();+ s.open_library(&library, false).unwrap();+ let interop = library.join("Interop");+ std::os::unix::fs::symlink(dir.join("elsewhere"), &interop).unwrap();+ let e = import(&mut s, &source, false).unwrap_err();+ assert!(e.to_string().contains(&format!("{} is a link or a file, not a folder", interop.display())), "{e}");+ assert_eq!(s.catalog.photos().count(), 0, "refused before anything is imported, so a retry can't duplicate");+ assert_eq!(std::fs::read_dir(dir.join("elsewhere")).unwrap().count(), 0, "nothing is written through the link");+ s.close_library().unwrap();+ std::fs::remove_dir_all(dir).unwrap();+}++/// An `Interop` folder that can't be written to would let the photos be imported and then fail+/// at the import index, after the import was saved (a retry could duplicate virtual copies). It is+/// refused before any photo is imported, with an error naming the folder.+#[cfg(unix)]+#[test]+fn an_unwritable_interop_folder_is_refused_before_anything_is_imported() {+ use std::os::unix::fs::PermissionsExt;+ let dir = std::env::temp_dir().join(format!("lightcraft-lrcat-interop-readonly-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ std::fs::create_dir_all(&dir).unwrap();+ let source = dir.join("source.lrcat");+ std::fs::write(&source, native_catalog(&photo_tables((dir.join("offline").to_str().unwrap(), "")))).unwrap();+ let library = dir.join("library");+ let mut s = crate::Session::new();+ s.open_library(&library, false).unwrap();+ let interop = library.join("Interop");+ std::fs::create_dir_all(&interop).unwrap();+ std::fs::set_permissions(&interop, std::fs::Permissions::from_mode(0o555)).unwrap();+ // root (some containers) ignores folder permissions: there is nothing to refuse+ if std::fs::write(interop.join("root-check"), b"").is_ok() {+ std::fs::set_permissions(&interop, std::fs::Permissions::from_mode(0o755)).unwrap();+ s.close_library().unwrap();+ std::fs::remove_dir_all(&dir).unwrap();+ lightcraft_testkit::skip!(System, "folder permissions are not enforced for this user (root)");+ return;+ }+ let e = import(&mut s, &source, false).unwrap_err();+ std::fs::set_permissions(&interop, std::fs::Permissions::from_mode(0o755)).unwrap();+ assert!(e.to_string().contains(&format!("the Lightroom import index can't be written in {}", interop.display())), "{e}");+ assert_eq!(s.catalog.photos().count(), 0, "refused before anything is imported, so a retry can't duplicate");+ assert_eq!(std::fs::read_dir(&interop).unwrap().count(), 0, "nothing was written");+ s.close_library().unwrap();+ std::fs::remove_dir_all(dir).unwrap();+}+ // Construct a small SQLite fixture from public file-format records, never private media. fn varint(mut n: u64) -> Vec<u8> { let mut out = vec![(n & 127) as u8];@@ -504,7 +798,9 @@ fn native_catalog_with_last_table_page(tables: &[(&str, &str, Vec<SqlValue>)], l /// table's root page. fn native_catalog_rows(tables: &[(&str, &str, Vec<Vec<SqlValue>>)], last_page: Option<i64>) -> Vec<u8> { use SqlValue::{Integer as I, Text as T};- let mut bytes = vec![0u8; 5 * 2048];+ // the schema page, then one page per table+ let pages = tables.len() + 1;+ let mut bytes = vec![0u8; pages * 2048]; bytes[..16].copy_from_slice(b"SQLite format 3\0"); bytes[16..18].copy_from_slice(&2048u16.to_be_bytes()); bytes[18] = 1;@@ -512,7 +808,7 @@ fn native_catalog_rows(tables: &[(&str, &str, Vec<Vec<SqlValue>>)], last_page: O bytes[21] = 64; bytes[22] = 32; bytes[23] = 32;- bytes[28..32].copy_from_slice(&5u32.to_be_bytes());+ bytes[28..32].copy_from_slice(&(pages as u32).to_be_bytes()); bytes[44..48].copy_from_slice(&4u32.to_be_bytes()); bytes[56..60].copy_from_slice(&1u32.to_be_bytes()); let mut schema = Vec::new();@@ -582,7 +878,7 @@ fn reads_native_sqlite_tables_without_lightroom_or_external_processes() { assert!(session.catalog.is_empty()); } std::fs::write(&path, b"bad catalog").unwrap();- assert!(read(&path).is_err());+ assert_eq!(read(&path).unwrap_err().kind(), crate::ErrorKind::Other); std::fs::remove_file(path).unwrap(); } @@ -640,7 +936,80 @@ fn a_virtual_copy_of_a_skipped_master_is_skipped_too() { catalog.skipped, [SkippedImage { image: 1, reason: SkipReason::MissingFile }, SkippedImage { image: 2, reason: SkipReason::MasterSkipped }] );- validate(&catalog).unwrap();+ let mut again = catalog.clone();+ skip_inconsistent_records(&mut again);+ assert_eq!(again.skipped, catalog.skipped, "the second pass finds nothing");+}++/// A keyword whose parents loop used to fail the whole read; it is now cut where the loop comes+/// back, with one warning however many photos have it.+#[test]+fn a_looping_keyword_hierarchy_is_cut_not_fatal() {+ use SqlValue::{Integer as I, Null as N, Text as T};+ let mut tables = one_row(photo_tables(("/catalog/", "")));+ tables.push((+ "AgLibraryKeyword",+ "id_local INTEGER PRIMARY KEY, name, parent",+ vec![vec![N, T("Travel".into()), I(2)], vec![N, T("Goa".into()), I(1)]],+ ));+ tables.push(("AgLibraryKeywordImage", "id_local INTEGER PRIMARY KEY, image, tag", vec![vec![N, I(1), I(1)], vec![N, I(1), I(1)]]));+ let catalog = read_tables(&tables, "keyword-loop");+ assert_eq!(catalog.photos[0].keywords, ["Goa|Travel", "Goa|Travel"]);+ assert_eq!(catalog.warnings, ["keyword “Travel”: its parent keywords form a loop or are nested more than 64 deep; imported as “Goa|Travel”"]);+ assert!(catalog.skipped.is_empty());+}++/// A catalog that can't be read at all is still an error, but it names the file and says what is+/// wrong (not "invalid parameters"), and nothing is changed.+#[test]+fn an_unreadable_catalog_is_an_error_naming_it_and_changes_nothing() {+ let dir = std::env::temp_dir().join(format!("lightcraft-lrcat-damaged-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ std::fs::create_dir_all(&dir).unwrap();+ let source = dir.join("source.lrcat");+ let mut no_files = photo_tables(("/catalog/", ""));+ no_files.remove(2);+ let cases = [+ // the Adobe_images table points past the end of the file+ (native_catalog_with_last_table_page(&photo_tables(("/catalog/", "")), Some(99)), "is damaged: table Adobe_images can't be read: "),+ (native_catalog(&no_files), "is not a Lightroom Classic catalog (or is damaged): it has no AgLibraryFile table"),+ (b"bad catalog".to_vec(), "can't be read: it is too short to be a SQLite database"),+ (vec![b'x'; 4096], "is damaged: not a readable SQLite database: "),+ ];+ for (bytes, expected) in cases {+ std::fs::write(&source, bytes).unwrap();+ let mut s = crate::Session::new();+ apply(&mut s, sample(), false).unwrap();+ let before = s.catalog.to_snapshot();+ let undo = s.undo.len();+ let e = import(&mut s, &source, false).unwrap_err();+ assert_eq!(e.kind(), crate::ErrorKind::Other, "{e}");+ let message = e.to_string();+ assert!(message.contains(&*source.to_string_lossy()) && message.contains(expected), "{message}");+ assert_eq!(s.catalog.to_snapshot(), before);+ assert_eq!(s.undo.len(), undo);+ }+ let e = import(&mut crate::Session::new(), &dir.join("gone.lrcat"), false).unwrap_err();+ assert_eq!(e.kind(), crate::ErrorKind::Other, "{e}");+ assert!(e.to_string().starts_with(&format!("the Lightroom catalog {} can't be read: ", dir.join("gone.lrcat").display())), "{e}");+ std::fs::remove_dir_all(dir).unwrap();+}++/// The library's import index that can't be read is an error naming it, not "invalid parameters".+#[test]+fn an_unreadable_import_index_is_an_error_naming_it() {+ let dir = std::env::temp_dir().join(format!("lightcraft-lr-index-unreadable-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ let index = dir.join("lightroom-index.json");+ std::fs::create_dir_all(&index).unwrap();+ let Err(e) = load_index(Some(&index)) else { panic!("a folder is not an index") };+ assert_eq!(e.kind(), crate::ErrorKind::Other, "{e}");+ assert!(e.to_string().starts_with(&format!("can't read the Lightroom import index {}: ", index.display())), "{e}");+ std::fs::remove_dir(&index).unwrap();+ std::fs::write(&index, b"{not json").unwrap();+ let Err(e) = load_index(Some(&index)) else { panic!("not an index") };+ assert!(e.to_string().contains("it is damaged"), "{e}");+ std::fs::remove_dir_all(dir).unwrap(); } #[test]@@ -804,7 +1173,7 @@ fn oversized_optional_table_warns_and_is_skipped() { let db = Database::open_with_wal(main, &[]).unwrap(); let reader = Reader { db, tables: vec![LiveTable { name: "optional_history".into(), rootpage: 2, column_names: Some(vec!["payload".into()]) }] }; let mut warnings = Vec::new();- let rows = reader.optional_table("optional_history", &mut warnings).unwrap();+ let rows = reader.optional_table("optional_history", &mut warnings); assert!(rows.is_empty()); assert!(warnings.iter().any(|warning| warning.contains("optional_history"))); }@@ -20,22 +20,72 @@ use crate::lightroom_catalog::{self, CatalogImport, ImportIndex}; const COMMAND: &str = "library.importLightroom"; const MAX_INDEX_BYTES: usize = 16 * 1024 * 1024; -fn error(message: impl Into<String>) -> crate::EngineError {- crate::EngineError::BadParams { cmd: COMMAND.into(), msg: message.into() }-}- /// The import was cancelled ([`crate::ErrorKind::Cancelled`]: not a failure). The message is the /// one it always had, so control-channel and MCP clients see the same `error` text. fn cancelled() -> crate::EngineError {- crate::EngineError::Cancelled(error("Lightroom import cancelled").to_string())+ let message = crate::EngineError::BadParams { cmd: COMMAND.into(), msg: "Lightroom import cancelled".into() };+ crate::EngineError::Cancelled(message.to_string()) } +/// Save the recovery archive. It is best-effort: nothing reads it back during import, so an archive+/// that can't be saved costs the archive (with a warning naming the folder), not the import. A+/// folder that can't be written to at all is refused first, for the import index's sake.+/// That warning goes first: the result window lists only the first few, and catalog warnings+/// (one per unreadable record) can be many. fn stage_archive(archive_dir: Option<&Path>, data: &mut CatalogImport) -> crate::Result<Option<PathBuf>> {- let archive_path = archive_dir.map(|dir| crate::lightroom_archive::store_best_effort(dir, data).map_err(error)).transpose()?.flatten();- if archive_dir.is_some() && archive_path.is_none() {- data.warnings.push("Lightroom source archive exceeded 32 MiB; archive skipped".into());+ let Some(dir) = archive_dir else { return Ok(None) };+ // The import index, unlike the archive, is required, and it is written only after the photos+ // are imported. Refuse here, before anything changes, a folder it can't be written to (a link+ // would take it outside the library), rather than import and then fail, where a retry could+ // duplicate virtual copies.+ require_index_folder(dir).map_err(|e| crate::EngineError::Other(format!("cannot import into this library: {e}")))?;+ match crate::lightroom_archive::store_best_effort(dir, data) {+ Ok(Some(stored)) => {+ // the archive is saved: the report names it, and the leftover is a warning of its own+ if let Some(problem) = stored.cleanup_failed {+ data.warnings.insert(+ 0,+ format!("Lightroom source archive saved as {}, but cleaning up {} failed: {problem}", stored.path.display(), dir.display()),+ );+ }+ Ok(Some(stored.path))+ }+ Ok(None) => {+ data.warnings.insert(0, "Lightroom source archive exceeded 32 MiB; archive skipped".into());+ Ok(None)+ }+ Err(e) => {+ data.warnings.insert(0, format!("Lightroom source archive not saved in {}: {e}; archive skipped", dir.display()));+ Ok(None)+ }+ }+}++/// Check that `lightroom-index.json` can be written in `dir` (creating the folder, as the archive+/// would): a real folder, not a link or a file, in which a file can be created and removed. The+/// probe is a plain exclusive create, never a hard link, so volumes without them (exFAT, #3114) pass.+fn require_index_folder(dir: &Path) -> Result<(), String> {+ if let Err(e) = std::fs::symlink_metadata(dir) {+ if e.kind() != std::io::ErrorKind::NotFound {+ return Err(format!("cannot read {}: {e}", dir.display()));+ }+ std::fs::create_dir_all(dir).map_err(|e| format!("cannot create the folder {}: {e}", dir.display()))?;+ }+ crate::lightroom_archive::require_real_directory(dir)?;+ let unwritable = |e: std::io::Error| format!("the Lightroom import index can't be written in {} ({e}); make the folder writable", dir.display());+ let pid = std::process::id();+ for slot in 0..64 {+ let probe = dir.join(format!(".lightroom-index-probe-{pid}-{slot}.tmp"));+ match std::fs::OpenOptions::new().write(true).create_new(true).open(&probe) {+ Ok(file) => {+ drop(file);+ return std::fs::remove_file(&probe).map_err(unwritable);+ }+ Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,+ Err(e) => return Err(unwritable(e)),+ } }- Ok(archive_path)+ Err(format!("no free name to check that {} is writable", dir.display())) } /// Memory identity of the library which owns a prepared completion.@@ -115,9 +165,16 @@ impl LightroomArchiveFinalization { pub fn finish(self) -> crate::Result<()> { let (Some(path), Some(bytes)) = (self.path, self.bytes) else { return Ok(()) }; if bytes.len() > MAX_INDEX_BYTES {- return Err(error("Lightroom import index exceeds 16 MiB"));+ return Err(crate::EngineError::Other(format!("could not write the Lightroom import index {}: it exceeds 16 MiB", path.display()))); }- lightcraft_catalog::safe_file::write_atomic(&path, &bytes).map_err(|e| error(e.to_string()))+ let failed =+ |e: &dyn std::fmt::Display| crate::EngineError::Other(format!("could not write the Lightroom import index {}: {e}", path.display()));+ // the archive refuses a linked `Interop` folder; now that its failure no longer stops the+ // import, the index must refuse it too, or it would be written outside the library+ if let Some(dir) = path.parent() {+ crate::lightroom_archive::require_real_directory(dir).map_err(|e| failed(&e))?;+ }+ lightcraft_catalog::safe_file::write_atomic(&path, &bytes).map_err(|e| failed(&e)) } pub fn path(&self) -> Option<&Path> {@@ -240,10 +297,10 @@ impl LightroomJob { if cancel.load(Ordering::Relaxed) { return Err(cancelled()); }- // (reading stops with a message when `cancel` is set: the flag says it was cancelled)+ // (reading stops when `cancel` is set: the flag says it was cancelled; a catalog that can't be+ // read is an error naming it, and a bad record is skipped, not an error) let mut data = lightroom_catalog::read_with_progress(&self.path, cancel, &self.progress.catalog_total, &self.progress.catalog_done)- .map_err(|e| if cancel.load(Ordering::Relaxed) { cancelled() } else { error(e) })?;- lightroom_catalog::validate_for_job(&data).map_err(error)?;+ .map_err(|e| if cancel.load(Ordering::Relaxed) { cancelled() } else { e })?; if cancel.load(Ordering::Relaxed) { return Err(cancelled()); }@@ -284,7 +341,7 @@ pub fn commit_prepared(s: &mut crate::Session, mut prepared: PreparedLightroom) } if !prepared.token.matches(s) { prepared.normal.rollback();- return Err(error("Lightroom import belongs to a different open library"));+ return Err(crate::EngineError::Other("Lightroom import belongs to a different open library".into())); } // A command-scoped Lightroom import must never rewrite source XMP sidecars. Direct depth-0 // callers do not set this transient flag, so they cannot affect a later command.@@ -310,7 +367,7 @@ pub fn commit_prepared(s: &mut crate::Session, mut prepared: PreparedLightroom) }, )?; let path = prepared.token.library_dir().map(|dir| dir.join("Interop/lightroom-index.json"));- let bytes = if path.is_some() { Some(serde_json::to_vec(&applied.index).map_err(|e| error(e.to_string()))?) } else { None };+ let bytes = if path.is_some() { Some(serde_json::to_vec(&applied.index).map_err(|e| crate::EngineError::Other(e.to_string()))?) } else { None }; Ok(LightroomCompletion { report: applied.report, finalization: LightroomArchiveFinalization { path, bytes } }) } @@ -478,6 +535,9 @@ mod tests { assert_eq!(e.to_string(), "invalid parameters for `library.importLightroom`: Lightroom import cancelled"); let Err(e) = job.prepare(&AtomicBool::new(false)) else { panic!("no such catalog") }; assert_ne!(e.kind(), crate::ErrorKind::Cancelled, "{e}");+ // a catalog that can't be read is not "invalid parameters": the error names it+ assert_eq!(e.kind(), crate::ErrorKind::Other, "{e}");+ assert!(e.to_string().starts_with("the Lightroom catalog /nonexistent/cancelled.lrcat can't be read: "), "{e}"); } #[test]@@ -564,6 +624,88 @@ mod tests { std::fs::remove_dir_all(archive_dir).unwrap(); } + /// The archive is best-effort: a folder it can't be written to costs the archive, not the import.+ #[test]+ fn an_archive_that_cannot_be_written_is_a_warning_naming_its_folder() {+ let dir = std::env::temp_dir().join(format!("lightcraft-lr-archive-unwritable-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ std::fs::create_dir_all(&dir).unwrap();+ let interop = dir.join("Interop");+ std::fs::create_dir_all(&interop).unwrap();+ // one more entry than the archive scans (`MAX_SCAN`)+ for i in 0..=1024 {+ std::fs::write(interop.join(format!("other-{i}")), b"").unwrap();+ }+ let mut data = empty_data();+ // more catalog warnings than the result window lists: the archive's must still be seen+ data.warnings = (0..10).map(|i| format!("record {i}: unreadable XMP")).collect();+ assert_eq!(stage_archive(Some(&interop), &mut data).unwrap(), None);+ assert_eq!(data.warnings.len(), 11);+ assert!(data.warnings[0].starts_with("Lightroom source archive not saved in "), "{}", data.warnings[0]);+ assert!(data.warnings[0].contains(&*interop.to_string_lossy()), "{}", data.warnings[0]);+ std::fs::remove_dir_all(dir).unwrap();+ }++ /// An archive that is saved but whose cleanup fails afterwards (here an old archive can't be+ /// retired) is still the import's archive: the report names it, and the cleanup is a warning.+ #[test]+ fn a_saved_archive_whose_cleanup_fails_is_reported_with_a_warning() {+ let dir = std::env::temp_dir().join(format!("lightcraft-lr-archive-cleanup-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&dir);+ let interop = dir.join("Interop");+ std::fs::create_dir_all(&interop).unwrap();+ let mut data = empty_data();+ // an older archive, then this catalog's, then enough others to retire the older one+ std::fs::write(interop.join("lightroom-import-0.lca"), b"older").unwrap();+ let saved = crate::lightroom_archive::store(&interop, &data).unwrap().path;+ assert_eq!(saved, interop.join("lightroom-import-1.lca"));+ for i in 2..=crate::lightroom_archive::MAX_MANAGED_ARCHIVES {+ std::fs::write(interop.join(format!("lightroom-import-{i}.lca")), format!("other {i}")).unwrap();+ }+ // retiring the older archive needs this name, which is taken+ std::fs::write(interop.join(format!(".lightroom-retire-{}-0.tmp", std::process::id())), b"taken").unwrap();+ assert_eq!(stage_archive(Some(&interop), &mut data).unwrap(), Some(saved.clone()));+ assert!(saved.is_file());+ assert_eq!(data.warnings.len(), 1, "{:?}", data.warnings);+ let warning = &data.warnings[0];+ assert!(warning.starts_with(&format!("Lightroom source archive saved as {}, but cleaning up", saved.display())), "{warning}");+ assert!(warning.contains("retention temporary path already exists"), "{warning}");+ assert_eq!(std::fs::read(interop.join("lightroom-import-0.lca")).unwrap(), b"older", "the older archive is kept");+ std::fs::remove_dir_all(dir).unwrap();+ }++ /// A failed index write is an I/O error naming the file, not "invalid parameters".+ #[test]+ fn an_index_write_failure_names_the_index_file() {+ let path = std::env::temp_dir().join(format!("lightcraft-lr-index-missing-{}", std::process::id())).join("lightroom-index.json");+ let e = LightroomArchiveFinalization { path: Some(path.clone()), bytes: Some(b"{}".to_vec()) }.finish().unwrap_err();+ assert!(matches!(e, crate::EngineError::Other(_)), "{e:?}");+ assert!(e.to_string().starts_with(&format!("could not write the Lightroom import index {}", path.display())), "{e}");+ }++ /// A linked `Interop` folder (which the archive already refuses) must not receive the index:+ /// it would land outside the library and could replace a file there.+ #[cfg(unix)]+ #[test]+ fn an_index_write_refuses_a_linked_interop_folder() {+ let root = std::env::temp_dir().join(format!("lightcraft-lr-index-linked-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&root);+ let (library, outside) = (root.join("library"), root.join("outside"));+ std::fs::create_dir_all(&library).unwrap();+ std::fs::create_dir_all(&outside).unwrap();+ std::fs::write(outside.join("lightroom-index.json"), b"not ours").unwrap();+ std::os::unix::fs::symlink(&outside, library.join("Interop")).unwrap();+ let path = library.join("Interop/lightroom-index.json");+ let e = LightroomArchiveFinalization { path: Some(path), bytes: Some(b"{}".to_vec()) }.finish().unwrap_err();+ assert!(e.to_string().contains("is a link or a file, not a folder"), "{e}");+ assert_eq!(std::fs::read(outside.join("lightroom-index.json")).unwrap(), b"not ours");+ // preparation refuses the link before anything is imported+ let mut data = empty_data();+ let e = stage_archive(Some(&library.join("Interop")), &mut data).unwrap_err();+ assert!(e.to_string().contains("is a link or a file, not a folder"), "{e}");+ std::fs::remove_dir_all(root).unwrap();+ }+ #[test] fn execute_fn_import_does_not_rewrite_source_sidecar() { let dir = std::env::temp_dir().join(format!("lightcraft-lr-job-sidecar-{}", std::process::id()));@@ -37,7 +37,10 @@ impl FileSource { return Err("catalog exceeds 16 GiB import limit".into()); } let mut header = vec![0; 100];- file.read_exact(&mut header).map_err(|e| format!("cannot read SQLite header: {e}"))?;+ file.read_exact(&mut header).map_err(|e| match e.kind() {+ std::io::ErrorKind::UnexpectedEof => "it is too short to be a SQLite database".to_string(),+ _ => format!("cannot read SQLite header: {e}"),+ })?; let source = Self { file: Mutex::new(file), path: path.into(), before, header }; source.ensure_unchanged()?; Ok(source)@@ -268,6 +268,7 @@ "{n} preset{}": "Přednastavení: {n}{:.0}", "{count} more warnings.": "Další varování: {count}.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "Neimportované obrázky katalogu: {skipped}. Jejich záznamy v katalogu jsou neúplné.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "Neimportované obrázky katalogu: {skipped}. Jejich záznamy v katalogu odporují jiným (opakované ID obrázku nebo virtuální kopie virtuální kopie).", "{photos} catalog records processed; {collections} collections.": "Zpracováno záznamů katalogu: {photos}; kolekcí: {collections}.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Chybějící originály: {missing}. Jejich záznamy jsou v katalogu dostupné; použijte Najít chybějící fotografie a znovu je propojte.", "Applied {name}": "Použito: {name}",@@ -270,6 +270,7 @@ "· {in_source} identical copies": "· {in_source} identische Kopien", "{count} more warnings.": "{count} weitere Warnungen.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} Katalogbilder wurden nicht importiert, weil ihre Einträge im Katalog unvollständig sind.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "{skipped} Katalogbilder wurden nicht importiert, weil ihre Einträge im Katalog anderen widersprechen (eine doppelte Bild-ID oder eine virtuelle Kopie einer virtuellen Kopie).", "{photos} catalog records processed; {collections} collections.": "{photos} Katalogeinträge verarbeitet; {collections} Sammlungen.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} Originale fehlen. Ihre Katalogeinträge sind verfügbar; verwende „Fehlende Fotos suchen“, um sie neu zu verknüpfen.", "Applied {name}": "{name} angewendet",@@ -270,6 +270,7 @@ "· {in_source} identical copies": "· {in_source} copias idénticas", "{count} more warnings.": "{count} advertencias más.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "No se importaron {skipped} imágenes del catálogo porque sus registros en el catálogo están incompletos.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "No se importaron {skipped} imágenes del catálogo porque sus registros en el catálogo contradicen a otros (un ID de imagen repetido o una copia virtual de una copia virtual).", "{photos} catalog records processed; {collections} collections.": "{photos} registros del catálogo procesados; {collections} colecciones.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Faltan {missing} originales. Sus registros del catálogo están disponibles; usa Buscar fotos que faltan para volver a vincularlos.", "Applied {name}": "Se aplicó {name}",@@ -258,6 +258,7 @@ "{sel_n} selected · {counted}": "Valittu: {sel_n} · {counted}", "{size} GB": "{size} Gt", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} catalog images were not imported because their records in the catalog are incomplete.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).", "{stage}… {:.0}%": "{stage}… {:.0}%", "{s} s of AV1 video": "{s} s AV1-videota", "{verb} {n} Photo{}": "{verb} kuvia: {n}{:.0}",@@ -270,6 +270,7 @@ "· {in_source} identical copies": "· {in_source} copies identiques", "{count} more warnings.": "{count} avertissements supplémentaires.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} images du catalogue n’ont pas été importées, car leurs entrées dans le catalogue sont incomplètes.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "{skipped} images du catalogue n’ont pas été importées, car leurs entrées dans le catalogue en contredisent d’autres (un identifiant d’image répété ou une copie virtuelle d’une copie virtuelle).", "{photos} catalog records processed; {collections} collections.": "{photos} entrées du catalogue traitées ; {collections} collections.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} originaux sont manquants. Leurs entrées de catalogue sont disponibles ; utilisez Rechercher les photos manquantes pour rétablir les liens.", "Applied {name}": "{name} appliqué",@@ -297,6 +297,7 @@ "{what} merge added at {mp}: its full size, {full}, needs more memory than this computer has": "Hasil penggabungan {what} ditambahkan dengan resolusi {mp}: resolusi penuhnya ({full}) memerlukan memori melebihi kapasitas komputer ini", "{what} merge added at {mp}, the largest a panorama can be (its full size is {full})": "Hasil penggabungan {what} ditambahkan dengan resolusi {mp}, resolusi maksimum untuk panorama (resolusi penuhnya {full})", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} catalog images were not imported because their records in the catalog are incomplete.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).", "Listing folders — folders: {folders}, files: {files}": "Listing folders — folders: {folders}, files: {files}", "Added {n} photo{}": "{n} foto ditambahkan{:.0}", "Could not read {name}: {e}": "Tidak dapat membaca {name}: {e}",@@ -251,6 +251,7 @@ "Delete {n} effect preset{}? This can't be undone.": "Eliminare i preset effetto ({n})?{:.0} L'operazione non si può annullare.", "{count} more warnings.": "Altri avvisi: {count}.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} immagini del catalogo non sono state importate perché i loro record nel catalogo sono incompleti.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "{skipped} immagini del catalogo non sono state importate perché i loro record nel catalogo ne contraddicono altri (un ID immagine ripetuto o una copia virtuale di una copia virtuale).", "· {in_source} identical copies": "· copie identiche: {in_source}", "{what} merge added at {mp}, the largest a panorama can be (its full size is {full})": "Unione {what} aggiunta a {mp}, la dimensione massima di un panorama (la dimensione piena è {full})", "{n} preset{}": "Preset: {n}{:.0}",@@ -274,6 +274,7 @@ "· {in_source} identical copies": "・同一のコピー {in_source} 件", "{count} more warnings.": "ほかに{count}件の警告があります。", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "カタログ内のレコードが不完全なため、{skipped}件の画像を読み込めませんでした。",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "カタログ内のレコードが他のレコードと矛盾しているため（画像IDの重複、または仮想コピーの仮想コピー）、{skipped}件の画像を読み込めませんでした。", "{photos} catalog records processed; {collections} collections.": "カタログの{photos}件のレコードを処理しました。コレクションは{collections}件です。", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing}件のオリジナルが見つかりません。カタログのレコードは利用できます。「見つからない写真を検索」で再リンクしてください。", "Applied {name}": "{name} を適用しました",@@ -280,6 +280,7 @@ "new, inside {parent}": "new, inside {parent}", "{count} more warnings.": "{count} more warnings.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "კატალოგის {skipped} სურათი არ დაიმპორტდა, რადგან მათი ჩანაწერები კატალოგში არასრულია.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "კატალოგის {skipped} სურათი არ დაიმპორტდა, რადგან მათი ჩანაწერები კატალოგში სხვა ჩანაწერებს ეწინააღმდეგება (გამეორებული სურათის ID ან ვირტუალური ასლის ვირტუალური ასლი).", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.", "{n} JPEG files": "{n} JPEG files", "{n} pages": "{n} pages",@@ -262,6 +262,7 @@ "{n} preset{}": "Ustawienia: {n}{:.0}", "{count} more warnings.": "Pozostałe ostrzeżenia: {count}.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "Nie zaimportowano obrazów z katalogu: {skipped}. Ich rekordy w katalogu są niekompletne.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "Nie zaimportowano obrazów z katalogu: {skipped}. Ich rekordy w katalogu są sprzeczne z innymi (powtórzony identyfikator obrazu lub kopia wirtualna kopii wirtualnej).", "{photos} catalog records processed; {collections} collections.": "Przetworzone rekordy katalogu: {photos}; kolekcje: {collections}.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Brakujące oryginały: {missing}. Ich rekordy w katalogu są dostępne; użyj polecenia Znajdź brakujące zdjęcia, aby je ponownie powiązać.", "Applied {name}": "Zastosowano {name}",@@ -274,6 +274,7 @@ "· {in_source} identical copies": "· {in_source} cópias idênticas", "{count} more warnings.": "Mais {count} avisos.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} imagens do catálogo não foram importadas porque seus registros no catálogo estão incompletos.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "{skipped} imagens do catálogo não foram importadas porque seus registros no catálogo contradizem outros (um ID de imagem repetido ou uma cópia virtual de uma cópia virtual).", "{photos} catalog records processed; {collections} collections.": "{photos} registros do catálogo processados; {collections} coleções.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} originais estão ausentes. Seus registros no catálogo estão disponíveis; use Encontrar fotos ausentes para vinculá-los novamente.", "Applied {name}": "{name} aplicado",@@ -270,6 +270,7 @@ "· {in_source} identical copies": "· идентичных копий: {in_source}", "{count} more warnings.": "Других предупреждений: {count}.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "Не импортированы изображения каталога: {skipped}. Их записи в каталоге неполные.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "Не импортированы изображения каталога: {skipped}. Их записи в каталоге противоречат другим (повторяющийся идентификатор изображения или виртуальная копия виртуальной копии).", "{photos} catalog records processed; {collections} collections.": "Обработано записей каталога: {photos}; коллекций: {collections}.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Не найдены оригиналы: {missing}. Их записи каталога доступны; используйте поиск отсутствующих фотографий, чтобы восстановить ссылки.", "Applied {name}": "Применено: {name}",@@ -297,6 +297,7 @@ "Moved {n} photo{} to “{}”. Edit → Undo moves them back.": "Moved {n} photo{} to “{}”. Edit → Undo moves them back.", "{n} selected photos · {pages} pages": "{n} selected photos · {pages} pages", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} catalog images were not imported because their records in the catalog are incomplete.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).", "Listing folders — folders: {folders}, files: {files}": "Listing folders — folders: {folders}, files: {files}", "Added {n} photo{}": "{n} foto tillagda{:.0}", "Could not read {name}: {e}": "Kunde inte läsa {name}: {e}",@@ -270,6 +270,7 @@ "· {in_source} identical copies": "· ідентичних копій: {in_source}", "{count} more warnings.": "Інших попереджень: {count}.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "Не імпортовано зображення каталогу: {skipped}. Їхні записи в каталозі неповні.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "Не імпортовано зображення каталогу: {skipped}. Їхні записи в каталозі суперечать іншим (повторений ідентифікатор зображення або віртуальна копія віртуальної копії).", "{photos} catalog records processed; {collections} collections.": "Оброблено записів каталогу: {photos}; колекцій: {collections}.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Не знайдено оригінали: {missing}. Їхні записи каталогу доступні; скористайтеся пошуком відсутніх фотографій, щоб відновити посилання.", "Applied {name}": "Застосовано: {name}",@@ -258,6 +258,7 @@ "{sel_n} selected · {counted}": "Đã chọn {sel_n} · {counted}", "{size} GB": "{size} GB", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} catalog images were not imported because their records in the catalog are incomplete.",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).", "{stage}… {:.0}%": "{stage}… {:.0}%", "{s} s of AV1 video": "{s} s of AV1 video", "{verb} {n} Photo{}": "{verb} {n} ảnh{:.0}",@@ -274,6 +274,7 @@ "· {in_source} identical copies": "· {in_source} 个相同副本", "{count} more warnings.": "另有 {count} 条警告。", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "有 {skipped} 张图像未导入，因为它们在目录中的记录不完整。",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "有 {skipped} 张图像未导入，因为它们在目录中的记录与其他记录矛盾（图像 ID 重复，或是虚拟副本的虚拟副本）。", "{photos} catalog records processed; {collections} collections.": "已处理 {photos} 条目录记录；{collections} 个收藏集。", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "有 {missing} 个原始文件缺失。其目录记录可用；请使用“查找缺失照片”重新链接文件。", "Applied {name}": "已应用 {name}",@@ -274,6 +274,7 @@ "· {in_source} identical copies": "· {in_source} 個相同副本", "{count} more warnings.": "另有 {count} 則警告。", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "有 {skipped} 張影像未匯入，因為它們在目錄中的記錄不完整。",+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).": "有 {skipped} 張影像未匯入，因為它們在目錄中的記錄與其他記錄矛盾（影像 ID 重複，或是虛擬副本的虛擬副本）。", "{photos} catalog records processed; {collections} collections.": "已處理 {photos} 筆目錄記錄；{collections} 個收藏集。", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "有 {missing} 個原始檔案遺失。其目錄記錄可用；請使用「尋找遺失相片」重新連結檔案。", "Applied {name}": "已套用 {name}",@@ -186,7 +186,7 @@ fn spawn_inspect( let result = lightcraft_engine::guard::catch("Lightroom inspect", || { lightcraft_engine::lightroom_catalog::read_with_progress(&path, &cancel, &progress.catalog_total, &progress.catalog_done) })- .and_then(|r| r.map(inspect_report));+ .and_then(|r| r.map(inspect_report).map_err(|e| e.to_string())); let _ = tx.send(Message::Inspected(result)); ctx.request_repaint(); })@@ -340,13 +340,21 @@ fn result_dialog(value: &Value, summary: &str) -> crate::state::Dialog { missing = missing )); }- // images whose catalog records are incomplete come as data (`skipped`), said here in the UI- // language; their ids are in the report for the CLI and agents- let skipped = report["skipped"].as_array().map_or(0, Vec::len);- if skipped > 0 {+ // images whose catalog records are incomplete or contradictory come as data (`skipped`), said+ // here in the UI language; their ids and reasons are in the report for the CLI and agents+ let skipped = report["skipped"].as_array().map_or(&[][..], Vec::as_slice);+ let contradictory = skipped.iter().filter(|s| matches!(s["reason"].as_str(), Some("duplicateId" | "masterIsCopy"))).count();+ let incomplete = skipped.len() - contradictory;+ if incomplete > 0 { details.push(crate::i18n::tr_format!( "{skipped} catalog images were not imported because their records in the catalog are incomplete.",- skipped = skipped+ skipped = incomplete+ ));+ }+ if contradictory > 0 {+ details.push(crate::i18n::tr_format!(+ "{skipped} catalog images were not imported because their records in the catalog contradict others (a repeated image id, or a virtual copy of a virtual copy).",+ skipped = contradictory )); } if let Some(warning) = value.get("indexWarning").and_then(Value::as_str) {@@ -713,6 +721,22 @@ mod tests { assert_eq!(&en[2..], ["w1", "w2"]); } + /// Images skipped for contradictory records (a damaged catalog, #1588) are their own line; a+ /// virtual copy whose master is gone is incomplete, like a missing file record.+ #[test]+ fn contradictory_records_are_their_own_line() {+ let value = json!({"report": {"photos": 3, "collections": 0, "skipped": [+ {"image": 7, "reason": "missingMaster"},+ {"image": 8, "reason": "duplicateId"},+ {"image": 9, "reason": "masterIsCopy"},+ ]}});+ crate::i18n::set_language(crate::i18n::Locale::En);+ let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") };+ assert_eq!(details.len(), 3, "{details:?}");+ assert_eq!(details[1], "1 catalog images were not imported because their records in the catalog are incomplete.");+ assert!(details[2].starts_with("2 catalog images were not imported because their records in the catalog contradict others"), "{details:?}");+ }+ #[test] fn skipped_images_do_not_take_the_warning_slots() { let warnings: Vec<_> = (0..10).map(|i| format!("w{i}")).collect();@@ -62,11 +62,29 @@ settings are imported. Missing originals remain catalogued for relinking, count separately from failures reading files that are present. Known-missing originals are not probed again. Previously browsed Local records join All Photos even when their originals are offline; their promotion is counted once and undone together with the catalog import.-An image whose file, folder or root folder record is gone from the catalog, or whose original path-is empty (Lightroom leaves these behind and ignores them), is skipped together with its virtual-copies instead of failing the whole import. The result lists them as data (`skipped`: each-image id with its reason, `missingFile`, `missingFolder`, `missingRoot`, `incompletePath` or-`masterSkipped`), also in `library.inspectLightroom`, and the desktop result window says how many in the UI language.+One bad record costs that record, not the import (issue #1588). An image whose file, folder or root folder+record is gone from the catalog, or whose original path is empty, is skipped together with its virtual copies, and+so is a virtual copy whose master is not in the catalog: these records are incomplete (Lightroom leaves them behind+and ignores them). So are the contradictory records of a damaged catalog: a second image with an id an earlier+image has (the first is imported), and a virtual copy whose master is itself a virtual copy. A duplicate is+left out before anything else looks at it, so it can't take the first image's records or virtual copies with it.+The result lists them as data (`skipped`: each image id with its reason,+`missingFile`, `missingFolder`, `missingRoot`, `incompletePath`, `masterSkipped`, `missingMaster`,+`masterIsCopy` or `duplicateId`), also in `library.inspectLightroom`, and the desktop result window says how many+in the UI language (one line for incomplete records, one for contradictory ones). A keyword or collection+hierarchy that loops, or is nested more than 64 levels deep, is cut with a warning instead of failing the import:+a keyword keeps its path up to where the loop comes back, and a collection loop is cut at its oldest collection+(the lowest id), which is imported at the top level. A second collection with the id of an earlier one is left out+with a warning naming both; the photos the catalog lists under that id go to the first. A capture time that isn't a+date is left out with a warning naming the image, which imports without it.++Only a catalog that can't be read at all still fails the import, before anything changes, with an error that+names the file and says what is wrong, not "invalid parameters": "the Lightroom catalog … can't be read: …" (it+is missing, not allowed, too large, or changed while it was read), "the Lightroom catalog … is damaged: …" (the+database or its write-ahead log is broken, or a table every catalog has — `AgLibraryRootFolder`,+`AgLibraryFolder`, `AgLibraryFile`, `Adobe_images` — can't be read), or "… is not a Lightroom Classic catalog+(or is damaged): it has no … table". A library import index (`Interop/lightroom-index.json`) that can't be read+is an error naming that file. These are `ErrorKind::Other`; a cancelled import stays `Cancelled`. Virtual copies count as added records too; reimport reports zero additions when all records already exist. Existing LightCraft edits are preserved by default; `updateExisting=true` explicitly replaces them. Persistent import@@ -76,12 +94,21 @@ catalog changes; any saved recovery archives remain available. Before changing records, the importer attempts to save source image records, decoded XMP, verbatim develop settings, history, snapshots and collection content to compressed `Interop/lightroom-import-N.lca` recovery archives in the LightCraft library. Archiving is-best-effort: a size limit or write failure produces a warning and does not prevent photo import.+best-effort: a size limit or write failure produces a warning (naming the `Interop` folder) and+does not prevent photo import. On a volume without hard links (exFAT, FAT, some network shares) an+archive is published by claiming its name with an exclusive create and renaming over it, so it+still never replaces an existing archive. Unchanged source data reuses its existing archive. Opaque SQL BLOBs are skipped. Each archive is capped at 32 MiB (uncompressed and on disk), with at most eight archives and 128 MiB total-retained. Oldest managed archives are retired; legacy `.json` archives remain untouched.-`Interop/lightroom-index.json` records imported identities. Source catalogs and originals are-never overwritten.+retained. Oldest managed archives are retired; legacy `.json` archives remain untouched. When an+archive is saved but cleanup afterwards fails (a temporary file or an old archive that can't be+removed), the report still names the archive and a separate warning names what was left behind.+`Interop/lightroom-index.json` records imported identities. Unlike the archive it is required, and+it is written after the photos are imported, so the importer checks its folder first: when+`Interop` is a link or a file rather than a folder, or a folder in which a file can't be created+(read-only, no permission), the import is refused before anything changes, with an error naming+the folder. Nothing lands outside the library, and a retry can't duplicate virtual copies (the index+write checks the folder again). Source catalogs and originals are never overwritten. Native import reads referenced database pages on demand (at most 64 KiB per page), rather than loading the whole `.lrcat`. Database files are capped at 16 GiB and 16,777,216 pages; the WAL@@ -94,7 +121,7 @@ importing: metadata checks detect ordinary concurrent changes, but are not SQLit The in-tree, read-only Rust SQLite B-tree reader follows SQLite's public file-format specification; it introduces no dependency or C runtime. Optional history, keyword and collection tables that exceed limits are skipped with warnings;-required photo tables still report errors. Unrelated unsupported schemas are ignored. A needed+required photo tables still report errors (naming the catalog and the table). Unrelated unsupported schemas are ignored. A needed WITHOUT ROWID or virtual table produces an explicit error, handled as a warning when optional. Develop settings reuse the existing XMP/preset mapper. Supported sliders, curves and supported@@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-25 · **Change:** minor (LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6+> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (LRC-LIB-CATALOG-IMPORT: a damaged catalog's contradictory records are skipped, not fatal, and an unreadable catalog names the file, #1588; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good@@ -699,7 +699,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-LIB-SURVEY | Survey view | P2 | ✅ | `cmd:view.survey` | N | | LRC-LIB-REFVIEW | Reference view | P2 | ✅ | `cmd:view.reference`, `cmd:photo.setReference`, `crates/ui-egui/src/panels/compare.rs` (`show_reference`) | View ▸ Reference View (⇧R): the reference photo beside the active one (edited with the Edit panel); photo menu ▸ Set as Reference Photo; each pane says “Rendering…” (widget `compare:rendering:<pane>`) or “This photo can't be opened” (`compare:renderFailed:<pane>`) as the loupe does. Escape → Detail without losing edits; pins are catalog-local; missing/deleted photos refused and deleted references no longer drawn (Windows regression/native smoke verified) | | LRC-LIB-CATALOG-PANEL | Catalog sets | P1 | ✅ | `cmd:library.source` (`missing`) | all, recently added, picks, recently deleted, missing photos (when any) |-| LRC-LIB-CATALOG-IMPORT | Native Lightroom Classic catalog import | P1 | 🟡 | `cmd:library.inspectLightroom`, `cmd:library.importLightroom`, `cmd:file.importLightroom`, `crates/engine/src/lightroom_catalog.rs`, `crates/ui-egui/src/dialogs/lightroom_destination.rs`, `crates/ui-egui/src/tests/lightroom_destination.rs`, `docs/lightroom-catalog-import.md` | before the import starts, File ▸ Import Lightroom Catalog… asks where the catalog goes: merge into the open library (named, with its photo count; preselected when it is empty) or create a new library (named after the catalog file, next to the open library, both editable), which is made and opened following File ▸ New Library…'s rules before the import; Cancel does nothing; Settings ▸ General ▸ "When importing a Lightroom catalog" (Ask where to import it / Always merge into the open library) or the window's "Don't ask again" turns it off; `file.importLightroom` takes `into` (`open` / `new`), `name`, `location` to choose without the window; a session in memory merges without asking (issue #2672); read-only `.lrcat` plus committed WAL pages (also WAL-only tables with main schema format 0), in-tree read-only Rust SQLite B-tree reader; on-demand main-file pages up to 16 GiB with existing table/record bounds (WAL still capped at 1 GiB, issue #798); background inspection/import with progress/cancellation, per-open library identity checks; persistent result with destination library, offline-original counts and Show in Library, dismissed when switching libraries; offline Local records are promoted without probing missing files; originals in place, ratings/flags/labels, XMP, hierarchical keywords, collection sets/regular collections, virtual copies, mapped develop settings; stable reimport identities, undo, missing-file placeholders, a root folder not found at its recorded path looked up relative to the catalog (Lightroom's `relativePathFromCatalog`, so a project folder copied from another computer stays linked; reported as a warning), existing edits preserved. Best-effort deduplicated recovery archives retain settings/history/snapshots/collection rules (32 MiB each, eight archives/128 MiB aggregate); archive limits/write failures and optional-table limits warn without blocking photo import; unrelated unsupported tables and opaque SQL BLOBs skipped. Custom white balance on relative-WB raws (`RAW` in Lightroom's terms) stays As Shot and is reported, since the catalog keeps no as-shot white to shift from (issue #510). An image whose file, folder or root folder record is missing from the catalog, or whose original path is empty, is skipped with its virtual copies instead of failing the whole import; the report lists them as data (`skipped`: image id and reason) and the result window counts them in the UI language (PR #2230). Adobe rendering, AI data and native history replay remain incomplete; no writes to Lightroom catalogs. Open Library given a Lightroom catalog folder, an `.lrdata` / `.lrcat-data` folder or the `.lrcat` makes no library there and offers this import with the catalog found (`cmd:app.openLibrary`, `crates/engine/src/lightroom_folder.rs`; issue #891) |+| LRC-LIB-CATALOG-IMPORT | Native Lightroom Classic catalog import | P1 | 🟡 | `cmd:library.inspectLightroom`, `cmd:library.importLightroom`, `cmd:file.importLightroom`, `crates/engine/src/lightroom_catalog.rs`, `crates/ui-egui/src/dialogs/lightroom_destination.rs`, `crates/ui-egui/src/tests/lightroom_destination.rs`, `docs/lightroom-catalog-import.md` | before the import starts, File ▸ Import Lightroom Catalog… asks where the catalog goes: merge into the open library (named, with its photo count; preselected when it is empty) or create a new library (named after the catalog file, next to the open library, both editable), which is made and opened following File ▸ New Library…'s rules before the import; Cancel does nothing; Settings ▸ General ▸ "When importing a Lightroom catalog" (Ask where to import it / Always merge into the open library) or the window's "Don't ask again" turns it off; `file.importLightroom` takes `into` (`open` / `new`), `name`, `location` to choose without the window; a session in memory merges without asking (issue #2672); read-only `.lrcat` plus committed WAL pages (also WAL-only tables with main schema format 0), in-tree read-only Rust SQLite B-tree reader; on-demand main-file pages up to 16 GiB with existing table/record bounds (WAL still capped at 1 GiB, issue #798); background inspection/import with progress/cancellation, per-open library identity checks; persistent result with destination library, offline-original counts and Show in Library, dismissed when switching libraries; offline Local records are promoted without probing missing files; originals in place, ratings/flags/labels, XMP, hierarchical keywords, collection sets/regular collections, virtual copies, mapped develop settings; stable reimport identities, undo, missing-file placeholders, a root folder not found at its recorded path looked up relative to the catalog (Lightroom's `relativePathFromCatalog`, so a project folder copied from another computer stays linked; reported as a warning), existing edits preserved. Best-effort deduplicated recovery archives retain settings/history/snapshots/collection rules (32 MiB each, eight archives/128 MiB aggregate); archive limits/write failures and optional-table limits warn without blocking photo import; unrelated unsupported tables and opaque SQL BLOBs skipped. Custom white balance on relative-WB raws (`RAW` in Lightroom's terms) stays As Shot and is reported, since the catalog keeps no as-shot white to shift from (issue #510). An image whose file, folder or root folder record is missing from the catalog, or whose original path is empty, is skipped with its virtual copies instead of failing the whole import; the report lists them as data (`skipped`: image id and reason) and the result window counts them in the UI language (PR #2230). A virtual copy whose master is gone is skipped as incomplete too, and contradictory records of a damaged catalog (a repeated image id, a virtual copy whose master is itself a copy) are skipped the same way, a keyword or collection hierarchy that loops is cut with a warning, and a repeated collection id or a capture time that isn't a date is left out with a warning; only a catalog that can't be read at all fails, before any change, with an error naming the file and what is wrong instead of "invalid parameters" (issue #1588). Adobe rendering, AI data and native history replay remain incomplete; no writes to Lightroom catalogs. Open Library given a Lightroom catalog folder, an `.lrdata` / `.lrcat-data` folder or the `.lrcat` makes no library there and offers this import with the catalog found (`cmd:app.openLibrary`, `crates/engine/src/lightroom_folder.rs`; issue #891) | | LRC-LIB-CATALOG-LOCK | Catalog open in one program at a time | P1 | ✅ | `cmd:app.openLibrary`, `crates/catalog/src/lock.rs`, `crates/engine/src/library.rs` (`open_library`) | exclusive OS lock on `catalog.lock` for the session (app and `lightcraft-cli --library`); a second opener is refused with who has it (`catalog.lock.owner`: program, process, computer); released by the OS on a crash, so never stale; file systems without locks open unprotected with a warning (the app's notices, `library.info` → `settingsWarnings`, and the CLI's stderr; issue #171); no read-only mode; the browser build (OPFS) has no guard across tabs yet. A library that can't be opened at launch (locked, unreadable, newer format, drive missing) shows a window — Try Again, Choose Another Library…, Continue Without Saving (then a banner; nothing written), Quit — instead of a silent demo session (`crates/ui-egui/src/panels/library_problem.rs`); the lock is released explicitly when the library closes, not by letting its file handle close, so a program another thread is starting at that moment (which holds a copy of every file handle until it is launched) cannot keep the library "already open" for a moment (issue #680) | | LRC-LIB-CATALOG-SWITCH | New / Open Recent catalog, catalog name shown | P1 | ✅ | `cmd:app.newLibrary`, `cmd:app.openLibrary`, `cmd:app.recentLibraries`, `cmd:app.clearRecentLibraries`, `crates/ui-egui/src/dialogs/settings.rs`, `crates/ui-egui/src/panels/left.rs`, `crates/ui-egui/src/panels/library_problem.rs` (`launch_plan`, `show_chooser`), `crates/ui-egui/src/tests/launch_library.rs` | File ▸ New Library… (name, then location), File ▸ Open Recent Library (last 8; missing ones greyed out, dropped when opened; Remove from List ▸ one entry, or `app.recentLibraries {remove}`, which forgets only the path; Clear), the open library's name under My Photos and in the window title (issue #902); Open/New refused during imports and their folder scans; a library that can't be opened leaves the current one and its reference pin intact; a successful switch clears the catalog-local reference pin; ⌘O (Ctrl+O on Windows and Linux) runs Open Library… and asks for a folder once (`crates/ui-egui/src/tests/keymap.rs`: `cmd_o_opens_the_library_picker_once`); a dialog or a text field with the focus keeps the key; Settings ▸ General ▸ At launch: Open the last library (default) or Ask me, a window listing the recent libraries (name, folder, when last opened; missing, unreadable or no-longer-library ones greyed out with the reason, checked read-only — nothing is locked or written, so a library another program has open is reported, with who has it, when Open is pressed; the last one selected; Open, Open Another…, New Library…, Remove from List, Quit; ↑ ↓ Return Esc), also shown once when Alt/Option is held in the first 2 s after the window appears (macOS reports an Option key held since before launch only once it changes); `--library` and `LIGHTCRAFT_LIBRARY` come first and `lightcraft-cli` never asks (issue #1290) | | LRC-LIB-FOLDERS | Disk folder tree | P1 | ✅ | `cmd:library.browse`, `cmd:folder.rename`, `cmd:folder.move`, `crates/ui-egui/src/panels/left.rs` (`local_section`, `folder_tree`) | standard places + the browsed folder + Browse Folder…, each an expandable tree of subfolders, breadcrumb navigation; context menu: Rename Folder…, Move Folder To…, Show in Finder (on disk, photos relinked; one undo step each: undo / redo move the folder back and forth, refused when the destination is taken) |@@ -799,6 +799,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---|+| 2026-10-27 | patch | LRC-LIB-CATALOG-IMPORT: a repeated image id or a virtual copy whose master is gone or is itself a copy is skipped (`skipped`: `duplicateId`, `missingMaster`, `masterIsCopy`), a looping keyword or collection hierarchy is cut with a warning, and a repeated collection id or a capture time that isn't a date is left out with a warning, instead of failing the whole import; an unreadable or damaged catalog fails with an error naming the file, not "invalid parameters" (#1588); status unchanged (🟡) | | 2026-10-25 | patch | LR-BEHAV-TITLEBAR: the top bar's title-bar gestures come from craft-window's `title_bar`: a double-click follows the system setting, and a press on a button, the search field or a panel floating over the bar never moves the window; status counts unchanged | | 2026-10-24 | minor | KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 (`keyword.nextSet`, `keyword.previousSet`) go to the next / previous keyword set; status counts unchanged (KEYC-METADATA stays 🟡) | | 2026-10-24 | minor | KEYC-METADATA: ⌘K / Ctrl+K (`panel.addKeywords`, Add Keywords) shows the Keywords panel, opens Keywording and puts the cursor in the Add keyword box; status counts unchanged (KEYC-METADATA stays 🟡) |@@ -1,5 +1,25 @@ # What's new in LightCraft +## 2026-10-27++### A damaged Lightroom catalog imports everything it can+- A Lightroom catalog with a few damaged records (two images with the same id, a virtual copy whose master+ is gone or is itself a copy, keywords or collections whose parents loop) no longer fails the whole import: those+ images are skipped and counted in the result window, and a looping keyword or collection is imported with a+ warning. So is a second collection with the id of another (its photos go to the first), and a capture time that+ isn't a date is left out with a warning. A catalog that can't be read at all now says which file and what is wrong ("the Lightroom catalog … is+ damaged: table Adobe_images can't be read: …") instead of "invalid parameters".++## 2026-10-26++### Lightroom catalogs import into libraries on exFAT drives+- Importing a Lightroom catalog into a library on an exFAT or FAT drive (or a network share without hard links) no+ longer fails with "cannot atomically publish archive: Operation not supported": the recovery archive in the+ library's `Interop` folder is saved another way that still never replaces an existing archive. When the archive+ can't be saved at all, the import goes on and warns, naming the folder, as documented. An `Interop` folder that can't+ be written to at all (or is a link) is refused before any photo is imported, naming it, since the import index must+ be saved there; a failed import-index write now names the file instead of reporting "invalid parameters".+ ## 2026-10-24 ### Relative-date filters and smart albums follow the clock