diff --git a/crates/update/src/dialog.rs b/crates/update/src/dialog.rs index 1b1759d2..5db9e1c8 100644 --- a/crates/update/src/dialog.rs +++ b/crates/update/src/dialog.rs @@ -35,6 +35,9 @@ pub struct StartupPrompt { /// [`StartupPrompt::set_note`] and [`StartupPrompt::set_message`] keep only one. pub status_note: Option, pub is_installing: bool, + /// The update is ready and waits for the app to close: the main button reads "Restart to + /// Update" and answers [`StartupPromptResponse::UpdateAndRestart`]. + pub is_ready: bool, /// Puts the card's words in the interface language ([`crate::translate`]); English by default. pub translate: Translate, } @@ -52,6 +55,7 @@ impl StartupPrompt { status_message: None, status_note: None, is_installing: false, + is_ready: false, translate: english, } } @@ -102,7 +106,7 @@ impl StartupPrompt { } let pal = Palette::new(ctx, self.app); - let Palette { is_dark, card_bg, border_color, text_primary, text_muted, fill, on_fill } = pal; + let Palette { card_bg, border_color, text_primary, text_muted, fill, on_fill, alert, alert_bg, .. } = pal; let t = self.translate; let version = self.release.version_label(); @@ -126,16 +130,16 @@ impl StartupPrompt { ui.add_space(8.0); if let Some(msg) = self.message() { - let alert_bg = if is_dark { Color32::from_rgb(50, 30, 20) } else { Color32::from_rgb(255, 242, 235) }; - let alert_color = Color32::from_rgb(225, 95, 30); egui::Frame::new() .fill(alert_bg) - .stroke(egui::Stroke::new(1.0, alert_color)) + .stroke(egui::Stroke::new(1.0, alert)) .corner_radius(egui::CornerRadius::ZERO) .inner_margin(egui::Margin::symmetric(10, 6)) .show(ui, |ui| { ui.set_width(ui.available_width()); - ui.label(RichText::new(msg).size(12.0).color(alert_color)); + bounded(ui, "craft_update.prompt_message", |ui| { + ui.label(RichText::new(msg).size(12.0).color(alert)); + }); }); ui.add_space(4.0); } @@ -190,13 +194,13 @@ impl StartupPrompt { response = StartupPromptResponse::Later; } } else { - if self.is_self_updating { - let update_btn = egui::Button::new( - RichText::new(button(t, "Update and Restart")).size(10.5).strong().monospace().color(on_fill), - ) - .fill(fill) - .corner_radius(egui::CornerRadius::ZERO) - .min_size(Vec2::new(140.0, 26.0)); + if self.is_self_updating || self.is_ready { + let label = if self.is_ready { "Restart to Update" } else { "Update and Restart" }; + let update_btn = + egui::Button::new(RichText::new(button(t, label)).size(10.5).strong().monospace().color(on_fill)) + .fill(fill) + .corner_radius(egui::CornerRadius::ZERO) + .min_size(Vec2::new(140.0, 26.0)); if ui.add(update_btn).clicked() { response = StartupPromptResponse::UpdateAndRestart; } @@ -243,6 +247,10 @@ struct Palette { fill: Color32, /// Text on `fill`. on_fill: Color32, + /// Errors and failure reasons, on the card and on `alert_bg`. + alert: Color32, + /// The box around a message in the prompt. + alert_bg: Color32, } impl Palette { @@ -257,6 +265,8 @@ impl Palette { text_muted: if is_dark { Color32::from_rgb(150, 150, 160) } else { Color32::from_rgb(105, 105, 115) }, fill, on_fill: ink_on(fill), + alert: if is_dark { Color32::from_rgb(225, 95, 30) } else { Color32::from_rgb(184, 66, 8) }, + alert_bg: if is_dark { Color32::from_rgb(44, 26, 18) } else { Color32::from_rgb(255, 242, 235) }, } } } @@ -352,18 +362,24 @@ pub enum CheckResultResponse { None, /// OK, the close button, Esc or a click outside: the app stops showing the dialog. Close, - /// Failed state only: the app starts another requested check. + /// Failed states only: the app starts another requested check. CheckAgain, + /// Ready state only: the app calls `Updater::restart_to_update`, which asks it to close. + RestartToUpdate, } /// The result of a manual "Check for Updates", in the same card as [`StartupPrompt`]: a spinner -/// while checking, "newest version" when up to date, the error and a retry when it failed. +/// while checking, "newest version" when up to date, the error and a retry when it failed, an +/// update installing or ready, and the launcher's choice when it manages the version. It also +/// shows what the last update came to at the first start after it. /// /// An app shows it while a requested check is in view: open it when the user starts a check -/// (`Updater::check_now`), call [`show`](Self::show) every frame with `updater.status()` until it -/// returns [`CheckResultResponse::Close`], then stop calling it. On -/// [`CheckResultResponse::CheckAgain`] start another requested check and keep showing it. An -/// available update goes to [`StartupPrompt`] instead; the app closes this dialog when the prompt opens. +/// (`Updater::check_now`) or when `updater.status()` is not [`Status::Idle`] after the check at +/// start, call [`show`](Self::show) every frame with `updater.status()` until it returns +/// [`CheckResultResponse::Close`], then call `Updater::dismiss_status` and stop calling it. On +/// [`CheckResultResponse::CheckAgain`] start another requested check and keep showing it; on +/// [`CheckResultResponse::RestartToUpdate`] call `Updater::restart_to_update`. An available update +/// goes to [`StartupPrompt`] instead; the app closes this dialog when the prompt opens. #[derive(Clone, Copy, Debug)] pub struct CheckResultDialog { app: &'static App, @@ -384,14 +400,15 @@ impl CheckResultDialog { self } - /// Draws the modal card for `status` and returns what the user chose. [`Status::Idle`] draws - /// nothing and returns [`CheckResultResponse::None`]. + /// Draws the card for `status` and returns what the user chose. [`Status::Idle`] draws + /// nothing and returns [`CheckResultResponse::None`]. The card is modal except for + /// [`Status::Updated`], a notice that leaves the app usable behind it. pub fn show(&self, ctx: &egui::Context, status: &Status) -> CheckResultResponse { if matches!(status, Status::Idle) { return CheckResultResponse::None; } let mut response = CheckResultResponse::None; - if backdrop(ctx) { + if !matches!(status, Status::Updated(_)) && backdrop(ctx) { response = CheckResultResponse::Close; } if ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) { @@ -399,7 +416,7 @@ impl CheckResultDialog { } let pal = Palette::new(ctx, self.app); - let Palette { card_bg, border_color, text_primary, text_muted, fill, on_fill, .. } = pal; + let Palette { card_bg, border_color, text_primary, text_muted, fill, on_fill, alert, .. } = pal; let t = self.translate; egui::Area::new(egui::Id::unique("craft_update.check_result")).anchor(Align2::CENTER_CENTER, Vec2::ZERO).order(egui::Order::Foreground).show( ctx, @@ -424,6 +441,33 @@ impl CheckResultDialog { ui.label(RichText::new(t("Checking for updates…", &[])).size(12.5).color(text_primary)); }); } + Status::Installing => { + ui.horizontal(|ui| { + ui.spinner(); + ui.label(RichText::new(t("Installing update…", &[])).size(12.5).color(text_primary)); + }); + } + Status::Ready => { + let text = t(crate::updater::READY_TEXT, &[("app", self.app.name)]); + ui.label(RichText::new(text).size(12.5).color(text_primary)); + } + Status::Managed => { + let text = t( + "{app} {version} follows the version chosen in the launcher, so it does not update itself.", + &[("app", self.app.name), ("version", self.running_version)], + ); + ui.label(RichText::new(text).size(12.5).color(text_muted)); + } + Status::Updated(version) => { + let text = t("{app} updated to {version}.", &[("app", self.app.name), ("version", version)]); + ui.label(RichText::new(text).size(12.5).color(text_primary)); + } + Status::InstallFailed(reason) => { + let text = t("The update could not be installed: {reason}", &[("reason", reason)]); + bounded(ui, "craft_update.install_failure", |ui| { + ui.add(egui::Label::new(RichText::new(text).size(12.5).color(alert)).selectable(true)); + }); + } Status::UpToDate => { let text = t( "{app} {version} is the newest version available.", @@ -432,9 +476,10 @@ impl CheckResultDialog { ui.label(RichText::new(text).size(12.5).color(text_muted)); } Status::Failed(error) => { - let alert_color = Color32::from_rgb(225, 95, 30); - ui.label(RichText::new(t("Could not check for updates.", &[])).size(12.5).strong().color(alert_color)); - ui.add(egui::Label::new(RichText::new(error).size(12.0).color(text_muted)).selectable(true)); + ui.label(RichText::new(t("Could not check for updates.", &[])).size(12.5).strong().color(alert)); + bounded(ui, "craft_update.check_failure", |ui| { + ui.add(egui::Label::new(RichText::new(error).size(12.0).color(text_muted)).selectable(true)); + }); ui.label( RichText::new(t("All releases are listed at {url}", &[("url", &self.app.releases_url())])) .size(11.5) @@ -458,7 +503,7 @@ impl CheckResultDialog { if ui.add(ok).clicked() { response = CheckResultResponse::Close; } - if matches!(status, Status::Failed(_)) { + if matches!(status, Status::Failed(_) | Status::InstallFailed(_)) { let again = egui::Button::new(RichText::new(button(t, "Check Again")).size(10.5).monospace().color(text_primary)) .corner_radius(egui::CornerRadius::ZERO) .min_size(Vec2::new(100.0, 26.0)); @@ -466,6 +511,15 @@ impl CheckResultDialog { response = CheckResultResponse::CheckAgain; } } + if matches!(status, Status::Ready) { + let restart = + egui::Button::new(RichText::new(button(t, "Restart to Update")).size(10.5).monospace().color(text_primary)) + .corner_radius(egui::CornerRadius::ZERO) + .min_size(Vec2::new(130.0, 26.0)); + if ui.add(restart).clicked() { + response = CheckResultResponse::RestartToUpdate; + } + } }); } }); @@ -475,6 +529,16 @@ impl CheckResultDialog { } } +/// The tallest an error or failure reason draws in a card; a longer one scrolls, so the card's +/// buttons stay on screen. +const REASON_MAX_HEIGHT: f32 = 160.0; + +/// Draws `add_contents` (an error or a failure reason) at most [`REASON_MAX_HEIGHT`] tall, +/// scrolling the rest. +fn bounded(ui: &mut egui::Ui, id: &str, add_contents: impl FnOnce(&mut egui::Ui)) { + egui::ScrollArea::vertical().id_salt(id).max_height(REASON_MAX_HEIGHT).auto_shrink([false, true]).show(ui, add_contents); +} + /// Backgrounds of an unfilled button (LATER, DISMISS) when hovered and when pressed: a step from /// the card, so its text keeps the contrast it has at rest. fn button_states(is_dark: bool) -> (Color32, Color32) { @@ -547,7 +611,7 @@ mod tests { use egui::{Context, Event, FullOutput, Key, Modifiers, RawInput, Rect}; const DARK_CARD: Color32 = Color32::from_rgb(26, 26, 28); - const DARK_STATUS: Color32 = Color32::from_rgb(50, 30, 20); + const DARK_STATUS: Color32 = Color32::from_rgb(44, 26, 18); fn screen() -> Rect { Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(640.0, 480.0)) @@ -573,6 +637,7 @@ mod tests { status_message: None, status_note: None, is_installing: false, + is_ready: false, translate: english, } } @@ -860,6 +925,54 @@ mod tests { } } + /// Errors and failure reasons reach WCAG AA's 4.5:1 on the card and in the prompt's message + /// box, in both themes. + #[test] + fn failure_text_reaches_aa_in_both_themes() { + for theme in [egui::ThemePreference::Dark, egui::ThemePreference::Light] { + let ctx = Context::default(); + ctx.set_theme(theme); + let pal = Palette::new(&ctx, &PHOTOCRAFT); + for (surface, bg) in [("card", pal.card_bg), ("message box", pal.alert_bg)] { + let ratio = contrast(pal.alert, bg); + assert!(ratio >= 4.5, "{theme:?} {surface}: {ratio:.2}:1"); + } + } + } + + /// A long error or failure reason scrolls inside the card, so the card's buttons stay on + /// screen. + #[test] + fn a_long_reason_keeps_the_buttons_on_screen() { + let long = "the installer reported an error. ".repeat(120); + let on_screen = |output: &FullOutput, label: &str| text_pos(output, label).is_some_and(|pos| screen().contains(pos)); + let dialog = CheckResultDialog::new(&PHOTOCRAFT, "0.1.0"); + for status in [Status::InstallFailed(long.clone()), Status::Failed(long.clone())] { + let ctx = dark_context(); + let mut output = None; + // The first frame of an area only sizes it; the second paints. + for _ in 0..2 { + let mut out = ctx.run_ui(RawInput { screen_rect: Some(screen()), ..RawInput::default() }, |ui| { + dialog.show(ui.ctx(), &status); + }); + out.textures_delta.clear(); + output = Some(out); + } + let output = output.unwrap(); + for label in ["OK", "CHECK AGAIN"] { + assert!(on_screen(&output, label), "{label} is off screen for a long {status:?}"); + } + } + let ctx = dark_context(); + let mut prompt = prompt_for(true); + prompt.set_message(long); + frame(&ctx, &mut prompt, Vec::new()); + let (_, output) = frame(&ctx, &mut prompt, Vec::new()); + for label in ["UPDATE AND RESTART", "LATER"] { + assert!(on_screen(&output, label), "{label} is off screen under a long message"); + } + } + /// Where the frame painted `text` (inside its galley, so a pointer there is over it). fn text_pos(output: &FullOutput, text: &str) -> Option { fn find(shape: &egui::Shape, text: &str) -> Option { @@ -1049,6 +1162,26 @@ mod tests { let (_, idle) = run(&ctx, &Status::Idle, Vec::new()); assert!(painted_text(&idle).is_empty()); + // An update under way, the launcher's choice, and what the last update came to. + let ready = crate::translate::fill(crate::updater::READY_TEXT, &[("app", "PhotoCraft")]); + let backdrop = Color32::from_black_alpha(150); + for (status, wants, modal) in [ + (Status::Installing, vec!["Installing update…", "OK"], true), + (Status::Ready, vec![ready.as_str(), "RESTART TO UPDATE", "OK"], true), + (Status::Managed, vec!["PhotoCraft 0.1.0 follows the version chosen in the launcher, so it does not update itself.", "OK"], true), + (Status::Updated("0.2.0".into()), vec!["PhotoCraft updated to 0.2.0.", "OK"], false), + (Status::InstallFailed("disk full".into()), vec!["The update could not be installed: disk full", "CHECK AGAIN", "OK"], true), + ] { + let (response, output) = run(&ctx, &status, Vec::new()); + let text = painted_text(&output); + for want in wants { + assert!(text.contains(want), "{status:?}: {want} missing in {text}"); + } + assert!(!text.contains("newest version"), "{status:?} is not called newest: {text}"); + assert_eq!(response, CheckResultResponse::None); + assert_eq!(rect_fills(&output).iter().any(|(_, fill)| *fill == backdrop), modal, "{status:?} modal"); + } + let escape = Event::Key { key: Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }; assert_eq!(run(&dark_context(), &Status::UpToDate, vec![escape]).0, CheckResultResponse::Close); } @@ -1070,19 +1203,20 @@ mod tests { #[test] fn the_prompt_draws_its_words_through_the_apps_translator() { - for (self_updating, installing) in [(false, false), (true, false), (true, true)] { + for (self_updating, installing, ready) in [(false, false, false), (true, false, false), (true, true, false), (true, false, true)] { let ctx = dark_context(); - let mut prompt = StartupPrompt { is_installing: installing, ..prompt_for(self_updating) }.with_translator(pseudo); + let mut prompt = StartupPrompt { is_installing: installing, is_ready: ready, ..prompt_for(self_updating) }.with_translator(pseudo); frame(&ctx, &mut prompt, Vec::new()); let (_, output) = frame(&ctx, &mut prompt, Vec::new()); let painted = painted_text(&output); assert_all_translated(&painted, &["P", "PhotoCraft"]); // Values reach the translation; buttons are looked up in title case and drawn in capitals. assert!(painted.contains("[0.1.0, 0.2.0 | v]") && painted.contains("[ArtCraft | by ]"), "{painted}"); - let button = match (self_updating, installing) { - (_, true) => "[ | DISMISS]", - (true, false) => "[ | UPDATE AND RESTART]", - (false, false) => "[ | DOWNLOAD]", + let button = match (self_updating, installing, ready) { + (_, true, _) => "[ | DISMISS]", + (_, false, true) => "[ | RESTART TO UPDATE]", + (true, false, false) => "[ | UPDATE AND RESTART]", + (false, false, false) => "[ | DOWNLOAD]", }; assert!(painted.contains(button) && painted.contains("[ | UPDATE]"), "{painted}"); } @@ -1091,7 +1225,16 @@ mod tests { #[test] fn the_result_dialog_draws_its_words_through_the_apps_translator() { let dialog = CheckResultDialog::new(&PHOTOCRAFT, "0.1.0").with_translator(pseudo); - for status in [Status::Checking, Status::UpToDate, Status::Failed("no network".into())] { + for status in [ + Status::Checking, + Status::UpToDate, + Status::Failed("no network".into()), + Status::Installing, + Status::Ready, + Status::Managed, + Status::Updated("0.2.0".into()), + Status::InstallFailed("no network".into()), + ] { let ctx = dark_context(); let mut output = None; for _ in 0..2 { diff --git a/crates/update/src/process.rs b/crates/update/src/process.rs index 1074cec4..e39eb378 100644 --- a/crates/update/src/process.rs +++ b/crates/update/src/process.rs @@ -2,7 +2,8 @@ use std::ffi::OsStr; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::{Child, Command, ExitStatus, Stdio}; +use std::sync::mpsc; use std::time::{Duration, Instant}; /// A `Command` that won't flash a console window on Windows, and clears AppImage-injected @@ -62,7 +63,15 @@ pub fn run_command(mut cmd: Command, program: &str, timeout: Duration) -> Result let mut child = cmd.stdin(Stdio::null()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().map_err(|e| format!("couldn't start {program} ({e})"))?; // Read both pipes while it runs, so a chatty helper can't block on a full one. - let (stdout, stderr) = (read_all(child.stdout.take()), read_all(child.stderr.take())); + let readers = read_all(child.stdout.take()).and_then(|stdout| Ok((stdout, read_all(child.stderr.take())?))); + let (stdout, stderr) = match readers { + Ok(readers) => readers, + Err(e) => { + let _ = child.kill(); + let _ = child.wait(); + return Err(format!("couldn't read the output of {program} ({e})")); + } + }; let start = Instant::now(); let status = loop { match child.try_wait() { @@ -93,8 +102,8 @@ pub fn run_command(mut cmd: Command, program: &str, timeout: Duration) -> Result } /// Captures at most 8 MB of a child's pipe and drains the rest on its own thread. -fn read_all(pipe: Option) -> std::thread::JoinHandle> { - std::thread::spawn(move || { +fn read_all(pipe: Option) -> std::io::Result>> { + std::thread::Builder::new().name("helper-output".into()).spawn(move || { let mut out = Vec::new(); if let Some(mut pipe) = pipe { let _ = std::io::Read::read_to_end(&mut std::io::Read::take(&mut pipe, 8 << 20), &mut out); @@ -106,13 +115,61 @@ fn read_all(pipe: Option) -> std::thread:: }) } -/// Starts `cmd` without waiting for it, and reaps it when it exits. +/// Starts `cmd` without waiting for it. +/// +/// On Unix the process is reaped when it exits: a thread that waits for the process starts +/// first and starts the process itself. When no thread can be started, the call returns an error +/// and does not start the process. On Windows the process starts on the calling thread, and no +/// thread waits for it. pub fn spawn_detached(mut cmd: Command) -> Result<(), String> { - let mut child = cmd.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()).spawn().map_err(|e| format!("couldn't start it ({e})"))?; - std::thread::spawn(move || { - let _ = child.wait(); - }); - Ok(()) + cmd.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()); + if cfg!(windows) { + return cmd.spawn().map(drop).map_err(|e| format!("couldn't start it ({e})")); + } + spawn_detached_with( + move || cmd.spawn(), + |wait| { + std::thread::Builder::new() + .name("reap-child".into()) + .spawn(move || { + let _ = wait(); + }) + .map(drop) + }, + ) +} + +/// The thread that [`spawn_detached`] starts on Unix: `start_thread` starts a thread that runs +/// the closure it is given, and that closure calls `spawn` and then waits for the process. +/// `spawn` is called on that thread only, so when `start_thread` fails nothing is started. +#[cfg_attr(windows, allow(dead_code))] +fn spawn_detached_with( + spawn: impl FnOnce() -> std::io::Result + Send + 'static, + start_thread: impl FnOnce(Box Option + Send>) -> std::io::Result<()>, +) -> Result<(), String> { + let (started, start_result) = mpsc::channel(); + start_thread(Box::new(move || start_and_wait(spawn, &started))).map_err(|e| format!("couldn't start a thread to wait for it ({e})"))?; + match start_result.recv() { + Ok(Ok(())) => Ok(()), + Ok(Err(e)) => Err(format!("couldn't start it ({e})")), + Err(_) => Err("couldn't start it (its thread ended before starting it)".into()), + } +} + +/// Calls `spawn`, sends whether that worked on `started`, and waits for the process to exit. +/// Returns its exit status, or `None` when it didn't start or couldn't be waited for. +#[cfg_attr(windows, allow(dead_code))] +fn start_and_wait(spawn: impl FnOnce() -> std::io::Result, started: &mpsc::Sender>) -> Option { + match spawn() { + Ok(mut child) => { + let _ = started.send(Ok(())); + child.wait().ok() + } + Err(e) => { + let _ = started.send(Err(e)); + None + } + } } /// Waits up to `timeout` for the process `pid` to exit, and says whether it has. A process that @@ -224,17 +281,22 @@ fn opener(target: &std::ffi::OsStr) -> Result { pub fn open_url_with_fallback(primary_url: &str, fallback_url: &str) { let primary = primary_url.to_string(); let fallback = fallback_url.to_string(); - std::thread::spawn(move || { + let opener = std::thread::Builder::new().name("open-url".into()).spawn(move || { if !open_and_wait_success(&primary, Duration::from_millis(1500)) { - log::warn!("couldn't open {primary}; falling back to {fallback}"); + log::warn!("couldn't open a link; opening its fallback"); let _ = open(std::ffi::OsStr::new(&fallback)); } }); + if let Err(e) = opener { + log::warn!("couldn't start a thread to open a link ({e})"); + } } /// Starts the default system opener for `target` and returns `true` unless it fails to start /// or exits in error within `timeout`. An opener still running after `timeout` counts as -/// success and is reaped in the background. +/// success, and the call waits for it to exit before returning. An opener whose state can't be +/// read is stopped and waited for, and counts as a failure. [`open_url_with_fallback`] calls this +/// on its own `open-url` thread. /// /// `explorer` exits with an error status even when it opens the target, so on Windows only a /// failure to start counts; the scheme check covers the rest. @@ -242,7 +304,7 @@ fn open_and_wait_success(target: &str, timeout: Duration) -> bool { let mut cmd = match opener(std::ffi::OsStr::new(target)) { Ok(cmd) => cmd, Err(error) => { - log::warn!("couldn't resolve the system opener for {target}: {error}"); + log::warn!("couldn't resolve the system opener: {error}"); return false; } }; @@ -252,7 +314,7 @@ fn open_and_wait_success(target: &str, timeout: Duration) -> bool { let mut child = match cmd.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null()).spawn() { Ok(c) => c, Err(e) => { - log::warn!("couldn't start the opener for {target}: {e}"); + log::warn!("couldn't start the system opener: {e}"); return false; } }; @@ -262,12 +324,14 @@ fn open_and_wait_success(target: &str, timeout: Duration) -> bool { Ok(Some(status)) => return status.success(), Ok(None) if start.elapsed() < timeout => std::thread::sleep(Duration::from_millis(25)), Ok(None) => { - std::thread::spawn(move || { - let _ = child.wait(); - }); + let _ = child.wait(); return true; } - Err(_) => return false, + Err(_) => { + let _ = child.kill(); + let _ = child.wait(); + return false; + } } } } @@ -472,6 +536,54 @@ mod run_tests { assert!(start.elapsed() < Duration::from_secs(5)); } + /// The thread that starts a detached process waits for it, so the process is reaped when it + /// exits: only a wait gives its exit status. + #[test] + fn a_detached_process_is_waited_for() { + let (program, args) = shell("exit 7"); + let mut cmd = command(program); + cmd.args(args); + let (status_sent, status) = mpsc::channel(); + let result = spawn_detached_with( + move || cmd.spawn(), + move |wait| { + std::thread::Builder::new() + .spawn(move || { + let _ = status_sent.send(wait()); + }) + .map(drop) + }, + ); + assert_eq!(result, Ok(())); + assert_eq!(status.recv().ok().flatten().and_then(|status| status.code()), Some(7), "the waiting thread gets the exit status"); + } + + /// When no thread can be started to wait for a process, the process is not started and the + /// call says why. + #[test] + fn no_process_starts_without_a_thread_to_wait_for_it() { + let (program, args) = shell("exit 0"); + let mut cmd = command(program); + cmd.args(args); + let spawned = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let spawn_called = std::sync::Arc::clone(&spawned); + let result = spawn_detached_with( + move || { + spawn_called.store(true, std::sync::atomic::Ordering::SeqCst); + cmd.spawn() + }, + |_wait| Err(std::io::Error::other("no threads")), + ); + assert_eq!(result, Err("couldn't start a thread to wait for it (no threads)".into())); + assert!(!spawned.load(std::sync::atomic::Ordering::SeqCst), "the process was started"); + } + + #[test] + fn a_detached_program_that_cannot_start_says_so() { + let result = spawn_detached(command("craft-update-test-no-such-program")); + assert!(matches!(&result, Err(e) if e.starts_with("couldn't start it (")), "{result:?}"); + } + #[test] fn a_silent_failure_and_a_success() { let (program, args) = shell("exit 4"); diff --git a/crates/update/src/self_update.rs b/crates/update/src/self_update.rs index 256081ce..01ec41da 100644 --- a/crates/update/src/self_update.rs +++ b/crates/update/src/self_update.rs @@ -244,11 +244,12 @@ fn msi_step(app: &App, exe: &Path) -> Result { crate::msi_migrate::windows::step(app, exe) } -/// Whether the helper starts the app after the update step: always after a successful one, and -/// after a failed one only while its exe is still there. +/// Whether the helper starts the app after the update step: never while the old app is still +/// running (a second copy of it would open), always after a successful step, and after a failed +/// one only while its exe is still there. #[cfg_attr(not(windows), allow(dead_code))] -fn restarts(step_succeeded: bool, exe_exists: bool) -> bool { - step_succeeded || exe_exists +fn restarts(app_exited: bool, step_succeeded: bool, exe_exists: bool) -> bool { + app_exited && (step_succeeded || exe_exists) } /// The exe to start after an MSI update of a registered copy: the one its registrations name now @@ -371,6 +372,60 @@ pub fn update( last: &dyn Fn() -> Result<(), String>, running_version: Option<&str>, ) -> Result<(), String> { + prepare_update(app, install, release, platform, http, paths, stop, phase, last, running_version)?.start() +} + +/// What remains of an update after [`prepare_update`]: starting the helper, which waits for this +/// process to exit. On Windows the helper then installs the update and starts the new version; +/// on macOS and Linux the update is already in place and the helper starts the new version. +#[must_use = "the update only installs or restarts once the restart is started"] +pub struct Restart { + helper: Option Result<(), String> + Send>>, +} + +impl Restart { + /// Nothing is left to start. + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn scheduled() -> Self { + Self { helper: None } + } + + /// `start` runs when the app closes. + #[cfg_attr(not(any(windows, unix, test)), allow(dead_code))] + pub(crate) fn later(start: impl FnOnce() -> Result<(), String> + Send + 'static) -> Self { + Self { helper: Some(Box::new(start)) } + } + + /// Starts the helper. Call it when the app's exit is certain: the helper waits only a minute + /// for the app to exit. On Windows it installs nothing when the app is still running after + /// that; on macOS and Linux the Flatpak helper starts nothing then, and the other helpers + /// start the new version anyway. + pub fn start(self) -> Result<(), String> { + self.helper.map_or(Ok(()), |start| start()) + } +} + +impl std::fmt::Debug for Restart { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Restart").field("pending", &self.helper.is_some()).finish() + } +} + +/// [`update`] without starting the helper: the caller starts the returned [`Restart`] when the +/// app's exit is certain. On macOS and Linux the update is in place when this returns. +#[allow(clippy::too_many_arguments)] +pub fn prepare_update( + app: &App, + install: &Install, + release: &Release, + platform: Platform, + http: &Http, + paths: &Paths, + stop: &Stop, + phase: &mut dyn FnMut(JobPhase), + last: &dyn Fn() -> Result<(), String>, + running_version: Option<&str>, +) -> Result { let cur_v = running_version.and_then(|v| current_version(app, v)); if let (Some(rel_v), Some(cur_v)) = (release.version.as_ref(), cur_v.as_ref()) && !version::offers_update(rel_v, cur_v) @@ -390,11 +445,13 @@ pub fn update( // The restart ends this process: nothing else may be under way. last()?; phase(JobPhase::Installing); - // Written before the swap starts: the Windows helper runs alongside this process and clears - // it when it fails, which a write after scheduling could undo. + // Written before the swap starts. On Windows the helper does the swap, so the record is + // written when it starts (see `windows_helper`): it clears the record when it fails, which a + // write after it started could undo. + #[cfg(not(windows))] mark_installed(paths, release); let pid = std::process::id(); - let scheduled = (|| -> Result<(), String> { + let scheduled = (|| -> Result { match install { #[cfg(target_os = "macos")] Install::MacApp(bundle) => { @@ -402,20 +459,20 @@ pub fn update( let placed = install::macos::install_dmg(app, &file, Some(&existing), None, false, paths, &install::macos::Expect::release(release)); let _ = std::fs::remove_file(&file); let placed = placed?; - relaunch_after_exit(app, pid, &[Path::new("/usr/bin/open").as_os_str(), placed.as_os_str()]) + Ok(relaunch_after_exit(app, pid, &[Path::new("/usr/bin/open").as_os_str(), placed.as_os_str()])) } #[cfg(target_os = "linux")] Install::AppImage(path) => { replace_appimage(&file, path)?; let _ = std::fs::remove_file(&file); - relaunch_after_exit(app, pid, &[path.as_os_str()]) + Ok(relaunch_after_exit(app, pid, &[path.as_os_str()])) } #[cfg(target_os = "linux")] Install::Package(kind, exe) => { let result = install::linux::install_package(*kind, &file, None, stop, phase); let _ = std::fs::remove_file(&file); result?; - relaunch_after_exit(app, pid, &[exe.as_os_str()]) + Ok(relaunch_after_exit(app, pid, &[exe.as_os_str()])) } #[cfg(target_os = "linux")] Install::Flatpak => { @@ -432,23 +489,23 @@ pub fn update( relaunch_flatpak_after_exit(app) } #[cfg(windows)] - Install::Msi(exe) => windows_helper(app, paths, pid, exe, &msi_finish(&file)?), + Install::Msi(exe) => windows_helper(app, paths, release, pid, exe, &msi_finish(&file)?), #[cfg(windows)] Install::Portable(exe) => { exe.parent().ok_or_else(|| format!("{} isn't in a folder", app.name))?; - windows_helper(app, paths, pid, exe, &Finish::Portable { zip: file, tmp: paths.cache.join(PORTABLE_STAGING) }) + windows_helper(app, paths, release, pid, exe, &Finish::Portable { zip: file, tmp: paths.cache.join(PORTABLE_STAGING) }) } #[cfg(windows)] Install::Standalone(exe) => { exe.parent().ok_or_else(|| format!("{} isn't in a folder", app.name))?; - windows_helper(app, paths, pid, exe, &Finish::Portable { zip: file, tmp: paths.cache.join(PORTABLE_STAGING) }) + windows_helper(app, paths, release, pid, exe, &Finish::Portable { zip: file, tmp: paths.cache.join(PORTABLE_STAGING) }) } #[cfg(unix)] Install::Standalone(exe) => { let result = replace_standalone(app, paths, &file, exe); let _ = std::fs::remove_file(&file); result?; - relaunch_after_exit(app, pid, &[exe.as_os_str()]) + Ok(relaunch_after_exit(app, pid, &[exe.as_os_str()])) } #[allow(unreachable_patterns)] _ => { @@ -457,19 +514,101 @@ pub fn update( } } })(); - unmark_unless_scheduled(paths, scheduled) + let scheduled = unmark_unless_scheduled(paths, scheduled); + // On macOS and Linux the update is in place once it is prepared. + #[cfg(not(windows))] + if scheduled.is_ok() { + record_done(paths, &release.version_label()); + } + scheduled } -/// Records `release` as just installed, so it isn't offered again for [`JUST_UPDATED_FOR`]. +/// Records `release` as just installed, so it isn't offered again for [`JUST_UPDATED_FOR`]: its +/// tag, the time, and the version it installs. The outcome records of an earlier update are +/// removed: this update writes its own. fn mark_installed(paths: &Paths, release: &Release) { - if let Err(e) = crate::paths::write_atomic(&marker(paths), format!("{}\n{}\n", release.tag, crate::now()).as_bytes()) { + let _ = std::fs::remove_file(failure_file(paths)); + let _ = std::fs::remove_file(done_file(paths)); + let text = format!("{}\n{}\n{}\n", release.tag, crate::now(), release.version_label()); + if let Err(e) = crate::paths::write_atomic(&marker(paths), text.as_bytes()) { log::warn!("couldn't record the self-update: {e}"); } } +/// Where a finished update records the version it installed, for the new version's first start. +fn done_file(paths: &Paths) -> PathBuf { + paths.data.join("self-update-done.txt") +} + +/// Records that the update to `version` has been installed. +fn record_done(paths: &Paths, version: &str) { + if let Err(e) = crate::paths::write_atomic(&done_file(paths), version.as_bytes()) { + log::warn!("couldn't record the finished self-update: {e}"); + } +} + +/// The version the scheduled update installs, from the record [`mark_installed`] wrote: its third +/// line, or the tag on its first line in a record without one. +#[cfg_attr(not(windows), allow(dead_code))] +fn scheduled_version(paths: &Paths) -> Option { + let text = read_record(&marker(paths))?; + let mut lines = text.lines(); + let tag = lines.next(); + let version = lines.nth(1).filter(|version| !version.trim().is_empty()).or(tag); + version.map(str::trim).filter(|version| !version.is_empty()).map(str::to_owned) +} + +/// The most of an outcome record that is read: a reason longer than this is cut short. +const RECORD_LIMIT: usize = 4096; + +/// The text of the record at `path`, at most [`RECORD_LIMIT`] bytes of it. `None` when it is +/// missing, unreadable or not a plain file (a link is not followed). +fn read_record(path: &Path) -> Option { + use std::io::Read; + if !is_plain_file(path) { + return None; + } + let mut bytes = Vec::new(); + std::fs::File::open(path).ok()?.take(RECORD_LIMIT as u64 + 1).read_to_end(&mut bytes).ok()?; + let cut = bytes.len() > RECORD_LIMIT; + bytes.truncate(RECORD_LIMIT); + let mut text = String::from_utf8_lossy(&bytes).into_owned(); + if cut { + // A multi-byte character split at the limit decodes as U+FFFD; drop it. + text = text.trim_end_matches('\u{FFFD}').to_owned(); + text.push('…'); + } + Some(text) +} + +/// What the last self-update came to, read once by the next start ([`take_outcome`]). +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Outcome { + /// The update installed this version, which is the one running now. + Installed(Version), + /// The update could not be installed, for this reason. + Failed(String), +} + +/// What the last self-update came to, once: the records are removed as they are read. A +/// recorded failure wins over a record of a finished update. A finished update counts only when +/// the version it installed is `running`, the version this copy counts as. +pub fn take_outcome(paths: &Paths, running: &str) -> Option { + let failure = take_failure(paths); + let done = read_record(&done_file(paths)); + let _ = std::fs::remove_file(done_file(paths)); + if let Some(reason) = failure { + let reason = reason.trim(); + let reason = if reason.is_empty() { "no reason was recorded" } else { reason }; + return Some(Outcome::Failed(reason.to_owned())); + } + let installed = version::parse(done?.trim())?; + (Some(&installed) == version::parse(running).as_ref()).then_some(Outcome::Installed(installed)) +} + /// Removes the record of [`mark_installed`] when the swap couldn't be scheduled, so the update /// stays on offer. -fn unmark_unless_scheduled(paths: &Paths, scheduled: Result<(), String>) -> Result<(), String> { +fn unmark_unless_scheduled(paths: &Paths, scheduled: Result) -> Result { if scheduled.is_err() { let _ = std::fs::remove_file(marker(paths)); } @@ -587,7 +726,6 @@ fn stage(from: &Path, to: &Path, executable: bool) -> Result { } /// A regular file, not a link to one. -#[cfg(unix)] fn is_plain_file(path: &Path) -> bool { std::fs::symlink_metadata(path).is_ok_and(|m| m.file_type().is_file()) } @@ -612,10 +750,24 @@ fn plain_files(dir: &Path) -> Result, String> { Ok(files) } -/// Starts `sh`, which waits for `pid` to exit and then runs `command`. Detached, so it outlives us. +/// A [`Restart`] that starts `sh`, which waits for `pid` to exit and then runs `command`. +/// Detached, so it outlives us. Nothing starts before the app's exit is certain: a close the user +/// cancels leaves no helper counting down its wait. #[cfg(unix)] -fn relaunch_after_exit(app: &App, pid: u32, command: &[&std::ffi::OsStr]) -> Result<(), String> { - process::spawn_detached(helper_command(app, pid, command)).map_err(|e| format!("couldn't schedule the restart ({e})")) +fn relaunch_after_exit(app: &App, pid: u32, command: &[&std::ffi::OsStr]) -> Restart { + relaunch_with(app, pid, command, process::spawn_detached) +} + +/// [`relaunch_after_exit`], with `spawn` starting the helper. +#[cfg(unix)] +fn relaunch_with( + app: &App, + pid: u32, + command: &[&std::ffi::OsStr], + spawn: impl FnOnce(std::process::Command) -> Result<(), String> + Send + 'static, +) -> Restart { + let cmd = helper_command(app, pid, command); + Restart::later(move || spawn(cmd).map_err(|e| format!("couldn't schedule the restart ({e})"))) } #[cfg(unix)] @@ -669,9 +821,10 @@ fn flatpak_host_pid() -> Option { parse_flatpak_host_pid(&out, &instance_id) } -/// Starts a host helper that waits for this Flatpak instance to exit, then relaunches it via Flatpak on the host. +/// A [`Restart`] that starts a host helper, which waits for this Flatpak instance to exit, then +/// relaunches it via Flatpak on the host. Nothing starts before the app's exit is certain. #[cfg(target_os = "linux")] -fn relaunch_flatpak_after_exit(app: &App) -> Result<(), String> { +fn relaunch_flatpak_after_exit(app: &App) -> Result { let host_pid = flatpak_host_pid(); let instance_id = flatpak_instance_id(); // Each wait clause polls until the old process is gone, then execs the relaunch command. @@ -690,7 +843,8 @@ fn relaunch_flatpak_after_exit(app: &App) -> Result<(), String> { (None, None) => return Err(format!("{} is updated; quit and reopen it to start the new version", app.name)), }; let script = format!("({wait_clause}) &"); - process::spawn_detached(flatpak_restart_command(app, &script, &target_arg)).map_err(|e| format!("couldn't schedule the Flatpak restart ({e})")) + let cmd = flatpak_restart_command(app, &script, &target_arg); + Ok(Restart::later(move || process::spawn_detached(cmd).map_err(|e| format!("couldn't schedule the Flatpak restart ({e})")))) } #[cfg(target_os = "linux")] @@ -759,11 +913,12 @@ const HELPER_DIR: &str = "launcher-update-helper"; #[cfg_attr(not(windows), allow(dead_code))] const PORTABLE_STAGING: &str = "launcher-update"; -/// Copies this program into the cache and starts the copy as the update helper, which waits for -/// `pid` to exit, does `finish` and starts `exe`. Run from the cache, the helper doesn't hold the -/// installed exe that the step replaces. +/// Copies this program into the cache as the update helper, which waits for `pid` to exit, does +/// `finish` and starts `exe`. Run from the cache, the helper doesn't hold the installed exe that +/// the step replaces. The returned [`Restart`] records `release` as installed and starts the +/// helper. #[cfg(windows)] -fn windows_helper(app: &App, paths: &Paths, pid: u32, exe: &Path, finish: &Finish) -> Result<(), String> { +fn windows_helper(app: &App, paths: &Paths, release: &Release, pid: u32, exe: &Path, finish: &Finish) -> Result { let program = std::env::current_exe().map_err(|e| format!("couldn't find this program ({e})"))?; let dir = paths.cache.join(HELPER_DIR); // The previous update's copy, when it's still there. @@ -774,7 +929,12 @@ fn windows_helper(app: &App, paths: &Paths, pid: u32, exe: &Path, finish: &Finis let mut cmd = process::command(&helper); cmd.args(helper_args(pid, exe, finish)); clear_pretend_version(&mut cmd, app); - process::spawn_detached(cmd).map_err(|e| format!("couldn't schedule the update ({e})")) + let (paths, release) = (paths.clone(), release.clone()); + Ok(Restart::later(move || { + mark_installed(&paths, &release); + let started = process::spawn_detached(cmd).map_err(|e| format!("couldn't schedule the update ({e})")); + unmark_unless_scheduled(&paths, started) + })) } /// Finishes an update when this program was started as the update helper; `args` are its @@ -799,23 +959,96 @@ pub fn finish(app: &App, args: &[OsString]) -> Option> { // Read before the step: the update may move or replace the old exe. let registered = matches!(step, Finish::Msi(_)) && matches!(crate::msi_copy::windows::find(app, &exe), Some(crate::msi_copy::MsiCopy::Registered { .. })); - let done = run_step(app, &me, pid, &exe, &step, std::time::Duration::from_secs(60)); + let wait = |pid| process::wait_for_exit(pid, std::time::Duration::from_secs(60)); + let start = |exe: &Path| { + signed_like(app, &me, exe) + .and_then(|()| process::spawn_detached(std::process::Command::new(exe)).map_err(|e| format!("couldn't start {} ({e})", exe.display()))) + }; + Some(finish_step(app, &me, &paths, pid, exe, &step, registered, &wait, &start)) +} + +/// The helper's work once its checks have passed: waits for `pid` with `wait`, does `step`, and +/// starts the app with `start` when [`restarts`] allows it. Every failure is recorded for the +/// app's next start. +#[cfg(windows)] +#[allow(clippy::too_many_arguments)] +fn finish_step( + app: &App, + reference: &Path, + paths: &Paths, + pid: u32, + exe: PathBuf, + step: &Finish, + registered: bool, + wait: &dyn Fn(u32) -> bool, + start: &dyn Fn(&Path) -> Result<(), String>, +) -> Result<(), String> { + let portable_before = has_portable_marker(&exe); + let stepped = step_after_wait(app, reference, pid, &exe, step, wait); + let done = stepped.result; if let Err(e) = &done { - record_failure(&paths, e); + record_failure(paths, e); } // The new app, or the old one when the step failed. let exe = if done.is_ok() { exe_after_msi(app, registered, &exe) } else { exe }; - // A failed move can leave no program to start; its recorded error already says how to get - // the app back, and a restart error must not replace it. - if !restarts(done.is_ok(), exe.is_file()) { - return Some(done); + // A standalone copy updated from the portable zip gains `portable.txt` and keeps its folders + // beside it from now on: the records the new app reads go there. + let version = scheduled_version(paths); + let moved = (done.is_ok() && has_portable_marker(&exe) != portable_before).then(|| Paths::for_exe(app, &exe).ok()).flatten(); + if let Some(after) = &moved { + move_marker(paths, after); } - let started = signed_like(app, &me, &exe) - .and_then(|()| process::spawn_detached(std::process::Command::new(&exe)).map_err(|e| format!("couldn't start {} ({e})", exe.display()))); - if let Err(e) = &started { - record_failure(&paths, e); + let paths = moved.as_ref().unwrap_or(paths); + // An app that is still running gets no second copy. A failed move can leave no program to + // start; its recorded error already says how to get the app back, and a restart error must + // not replace it. + if !restarts(stepped.exited, done.is_ok(), exe.is_file()) { + return done; + } + // Written before the new app starts, so its first start finds it. A failed start removes it. + if done.is_ok() + && let Some(version) = &version + { + record_done(paths, version); + } + let started = start(&exe); + let restarted = started.is_ok(); + let result = with_restart(done, started); + if !restarted && let Err(e) = &result { + record_failure(paths, e); + } + result +} + +/// The helper's result once it has tried to start the app: when both the update step and the +/// start fail, the error names both, the step's first. +#[cfg_attr(not(windows), allow(dead_code))] +fn with_restart(done: Result<(), String>, started: Result<(), String>) -> Result<(), String> { + match (done, started) { + (done, Ok(())) => done, + (Ok(()), Err(restart)) => Err(restart), + (Err(update), Err(restart)) => Err(format!("{update}; {restart}")), + } +} + +/// Whether `exe` has `portable.txt` beside it, which makes [`Paths::for_exe`] keep its folders +/// there. +#[cfg(windows)] +fn has_portable_marker(exe: &Path) -> bool { + exe.parent().is_some_and(|dir| dir.join("portable.txt").is_file()) +} + +/// Moves the record of the scheduled update from `from` to `to`, the folders the updated app +/// reads, and removes outcome records left in `to` by an earlier update. +#[cfg(windows)] +fn move_marker(from: &Paths, to: &Paths) { + let _ = std::fs::remove_file(failure_file(to)); + let _ = std::fs::remove_file(done_file(to)); + if let Some(text) = read_record(&marker(from)) + && crate::paths::write_atomic(&marker(to), text.as_bytes()).is_ok() + { + let _ = std::fs::remove_file(marker(from)); } - Some(done.and(started)) } /// Finishes an update on non-Windows platforms (no-op helper). @@ -829,20 +1062,25 @@ fn failure_file(paths: &Paths) -> PathBuf { paths.logs().join("self-update-error.txt") } -/// Records a failed update: the next check offers the update again, and the next start logs `error`. -#[cfg_attr(not(windows), allow(dead_code))] -fn record_failure(paths: &Paths, error: &str) { +/// Records a failed update: the next check offers the update again, and the next start shows +/// `error`. A record of the update as finished is removed. +#[cfg_attr(not(any(windows, feature = "egui")), allow(dead_code))] +pub(crate) fn record_failure(paths: &Paths, error: &str) { let _ = std::fs::remove_file(marker(paths)); + let _ = std::fs::remove_file(done_file(paths)); let _ = std::fs::create_dir_all(paths.logs()); let _ = crate::paths::write_atomic(&failure_file(paths), error.as_bytes()); } -/// The error of the last update, if it failed, once. +/// The error of the last update, if it failed, once. A long error is cut short at +/// [`RECORD_LIMIT`] bytes. pub fn take_failure(paths: &Paths) -> Option { let file = failure_file(paths); - let error = std::fs::read_to_string(&file).ok()?; - let _ = std::fs::remove_file(&file); - Some(error) + let error = read_record(&file); + if is_plain_file(&file) { + let _ = std::fs::remove_file(&file); + } + error } /// Whether `exe` is named like this app's own executable (case-insensitive, any of its ids). @@ -941,65 +1179,26 @@ fn check_helper(app: &App, me: &Path, paths: &Paths, exe: &Path, step: &Finish) if allowed { Ok(()) } else { Err(refused()) } } -/// Lets a Windows test observe both sides of `run_step`'s wait for the old launcher. Compiled only -/// for tests, so production behavior is unchanged. -#[cfg(all(test, windows))] -mod wait_watch { - use std::sync::mpsc::{self, Receiver, Sender}; - use std::sync::{Mutex, PoisonError}; - - /// A test's two observations of one wait, keyed by the launcher's pid. - pub(super) struct Channels { - /// Sent just before `run_step` calls the wait. - pub(super) entered: Receiver<()>, - /// Sent right after the wait returns, before any extraction, and carries the wait's result. - pub(super) returned: Receiver, - } - - struct Watch { - pid: u32, - entered: Option>, - returned: Option>, - } - - static WATCH: Mutex> = Mutex::new(None); - - pub(super) fn watch(pid: u32) -> Channels { - let (entered, entered_rx) = mpsc::channel(); - let (returned, returned_rx) = mpsc::channel(); - *WATCH.lock().unwrap_or_else(PoisonError::into_inner) = Some(Watch { pid, entered: Some(entered), returned: Some(returned) }); - Channels { entered: entered_rx, returned: returned_rx } - } - - /// Notes that `run_step` is about to wait. Only the first call for a watched pid is kept. - pub(super) fn entered(pid: u32) { - let mut watch = WATCH.lock().unwrap_or_else(PoisonError::into_inner); - let sender = watch.as_mut().filter(|watch| watch.pid == pid).and_then(|watch| watch.entered.take()); - if let Some(sender) = sender { - let _ = sender.send(()); - } - } - - /// Notes the wait's result as soon as it returns; the step hasn't run yet. - pub(super) fn returned(pid: u32, waited: bool) { - let mut watch = WATCH.lock().unwrap_or_else(PoisonError::into_inner); - let sender = watch.as_mut().filter(|watch| watch.pid == pid).and_then(|watch| watch.returned.take()); - if let Some(sender) = sender { - let _ = sender.send(waited); - } - } +/// [`step_after_wait`], waiting up to `wait` for `pid` to exit. +#[cfg(all(windows, test))] +fn run_step(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wait: std::time::Duration) -> Result<(), String> { + step_after_wait(app, reference, pid, exe, step, &|pid| process::wait_for_exit(pid, wait)).result } -/// Waits up to `wait` for `pid` to exit, does `step` once it has, and removes the download either -/// way. The MSI, and every program in the portable zip, must be signed like `reference`. +/// What [`step_after_wait`] came to. #[cfg(windows)] -fn run_step(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wait: std::time::Duration) -> Result<(), String> { - #[cfg(test)] - wait_watch::entered(pid); - // The test hook needs the result, so it is stored rather than consumed by the branch. - let waited = process::wait_for_exit(pid, wait); - #[cfg(test)] - wait_watch::returned(pid, waited); +struct Stepped { + /// The old app had exited when the wait returned. + exited: bool, + result: Result<(), String>, +} + +/// Calls `wait`, which says whether `pid` exited in time, does `step` once it has, and removes +/// the download either way. The MSI, and every program in the portable zip, must be signed like +/// `reference`. +#[cfg(windows)] +fn step_after_wait(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wait: &dyn Fn(u32) -> bool) -> Stepped { + let waited = wait(pid); let result = if waited { match step { Finish::Msi(msi) => { @@ -1025,7 +1224,7 @@ fn run_step(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wa let _ = std::fs::remove_file(zip); } } - result + Stepped { exited: waited, result } } #[cfg(test)] @@ -1178,18 +1377,23 @@ mod tests { let tmp = dir.path().join("cache").join("launcher-update"); let step = Finish::Portable { zip: zip.clone(), tmp: tmp.clone() }; - let mut old = std::process::Command::new("ping").args(["-n", "3", "127.0.0.1"]).stdout(std::process::Stdio::null()).spawn().unwrap(); + let mut old = BlockedLauncher::start(); let exe = folder.join("artcraft-launcher.exe"); - let helper = { - let (exe, step, pid) = (exe.clone(), step.clone(), old.id()); - std::thread::spawn(move || { - run_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), pid, &exe, &step, std::time::Duration::from_secs(30)) - }) - }; - std::thread::sleep(std::time::Duration::from_millis(500)); - assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0", "replaced while the old launcher was still running"); - old.wait().unwrap(); - helper.join().unwrap().unwrap(); + std::thread::scope(|scope| { + // Inside the scope, so a failed assertion drops the gate and releases the helper + // before the scope joins it. + let (gate, wait) = WaitGate::new(); + let (helper_exe, pid) = (exe.clone(), old.id()); + let helper = scope.spawn(move || step_after_wait(&crate::suite::LAUNCHER, &system_program("cmd.exe"), pid, &helper_exe, &step, &wait)); + gate.entered(); + old.assert_running("while the helper was waiting"); + assert_eq!(std::fs::read(&exe).unwrap(), b"0.1.0", "replaced while the old launcher was still running"); + old.finish().expect("the stand-in launcher couldn't be reaped"); + gate.release(); + let stepped = helper.join().unwrap(); + assert!(stepped.exited, "the wait reported that the launcher was still running although it had exited"); + stepped.result.unwrap(); + }); assert_eq!(std::fs::read(&exe).unwrap(), signed_exe); assert_eq!(std::fs::read_to_string(folder.join("ArtCraftLauncherData").join("settings.json")).unwrap(), "{}"); assert!(!folder.join("artcraft-launcher-0.2.0-windows-x64-portable").exists(), "the zip's folder must not be nested in the launcher's"); @@ -1248,20 +1452,52 @@ mod tests { } } + /// A wait for [`step_after_wait`] that the test controls: it reports when the helper has + /// reached its wait, blocks until the test releases it, and then asks the real + /// [`process::wait_for_exit`] whether the launcher has exited, without waiting. + #[cfg(windows)] + struct WaitGate { + entered: std::sync::mpsc::Receiver<()>, + release: std::sync::mpsc::Sender<()>, + } + + #[cfg(windows)] + impl WaitGate { + /// The gate and the wait to hand to the helper. The wait holds the only sender of + /// `entered`, so a helper that ends without reaching its wait fails [`WaitGate::entered`] + /// instead of blocking it, and dropping the gate releases the wait. + fn new() -> (Self, impl Fn(u32) -> bool + Send) { + let (entered_tx, entered) = std::sync::mpsc::channel(); + let (release, released) = std::sync::mpsc::channel::<()>(); + let released = std::sync::Mutex::new(released); + let wait = move |pid| { + let _ = entered_tx.send(()); + let _ = released.lock().map(|released| released.recv()); + process::wait_for_exit(pid, std::time::Duration::ZERO) + }; + (Self { entered, release }, wait) + } + + /// Blocks until the helper has reached its wait. + fn entered(&self) { + self.entered.recv().expect("the helper never reached its wait"); + } + + /// Lets the helper's wait return. + fn release(&self) { + self.release.send(()).expect("the helper stopped before its wait was released"); + } + } + /// A launcher without `portable.txt` is a standalone installation, and the helper still /// replaces its own executable after it exits. The test holds a stand-in launcher alive - /// through a pipe and observes both sides of the helper's wait: the first assertion runs only - /// after the helper announces it is about to wait. A wait that returns while the stand-in is alive - /// fails the test before any extraction can replace the executable. - /// - /// The "still blocked" check is a bounded observation, not proof: a helper descheduled for the - /// whole window could hide a missing wait until the release below. A premature timeout longer - /// than this observation window is also outside this test's coverage. + /// through a pipe and holds the helper inside its wait until the stand-in has exited, so the + /// executable must still be the old one while the helper waits, and the real + /// [`process::wait_for_exit`] must report the live stand-in as running and the exited one as gone. #[cfg(windows)] #[test] fn a_standalone_update_replaces_the_launcher_after_it_exits() { use std::io::Write; - use std::sync::mpsc::RecvTimeoutError; let dir = tempfile::tempdir().unwrap(); let folder = dir.path().join("ArtCraft Launcher"); std::fs::create_dir_all(folder.join("ArtCraftLauncherData")).unwrap(); @@ -1282,49 +1518,23 @@ mod tests { let tmp = dir.path().join("cache").join(PORTABLE_STAGING); let step = Finish::Portable { zip: zip.clone(), tmp: tmp.clone() }; - // Timeouts only bound the test; each failure names the observation that never arrived. - let entered_timeout = std::time::Duration::from_secs(10); - let blocked_window = std::time::Duration::from_secs(2); - let returned_timeout = std::time::Duration::from_secs(30); - + let mut old = BlockedLauncher::start(); + let pid = old.id(); std::thread::scope(|scope| { - let mut old = BlockedLauncher::start(); - let pid = old.id(); - let watch = wait_watch::watch(pid); + // Inside the scope, so a failed assertion drops the gate and releases the helper + // before the scope joins it. + let (gate, wait) = WaitGate::new(); let helper_exe = exe.clone(); - let step = step.clone(); - let helper = scope.spawn(move || { - run_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), pid, &helper_exe, &step, std::time::Duration::from_secs(30)) - }); - match watch.entered.recv_timeout(entered_timeout) { - Ok(()) => {} - Err(RecvTimeoutError::Timeout) => { - panic!("the helper didn't reach its wait within {entered_timeout:?} (helper already finished: {})", helper.is_finished()) - } - Err(RecvTimeoutError::Disconnected) => panic!("the entered channel disconnected before the helper reached its wait"), - } - old.assert_running("before the first replacement check"); - assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0", "replaced while the old launcher was still running"); - - // A correct wait can't return while the stand-in holds the pipe. - match watch.returned.recv_timeout(blocked_window) { - Err(RecvTimeoutError::Timeout) => {} - Ok(waited) => panic!("run_step left its wait while the old launcher was still running (waited: {waited})"), - Err(RecvTimeoutError::Disconnected) => panic!("the returned channel disconnected while the old launcher was still running"), - } - old.assert_running("after the wait was expected to stay blocked"); - assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0", "replaced while the old launcher was still running"); - + let helper = scope.spawn(move || step_after_wait(&crate::suite::LAUNCHER, &system_program("cmd.exe"), pid, &helper_exe, &step, &wait)); + gate.entered(); + old.assert_running("while the helper was waiting"); + assert_eq!(std::fs::read(&exe).unwrap(), b"0.1.0", "replaced while the old launcher was still running"); + assert!(!process::wait_for_exit(pid, std::time::Duration::ZERO), "the wait reported the running launcher as exited"); old.finish().expect("the stand-in launcher couldn't be reaped"); - match watch.returned.recv_timeout(returned_timeout) { - Ok(waited) => assert!(waited, "the wait reported that the launcher was still running although it had exited"), - Err(RecvTimeoutError::Timeout) => panic!( - "the helper didn't leave its wait within {returned_timeout:?} after the launcher exited (helper already finished: {})", - helper.is_finished() - ), - Err(RecvTimeoutError::Disconnected) => panic!("the returned channel disconnected before reporting the wait's result"), - } - helper.join().unwrap().unwrap(); + gate.release(); + let stepped = helper.join().unwrap(); + assert!(stepped.exited, "the wait reported that the launcher was still running although it had exited"); + stepped.result.unwrap(); }); assert_eq!(std::fs::read(&exe).unwrap(), signed_exe); assert_eq!(std::fs::read_to_string(folder.join("ArtCraftLauncherData").join("settings.json")).unwrap(), "{}"); @@ -1434,7 +1644,7 @@ mod tests { std::fs::create_dir_all(&paths.data).unwrap(); let release = Release { tag: "v0.2.0".into(), ..Release::default() }; mark_installed(&paths, &release); - assert!(unmark_unless_scheduled(&paths, Err("couldn't schedule the update".into())).is_err()); + assert!(unmark_unless_scheduled(&paths, Err::<(), _>("couldn't schedule the update".into())).is_err()); assert!(!just_updated_to(&paths, &release)); mark_installed(&paths, &release); assert!(unmark_unless_scheduled(&paths, Ok(())).is_ok()); @@ -1461,6 +1671,96 @@ mod tests { assert_eq!(take_failure(&paths), None); } + /// The outcome is read once; a failure wins over a finished update, which counts only for the + /// version it installed. + #[test] + fn the_outcome_of_an_update_is_read_once() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + assert_eq!(take_outcome(&paths, "0.2.0"), None); + record_done(&paths, "v0.2.0"); + assert_eq!(take_outcome(&paths, "0.2.0"), Some(Outcome::Installed(Version::new(0, 2, 0)))); + assert_eq!(take_outcome(&paths, "0.2.0"), None, "read once"); + + record_done(&paths, "v0.2.0"); + assert_eq!(take_outcome(&paths, "0.1.0"), None, "the old version still runs"); + assert!(!done_file(&paths).exists(), "a record that doesn't match is removed too"); + + record_done(&paths, "v0.2.0"); + record_failure(&paths, "the Windows Installer stopped with error 1603"); + assert!(!done_file(&paths).exists(), "a failure removes the record of a finished update"); + record_done(&paths, "v0.2.0"); + assert_eq!(take_outcome(&paths, "0.2.0"), Some(Outcome::Failed("the Windows Installer stopped with error 1603".into()))); + assert_eq!(take_outcome(&paths, "0.2.0"), None); + + // Damaged records: no version, an empty reason, a reason longer than the limit. + crate::paths::write_atomic(&done_file(&paths), b"\xff\xfe not a version").unwrap(); + assert_eq!(take_outcome(&paths, "0.2.0"), None); + record_failure(&paths, " \n"); + assert_eq!(take_outcome(&paths, "0.2.0"), Some(Outcome::Failed("no reason was recorded".into()))); + record_failure(&paths, &"é".repeat(RECORD_LIMIT)); + let Some(Outcome::Failed(reason)) = take_outcome(&paths, "0.2.0") else { panic!("a long reason is still a failure") }; + assert!(reason.len() <= RECORD_LIMIT + '…'.len_utf8() && reason.ends_with("é…"), "{}", reason.len()); + // A folder where a record goes is not read, and not removed. + std::fs::create_dir_all(failure_file(&paths)).unwrap(); + std::fs::create_dir_all(done_file(&paths)).unwrap(); + assert_eq!(take_outcome(&paths, "0.2.0"), None); + assert!(failure_file(&paths).is_dir()); + } + + /// A link where a record goes is not followed: neither record is read through it, and the + /// file it points to stays. Skipped where the system refuses to create the link (Windows + /// without the privilege for symbolic links). + #[test] + fn a_record_that_is_a_link_is_not_read() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + std::fs::create_dir_all(paths.logs()).unwrap(); + std::fs::create_dir_all(&paths.data).unwrap(); + let elsewhere = dir.path().join("elsewhere.txt"); + std::fs::write(&elsewhere, "v0.2.0").unwrap(); + #[cfg(unix)] + let link = |to: &Path, at: &Path| std::os::unix::fs::symlink(to, at); + #[cfg(windows)] + let link = |to: &Path, at: &Path| std::os::windows::fs::symlink_file(to, at); + if let Err(e) = link(&elsewhere, &failure_file(&paths)).and_then(|()| link(&elsewhere, &done_file(&paths))) { + eprintln!("skipped: couldn't create a symbolic link ({e})"); + return; + } + assert_eq!(take_failure(&paths), None); + assert_eq!(take_outcome(&paths, "0.2.0"), None); + assert_eq!(std::fs::read_to_string(&elsewhere).unwrap(), "v0.2.0"); + } + + /// Scheduling a new update clears the outcome records of an earlier one, so a stale failure + /// is not reported after it. + #[test] + fn a_new_update_clears_the_last_ones_outcome() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + record_failure(&paths, "the Windows Installer stopped with error 1603"); + mark_installed(&paths, &Release { tag: "v0.3.0".into(), ..Release::default() }); + assert_eq!(take_failure(&paths), None); + assert_eq!(scheduled_version(&paths).as_deref(), Some("v0.3.0")); + } + + /// A release whose tag is not its version (an app may build its own `Release`) is recorded + /// by its version, so the new version's first start still reports the update. + #[test] + fn a_finished_update_is_recorded_by_its_version_not_its_tag() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let release = Release { tag: "stable".into(), version: version::parse("0.2.0"), ..Release::default() }; + mark_installed(&paths, &release); + assert!(just_updated_to(&paths, &release), "the tag still guards against offering it again"); + assert_eq!(scheduled_version(&paths).as_deref(), Some("0.2.0")); + record_done(&paths, &scheduled_version(&paths).unwrap()); + assert_eq!(take_outcome(&paths, "0.2.0"), Some(Outcome::Installed(Version::new(0, 2, 0)))); + // A record without the version line, from before it was written, names the tag. + crate::paths::write_atomic(&marker(&paths), format!("v0.3.0\n{}\n", crate::now()).as_bytes()).unwrap(); + assert_eq!(scheduled_version(&paths).as_deref(), Some("v0.3.0")); + } + /// A launcher that doesn't quit keeps its files, and the download is still removed. #[cfg(windows)] #[test] @@ -1471,19 +1771,11 @@ mod tests { let msi = dir.path().join("artcraft-launcher-0.2.0-windows-x64.msi"); std::fs::write(&msi, "not a package").unwrap(); let running = std::process::id(); - let started = std::time::Instant::now(); - // The test process itself never exits within the wait, so the package is never run. - let error = run_step( - &crate::suite::LAUNCHER, - &system_program("cmd.exe"), - running, - &exe, - &Finish::Msi(msi.clone()), - std::time::Duration::from_millis(300), - ) - .unwrap_err(); + // The test process itself is still running when the wait asks, so the package is never run. + let error = + run_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), running, &exe, &Finish::Msi(msi.clone()), std::time::Duration::ZERO) + .unwrap_err(); assert!(error.contains("still running"), "{error}"); - assert!(started.elapsed() < std::time::Duration::from_secs(30)); assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0"); assert!(!msi.exists()); } @@ -1718,22 +2010,102 @@ mod tests { } } - /// The helper waits for the old launcher to exit before it starts the new one. + /// The restart helper starts only when the restart is started, at the app's exit: a close the + /// user cancels leaves no helper counting down its one-minute wait. The test starts the helper + /// with its standard streams set to null, as `spawn_detached` does, and a thread that waits + /// for the helper to exit writes `exited`, so a helper that exits without writing `started` + /// fails the test. + #[cfg(unix)] + #[test] + fn the_restart_helper_starts_only_when_the_restart_is_started() { + use std::io::{BufRead, Write}; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; + let dir = tempfile::tempdir().unwrap(); + let events = dir.path().join("events"); + assert!(std::process::Command::new("mkfifo").arg(&events).status().unwrap().success()); + // Open for reading and writing, so neither this open nor the helper's blocks. + let pipe = std::fs::OpenOptions::new().read(true).write(true).open(&events).unwrap(); + let mut exited = pipe.try_clone().unwrap(); + let mut lines = std::io::BufReader::new(pipe).lines(); + let mut gone = std::process::Command::new("true").spawn().unwrap(); + gone.wait().unwrap(); + let started = r#"printf 'started\n' > "$1""#; + let command = [std::ffi::OsStr::new("/bin/sh"), "-c".as_ref(), started.as_ref(), "sh".as_ref(), events.as_os_str()]; + let spawned = Arc::new(AtomicBool::new(false)); + let flag = Arc::clone(&spawned); + let spawn = move |mut cmd: std::process::Command| -> Result<(), String> { + flag.store(true, Ordering::SeqCst); + let mut helper = cmd + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .map_err(|e| e.to_string())?; + std::thread::spawn(move || { + let _ = helper.wait(); + let _ = exited.write_all(b"exited\n"); + }); + Ok(()) + }; + let restart = relaunch_with(&crate::suite::LAUNCHER, gone.id(), &command, spawn); + assert!(!spawned.load(Ordering::SeqCst), "the helper must wait for the restart to be started"); + assert_eq!(format!("{restart:?}"), "Restart { pending: true }", "the helper must wait for the restart to be started"); + assert_eq!(restart.start(), Ok(())); + assert_eq!(lines.next().unwrap().unwrap(), "started", "the helper exited without starting the command"); + assert_eq!(lines.next().unwrap().unwrap(), "exited"); + } + + /// The helper waits for the old launcher to exit before it starts the new one. The helper's + /// `sleep` is a script on `PATH` that reports each pause through a named pipe and pauses until + /// the test releases it, and the started command reports whether the old launcher was still + /// running. The test holds both pipes open for reading and writing, so no open blocks, and a + /// thread that waits for the helper to exit writes `exited`, so every read gets a line. #[cfg(unix)] #[test] fn the_restart_waits_for_the_old_process_to_exit() { + use std::io::{BufRead, Write}; + use std::os::unix::fs::PermissionsExt; let dir = tempfile::tempdir().unwrap(); - let marker = dir.path().join("started"); - let mut old = std::process::Command::new("sleep").arg("1").spawn().unwrap(); - relaunch_after_exit(&crate::suite::LAUNCHER, old.id(), &[std::ffi::OsStr::new("touch"), marker.as_os_str()]).unwrap(); - std::thread::sleep(std::time::Duration::from_millis(400)); - assert!(!marker.exists(), "started while the old launcher was still running"); - old.wait().unwrap(); - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); - while !marker.exists() && std::time::Instant::now() < deadline { - std::thread::sleep(std::time::Duration::from_millis(50)); + let (events, release) = (dir.path().join("events"), dir.path().join("release")); + for fifo in [&events, &release] { + assert!(std::process::Command::new("mkfifo").arg(fifo).status().unwrap().success()); } - assert!(marker.exists(), "never started after the old launcher exited"); + let bin = dir.path().join("bin"); + std::fs::create_dir(&bin).unwrap(); + let sleep = bin.join("sleep"); + std::fs::write(&sleep, "#!/bin/sh\nprintf 'waiting\\n' > \"$CRAFT_TEST_EVENTS\"\nread _ < \"$CRAFT_TEST_RELEASE\"\n").unwrap(); + std::fs::set_permissions(&sleep, std::fs::Permissions::from_mode(0o755)).unwrap(); + let mut path = std::ffi::OsString::from(bin.as_os_str()); + if let Some(rest) = std::env::var_os("PATH") { + path.push(":"); + path.push(rest); + } + + let mut old = std::process::Command::new("cat").stdin(std::process::Stdio::piped()).stdout(std::process::Stdio::null()).spawn().unwrap(); + let pid = old.id().to_string(); + let started = r#"if kill -0 "$1" 2>/dev/null; then s=alive; else s=gone; fi; printf 'started %s\n' "$s" > "$CRAFT_TEST_EVENTS""#; + let command = [std::ffi::OsStr::new("/bin/sh"), "-c".as_ref(), started.as_ref(), "sh".as_ref(), pid.as_ref()]; + let open = |fifo: &Path| std::fs::OpenOptions::new().read(true).write(true).open(fifo).unwrap(); + let (events_pipe, mut release_pipe) = (open(&events), open(&release)); + let mut exited = events_pipe.try_clone().unwrap(); + let mut lines = std::io::BufReader::new(events_pipe).lines(); + let mut next = move || lines.next().unwrap().unwrap(); + + let mut helper = helper_command(&crate::suite::LAUNCHER, old.id(), &command); + helper.env("PATH", path).env("CRAFT_TEST_EVENTS", &events).env("CRAFT_TEST_RELEASE", &release); + let mut helper = helper.stdin(std::process::Stdio::null()).stdout(std::process::Stdio::null()).spawn().unwrap(); + std::thread::spawn(move || { + let _ = helper.wait(); + let _ = exited.write_all(b"exited\n"); + }); + + assert_eq!(next(), "waiting", "the helper didn't wait while the old launcher was still running"); + drop(old.stdin.take()); + old.wait().unwrap(); + release_pipe.write_all(b"go\n").unwrap(); + assert_eq!(next(), "started gone", "didn't start once the old launcher had exited"); + assert_eq!(next(), "exited"); } /// A real disk image: the new `ArtCraft Launcher.app` replaces the old one in place. @@ -1986,10 +2358,256 @@ mod tests { #[test] fn a_failed_step_that_removed_the_app_starts_nothing_and_keeps_its_message() { - assert!(restarts(true, true)); - assert!(restarts(true, false)); - assert!(restarts(false, true)); - assert!(!restarts(false, false)); + assert!(restarts(true, true, true)); + assert!(restarts(true, true, false)); + assert!(restarts(true, false, true)); + assert!(!restarts(true, false, false)); + } + + /// An app still running after the helper's wait never gets a second copy, whatever the step + /// and the exe would allow. + #[test] + fn an_app_still_running_is_never_started_again() { + for (step_succeeded, exe_exists) in [(true, true), (true, false), (false, true), (false, false)] { + assert!(!restarts(false, step_succeeded, exe_exists), "{step_succeeded} {exe_exists}"); + } + } + + /// A helper whose wait ends with the old app still running installs nothing, starts no + /// program, records why, and removes the download. + #[cfg(windows)] + #[test] + fn the_helper_starts_nothing_while_the_old_app_runs() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let exe = dir.path().join("artcraft-launcher.exe"); + std::fs::write(&exe, "0.1.0").unwrap(); + let zip = dir.path().join("artcraft-launcher-0.2.0-windows-x64-portable.zip"); + std::fs::write(&zip, "zip").unwrap(); + let step = Finish::Portable { zip: zip.clone(), tmp: dir.path().join(PORTABLE_STAGING) }; + let started = std::sync::Mutex::new(Vec::::new()); + let start = |exe: &Path| { + started.lock().unwrap().push(exe.to_path_buf()); + Ok(()) + }; + let never_exits = |_pid| false; + let error = finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &never_exits, &start) + .unwrap_err(); + assert!(error.contains("still running"), "{error}"); + assert_eq!(*started.lock().unwrap(), Vec::::new(), "no copy of the app may start while it still runs"); + assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0"); + assert!(!zip.exists(), "the download is removed"); + assert_eq!(take_failure(&paths).as_deref(), Some(error.as_str())); + } + + /// A step that fails after the old app exited starts the old app again, as it was. + #[cfg(windows)] + #[test] + fn the_helper_starts_the_old_app_after_a_failed_step() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let exe = dir.path().join("artcraft-launcher.exe"); + std::fs::write(&exe, "0.1.0").unwrap(); + // Not a zip, so the step fails. + let zip = dir.path().join("artcraft-launcher-0.2.0-windows-x64-portable.zip"); + std::fs::write(&zip, "not a zip").unwrap(); + let step = Finish::Portable { zip, tmp: dir.path().join(PORTABLE_STAGING) }; + let started = std::sync::Mutex::new(Vec::::new()); + let start = |exe: &Path| { + started.lock().unwrap().push(exe.to_path_buf()); + Ok(()) + }; + let exited = |_pid| true; + assert!(finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &exited, &start).is_err()); + assert_eq!(*started.lock().unwrap(), vec![exe.clone()]); + assert_eq!(std::fs::read_to_string(&exe).unwrap(), "0.1.0"); + } + + /// When the update step fails and the old app can't be started again either, the error kept + /// for the next start names both. + #[cfg(windows)] + #[test] + fn a_failed_step_and_a_failed_restart_are_both_recorded() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let exe = dir.path().join("artcraft-launcher.exe"); + std::fs::write(&exe, "0.1.0").unwrap(); + // Not a zip, so the step fails. + let zip = dir.path().join("artcraft-launcher-0.2.0-windows-x64-portable.zip"); + std::fs::write(&zip, "not a zip").unwrap(); + let step = Finish::Portable { zip, tmp: dir.path().join(PORTABLE_STAGING) }; + let start = |_exe: &Path| Err("couldn't start artcraft-launcher.exe (denied)".to_string()); + let exited = |_pid| true; + let error = + finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &exited, &start).unwrap_err(); + assert!(error.ends_with("; couldn't start artcraft-launcher.exe (denied)"), "{error}"); + assert!(error.len() > "; couldn't start artcraft-launcher.exe (denied)".len(), "the step's error comes first: {error}"); + assert_eq!(take_failure(&paths).as_deref(), Some(error.as_str())); + } + + /// The helper's result keeps the update step's error when the start fails too. + #[test] + fn a_failed_restart_keeps_the_step_error() { + assert_eq!(with_restart(Ok(()), Ok(())), Ok(())); + assert_eq!(with_restart(Err("step".into()), Ok(())), Err("step".into())); + assert_eq!(with_restart(Ok(()), Err("start".into())), Err("start".into())); + assert_eq!(with_restart(Err("step".into()), Err("start".into())), Err("step; start".into())); + } + + /// A successful step starts the new app once. + #[cfg(windows)] + #[test] + fn the_helper_starts_the_new_app_after_the_update() { + use std::io::Write; + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let folder = dir.path().join("ArtCraft Launcher"); + std::fs::create_dir_all(&folder).unwrap(); + let exe = folder.join("artcraft-launcher.exe"); + std::fs::write(&exe, "0.1.0").unwrap(); + let zip = dir.path().join("artcraft-launcher-0.2.0-windows-x64-portable.zip"); + let mut writer = zip::ZipWriter::new(std::fs::File::create(&zip).unwrap()); + let signed_exe = std::fs::read(system_program("cmd.exe")).unwrap(); + writer.start_file("artcraft-launcher-0.2.0-windows-x64-portable/artcraft-launcher.exe", zip::write::SimpleFileOptions::default()).unwrap(); + writer.write_all(&signed_exe).unwrap(); + writer.finish().unwrap(); + let step = Finish::Portable { zip, tmp: dir.path().join(PORTABLE_STAGING) }; + let started = std::sync::Mutex::new(Vec::<(PathBuf, Vec)>::new()); + let start = |exe: &Path| { + started.lock().unwrap().push((exe.to_path_buf(), std::fs::read(exe).unwrap())); + Ok(()) + }; + let exited = |_pid| true; + mark_installed(&paths, &Release { tag: "v0.2.0".into(), ..Release::default() }); + assert_eq!( + finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &exited, &start), + Ok(()) + ); + assert_eq!(*started.lock().unwrap(), vec![(exe, signed_exe)], "the new app starts once"); + assert_eq!(take_outcome(&paths, "0.2.0"), Some(Outcome::Installed(Version::new(0, 2, 0))), "the new version's first start says so"); + } + + /// A standalone copy updated from the portable zip becomes portable: the outcome and the + /// record of the update go to the folders beside it, which the new app reads. + #[cfg(windows)] + #[test] + fn a_standalone_copy_that_becomes_portable_finds_the_outcome_beside_it() { + use std::io::Write; + let dir = tempfile::tempdir().unwrap(); + // The per-user folders the old, standalone copy used. + let paths = Paths::under(&dir.path().join("per-user")); + let folder = dir.path().join("ArtCraft Launcher"); + std::fs::create_dir_all(&folder).unwrap(); + let exe = folder.join("artcraft-launcher.exe"); + std::fs::write(&exe, "0.1.0").unwrap(); + let zip = dir.path().join("artcraft-launcher-0.2.0-windows-x64-portable.zip"); + let mut writer = zip::ZipWriter::new(std::fs::File::create(&zip).unwrap()); + writer.start_file("artcraft-launcher-0.2.0-windows-x64-portable/artcraft-launcher.exe", zip::write::SimpleFileOptions::default()).unwrap(); + writer.write_all(&std::fs::read(system_program("cmd.exe")).unwrap()).unwrap(); + writer.start_file("artcraft-launcher-0.2.0-windows-x64-portable/portable.txt", zip::write::SimpleFileOptions::default()).unwrap(); + writer.finish().unwrap(); + let step = Finish::Portable { zip, tmp: dir.path().join(PORTABLE_STAGING) }; + let release = Release { tag: "v0.2.0".into(), version: version::parse("0.2.0"), ..Release::default() }; + mark_installed(&paths, &release); + let exited = |_pid| true; + let start = |_: &Path| Ok(()); + assert_eq!( + finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &exited, &start), + Ok(()) + ); + let after = Paths::for_exe(&crate::suite::LAUNCHER, &exe).unwrap(); + assert_eq!(after.data, folder.join("ArtCraftLauncherData").join("data")); + assert!(just_updated_to(&after, &release), "the new app does not offer the same update again"); + assert_eq!(take_outcome(&after, "0.2.0"), Some(Outcome::Installed(Version::new(0, 2, 0)))); + assert_eq!(take_outcome(&paths, "0.2.0"), None, "nothing is left where the new app no longer looks"); + + // A new app that cannot start is reported there too. + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(&dir.path().join("per-user")); + let (exe, step) = portable_update(dir.path()); + if let Finish::Portable { zip, .. } = &step { + let mut writer = zip::ZipWriter::new(std::fs::File::create(zip).unwrap()); + writer + .start_file("artcraft-launcher-0.2.0-windows-x64-portable/artcraft-launcher.exe", zip::write::SimpleFileOptions::default()) + .unwrap(); + writer.write_all(&std::fs::read(system_program("cmd.exe")).unwrap()).unwrap(); + writer.start_file("artcraft-launcher-0.2.0-windows-x64-portable/portable.txt", zip::write::SimpleFileOptions::default()).unwrap(); + writer.finish().unwrap(); + } + mark_installed(&paths, &release); + let denied = |_: &Path| Err("couldn't start the launcher (denied)".to_owned()); + assert!(finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe.clone(), &step, false, &exited, &denied).is_err()); + let after = Paths::for_exe(&crate::suite::LAUNCHER, &exe).unwrap(); + assert_eq!(take_outcome(&after, "0.2.0"), Some(Outcome::Failed("couldn't start the launcher (denied)".into()))); + } + + /// A portable update of the launcher to 0.2.0 in `dir`: its exe and the step. + #[cfg(windows)] + fn portable_update(dir: &Path) -> (PathBuf, Finish) { + use std::io::Write; + let folder = dir.join("ArtCraft Launcher"); + std::fs::create_dir_all(&folder).unwrap(); + let exe = folder.join("artcraft-launcher.exe"); + std::fs::write(&exe, "0.1.0").unwrap(); + let zip = dir.join("artcraft-launcher-0.2.0-windows-x64-portable.zip"); + let mut writer = zip::ZipWriter::new(std::fs::File::create(&zip).unwrap()); + writer.start_file("artcraft-launcher-0.2.0-windows-x64-portable/artcraft-launcher.exe", zip::write::SimpleFileOptions::default()).unwrap(); + writer.write_all(&std::fs::read(system_program("cmd.exe")).unwrap()).unwrap(); + writer.finish().unwrap(); + (exe, Finish::Portable { zip, tmp: dir.join(PORTABLE_STAGING) }) + } + + /// A finished step whose new app can't be started is a failure at the next start, never a + /// finished update. + #[cfg(windows)] + #[test] + fn a_failed_relaunch_is_not_reported_as_a_finished_update() { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let (exe, step) = portable_update(dir.path()); + mark_installed(&paths, &Release { tag: "v0.2.0".into(), ..Release::default() }); + let start = |_: &Path| Err("couldn't start the launcher (denied)".to_owned()); + let exited = |_pid| true; + let result = finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe, &step, false, &exited, &start); + assert_eq!(result, Err("couldn't start the launcher (denied)".into())); + assert_eq!(take_outcome(&paths, "0.2.0"), Some(Outcome::Failed("couldn't start the launcher (denied)".into()))); + } + + /// A helper that times out or whose step fails records no finished update. + #[cfg(windows)] + #[test] + fn a_timed_out_or_failed_step_is_not_reported_as_a_finished_update() { + for (exits, zip_ok) in [(false, true), (true, false)] { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let (exe, step) = portable_update(dir.path()); + if !zip_ok && let Finish::Portable { zip, .. } = &step { + std::fs::write(zip, "not a zip").unwrap(); + } + mark_installed(&paths, &Release { tag: "v0.2.0".into(), ..Release::default() }); + let start = |_: &Path| Ok(()); + let wait = |_pid| exits; + assert!(finish_step(&crate::suite::LAUNCHER, &system_program("cmd.exe"), &paths, 4242, exe, &step, false, &wait, &start).is_err()); + assert!(!done_file(&paths).exists(), "exits {exits}, zip {zip_ok}"); + assert!(matches!(take_outcome(&paths, "0.2.0"), Some(Outcome::Failed(_))), "exits {exits}, zip {zip_ok}"); + } + } + + /// A prepared Windows restart does nothing until it is started, and runs once when it is. + #[test] + fn a_prepared_restart_runs_only_when_started() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + let runs = Arc::new(AtomicUsize::new(0)); + let counter = Arc::clone(&runs); + let restart = Restart::later(move || { + counter.fetch_add(1, Ordering::SeqCst); + Ok(()) + }); + assert_eq!(runs.load(Ordering::SeqCst), 0, "nothing starts before the app closes"); + assert_eq!(restart.start(), Ok(())); + assert_eq!(runs.load(Ordering::SeqCst), 1); + assert_eq!(Restart::scheduled().start(), Ok(())); } #[test] diff --git a/crates/update/src/updater.rs b/crates/update/src/updater.rs index 9211f257..c45da32c 100644 --- a/crates/update/src/updater.rs +++ b/crates/update/src/updater.rs @@ -4,17 +4,22 @@ //! handling of failures, dropped workers, repaints and quitting. //! //! An app the launcher keeps on another version (see [`crate::pins`]) offers nothing itself: the -//! check reads the shared choice first, and a requested check reports [`Status::UpToDate`] while -//! [`Updater::managed`] says why. +//! check reads the shared choice first, and a requested check reports [`Status::Managed`] while +//! [`Updater::managed`] says which choice it is. +//! +//! An update ends with the app closing. On Windows the update installs only after the app has +//! exited, so the updater says it is ready rather than installed, and the first start afterwards +//! reports what came of it: [`Status::Updated`] or [`Status::InstallFailed`]. use std::path::PathBuf; -use std::sync::Arc; -use std::sync::mpsc::{Receiver, TryRecvError, channel}; +use std::sync::mpsc::{Receiver, SendError, Sender, TryRecvError, channel}; +use std::sync::{Arc, Mutex, PoisonError}; use std::time::Duration; use crate::dialog::{StartupPrompt, StartupPromptResponse}; use crate::pins::{Choice, Pins}; use crate::release::Release; +use crate::self_update::{Outcome, Restart}; use crate::translate::{Note, Translate, english}; use crate::{App, Http, Paths, Platform, Stop, check_for_update, self_update}; @@ -22,7 +27,8 @@ use crate::{App, Http, Paths, Platform, Stop, check_for_update, self_update}; /// with one; without it the check asks GitHub. pub type CheckSource = Arc Result, String> + Send + Sync>; -/// What a requested check came to, for a window that answers Help ▸ Check for Updates. +/// What a requested check came to, for a window that answers Help ▸ Check for Updates, and what +/// the last update came to, at the first start after it. #[derive(Clone, Debug, PartialEq, Eq)] pub enum Status { /// Nothing to report. @@ -33,16 +39,40 @@ pub enum Status { UpToDate, /// A requested check failed. Failed(String), + /// A requested check found an update downloading and installing; the startup prompt shows + /// its progress. + Installing, + /// A requested check found an update ready and waiting for the app to close + /// ([`READY_TEXT`]). [`Updater::restart_to_update`] asks the app to close. + Ready, + /// A requested check found that the launcher keeps this app on a version of its choosing + /// ([`Updater::managed`]), so it does not update itself. + Managed, + /// This start follows an update that installed this version. + Updated(String), + /// The last update could not be installed, for this reason. + InstallFailed(String), } +/// What [`Status::Ready`] says: on Windows the update installs after the app exits; elsewhere it +/// is already in place and takes effect when the app restarts. +pub const READY_TEXT: &str = + if cfg!(windows) { "Update ready. Close {app} to install it." } else { "The update is installed. Close {app} to finish; it restarts by itself." }; + /// What the app should do after [`Updater::show`]. #[derive(Clone, Debug, PartialEq, Eq)] pub enum Action { None, /// Open this page in the browser. OpenUrl(String), - /// Close the app through its normal close path (which asks about unsaved work): the update is - /// installed and a helper restarts the app once it has exited. + /// Close the app through its normal close path (which asks about unsaved work): an update is + /// ready and a helper installs it and restarts the app once the app has exited. This is a + /// request: the user may cancel the close, and the update then stays ready, with + /// [`Status::Ready`] for a requested check and Restart to Update in the prompt. The helper + /// starts only at the exit: when the app drops the [`Updater`] as it exits, or + /// calls [`Updater::start_pending_update`] in `on_exit`, which an app that ends the process + /// there without dropping the `Updater` must do. An installation still in progress when + /// such an app ends the process is lost, since its worker thread ends with the process. Close, } @@ -66,6 +96,23 @@ impl Failure { fn dropped() -> Self { Self::Updater(Note::new("The update stopped unexpectedly.", &[])) } + + /// The reason in English, for the log and for the record the next start reads. + fn reason(&self) -> String { + match self { + Self::Installer(e) => e.clone(), + Self::Updater(note) => note.translated(english), + } + } +} + +/// Logs a failed update and records it under `paths`, so that the next start reports it +/// ([`Status::InstallFailed`]). Used at the exit, where no prompt can show the reason. +fn record(app: &App, paths: Option<&Paths>, reason: &str) { + log::warn!("{} self-update failed: {reason}", app.name); + if let Some(paths) = paths { + self_update::record_failure(paths, reason); + } } impl From for Failure { @@ -89,11 +136,26 @@ pub struct Updater { requested: bool, started: bool, check: Option>, - install: Option<(Receiver>, Release)>, + install: Option<(Receiver>, Release)>, prompt: Option, - restart: bool, - /// An update is installed and waits for the app to exit; nothing starts a second one. + /// A finished installation waiting for the app to exit: what still has to start then, and + /// the release it installs. + restart: Option<(Restart, Release)>, + /// An update is ready and waits for the app to exit; nothing starts a second one. installed: bool, + /// [`Self::show_if`] returns [`Action::Close`] at its next frame that may close: once when an + /// installation finishes, and again for each [`Self::restart_to_update`]. + close_wanted: bool, + /// The folders that hold the outcome records ([`self_update::take_outcome`]) in place of the + /// app's own (tests). + paths: Option, + /// Why the last update could not be installed, until a prompt that offers the update again + /// shows it. + failed_reason: Option, + /// Set when the app exits: the `Updater` is dropped or [`Updater::start_pending_update`] is + /// called. An install worker sends its outcome while holding it, and starts a finished + /// installation's helper itself once it is set. + exiting: Arc>, /// The version-choice file to read in place of [`Pins::default_path`] (tests). pins_path: Option, /// The launcher's choice for this app when it is not stable, as of the last check or offer. @@ -116,8 +178,12 @@ impl Updater { check: None, install: None, prompt: None, - restart: false, + restart: None, installed: false, + close_wanted: false, + paths: None, + failed_reason: None, + exiting: Arc::new(Mutex::new(false)), pins_path: None, managed: None, translate: english, @@ -141,9 +207,19 @@ impl Updater { StartupPrompt::new(self.app, release, self.version.clone()).with_translator(self.translate) } - /// "The update is installed…", translated when the prompt draws. + /// Opens the prompt offering `release`. A failed update offered again says why the last + /// attempt failed, once. + fn open_offer(&mut self, release: Release) { + let mut prompt = self.prompt_for(release); + if let Some(reason) = self.failed_reason.take() { + prompt.set_note(Note::new("The update could not be installed: {reason}", &[("reason", &reason)])); + } + self.prompt = Some(prompt); + } + + /// [`READY_TEXT`], translated when the prompt draws. fn installed_note(&self) -> Note { - Note::new("The update is installed. Close {app} to finish; it restarts by itself.", &[("app", self.app.name)]) + Note::new(READY_TEXT, &[("app", self.app.name)]) } /// Replaces the network with `source` (tests). @@ -152,6 +228,34 @@ impl Updater { self } + /// Reads and writes the update's outcome records under `paths` instead of the app's own + /// folders (tests). + pub fn with_paths(mut self, paths: Paths) -> Self { + self.paths = Some(paths); + self + } + + /// The folders of the outcome records: [`Self::with_paths`]'s, else the app's own. Unit tests + /// never read or write the app's own records. + fn outcome_paths(&self) -> Option { + self.paths.clone().or_else(|| if cfg!(test) { None } else { Paths::for_app(self.app).ok() }) + } + + /// What the last update came to, once: [`Status::Updated`] when it installed the version + /// running now, [`Status::InstallFailed`] with the reason when it failed. + fn read_outcome(&mut self) { + let Some(paths) = self.outcome_paths() else { return }; + match self_update::take_outcome(&paths, &self.version) { + Some(Outcome::Installed(version)) => self.status = Status::Updated(version.to_string()), + Some(Outcome::Failed(reason)) => { + log::warn!("{} self-update failed: {reason}", self.app.name); + self.failed_reason = Some(reason.clone()); + self.status = Status::InstallFailed(reason); + } + None => {} + } + } + /// Reads the version choices from `path` instead of the launcher's file (tests). pub fn with_pins_path(mut self, path: impl Into) -> Self { self.pins_path = Some(path.into()); @@ -183,24 +287,39 @@ impl Updater { self.managed.is_some() } - /// The check at start, once. Later calls do nothing. + /// The check at start, once. Later calls do nothing. It first reads what the last update came + /// to, once: [`Self::status`] is then [`Status::Updated`] or [`Status::InstallFailed`], and + /// the app shows its [`CheckResultDialog`](crate::dialog::CheckResultDialog) while the status + /// is not [`Status::Idle`]. The check itself stays quiet unless it finds a release. pub fn check_at_start(&mut self, ctx: &egui::Context) { if !std::mem::replace(&mut self.started, true) { + self.read_outcome(); self.begin(ctx, false); } } /// Help ▸ Check for Updates: a check whose outcome [`Self::status`] reports. A check already - /// running becomes a requested one; nothing starts while an update installs. + /// running becomes a requested one. While an update installs or waits for the app to close, + /// nothing starts and the status says so ([`Status::Installing`], [`Status::Ready`]). pub fn check_now(&mut self, ctx: &egui::Context) { self.started = true; self.begin(ctx, true); } - /// Opens the startup prompt for a release the app found itself. + /// Asks the app to close so that a ready update installs: the next [`Self::show_if`] that may + /// close returns [`Action::Close`]. Restart to Update calls it after the user canceled an + /// earlier close. No close is asked for while no update is ready. + pub fn restart_to_update(&mut self) { + self.close_wanted = true; + } + + /// Opens the startup prompt for a release the app found itself. After a failed update the + /// prompt says why it failed, like the one a check opens. Opening the prompt sets + /// [`Self::status`] to [`Status::Idle`]. pub fn offer(&mut self, release: Release) { if self.install.is_none() && !self.installed && !self.read_choice() { - self.prompt = Some(self.prompt_for(release)); + self.open_offer(release); + self.status = Status::Idle; } } @@ -222,6 +341,9 @@ impl Updater { fn begin(&mut self, ctx: &egui::Context, requested: bool) { if self.install.is_some() || self.installed { + if requested { + self.status = self.pending_status(); + } return; } if requested { @@ -254,28 +376,83 @@ impl Updater { } } + /// The answer to a requested check while an update installs or waits for the app to exit. A + /// helper that already started while the app kept running may have failed: its recorded + /// reason is the answer then, the update is on offer again, and an open prompt says why + /// instead of offering Restart to Update. + fn pending_status(&mut self) -> Status { + if self.install.is_some() { + return Status::Installing; + } + if self.restart.is_none() + && let Some(reason) = self.outcome_paths().and_then(|paths| self_update::take_failure(&paths)) + { + log::warn!("{} self-update failed: {reason}", self.app.name); + let reason = reason.trim().to_owned(); + self.installed = false; + self.close_wanted = false; + match &mut self.prompt { + Some(prompt) => { + prompt.is_installing = false; + prompt.is_ready = false; + prompt.set_note(Note::new("The update could not be installed: {reason}", &[("reason", &reason)])); + } + // The next prompt that offers the update says why. + None => self.failed_reason = Some(reason.clone()), + } + return Status::InstallFailed(reason); + } + Status::Ready + } + + /// Sets the outcome of a check. The check at start leaves the status as it is: nothing, or + /// the outcome of the last update ([`Self::read_outcome`]). fn finish_check(&mut self, answer: Checked) { let requested = std::mem::take(&mut self.requested); + // A check that was running when an installation started answers with the installation's + // state: the update installing, ready, or the recorded helper failure. + if self.install.is_some() || self.installed { + match &answer { + Ok(Some(release)) => log::info!("{} update check found {} while an update installs", self.app.name, release.tag), + Ok(None) => log::info!("{} update check found nothing newer while an update installs", self.app.name), + Err(e) => log::warn!("{} update check failed while an update installs: {e}", self.app.name), + } + if requested { + self.status = self.pending_status(); + } + return; + } // The launcher may have pinned or unpinned the app while the check ran: whatever the - // answer, an app it manages offers nothing and reports nothing to do. + // answer, an app it manages offers nothing and says so. if self.read_choice() { if let Err(e) = &answer { log::info!("{} update check failed ({e}), but the launcher manages its version", self.app.name); } - self.status = if requested { Status::UpToDate } else { Status::Idle }; + if requested { + self.status = Status::Managed; + } return; } match answer { Ok(Some(release)) => { - self.status = Status::Idle; - if self.prompt.is_none() && self.install.is_none() && !self.installed { - self.prompt = Some(self.prompt_for(release)); + match &mut self.prompt { + None => self.open_offer(release), + // A prompt the app opened before (`offer`) says why the last update failed. + Some(prompt) => { + if let Some(reason) = self.failed_reason.take() { + prompt.set_note(Note::new("The update could not be installed: {reason}", &[("reason", &reason)])); + } + } } + self.status = Status::Idle; } - Ok(None) => self.status = if requested { Status::UpToDate } else { Status::Idle }, + Ok(None) if requested => self.status = Status::UpToDate, + Ok(None) => {} Err(e) => { log::warn!("{} update check failed: {e}", self.app.name); - self.status = if requested { Status::Failed(e) } else { Status::Idle }; + if requested { + self.status = Status::Failed(e); + } } } } @@ -308,20 +485,29 @@ impl Updater { if let Some(outcome) = outcome { let release = self.install.take().map(|(_, release)| release); match outcome { - Ok(()) => { + Ok(restart) => { self.installed = true; - self.restart = true; + self.restart = release.map(|release| (restart, release)); + self.close_wanted = true; let note = self.installed_note(); if let Some(prompt) = &mut self.prompt { prompt.is_installing = false; + prompt.is_ready = true; prompt.set_note(note); } + if matches!(self.status, Status::Installing) { + self.status = Status::Ready; + } } Err(failure) => { match &failure { Failure::Installer(e) => log::warn!("{} self-update failed: {e}", self.app.name), Failure::Updater(note) => log::warn!("{} self-update failed: {}", self.app.name, note.translated(english)), } + // The prompt says why; a result window showing the installation closes. + if matches!(self.status, Status::Installing) { + self.status = Status::Idle; + } // The prompt may have been dismissed meanwhile: bring it back with the reason. if self.prompt.is_none() && let Some(release) = release @@ -346,14 +532,14 @@ impl Updater { self.show_if(ctx, true) } - /// Like [`Self::show`], but an installation only starts when `can_start` is true. An app passes - /// `false` while it holds unsaved work: the restart helper is scheduled before the app closes, - /// and a close that the user then cancels would leave it waiting for an exit that never comes. + /// Like [`Self::show`], but an installation only starts, and a ready update only asks the app + /// to close, when `can_start` is true. An app passes `false` while it holds unsaved work. pub fn show_if(&mut self, ctx: &egui::Context, can_start: bool) -> Action { // The close waits for the app to hold no unsaved work: the user may have edited while the - // update installed, and a close they cancel is asked again once the work is saved. - if self.restart && can_start { - self.restart = false; + // update installed. It is only asked for: the helper, which waits just a minute for the + // app to exit, starts at the exit itself (`start_pending_update`), so a close the user + // cancels leaves the update ready. + if can_start && self.restart.is_some() && std::mem::take(&mut self.close_wanted) { return Action::Close; } let Some(prompt) = &mut self.prompt else { return Action::None }; @@ -376,6 +562,45 @@ impl Updater { } } + /// Starts the helper of a ready update, so that the update installs once the app has exited. + /// Call it once the app's exit is certain (a close nothing can cancel any more, in `on_exit`), + /// whether or not the exit came through [`Action::Close`]; dropping the `Updater` calls it + /// too. Later calls do nothing. An installation still running at the first call starts its + /// helper itself when it finishes, as long as the process is still running then, and no + /// longer reports through [`Updater::tick`]; an app that ends the process in `on_exit` loses + /// an installation still in progress, since its worker thread ends with the process. A + /// helper that cannot start, or an installation that failed and was not reported yet, is + /// recorded, and the next start reports it ([`Status::InstallFailed`]). + pub fn start_pending_update(&mut self) { + // Held while the channel is read: a worker sent its outcome before, and it is read here, + // or it sends after and starts its helper (or records its failure) itself. + let exiting = Arc::clone(&self.exiting); + let mut exiting = exiting.lock().unwrap_or_else(PoisonError::into_inner); + *exiting = true; + // An installation that finished since the last `tick`, or whose worker ended without a word. + // One still running is let go: it now reports through `deliver` alone. + if let Some((rx, release)) = self.install.take() + && let Some(outcome) = match rx.try_recv() { + Ok(outcome) => Some(outcome), + Err(TryRecvError::Empty) => None, + Err(TryRecvError::Disconnected) => Some(Err(Failure::dropped())), + } + { + match outcome { + Ok(restart) => { + self.restart = Some((restart, release)); + self.installed = true; + } + Err(failure) => record(self.app, self.outcome_paths().as_ref(), &failure.reason()), + } + } + if let Some((restart, _)) = self.restart.take() + && let Err(e) = restart.start() + { + record(self.app, self.outcome_paths().as_ref(), &e); + } + } + fn request_install(&mut self, ctx: &egui::Context, can_start: bool) { if can_start { self.start_install(ctx); @@ -390,9 +615,14 @@ impl Updater { return; } let installed_note = self.installed_note(); + let paths = self.outcome_paths(); let Some(prompt) = &mut self.prompt else { return }; if self.installed { + // Restart to update: the app is asked to close again. prompt.set_note(installed_note); + if self.restart.is_some() { + self.close_wanted = true; + } return; } let Some(installed) = self_update::current(self.app) else { @@ -402,13 +632,15 @@ impl Updater { }; prompt.is_installing = true; prompt.clear_message(); + self.failed_reason = None; let (tx, rx) = channel(); let (app, version, ctx, release) = (self.app, self.version.clone(), ctx.clone(), prompt.release.clone()); let worker_release = release.clone(); + let exiting = handoff(&mut self.exiting); let spawned = std::thread::Builder::new().name("self-update".into()).spawn(move || { let run = || install(app, &installed, worker_release, &version).map_err(Failure::Installer); let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(run)).unwrap_or_else(|_| Err(Failure::panicked())); - let _ = tx.send(outcome); + deliver(app, &tx, &exiting, paths.as_ref(), outcome); ctx.request_repaint(); }); match spawned { @@ -421,7 +653,45 @@ impl Updater { } } -fn install(app: &'static App, installed: &self_update::Install, mut release: Release, version: &str) -> Result<(), String> { +impl Drop for Updater { + /// The app is exiting: an installed update that is still waiting starts its helper, which + /// waits for this process to exit before it installs. + fn drop(&mut self) { + self.start_pending_update(); + } +} + +/// The flag a new install worker sends its outcome under (the `Updater`'s `exiting`). After +/// [`Updater::start_pending_update`] set it while the app kept running (a helper that then failed +/// puts the update on offer again), a new flag replaces it, so that the new worker reports +/// through `tick` instead of starting its helper at once. A worker let go before keeps the old one. +fn handoff(exiting: &mut Arc>) -> Arc> { + if *exiting.lock().unwrap_or_else(PoisonError::into_inner) { + *exiting = Arc::new(Mutex::new(false)); + } + Arc::clone(exiting) +} + +/// Sends the install worker's outcome to the [`Updater`]. When the app is exiting (the `Updater` +/// was dropped or [`Updater::start_pending_update`] was called), the worker starts a finished +/// installation's helper itself, since nothing will start it at a close. A helper that cannot +/// start, or a failed installation, is then recorded under `paths` for the next start. +fn deliver(app: &App, tx: &Sender>, exiting: &Mutex, paths: Option<&Paths>, outcome: Result) { + let gone = exiting.lock().unwrap_or_else(PoisonError::into_inner); + let unsent = if *gone { Some(outcome) } else { tx.send(outcome).err().map(|SendError(outcome)| outcome) }; + drop(gone); + match unsent { + Some(Ok(restart)) => { + if let Err(e) = restart.start() { + record(app, paths, &e); + } + } + Some(Err(failure)) => record(app, paths, &failure.reason()), + None => {} + } +} + +fn install(app: &'static App, installed: &self_update::Install, mut release: Release, version: &str) -> Result { let http = Http::new(&format!("{}/Update", app.name)).map_err(|e| format!("could not initialize HTTP: {e}"))?; if release.assets.is_empty() && let Some(full) = check_for_update(app, &http, version).map_err(|e| format!("could not fetch release assets: {e}"))? @@ -430,7 +700,7 @@ fn install(app: &'static App, installed: &self_update::Install, mut release: Rel } let paths = Paths::for_app(app).map_err(|e| format!("could not find paths: {e}"))?; let platform = Platform::current().ok_or("unsupported platform for self update")?; - self_update::update(app, installed, &release, platform, &http, &paths, &Stop::default(), &mut |_| {}, &|| Ok(()), Some(version)) + self_update::prepare_update(app, installed, &release, platform, &http, &paths, &Stop::default(), &mut |_| {}, &|| Ok(()), Some(version)) } /// Download in the prompt: the release page, when it is a safe web link (an app may build its @@ -460,13 +730,19 @@ mod tests { std::env::temp_dir().join("craft-update-tests-no-such-dir").join("versions.json") } - /// Ticks until the running check has reported. + /// Waits for the running check's answer on its channel (its thread always sends one, or ends + /// and closes the channel), then hands it to `tick` as a frame would. pub(super) fn settle(updater: &mut Updater, ctx: &egui::Context) { - let deadline = std::time::Instant::now() + Duration::from_secs(10); - while updater.check.is_some() && std::time::Instant::now() < deadline { - updater.tick(ctx); - std::thread::sleep(Duration::from_millis(5)); + let Some(rx) = updater.check.take() else { return }; + let (tx, answer) = channel(); + if let Ok(checked) = rx.recv() { + assert!(tx.send(checked).is_ok()); } + // Without an answer `tx` is dropped here, and `tick` sees a check that stopped. + drop(tx); + updater.check = Some(answer); + updater.tick(ctx); + assert!(updater.check.is_none(), "the check has reported"); } /// #391: Download opens only a safe web link, whoever built the `Release`. @@ -642,7 +918,7 @@ mod tests { fn a_dropped_install_worker_is_an_error() { let ctx = egui::Context::default(); let mut u = updater(Ok(None)); - let (tx, rx) = channel::>(); + let (tx, rx) = channel::>(); u.install = Some((rx, newer())); drop(tx); u.tick(&ctx); @@ -701,7 +977,7 @@ mod tests { let mut u = updater(Ok(Some(newer()))); let (tx, rx) = channel(); u.install = Some((rx, newer())); - assert!(tx.send(Ok(())).is_ok()); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); u.tick(&ctx); assert_eq!(u.show(&ctx), Action::Close); u.prompt = None; @@ -739,14 +1015,35 @@ mod tests { assert!(message(&u).starts_with("Save or close"), "{}", message(&u)); FRENCH.store(true, Ordering::Relaxed); assert!(message(&u).starts_with("FR Save or close"), "{}", message(&u)); - // An installed update names the app, in the language of the moment. + // A ready update names the app, in the language of the moment. let (tx, rx) = channel(); u.install = Some((rx, newer())); - assert!(tx.send(Ok(())).is_ok()); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); u.tick(&ctx); - assert!(message(&u).starts_with("FR The update is installed. Close PhotoCraft"), "{}", message(&u)); + let ready = crate::translate::fill(READY_TEXT, &[("app", "PhotoCraft")]); + assert_eq!(message(&u), format!("FR {ready}")); FRENCH.store(false, Ordering::Relaxed); - assert!(message(&u).starts_with("The update is installed."), "{}", message(&u)); + assert_eq!(message(&u), ready); + } + + /// On Windows the update installs after the app exits, so a finished preparation says it is + /// ready, never that it is installed. + #[test] + fn a_prepared_update_says_when_it_installs() { + if cfg!(windows) { + assert_eq!(READY_TEXT, "Update ready. Close {app} to install it."); + } else { + assert!(READY_TEXT.starts_with("The update is installed."), "{READY_TEXT}"); + } + let ctx = egui::Context::default(); + let mut u = updater(Ok(None)); + u.offer(newer()); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); + u.tick(&ctx); + let prompt = u.prompt.as_ref().map(|p| (p.is_installing, p.is_ready, p.message())); + assert_eq!(prompt, Some((false, true, Some(crate::translate::fill(READY_TEXT, &[("app", "PhotoCraft")]))))); } /// One frame with the updater's prompt drawn. @@ -763,7 +1060,7 @@ mod tests { u.offer(newer()); let (tx, rx) = channel(); u.install = Some((rx, newer())); - assert!(tx.send(Ok(())).is_ok()); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); u.tick(&ctx); assert_eq!(frame(&mut u, &ctx, false), Action::None, "unsaved work holds the close back"); assert_eq!(frame(&mut u, &ctx, true), Action::Close); @@ -776,11 +1073,565 @@ mod tests { let mut u = updater(Ok(None)); let (tx, rx) = channel(); u.install = Some((rx, newer())); - assert!(tx.send(Ok(())).is_ok()); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); u.tick(&ctx); assert_eq!(u.show(&ctx), Action::Close); assert_eq!(u.show(&ctx), Action::None); } + + /// A finished installation that still has to start its helper asks the app to close only + /// when it may, and starts the helper only at the exit, once. + #[test] + fn the_helper_starts_once_when_the_app_exits() { + use std::sync::atomic::{AtomicUsize, Ordering}; + let ctx = egui::Context::default(); + let mut u = updater(Ok(None)); + u.offer(newer()); + let starts = Arc::new(AtomicUsize::new(0)); + let counter = Arc::clone(&starts); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + let restart = Restart::later(move || { + counter.fetch_add(1, Ordering::SeqCst); + Ok(()) + }); + assert!(tx.send(Ok(restart)).is_ok()); + u.tick(&ctx); + assert_eq!(starts.load(Ordering::SeqCst), 0, "the helper waits for the close"); + assert_eq!(frame(&mut u, &ctx, false), Action::None); + assert_eq!(starts.load(Ordering::SeqCst), 0, "unsaved work holds the helper back"); + assert_eq!(frame(&mut u, &ctx, true), Action::Close); + assert_eq!(starts.load(Ordering::SeqCst), 0, "a close is a request; the helper waits for the exit"); + assert_eq!(frame(&mut u, &ctx, true), Action::None); + u.start_pending_update(); + assert_eq!(starts.load(Ordering::SeqCst), 1); + u.start_pending_update(); + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 1, "the helper starts once"); + } + + /// A close the user cancels starts no helper and keeps the update ready: a requested check + /// says so, and Restart to Update asks for the close again, from the result window or from + /// the prompt. + #[test] + fn a_canceled_close_keeps_the_update_ready() { + use std::sync::atomic::Ordering; + let ctx = egui::Context::default(); + let mut u = updater(Ok(None)); + u.offer(newer()); + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(restart)).is_ok()); + u.tick(&ctx); + assert_eq!(frame(&mut u, &ctx, true), Action::Close); + // The app's close was canceled: it keeps running and drawing. + for _ in 0..3 { + assert_eq!(frame(&mut u, &ctx, true), Action::None, "a canceled close is not asked for again by itself"); + } + assert_eq!(starts.load(Ordering::SeqCst), 0, "no helper starts while the app runs"); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Ready); + assert!(u.check.is_none(), "no check runs while an update is ready"); + assert!(u.prompt.as_ref().is_some_and(|p| p.is_ready), "the prompt offers Restart to Update"); + + u.restart_to_update(); + assert_eq!(frame(&mut u, &ctx, false), Action::None, "unsaved work still holds the close back"); + assert_eq!(frame(&mut u, &ctx, true), Action::Close); + assert_eq!(frame(&mut u, &ctx, true), Action::None); + // The prompt's button does the same. + u.request_install(&ctx, true); + assert_eq!(frame(&mut u, &ctx, true), Action::Close); + assert_eq!(starts.load(Ordering::SeqCst), 0); + + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 1, "the exit starts the helper once"); + } + + /// Restart to Update does nothing when no update is ready. + #[test] + fn restart_to_update_needs_a_ready_update() { + let ctx = egui::Context::default(); + let mut u = updater(Ok(None)); + u.restart_to_update(); + assert_eq!(frame(&mut u, &ctx, true), Action::None); + } + + /// A counter and a helper that adds one to it each time it starts. + fn counted_restart() -> (Arc, Restart) { + let starts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let counter = Arc::clone(&starts); + let restart = Restart::later(move || { + counter.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Ok(()) + }); + (starts, restart) + } + + /// An app that exits without [`Action::Close`] still starts the waiting helper, once, when + /// the updater is dropped. + #[test] + fn dropping_the_updater_starts_a_waiting_helper_once() { + use std::sync::atomic::Ordering; + let ctx = egui::Context::default(); + let mut u = updater(Ok(None)); + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(restart)).is_ok()); + u.tick(&ctx); + assert_eq!(frame(&mut u, &ctx, false), Action::None, "unsaved work holds the close back"); + assert_eq!(starts.load(Ordering::SeqCst), 0); + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 1); + + // Picked up from the worker without a `tick` first. + let mut u = updater(Ok(None)); + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(restart)).is_ok()); + u.start_pending_update(); + assert_eq!(starts.load(Ordering::SeqCst), 1); + u.start_pending_update(); + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 1, "the helper starts once"); + } + + /// An installation that finishes after the updater was dropped starts its helper once; one + /// the updater receives waits for the close. + #[test] + fn an_install_finishing_after_the_updater_is_dropped_starts_its_helper() { + use std::sync::atomic::Ordering; + let mut u = updater(Ok(None)); + let exiting = Arc::clone(&u.exiting); + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 0); + deliver(&crate::suite::LAUNCHER, &tx, &exiting, None, Ok(restart)); + assert_eq!(starts.load(Ordering::SeqCst), 1); + + // Sent while the drop has read the channel but not yet released the receiver. + let (starts, restart) = counted_restart(); + let (tx, _rx) = channel(); + deliver(&crate::suite::LAUNCHER, &tx, &Mutex::new(true), None, Ok(restart)); + assert_eq!(starts.load(Ordering::SeqCst), 1, "a receiver about to be dropped is not sent to"); + + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + deliver(&crate::suite::LAUNCHER, &tx, &Mutex::new(false), None, Ok(restart)); + assert_eq!(starts.load(Ordering::SeqCst), 0, "the updater starts it at the close"); + assert!(rx.try_recv().is_ok_and(|outcome| outcome.is_ok())); + } + + /// An installation still running when the app calls `start_pending_update` in `on_exit`, and + /// that finishes before the updater is dropped, starts its helper once; the worker's end is + /// not then recorded as a failure. + #[test] + fn an_install_finishing_after_start_pending_update_starts_its_helper_once() { + use std::sync::atomic::Ordering; + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + let exiting = Arc::clone(&u.exiting); + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + u.start_pending_update(); + assert_eq!(starts.load(Ordering::SeqCst), 0, "nothing has finished yet"); + deliver(&PHOTOCRAFT, &tx, &exiting, Some(&paths), Ok(restart)); + drop(tx); + assert_eq!(starts.load(Ordering::SeqCst), 1, "the worker starts the helper itself"); + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 1, "the helper starts once"); + let mut next = reading(&paths, "0.1.0"); + start_answered(&mut next, &ctx, Ok(None)); + assert_eq!(next.status(), &Status::Idle, "a helper that started is not a failure"); + } + + /// A failed installation picked up at exit leaves nothing installed and nothing waiting. + #[test] + fn a_failed_install_picked_up_at_exit_leaves_nothing_waiting() { + let mut u = updater(Ok(None)); + let (tx, rx) = channel::>(); + u.install = Some((rx, newer())); + assert!(tx.send(Err("couldn't download the update".into())).is_ok()); + u.start_pending_update(); + assert!(u.install.is_none(), "the finished job is cleared"); + assert!(!u.installed); + assert!(u.restart.is_none()); + } + + /// After [`Action::Close`], the app's exit (the updater dropped) starts the helper once. + #[test] + fn the_exit_after_a_close_starts_the_helper_once() { + use std::sync::atomic::Ordering; + let ctx = egui::Context::default(); + let mut u = updater(Ok(None)); + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(restart)).is_ok()); + u.tick(&ctx); + assert_eq!(frame(&mut u, &ctx, true), Action::Close); + assert_eq!(frame(&mut u, &ctx, true), Action::None, "the close is asked for once"); + assert_eq!(starts.load(Ordering::SeqCst), 0); + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 1); + } + + /// Outcome records in a fresh folder, and an updater reading them that runs `version`. + fn with_records(version: &str) -> (tempfile::TempDir, Paths, Updater) { + let dir = tempfile::tempdir().unwrap(); + let paths = Paths::under(dir.path()); + let u = reading(&paths, version); + (dir, paths, u) + } + + /// An updater running `version` that reads the outcome records under `paths`. + fn reading(paths: &Paths, version: &str) -> Updater { + Updater::new(&PHOTOCRAFT, version).with_source(Arc::new(|| Ok(None))).with_pins_path(no_pins()).with_paths(paths.clone()) + } + + /// The check at start, answered with `answer` without waiting on its thread. + fn start_answered(u: &mut Updater, ctx: &egui::Context, answer: Checked) { + u.check_at_start(ctx); + let (tx, rx) = channel(); + u.check = Some(rx); + assert!(tx.send(answer).is_ok()); + u.tick(ctx); + } + + /// A helper that can't start at the exit is recorded, and the next start says why, once. + #[test] + fn a_helper_that_cannot_start_at_the_exit_is_reported_at_the_next_start() { + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(Restart::later(|| Err("couldn't schedule the update (denied)".into())))).is_ok()); + u.tick(&ctx); + assert_eq!(frame(&mut u, &ctx, true), Action::Close); + drop(u); + + let mut next = reading(&paths, "0.1.0"); + start_answered(&mut next, &ctx, Ok(None)); + assert_eq!(next.status(), &Status::InstallFailed("couldn't schedule the update (denied)".into())); + let mut later = reading(&paths, "0.1.0"); + start_answered(&mut later, &ctx, Ok(None)); + assert_eq!(later.status(), &Status::Idle, "a failure is reported once"); + } + + /// The first start after a helper failure shows the reason once, and the quiet check at start + /// does not replace it. A failure wins over a stale record of a finished update. + #[test] + fn a_recorded_failure_is_shown_once_at_the_next_start() { + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + self_update::record_failure(&paths, "the Windows Installer stopped with error 1603"); + crate::paths::write_atomic(&paths.data.join("self-update-done.txt"), b"v0.1.0").unwrap(); + start_answered(&mut u, &ctx, Ok(None)); + assert_eq!(u.status(), &Status::InstallFailed("the Windows Installer stopped with error 1603".into())); + u.dismiss_status(); + assert_eq!(u.status(), &Status::Idle); + + let mut next = reading(&paths, "0.1.0"); + start_answered(&mut next, &ctx, Ok(None)); + assert_eq!(next.status(), &Status::Idle, "neither record is read twice"); + } + + /// A failed update that the check at start offers again says in its prompt why it failed. + #[test] + fn a_failed_update_offered_again_says_why_it_failed() { + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + self_update::record_failure(&paths, "disk full"); + start_answered(&mut u, &ctx, Ok(Some(newer()))); + assert_eq!(u.status(), &Status::Idle, "the prompt carries the reason"); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message).as_deref(), Some("The update could not be installed: disk full")); + } + + /// Check Again after a failed update, with the failure already dismissed, still says in the + /// prompt that offers the update why it failed, and only that prompt does. + #[test] + fn check_again_after_a_failed_update_says_why_it_failed() { + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + self_update::record_failure(&paths, "disk full"); + start_answered(&mut u, &ctx, Ok(None)); + assert_eq!(u.status(), &Status::InstallFailed("disk full".into())); + u.dismiss_status(); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Checking); + let (tx, rx) = channel(); + u.check = Some(rx); + assert!(tx.send(Ok(Some(newer()))).is_ok()); + u.tick(&ctx); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message).as_deref(), Some("The update could not be installed: disk full")); + + u.prompt = None; + u.check_now(&ctx); + let (tx, rx) = channel(); + u.check = Some(rx); + assert!(tx.send(Ok(Some(newer()))).is_ok()); + u.tick(&ctx); + assert!(u.prompt.is_some()); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message), None, "the reason is shown once"); + } + + /// The new version's first start says it was updated, once, and only when the finished + /// update installed the version running now. + #[test] + fn a_finished_update_is_announced_once_by_the_version_it_installed() { + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.2.0"); + crate::paths::write_atomic(&paths.data.join("self-update-done.txt"), b"v0.2.0").unwrap(); + start_answered(&mut u, &ctx, Ok(None)); + assert_eq!(u.status(), &Status::Updated("0.2.0".into())); + let mut next = reading(&paths, "0.2.0"); + start_answered(&mut next, &ctx, Ok(None)); + assert_eq!(next.status(), &Status::Idle, "announced once"); + + // The old version started again (the update didn't take): no claim, and the record goes. + crate::paths::write_atomic(&paths.data.join("self-update-done.txt"), b"v0.3.0").unwrap(); + let mut old = reading(&paths, "0.2.0"); + start_answered(&mut old, &ctx, Ok(None)); + assert_eq!(old.status(), &Status::Idle); + assert!(!paths.data.join("self-update-done.txt").exists()); + } + + /// A requested check while an update downloads says it is installing, and while it waits for + /// the close says it is ready, after dismissal and on repeated clicks too, and starts nothing. + #[test] + fn a_requested_check_during_an_update_says_where_it_is() { + let ctx = egui::Context::default(); + let mut u = updater(Ok(Some(newer()))); + u.offer(newer()); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + for _ in 0..2 { + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Installing); + assert!(u.check.is_none(), "no check starts"); + } + u.dismiss_status(); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Installing, "asked again after dismissal"); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); + u.tick(&ctx); + assert_eq!(u.status(), &Status::Ready, "the open window follows the update"); + u.dismiss_status(); + for _ in 0..2 { + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Ready); + assert!(u.check.is_none() && u.install.is_none(), "nothing starts"); + } + } + + /// A requested check that answers after an installation started reports the installation, + /// not its own answer, whether the update is still installing or already waits for the close. + #[test] + fn a_check_answering_during_an_update_reports_the_update() { + let ctx = egui::Context::default(); + for answer in [Ok(None), Err("offline".to_owned()), Ok(Some(newer()))] { + let mut u = updater(Ok(None)); + u.offer(newer()); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Checking); + let (check_tx, check_rx) = channel(); + u.check = Some(check_rx); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(check_tx.send(answer.clone()).is_ok()); + u.tick(&ctx); + assert_eq!(u.status(), &Status::Installing, "a check answering {answer:?} while installing"); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); + u.tick(&ctx); + assert_eq!(u.status(), &Status::Ready, "after {answer:?}"); + + let mut u = updater(Ok(None)); + u.offer(newer()); + u.check_now(&ctx); + let (check_tx, check_rx) = channel(); + u.check = Some(check_rx); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); + u.tick(&ctx); + assert!(u.check.is_some(), "the check is still running"); + assert!(check_tx.send(answer.clone()).is_ok()); + u.tick(&ctx); + assert_eq!(u.status(), &Status::Ready, "a check answering {answer:?} after the install finished"); + } + } + + /// A failed installation closes a result window showing it; the prompt says why. + #[test] + fn a_failed_install_ends_the_installing_status() { + let ctx = egui::Context::default(); + let mut u = updater(Ok(None)); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Installing); + assert!(tx.send(Err("disk full".into())).is_ok()); + u.tick(&ctx); + assert_eq!(u.status(), &Status::Idle); + assert_eq!(u.prompt.as_ref().and_then(|p| p.status_message.clone()).as_deref(), Some("disk full")); + } + + /// A helper that started while the app kept running and then failed is found by the next + /// requested check, which offers the update again. + #[test] + fn a_requested_check_finds_a_helper_failure_while_the_app_still_runs() { + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + u.offer(newer()); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); + u.tick(&ctx); + u.start_pending_update(); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Ready, "the helper is still waiting"); + assert!(u.prompt.as_ref().is_some_and(|p| p.is_ready), "the prompt offers Restart to Update"); + self_update::record_failure(&paths, "ArtCraft Launcher is still running"); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::InstallFailed("ArtCraft Launcher is still running".into())); + assert!(!u.installed, "the update is on offer again"); + let prompt = u.prompt.as_ref().map(|p| (p.is_installing, p.is_ready, p.message())); + let reason = "The update could not be installed: ArtCraft Launcher is still running".to_owned(); + assert_eq!(prompt, Some((false, false, Some(reason))), "the prompt no longer offers Restart to Update"); + assert!(!u.close_wanted, "no close is asked for"); + assert_eq!(frame(&mut u, &ctx, true), Action::None); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::Checking, "Check Again runs a check"); + // The prompt showed the reason: a later prompt does not repeat it. + u.prompt = None; + let (tx, rx) = channel(); + u.check = Some(rx); + assert!(tx.send(Ok(Some(newer()))).is_ok()); + u.tick(&ctx); + assert!(u.prompt.is_some()); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message), None, "the reason is shown once"); + } + + /// A retry after a helper failed while the app kept running reports through `tick` and waits + /// for the exit, like the first installation, instead of starting its helper at once. + #[test] + fn a_retry_after_a_helper_failure_while_the_app_runs_waits_for_the_exit() { + use std::sync::atomic::Ordering; + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + u.offer(newer()); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + assert!(tx.send(Ok(Restart::scheduled())).is_ok()); + u.tick(&ctx); + u.start_pending_update(); + self_update::record_failure(&paths, "ArtCraft Launcher is still running"); + u.check_now(&ctx); + assert_eq!(u.status(), &Status::InstallFailed("ArtCraft Launcher is still running".into())); + + // The retry's worker, as `start_install` starts it. + let exiting = handoff(&mut u.exiting); + let (starts, restart) = counted_restart(); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + deliver(&PHOTOCRAFT, &tx, &exiting, Some(&paths), Ok(restart)); + assert_eq!(starts.load(Ordering::SeqCst), 0, "the helper waits for the exit"); + u.tick(&ctx); + assert!(u.installed && u.restart.is_some(), "tick picks up the retry"); + assert_eq!(frame(&mut u, &ctx, true), Action::Close); + drop(u); + assert_eq!(starts.load(Ordering::SeqCst), 1, "the exit starts the helper once"); + let mut next = reading(&paths, "0.1.0"); + start_answered(&mut next, &ctx, Ok(None)); + assert_eq!(next.status(), &Status::Idle, "the retry is not recorded as a failure"); + } + + /// A release the app offers itself after a failed update says in its prompt why it failed, + /// once. + #[test] + fn an_offer_after_a_failed_update_says_why_it_failed() { + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + self_update::record_failure(&paths, "disk full"); + start_answered(&mut u, &ctx, Ok(None)); + assert_eq!(u.status(), &Status::InstallFailed("disk full".into())); + u.offer(newer()); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message).as_deref(), Some("The update could not be installed: disk full")); + assert_eq!(u.status(), &Status::Idle, "the prompt carries the reason"); + u.prompt = None; + u.offer(newer()); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message), None, "the reason is shown once"); + } + + /// An offer the app made before the check at start keeps its prompt; when the check finds the + /// release too, that prompt says why the last update failed, once. + #[test] + fn an_offer_before_the_check_at_start_says_why_the_last_update_failed() { + let ctx = egui::Context::default(); + let (_dir, paths, mut u) = with_records("0.1.0"); + self_update::record_failure(&paths, "disk full"); + u.offer(newer()); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message), None, "the failure is not read before the check at start"); + start_answered(&mut u, &ctx, Ok(Some(newer()))); + assert_eq!(u.status(), &Status::Idle, "the prompt carries the reason"); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message).as_deref(), Some("The update could not be installed: disk full")); + u.prompt = None; + u.offer(newer()); + assert_eq!(u.prompt.as_ref().and_then(StartupPrompt::message), None, "the reason is shown once"); + } + + /// An installation that finishes after the updater was dropped and whose helper cannot start, + /// or that failed, is recorded, and the next start says why. + #[test] + fn an_install_failing_after_the_updater_is_dropped_is_reported_at_the_next_start() { + let ctx = egui::Context::default(); + let failures: [(Result, &str); 3] = [ + (Ok(Restart::later(|| Err("couldn't schedule the update (denied)".into()))), "couldn't schedule the update (denied)"), + (Err("couldn't download the update".into()), "couldn't download the update"), + (Err(Failure::panicked()), "The update failed unexpectedly."), + ]; + for (outcome, want) in failures { + let (_dir, paths, mut u) = with_records("0.1.0"); + let exiting = Arc::clone(&u.exiting); + let (tx, rx) = channel(); + u.install = Some((rx, newer())); + drop(u); + deliver(&PHOTOCRAFT, &tx, &exiting, Some(&paths), outcome); + let mut next = reading(&paths, "0.1.0"); + start_answered(&mut next, &ctx, Ok(None)); + assert_eq!(next.status(), &Status::InstallFailed(want.into()), "{want}"); + } + } + + /// A failed installation picked up at the exit is recorded, and the next start says why. + #[test] + fn a_failed_install_picked_up_at_exit_is_reported_at_the_next_start() { + let ctx = egui::Context::default(); + for (failure, want) in [(Failure::from("disk full"), "disk full"), (Failure::dropped(), "The update stopped unexpectedly.")] { + let (_dir, paths, mut u) = with_records("0.1.0"); + let (tx, rx) = channel::>(); + u.install = Some((rx, newer())); + assert!(tx.send(Err(failure)).is_ok()); + drop(u); + let mut next = reading(&paths, "0.1.0"); + start_answered(&mut next, &ctx, Ok(None)); + assert_eq!(next.status(), &Status::InstallFailed(want.into()), "{want}"); + } + // A worker that ended without sending anything. + let (_dir, paths, mut u) = with_records("0.1.0"); + let (tx, rx) = channel::>(); + u.install = Some((rx, newer())); + drop(tx); + drop(u); + let mut next = reading(&paths, "0.1.0"); + start_answered(&mut next, &ctx, Ok(None)); + assert_eq!(next.status(), &Status::InstallFailed("The update stopped unexpectedly.".into())); + } } #[cfg(test)] diff --git a/crates/update/src/updater_pin_tests.rs b/crates/update/src/updater_pin_tests.rs index 2a4f58f1..8a4b73d7 100644 --- a/crates/update/src/updater_pin_tests.rs +++ b/crates/update/src/updater_pin_tests.rs @@ -30,7 +30,7 @@ fn a_pinned_app_is_not_prompted_and_the_network_is_not_asked() { assert_eq!(u.status(), &Status::Idle); u.check_now(&ctx); settle(&mut u, &ctx); - assert_eq!(u.status(), &Status::UpToDate, "{track:?}"); + assert_eq!(u.status(), &Status::Managed, "{track:?}"); assert_eq!(u.managed(), Some(&Choice::Managed(track.clone()))); u.offer(newer()); assert!(!u.prompt_open(), "an offer is held too: {track:?}"); @@ -109,7 +109,7 @@ fn a_check_after_the_pin_closes_a_prompt_opened_before_it() { u.check_now(&ctx); settle(&mut u, &ctx); assert!(!u.prompt_open()); - assert_eq!(u.status(), &Status::UpToDate); + assert_eq!(u.status(), &Status::Managed); } #[test] @@ -140,7 +140,7 @@ fn a_pin_made_while_the_check_runs_still_holds_its_answer() { u.check_now(&ctx); settle(&mut u, &ctx); assert!(!u.prompt_open()); - assert_eq!(u.status(), &Status::UpToDate); + assert_eq!(u.status(), &Status::Managed); } #[test] @@ -160,7 +160,7 @@ fn every_answer_after_a_pin_made_mid_check_reports_the_launchers_choice() { let mut u = Updater::new(&PHOTOCRAFT, "0.13.0").with_source(source).with_pins_path(&path); u.check_now(&ctx); settle(&mut u, &ctx); - assert_eq!(u.status(), &Status::UpToDate); + assert_eq!(u.status(), &Status::Managed); assert_eq!(u.managed(), Some(&Choice::Managed(crate::pins::VersionTrack::Nightly))); } }