diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs index d978641f..6c5e6e44 100644 --- a/apps/lightcraft-cli/src/main.rs +++ b/apps/lightcraft-cli/src/main.rs @@ -102,6 +102,14 @@ static ALLOC: dhat::Alloc = dhat::Alloc; /// Why `--library DIR` can't be opened; for a library open in another program, how to work with /// that one instead (issue #99). +/// Print the open library's warnings (an unlockable library, damaged settings files) on stderr: +/// stdout carries command results / MCP protocol messages only. +fn library_warnings(session: &mut Session, who: &str) { + for w in session.take_library_warnings() { + eprintln!("{who}: warning: {w}"); + } +} + fn library_error(dir: &str, e: lightcraft_engine::EngineError) -> String { match e { lightcraft_engine::EngineError::LibraryInUse(why) => format!( @@ -317,6 +325,7 @@ fn mcp(args: &[String]) -> Result<(), String> { let mut h = Headless::default(); let r = h.session.open_library(dir, demo).map_err(|e| library_error(dir, e))?; eprintln!("lightcraft-cli mcp: opened library {dir} ({r:?})"); + library_warnings(&mut h.session, "lightcraft-cli mcp"); h } None if demo => Headless::demo(), @@ -553,6 +562,7 @@ fn run(args: &[String]) -> Result<(), String> { Some(dir) => { let mut h = Headless::default(); h.session.open_library(dir, demo).map_err(|e| library_error(dir, e))?; + library_warnings(&mut h.session, "lightcraft-cli"); h } None if demo => Headless::demo(), diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index 3e89b824..7f902843 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -395,7 +395,7 @@ impl Session { forgot_local: None, presets_written, view_written, - settings_warnings: settings.warnings, + settings_warnings: unlocked_warning(lock.as_ref()).into_iter().chain(settings.warnings).collect(), warnings_reported: 0, blocked: settings.blocked, lock: lock.take(), @@ -570,7 +570,8 @@ impl Session { lib.files.write_atomic("prefs.json", &v).map_err(|e| EngineError::Other(format!("prefs: {e}"))) } - /// Settings-file warnings of the open library not handed out yet (the UI shows each once). + /// Warnings about the open library not handed out yet: its settings files, and a lock that + /// couldn't be taken (the UI shows each once; the CLI and MCP print them on stderr). pub fn take_library_warnings(&mut self) -> Vec { let Some(lib) = self.library.as_mut() else { return vec![] }; let new = lib.settings_warnings[lib.warnings_reported..].to_vec(); @@ -625,3 +626,15 @@ impl Session { Ok(()) } } + +/// The warning for a library opened without its one-program-at-a-time lock (issue #171): its +/// file system can't lock `catalog.lock` (some network shares). It opens anyway, as documented — +/// refusing would lock the user out — but the user must learn that a second program could open it. +fn unlocked_warning(lock: Option<&LibraryLock>) -> Option { + lock.filter(|l| !l.held()).map(|_| { + "This library could not be locked (its catalog.lock file can't be locked where it is stored, e.g. on some network \ + shares), so it is open without protection against a second program: use it in one LightCraft app or command at \ + a time, or changes made in one of them can be lost." + .to_string() + }) +} diff --git a/crates/engine/src/tests_settings_files.rs b/crates/engine/src/tests_settings_files.rs index ddc81f20..f9f24294 100644 --- a/crates/engine/src/tests_settings_files.rs +++ b/crates/engine/src/tests_settings_files.rs @@ -95,3 +95,28 @@ fn unreadable_settings_are_not_overwritten() { assert_eq!(files.files.get("presets.json").unwrap(), presets); assert_eq!(files.files.get("prefs.json").unwrap(), prefs); } + +/// Issue #171: a library whose `catalog.lock` can't be locked (stood in for here by a directory +/// of that name, which takes the same path as a file system without locks) still opens, and the +/// user is told it is unprotected — in `library.info` and once through the library warnings the +/// app's notices and the CLI show. A normally locked library says nothing. +#[test] +fn an_unlockable_library_opens_with_a_warning() { + let lib = temp_dir("unlocked"); + std::fs::create_dir_all(lib.join("catalog.lock")).unwrap(); + let mut s = Session::new().with_fs(); + s.open_library(&lib, false).unwrap(); + let info = s.execute("library.info", &json!({})).unwrap(); + let listed = info["settingsWarnings"].as_array().cloned().unwrap_or_default(); + assert!(listed.iter().any(|w| w.as_str().is_some_and(|w| w.contains("without protection against a second program"))), "{info}"); + let warnings = s.take_library_warnings(); + assert_eq!(warnings.len(), 1, "{warnings:?}"); + drop(s); + // the same library with a lockable lock file: no warning + std::fs::remove_dir_all(lib.join("catalog.lock")).unwrap(); + let mut s = Session::new().with_fs(); + s.open_library(&lib, false).unwrap(); + assert!(s.take_library_warnings().is_empty()); + drop(s); + let _ = std::fs::remove_dir_all(&lib); +} diff --git a/docs/parity.md b/docs/parity.md index 41c160ce..5054c113 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -729,7 +729,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 | | LRC-LIB-CATALOG-PANEL | Catalog sets | P1 | ✅ | `cmd:library.source` (`missing`) | all, recently added, picks, recently deleted, missing photos (when any) | -| 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; 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`) | +| 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`) | | 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) | | LRC-LIB-LIBFOLDERS | Library folders | P1 | ✅ | `cmd:library.folders`, `cmd:library.source` (`libraryFolder`), `cmd:library.filter` (`libraryFolder`), `cmd:library.removeFolder`, `cmd:folder.rename`, `cmd:folder.move`, `crates/catalog/src/folders.rs`, `crates/ui-egui/src/panels/left.rs` (`folders_section`, `folder_menu_for_library`, `row_menu`), `crates/ui-egui/src/tests_panels.rs` | sidebar Folders: the folders the library's photos were imported from, with photo counts (subfolders included), as another way to explore the library next to By Date and Keywords; Local stays the way to browse any folder on disk. Each disk is a row of its own and starts open: a drive letter, a Windows share (`\\server\share`, `\\?\UNC\…`, `\\wsl$\…`), a macOS share or disk (`/Volumes/tokyo`), a Linux mount (`/media`, `/run/media`, `/mnt`, gvfs `smb-share:…`, `/net/`), NFS (`/Network/Servers/`), else “This Computer” — told apart from the stored paths alone, so an offline share lists like a mounted one and the sidebar never waits on a network. Only folders that hold imported photos are listed (browsed, deleted and demo photos don't count); every folder is a row of its own (`Users` → `me` → `Pictures`), and a folder that holds nothing itself and leads to one folder starts open; every level indents one step more than its parent however deep it goes: the whole sidebar (Albums, Local, By Date, Folders, Keywords) scrolls sideways when its widest row is wider than the panel (rows are measured one frame behind; Create Album and the counts stay at the visible edge); a name that does not fit ends in an ellipsis at the count (the full name on hover; a row asks for room for at most 140 pt of its name, so depth, not one long name, makes the sidebar scroll), as does the grid title; selection and hover bars end at the panel's edge; click makes the folder the source, like an album or a Local folder (its photos and those of the folders inside it fill the grid, titled “photos/travel”; the filter bar narrows it further; `library.source` `libraryFolder` + `path`; kept with the library's view), and its row is always opened into view; the shown folder follows a rename or move and their undo (engine-tested; the sidebar test covers rename), is dropped back to All Photos when it is removed or no longer holds a photo, and a smart album made from the view keeps it, also when its rules are edited (the editor has no folder field). Only a folder that shows exactly its own photos can be shown (not the startup disk, not `/Volumes`-like folders above disks, not an empty one). A row that would cover other disks' photos too (the startup disk, `/Volumes`, `/mnt`…) only opens. Right-click (row or triangle): Rename Folder…, Move Folder To…, Show in Finder (on disk, photos relinked, as in Local) and Remove from Library… (asks; moves the folder's photos to Recently Deleted, one undo step, no file touched); a disk row offers Remove Disk from Library… (the command needs `disk: true` for a whole disk or a folder spanning several). Limits: names are compared as written (macOS `/Volumes/tokyo` vs `/Volumes/TOKYO` are two disks; `/Volumes/Macintosh HD/…` and `/…` two rows); no disk capacity / online status; no “include subfolders” switch (a folder always includes the ones inside it); creating a folder and moving photos between folders are not here | | LRC-LIB-COLLECTIONS | Collections & sets | P1 | ✅ | `cmd:album.create`, `cmd:album.createSmart`, `cmd:album.toggleTarget`, `cmd:album.setTarget`, `cmd:album.clearQuick` | albums + folders (sets), smart albums (rule editor; rules incl. File Path — the whole string, either separator, case-insensitive), Quick Collection and target album (B in the grids adds / removes the selection; album menu ▸ Set as Target Album; the target is marked +) |