Merge PR #2979: LRC-LIB-CATALOG-SWITCH: a library's catalog.snap or folder given at launch opens that library

docs/parity-checklist.md: header keeps main's newer date plus the PR's item; revision rows union.

apps/lightcraft/src/launch.rs

@@ -3,7 +3,8 @@ use super::prefs::config_dir;
 pub(crate) const HELP: &str = "\
 lightcraft — photo library + raw developer
 
-USAGE: lightcraft [OPTIONS] [FILES/FOLDERS…]   (files and folders are imported)
+USAGE: lightcraft [OPTIONS] [FILES/FOLDERS…]   (files and folders are imported; a library's catalog.snap or
+                                               folder opens that library)
 
 OPTIONS:
   --library DIR    open (or create) this library (env LIGHTCRAFT_LIBRARY; default: the last library opened, or
@@ -30,17 +31,48 @@ ENVIRONMENT:
 
 /// The launch arguments' files and folders to import (absolute), and a notice for each one that
 /// is not imported (see [`lightcraft_engine::import::launch_path`]; issue #374).
-pub(crate) fn launch_imports(args: &[String]) -> (Vec<String>, Vec<String>) {
+pub(crate) fn launch_imports(args: &[String]) -> LaunchArgs {
     let cwd = std::env::current_dir().ok();
     let home = std::env::var_os(if cfg!(windows) { "USERPROFILE" } else { "HOME" }).filter(|h| !h.is_empty()).map(std::path::PathBuf::from);
     launch_imports_in(args, cwd.as_deref(), home.as_deref())
 }
 
-pub(crate) fn launch_imports_in(args: &[String], cwd: Option<&std::path::Path>, home: Option<&std::path::Path>) -> (Vec<String>, Vec<String>) {
-    use lightcraft_engine::import::{LaunchPath, launch_path};
-    let (mut files, mut notices) = (Vec::new(), Vec::new());
+/// The library a launch argument names, after `--library` (`flag`) and `LIGHTCRAFT_LIBRARY` (`env`):
+/// the library to pass on as the flag, and whether the argument chose it (it is then remembered).
+pub(crate) fn launch_library_choice(
+    flag: Option<std::path::PathBuf>,
+    env: Option<&std::path::PathBuf>,
+    argument: Option<std::path::PathBuf>,
+) -> (Option<std::path::PathBuf>, bool) {
+    match (flag, env, argument) {
+        (Some(flag), _, _) => (Some(flag), false),
+        (None, None, Some(argument)) => (Some(argument), true),
+        _ => (None, false),
+    }
+}
+
+/// What the launch arguments ask for: files and folders to import, a library to open (a library's `catalog.snap`
+/// or folder, double-clicked in the file manager), and a notice for each argument
+/// left alone.
+#[derive(Debug, Default, PartialEq)]
+pub(crate) struct LaunchArgs {
+    pub(crate) files: Vec<String>,
+    pub(crate) library: Option<std::path::PathBuf>,
+    pub(crate) notices: Vec<String>,
+}
+
+pub(crate) fn launch_imports_in(args: &[String], cwd: Option<&std::path::Path>, home: Option<&std::path::Path>) -> LaunchArgs {
+    use lightcraft_engine::import::{LaunchPath, launch_deadline, launch_path_until};
+    let (mut files, mut notices, mut library) = (Vec::new(), Vec::new(), None);
+    // one deadline for every argument's library check: they all run before the window opens
+    let deadline = launch_deadline();
     for a in args {
-        match launch_path(a, cwd, home, lightcraft_engine::devices::dcim_in) {
+        match launch_path_until(a, cwd, home, lightcraft_engine::devices::dcim_in, deadline) {
+            Some(LaunchPath::Library(dir)) if library.is_none() => library = Some(std::path::PathBuf::from(dir)),
+            Some(LaunchPath::Library(dir)) => {
+                log::warn!("one library at a time: not opening {dir} given at launch");
+                notices.push(format!("{dir} was not opened: LightCraft opens one library at a time. Open it with File ▸ Open Library…"));
+            }
             Some(LaunchPath::Import(p)) => files.push(p),
             Some(LaunchPath::Refused { path, why }) => {
                 log::warn!("not importing {path} given at launch: {why}");
@@ -50,7 +82,7 @@ pub(crate) fn launch_imports_in(args: &[String], cwd: Option<&std::path::Path>,
             None => {}
         }
     }
-    (files, notices)
+    LaunchArgs { files, library, notices }
 }
 
 /// Where the log files live: `logs` in the settings folder, next to `ui.json` (see `logging`).

apps/lightcraft/src/main.rs

@@ -55,7 +55,10 @@ use gpu_crash::{gpu_crash_check, gpu_lost_check_at, gpu_lost_marker_path, gpu_lo
 use gpu_crash::{gpu_crash_check_at, gpu_crash_notice, window_wgpu_options};
 #[cfg(test)]
 use launch::launch_imports_in;
-use launch::{HELP, control_port_from, launch_imports, log_dir, panic_log_path_with, startup_failed, stopped_text, stopped_while_running};
+use launch::{
+    HELP, LaunchArgs, control_port_from, launch_imports, launch_library_choice, log_dir, panic_log_path_with, startup_failed, stopped_text,
+    stopped_while_running,
+};
 #[cfg(test)]
 use platform::{explorer_select_arg, explorer_select_command, windows_open_url_command};
 use prefs::{PrefsWriter, as_remembered, load_prefs, prefs_path};
@@ -198,7 +201,11 @@ fn main() -> eframe::Result {
     }
     // what the arguments name, resolved now: an empty or relative argument never becomes a drive
     // root, and a whole drive or home folder is not walked (issue #374)
-    let (files, launch_notices) = launch_imports(&files);
+    let LaunchArgs { files, library: launch_library, notices: launch_notices } = launch_imports(&files);
+    // a library double-clicked (its catalog.snap or folder) is the one that opens, not the last one
+    // used; --library and LIGHTCRAFT_LIBRARY still win
+    let env_library: Option<std::path::PathBuf> = std::env::var_os("LIGHTCRAFT_LIBRARY").filter(|p| !p.is_empty()).map(Into::into);
+    let (library_dir, opened_by_argument) = launch_library_choice(library_dir, env_library.as_ref(), launch_library);
     let (prefs, prefs_warning, keep_prefs_file) = load_prefs(in_memory);
     // the saved language from the start, so a failed start is reported in it too
     if let Some(ui) = &prefs {
@@ -209,7 +216,7 @@ fn main() -> eframe::Result {
     let launch = lightcraft_ui_egui::panels::library_problem::launch_plan(
         in_memory,
         library_dir,
-        std::env::var_os("LIGHTCRAFT_LIBRARY").filter(|p| !p.is_empty()).map(Into::into),
+        env_library,
         prefs.as_ref().map(|u| &u.settings),
         lightcraft_engine::library::default_dir(),
     );
@@ -301,6 +308,20 @@ fn main() -> eframe::Result {
                     keep_file: keep_prefs_file,
                     ..Default::default()
                 };
+                // a library opened from its catalog file is the one in use from now on, as Open Library makes it
+                // (after the snapshot above, so the writer saves it)
+                if opened_by_argument
+                    && problem.is_none()
+                    && let Some(dir) = app.session.library.as_ref().filter(|l| l.on_disk).map(|l| l.dir.to_string_lossy().into_owned())
+                {
+                    let settings = &mut app.ui.settings;
+                    if settings.library_path != dir && !settings.library_path.is_empty() {
+                        let previous = settings.library_path.clone();
+                        lightcraft_ui_egui::panels::settings::remember_library(&mut settings.recent_libraries, &previous);
+                    }
+                    lightcraft_ui_egui::panels::settings::remember_library(&mut settings.recent_libraries, &dir);
+                    settings.library_path = dir;
+                }
                 // after the snapshot above, so the writer saves the switched-off preference
                 if let Some(notice) = gpu_crash {
                     app.ui.settings.gpu = false;

apps/lightcraft/src/tests.rs

@@ -14,17 +14,57 @@ fn launch_arguments_never_walk_a_drive_or_the_home_folder() {
     let home = dir("launch");
     std::fs::create_dir_all(home.join("Trip")).unwrap();
     let args: Vec<String> = ["", "  ", ".", "Trip", &home.to_string_lossy()].iter().map(|s| s.to_string()).collect();
-    let (files, notices) = launch_imports_in(&args, Some(&home), Some(&home));
+    let LaunchArgs { files, notices, library } = launch_imports_in(&args, Some(&home), Some(&home));
+    assert_eq!(library, None);
     assert_eq!(files, [home.join("Trip").to_string_lossy().to_string()]);
     assert_eq!(notices.len(), 2, "{notices:?}");
     assert!(notices.iter().all(|n| n.contains("was not imported")), "{notices:?}");
     // the working folder is a drive root (a launcher's choice): `.` is never the whole drive
     let root = home.ancestors().last().unwrap().to_path_buf();
-    let (files, notices) = launch_imports_in(&[".".to_string()], Some(&root), None);
+    let LaunchArgs { files, notices, .. } = launch_imports_in(&[".".to_string()], Some(&root), None);
     assert!(files.iter().all(|f| f.ends_with("DCIM")) && files.len() + notices.len() == 1, "{files:?} {notices:?}");
     let _ = std::fs::remove_dir_all(&home);
 }
 
+/// `--library` and `LIGHTCRAFT_LIBRARY` win over a library given as an argument, which is
+/// then not remembered either.
+#[test]
+fn the_library_flag_and_variable_win_over_a_library_argument() {
+    use std::path::PathBuf;
+    let (flag, env, arg) = (PathBuf::from("flag"), PathBuf::from("env"), PathBuf::from("arg"));
+    assert_eq!(launch_library_choice(Some(flag.clone()), Some(&env), Some(arg.clone())), (Some(flag.clone()), false));
+    assert_eq!(launch_library_choice(None, Some(&env), Some(arg.clone())), (None, false), "the variable is passed on as itself");
+    assert_eq!(launch_library_choice(None, None, Some(arg.clone())), (Some(arg), true));
+    assert_eq!(launch_library_choice(None, None, None), (None, false));
+    assert_eq!(launch_library_choice(Some(flag.clone()), None, None), (Some(flag), false));
+}
+
+/// A library's catalog.snap (double-clicked in Explorer) or its folder given at
+/// launch opens that library and is never imported as photos; a second library is left with a notice.
+#[test]
+fn a_librarys_catalog_given_at_launch_opens_that_library() {
+    let home = dir("launch-library");
+    let lib = home.join("Wedding LC");
+    let mut s = lightcraft_engine::Session::new().with_fs();
+    s.open_library(&lib, false).unwrap();
+    s.close_library().unwrap();
+    let snap = lib.join("catalog.snap");
+    assert!(snap.is_file(), "a library keeps its catalog.snap");
+    let other = home.join("Other LC");
+    let mut s = lightcraft_engine::Session::new().with_fs();
+    s.open_library(&other, false).unwrap();
+    s.close_library().unwrap();
+    for arg in [snap.to_string_lossy().to_string(), lib.to_string_lossy().to_string(), "Wedding LC/catalog.snap".to_string()] {
+        let got = launch_imports_in(std::slice::from_ref(&arg), Some(&home), Some(&home.join("elsewhere")));
+        assert_eq!(got.library.as_deref(), Some(lib.as_path()), "{arg}");
+        assert!(got.files.is_empty() && got.notices.is_empty(), "{arg}: never imported: {got:?}");
+    }
+    let got = launch_imports_in(&[snap.to_string_lossy().to_string(), other.to_string_lossy().to_string()], Some(&home), None);
+    assert_eq!(got.library.as_deref(), Some(lib.as_path()), "the first one");
+    assert_eq!(got.notices.len(), 1, "{got:?}");
+    let _ = std::fs::remove_dir_all(&home);
+}
+
 /// Issue #236: the window answers control requests on its UI thread, so the desktop session
 /// adds to Apple Photos on a worker (headless sessions wait instead).
 #[test]

crates/engine/src/import.rs

@@ -45,7 +45,9 @@ mod types;
 pub(crate) use commit::workers;
 pub use commit::{batch_size, civil, commit_prepared, system_clock};
 pub use defaults::{CameraDefault, ImportDefaults, apply_import_defaults, has_import_look, import_defaults};
-pub use files::{EXTENSIONS, LaunchPath, accept_list, expand, expand_within, is_supported, launch_path};
+pub use files::{
+    EXTENSIONS, LAUNCH_PROBE_LIMIT, LaunchPath, accept_list, expand, expand_within, is_supported, launch_deadline, launch_path, launch_path_until,
+};
 pub(crate) use files::{expand_noting, probe_all, probe_paths, probe_with};
 #[cfg(test)]
 use job::PreparedItem;

crates/engine/src/import/files.rs

@@ -121,6 +121,53 @@ pub enum LaunchPath {
     /// Not imported: `path` (absolute) is a whole drive or the whole home folder, which is never
     /// walked without being chosen in the app (issue #374).
     Refused { path: String, why: String },
+    /// Open this (absolute) library folder: the argument was a library's `catalog.snap` or its folder, as a
+    /// double-click on a catalog in the file manager gives. Never imported.
+    Library(String),
+}
+
+/// The library folder `path` names: a library's `catalog.snap` (its folder), or a folder holding one. `None` for
+/// anything else (photos, folders of photos). The folder must pass the library opener's own test
+/// ([`crate::library::is_library`]), so a `CATALOG.SNAP` the opener would not read (a case-sensitive file system)
+/// never opens its folder as a new, empty library.
+pub fn library_dir_of(path: &Path) -> Option<PathBuf> {
+    let snapshot = lightcraft_catalog::journal::SNAPSHOT;
+    let named = path.file_name().is_some_and(|n| n.to_string_lossy().eq_ignore_ascii_case(snapshot));
+    if named && path.is_file() {
+        return path.parent().filter(|dir| crate::library::is_library(dir)).map(Path::to_path_buf);
+    }
+    (path.is_dir() && crate::library::is_library(path)).then(|| path.to_path_buf())
+}
+
+/// How long the launch arguments' library checks may take, all of them together, before LightCraft gives up and
+/// treats the arguments not yet checked as photos to import (in the background). A path on a dead network mount
+/// can block a file-system query for minutes, and these checks run before the window opens.
+pub const LAUNCH_PROBE_LIMIT: std::time::Duration = std::time::Duration::from_secs(2);
+
+/// When the launch arguments' library checks, starting now, must be done: [`LAUNCH_PROBE_LIMIT`] from now.
+pub fn launch_deadline() -> std::time::Instant {
+    let now = std::time::Instant::now();
+    now.checked_add(LAUNCH_PROBE_LIMIT).unwrap_or(now)
+}
+
+/// `probe`'s answer, or `None` when it takes longer than `limit`: it runs on its own thread, which is left to
+/// finish (or stay blocked) on its own. Also `None` when no thread can be started.
+pub(crate) fn within<T: Send + 'static>(limit: std::time::Duration, probe: impl FnOnce() -> Option<T> + Send + 'static) -> Option<T> {
+    let (tx, rx) = std::sync::mpsc::channel();
+    let started = std::thread::Builder::new().name("launch-probe".into()).spawn(move || {
+        let _ = tx.send(probe());
+    });
+    if let Err(e) = started {
+        log::warn!("could not start the launch-path check: {e}");
+        return None;
+    }
+    match rx.recv_timeout(limit) {
+        Ok(found) => found,
+        Err(_) => {
+            log::warn!("the launch-path check took longer than {limit:?}; treating the argument as photos to import");
+            None
+        }
+    }
 }
 
 /// Resolve a launch argument for importing (issue #374). An empty argument is ignored (`None`);
@@ -130,6 +177,19 @@ pub enum LaunchPath {
 /// or file-system root is replaced by its camera folder (`DCIM`, from `dcim`: a memory card) or
 /// refused, and so is the home folder `home`: either would walk most of the disk.
 pub fn launch_path(arg: &str, cwd: Option<&Path>, home: Option<&Path>, dcim: impl Fn(&Path) -> Option<PathBuf>) -> Option<LaunchPath> {
+    launch_path_until(arg, cwd, home, dcim, launch_deadline())
+}
+
+/// [`launch_path`], with its library check bounded by `deadline` (from [`launch_deadline`]), which every argument
+/// of one launch shares: several paths on a dead mount wait [`LAUNCH_PROBE_LIMIT`] in all, not each. Once it has
+/// passed, an argument is imported without the check.
+pub fn launch_path_until(
+    arg: &str,
+    cwd: Option<&Path>,
+    home: Option<&Path>,
+    dcim: impl Fn(&Path) -> Option<PathBuf>,
+    deadline: std::time::Instant,
+) -> Option<LaunchPath> {
     if arg.trim().is_empty() {
         return None;
     }
@@ -164,6 +224,13 @@ pub fn launch_path(arg: &str, cwd: Option<&Path>, home: Option<&Path>, dcim: imp
     if home.is_some_and(|h| key(&h.to_string_lossy()) == key(&shown)) {
         return Some(LaunchPath::Refused { path: shown, why: "the whole home folder".into() });
     }
+    let left = deadline.saturating_duration_since(std::time::Instant::now());
+    let probed = abs.clone();
+    if !left.is_zero()
+        && let Some(dir) = within(left, move || library_dir_of(&probed))
+    {
+        return Some(LaunchPath::Library(dir.to_string_lossy().to_string()));
+    }
     Some(LaunchPath::Import(shown))
 }
 
@@ -363,6 +430,67 @@ mod prepared_tests {
     use super::*;
     use lightcraft_catalog::{Op, Photo, PhotoId, Source};
 
+    fn temp_dir(tag: &str) -> PathBuf {
+        let d = std::env::temp_dir().join(format!("lc-launch-{tag}-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&d);
+        std::fs::create_dir_all(&d).unwrap();
+        d
+    }
+
+    /// A launch argument names a library only when the library opener would read that folder: a `CATALOG.SNAP`
+    /// on a case-sensitive file system (no `catalog.snap` or `catalog.log` beside it) is not one.
+    #[test]
+    fn a_launch_argument_names_a_library_only_when_the_opener_can_read_it() {
+        let dir = temp_dir("upper");
+        let upper = dir.join("CATALOG.SNAP");
+        std::fs::write(&upper, b"x").unwrap();
+        let found = library_dir_of(&upper);
+        assert_eq!(found.is_some(), crate::library::is_library(&dir), "{found:?}");
+        if let Some(d) = &found {
+            assert!(crate::library::is_library(d), "{} opened as a library the opener can't read", d.display());
+        }
+        // a folder holding only the journal is a library to the opener, so it is one here too
+        let journal = temp_dir("journal");
+        std::fs::write(journal.join(lightcraft_catalog::journal::LOG), b"x").unwrap();
+        assert_eq!(library_dir_of(&journal), Some(journal.clone()));
+        // a folder of photos is not
+        let photos = temp_dir("photos");
+        std::fs::write(photos.join("a.jpg"), b"x").unwrap();
+        assert_eq!(library_dir_of(&photos), None);
+        for d in [dir, journal, photos] {
+            let _ = std::fs::remove_dir_all(d);
+        }
+    }
+
+    /// The library check runs before the window opens; one that blocks (a dead network mount) gives up after the
+    /// limit and the argument is imported instead, so LightCraft still starts.
+    #[test]
+    fn a_blocked_launch_probe_gives_up_after_its_limit() {
+        let start = std::time::Instant::now();
+        let found = within(std::time::Duration::from_millis(50), || {
+            std::thread::sleep(std::time::Duration::from_secs(5));
+            Some(PathBuf::from("late"))
+        });
+        assert_eq!(found, None);
+        assert!(start.elapsed() < std::time::Duration::from_secs(2), "waited {:?}", start.elapsed());
+        assert_eq!(within(LAUNCH_PROBE_LIMIT, || Some(1)), Some(1), "a quick answer is kept");
+    }
+
+    /// The launch arguments share one deadline (CodeRabbit on #2979): once it has passed, an argument is imported
+    /// without its library check, so several paths on a dead mount can't each wait the limit.
+    #[test]
+    fn launch_arguments_share_one_deadline() {
+        let dir = temp_dir("deadline");
+        std::fs::write(dir.join(lightcraft_catalog::journal::LOG), b"x").unwrap();
+        let arg = dir.to_string_lossy().to_string();
+        let none = |_: &Path| None;
+        assert_eq!(launch_path_until(&arg, None, None, none, launch_deadline()), Some(LaunchPath::Library(arg.clone())));
+        let start = std::time::Instant::now();
+        assert_eq!(launch_path_until(&arg, None, None, none, start), Some(LaunchPath::Import(arg.clone())), "deadline passed");
+        assert!(start.elapsed() < std::time::Duration::from_millis(500), "waited {:?}", start.elapsed());
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
     /// Extensions the picker may offer: every one in [`EXTENSIONS`], minus the
     /// files this build cannot import (see [`accept_list`]).
     fn pickable_extensions() -> Vec<&'static str> {