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]