From 4671e213a21ca27ebd7a9b373b2b8af528cdd4f6 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 7 Nov 2026 21:53:25 -0800 Subject: [PATCH] craft-update: restart helpers start at the exit and outcomes reach the new app - On macOS and Linux the restart helper starts at the exit too (Restart::start), instead of when the update is prepared. A close the user cancels no longer leaves a helper that starts a second copy after its one-minute wait, or a Flatpak helper that gives up before Restart to Update. - The record of a finished update names the release's version instead of its tag, so a release whose tag is not a version is still reported. - A standalone Windows copy updated from the portable zip becomes portable. The helper now writes the outcome and the record of the update to the folders beside it, which the new app reads. - When the check at start finds a release for a prompt the app already opened with Updater::offer, that prompt shows why the last update failed. - Errors and failure reasons reach 4.5:1 contrast on the card and in the prompt's message box in both themes, and a long one scrolls inside a 160 px area so the card's buttons stay on screen. --- crates/update/src/dialog.rs | 89 +++++++++++-- crates/update/src/self_update.rs | 206 +++++++++++++++++++++++++------ crates/update/src/updater.rs | 37 ++++-- 3 files changed, 279 insertions(+), 53 deletions(-) diff --git a/crates/update/src/dialog.rs b/crates/update/src/dialog.rs index 5f471d47..5db9e1c8 100644 --- a/crates/update/src/dialog.rs +++ b/crates/update/src/dialog.rs @@ -106,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(); @@ -130,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); } @@ -247,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 { @@ -261,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) }, } } } @@ -410,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, @@ -428,7 +434,6 @@ impl CheckResultDialog { } ui.add_space(8.0); - let alert_color = Color32::from_rgb(225, 95, 30); match status { Status::Checking => { ui.horizontal(|ui| { @@ -459,7 +464,9 @@ impl CheckResultDialog { } Status::InstallFailed(reason) => { let text = t("The update could not be installed: {reason}", &[("reason", reason)]); - ui.add(egui::Label::new(RichText::new(text).size(12.5).color(alert_color)).selectable(true)); + 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( @@ -469,8 +476,10 @@ impl CheckResultDialog { ui.label(RichText::new(text).size(12.5).color(text_muted)); } Status::Failed(error) => { - 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) @@ -520,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) { @@ -592,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)) @@ -906,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 { diff --git a/crates/update/src/self_update.rs b/crates/update/src/self_update.rs index 3b4d9cf8..d044ff6e 100644 --- a/crates/update/src/self_update.rs +++ b/crates/update/src/self_update.rs @@ -375,29 +375,31 @@ pub fn update( prepare_update(app, install, release, platform, http, paths, stop, phase, last, running_version)?.start() } -/// What remains of an update after [`prepare_update`]: starting the Windows helper, which waits -/// for this process to exit and then installs the update. On macOS and Linux the update is -/// already in place and its restart already scheduled, so [`Restart::start`] does nothing. -#[must_use = "on Windows the update is only installed once the restart is started"] +/// 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(all(windows, not(test)), allow(dead_code))] + #[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, test)), allow(dead_code))] + #[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 closes: the helper waits only a minute for the - /// app to exit, and installs nothing when the app is still running after that. + /// 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()) } @@ -409,8 +411,8 @@ impl std::fmt::Debug for Restart { } } -/// [`update`] without starting the Windows helper: the caller starts the returned [`Restart`] -/// when the app closes. On macOS and Linux the result is the same as [`update`]'s. +/// [`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, @@ -457,20 +459,20 @@ pub fn prepare_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()]).map(|()| Restart::scheduled()) + 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()]).map(|()| Restart::scheduled()) + 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()]).map(|()| Restart::scheduled()) + Ok(relaunch_after_exit(app, pid, &[exe.as_os_str()])) } #[cfg(target_os = "linux")] Install::Flatpak => { @@ -484,7 +486,7 @@ pub fn prepare_update( let result = install::linux::update_flatpak_bundle(&file, &app.bundle_id(), system, stop, phase); let _ = std::fs::remove_file(&file); result?; - relaunch_flatpak_after_exit(app).map(|()| Restart::scheduled()) + relaunch_flatpak_after_exit(app) } #[cfg(windows)] Install::Msi(exe) => windows_helper(app, paths, release, pid, exe, &msi_finish(&file)?), @@ -503,7 +505,7 @@ pub fn prepare_update( let result = replace_standalone(app, paths, &file, exe); let _ = std::fs::remove_file(&file); result?; - relaunch_after_exit(app, pid, &[exe.as_os_str()]).map(|()| Restart::scheduled()) + Ok(relaunch_after_exit(app, pid, &[exe.as_os_str()])) } #[allow(unreachable_patterns)] _ => { @@ -513,20 +515,22 @@ pub fn prepare_update( } })(); let scheduled = unmark_unless_scheduled(paths, scheduled); - // On macOS and Linux the update is in place once it is 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.tag); + record_done(paths, &release.version_label()); } scheduled } -/// Records `release` as just installed, so it isn't offered again for [`JUST_UPDATED_FOR`]. The -/// outcome records of an earlier update are removed: this update writes its own. +/// 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) { let _ = std::fs::remove_file(failure_file(paths)); let _ = std::fs::remove_file(done_file(paths)); - if let Err(e) = crate::paths::write_atomic(&marker(paths), format!("{}\n{}\n", release.tag, crate::now()).as_bytes()) { + 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}"); } } @@ -536,17 +540,22 @@ fn done_file(paths: &Paths) -> PathBuf { paths.data.join("self-update-done.txt") } -/// Records that the update to `tag` has been installed. -fn record_done(paths: &Paths, tag: &str) { - if let Err(e) = crate::paths::write_atomic(&done_file(paths), tag.as_bytes()) { +/// 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. +/// 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_tag(paths: &Paths) -> Option { - read_record(&marker(paths)).and_then(|text| text.lines().next().map(str::to_owned)).filter(|tag| !tag.trim().is_empty()) +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. @@ -741,10 +750,13 @@ 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 { + let cmd = helper_command(app, pid, command); + Restart::later(move || process::spawn_detached(cmd).map_err(|e| format!("couldn't schedule the restart ({e})"))) } #[cfg(unix)] @@ -798,9 +810,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. @@ -819,7 +832,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")] @@ -958,6 +972,7 @@ fn finish_step( 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 { @@ -965,6 +980,14 @@ fn finish_step( } // 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 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 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. @@ -973,9 +996,9 @@ fn finish_step( } // Written before the new app starts, so its first start finds it. A failed start removes it. if done.is_ok() - && let Some(tag) = scheduled_tag(paths) + && let Some(version) = &version { - record_done(paths, &tag); + record_done(paths, version); } let started = start(&exe); if let Err(e) = &started { @@ -984,6 +1007,26 @@ fn finish_step( done.and(started) } +/// 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)); + } +} + /// Finishes an update on non-Windows platforms (no-op helper). #[cfg(not(windows))] pub fn finish(_app: &App, _args: &[OsString]) -> Option> { @@ -1674,7 +1717,24 @@ mod tests { 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_tag(&paths).as_deref(), Some("v0.3.0")); + 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. @@ -1926,6 +1986,28 @@ mod tests { } } + /// 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. + #[cfg(unix)] + #[test] + fn the_restart_helper_starts_only_when_the_restart_is_started() { + use std::io::BufRead; + 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 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 restart = relaunch_after_exit(&crate::suite::LAUNCHER, gone.id(), &command); + 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 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 @@ -2326,6 +2408,60 @@ mod tests { 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) { diff --git a/crates/update/src/updater.rs b/crates/update/src/updater.rs index da174874..196c862e 100644 --- a/crates/update/src/updater.rs +++ b/crates/update/src/updater.rs @@ -68,8 +68,8 @@ pub enum Action { /// 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. On Windows - /// the helper starts only at the exit: when the app drops the [`Updater`] as it exits, or + /// [`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. @@ -434,8 +434,14 @@ impl Updater { } match answer { Ok(Some(release)) => { - if self.prompt.is_none() && self.install.is_none() && !self.installed { - self.open_offer(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; } @@ -529,9 +535,9 @@ impl Updater { /// 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. It is only asked for: the Windows 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. + // 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; } @@ -1561,6 +1567,23 @@ mod tests { 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]