mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-14 19:27:01 -04:00
Fix #171: a library that can't be locked opens with a warning the user sees
When catalog.lock can't be locked (a file system without locks, such as some network shares), the library opens unprotected, as documented, but the promised warning went only to the log. It is now the first of the library's warnings: library.info lists it under settingsWarnings, the app shows it once in its notices, and lightcraft-cli run / mcp print the library's warnings on stderr (stdout stays results / protocol only). A normally locked library shows none. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01912WaK6Rmew3wxZuPyX7E8
This commit is contained in:
4 files changed
+51
-3
No files matched your search
@@ -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(),
|
||||
|
||||
@@ -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<String> {
|
||||
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<String> {
|
||||
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()
|
||||
})
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
+1
-1
@@ -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/<host>`), NFS (`/Network/Servers/<host>`), 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 +) |
|
||||
|
||||
Reference in new issue
Block a user