diff --git a/crates/update/src/lib.rs b/crates/update/src/lib.rs index 5d68bc6a..974af700 100644 --- a/crates/update/src/lib.rs +++ b/crates/update/src/lib.rs @@ -51,6 +51,8 @@ pub use dialog::{StartupPrompt, StartupPromptResponse}; /// Checks for an update against GitHub releases for the given app. pub fn check_for_update(app: &App, http: &Http, current_version: &str) -> Result, String> { + let version = self_update::current_version(app, current_version).map(|v| v.to_string()); + let current_version = version.as_deref().unwrap_or(current_version); let url = format!("https://api.github.com/repos/{}/releases?per_page=30", app.repo); match http.get_api(&url, None)? { crate::http::Fetched::Body { text, .. } => offered_release(&text, app.repo, current_version), diff --git a/crates/update/src/self_update.rs b/crates/update/src/self_update.rs index 9ef3e377..a62999d6 100644 --- a/crates/update/src/self_update.rs +++ b/crates/update/src/self_update.rs @@ -85,7 +85,7 @@ pub const DEV_VAR: &str = "CRAFT_UPDATE_DEV"; /// The version this app counts as (see [`PRETEND_VAR`]). pub fn current_version(app: &App, fallback_pkg_version: &str) -> Option { - let app_pretend = format!("{}_PRETEND_VERSION", app.id.replace('-', "_").to_uppercase()); + let app_pretend = pretend_var(app); std::env::var(&app_pretend) .or_else(|_| std::env::var(PRETEND_VAR)) .ok() @@ -93,6 +93,15 @@ pub fn current_version(app: &App, fallback_pkg_version: &str) -> Option .or_else(|| version::parse(fallback_pkg_version)) } +fn pretend_var(app: &App) -> String { + format!("{}_PRETEND_VERSION", app.id.replace('-', "_").to_uppercase()) +} + +/// A pretend version lasts one run, including when an update schedules a restart. +fn clear_pretend_version(cmd: &mut std::process::Command, app: &App) { + cmd.env_remove(PRETEND_VAR).env_remove(pretend_var(app)); +} + /// How long a just-installed version stays exempt from being offered again. const JUST_UPDATED_FOR: i64 = 24 * 3600; @@ -319,7 +328,8 @@ pub fn update( if let (Some(rel_v), Some(cur_v)) = (release.version.as_ref(), cur_v.as_ref()) && !version::offers_update(rel_v, cur_v) { - // A build from source is only replaced by a strictly higher release (see `offers_update`). + // Compare the effective version: an explicit pretend version also exercises installation. + // Without that opt-in, a build from source requires a strictly higher release. return Err(format!("{} {} is not newer than currently running version {}", app.name, release.version_label(), cur_v)); } let (asset, _) = pick_asset(release, &[app.id], platform, &[install.kind()]) @@ -345,20 +355,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(pid, &[Path::new("/usr/bin/open").as_os_str(), placed.as_os_str()]) + 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(pid, &[path.as_os_str()]) + 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, stop, phase); let _ = std::fs::remove_file(&file); result?; - relaunch_after_exit(pid, &[exe.as_os_str()]) + relaunch_after_exit(app, pid, &[exe.as_os_str()]) } #[cfg(target_os = "linux")] Install::Flatpak => { @@ -384,7 +394,7 @@ pub fn update( let result = replace_standalone(app, &file, exe); let _ = std::fs::remove_file(&file); result?; - relaunch_after_exit(pid, &[exe.as_os_str()]) + relaunch_after_exit(app, pid, &[exe.as_os_str()]) } #[allow(unreachable_patterns)] _ => { @@ -468,20 +478,19 @@ fn replace_standalone(app: &App, tarball: &Path, exe: &Path) -> Result<(), Strin /// Starts `sh`, which waits for `pid` to exit and then runs `command`. Detached, so it outlives us. #[cfg(unix)] -fn relaunch_after_exit(pid: u32, command: &[&std::ffi::OsStr]) -> Result<(), String> { - process::spawn_detached(helper_command(pid, command)).map_err(|e| format!("couldn't schedule the restart ({e})")) +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})")) } #[cfg(unix)] -fn helper_command(pid: u32, command: &[&std::ffi::OsStr]) -> std::process::Command { +fn helper_command(app: &App, pid: u32, command: &[&std::ffi::OsStr]) -> std::process::Command { let mut cmd = process::command("/bin/sh"); cmd.arg("-c") .arg(r#"pid=$1; shift; i=0; while kill -0 "$pid" 2>/dev/null && [ $i -lt 600 ]; do sleep 0.1; i=$((i+1)); done; exec "$@""#) .arg("sh") .arg(pid.to_string()) - .args(command) - // The new launcher reports its real version; a pretend one was for this run only. - .env_remove(PRETEND_VAR); + .args(command); + clear_pretend_version(&mut cmd, app); cmd } @@ -545,10 +554,19 @@ 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})")) +} + +#[cfg(target_os = "linux")] +fn flatpak_restart_command(app: &App, script: &str, target: &str) -> std::process::Command { let app_id = app.bundle_id(); + // flatpak run reapplies persistent overrides; removing inherited variables alone is insufficient. + let global_unset = format!("--unset-env={PRETEND_VAR}"); + let app_unset = format!("--unset-env={}", pretend_var(app)); let mut cmd = process::command("flatpak-spawn"); - cmd.args(["--host", "sh", "-c", &script, "sh", &target_arg, "flatpak", "run", &app_id]).env_remove(PRETEND_VAR); - process::spawn_detached(cmd).map_err(|e| format!("couldn't schedule the Flatpak restart ({e})")) + cmd.args(["--host", "sh", "-c", script, "sh", target, "flatpak", "run", &global_unset, &app_unset, &app_id]); + clear_pretend_version(&mut cmd, app); + cmd } /// The first argument of the Windows update helper: a copy of this program that finishes an @@ -609,7 +627,7 @@ const PORTABLE_STAGING: &str = "launcher-update"; /// `pid` to exit, does `finish` and starts `exe`. Run from the cache, the helper doesn't hold the /// installed exe that the step replaces. #[cfg(windows)] -fn windows_helper(_app: &App, paths: &Paths, pid: u32, exe: &Path, finish: &Finish) -> Result<(), String> { +fn windows_helper(app: &App, paths: &Paths, pid: u32, exe: &Path, finish: &Finish) -> Result<(), String> { 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. @@ -618,7 +636,8 @@ fn windows_helper(_app: &App, paths: &Paths, pid: u32, exe: &Path, finish: &Fini let helper = dir.join(program.file_name().ok_or("this program has no file name")?); std::fs::copy(&program, &helper).map_err(|e| format!("couldn't schedule the update ({e})"))?; let mut cmd = process::command(&helper); - cmd.args(helper_args(pid, exe, finish)).env_remove(PRETEND_VAR); + 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})")) } @@ -1466,10 +1485,38 @@ mod tests { #[cfg(unix)] #[test] fn the_restarted_launcher_does_not_inherit_a_pretend_version() { - let cmd = helper_command(1, &[std::ffi::OsStr::new("/usr/bin/true")]); + let cmd = helper_command(&crate::suite::LAUNCHER, 1, &[std::ffi::OsStr::new("/usr/bin/true")]); assert!(cmd.get_envs().any(|(k, v)| k == PRETEND_VAR && v.is_none()), "the helper must remove {PRETEND_VAR}"); } + #[test] + fn restarted_apps_do_not_inherit_their_pretend_versions() { + for app in crate::suite::SUITE.iter().chain(std::iter::once(&crate::suite::LAUNCHER)) { + let mut cmd = std::process::Command::new("unused"); + clear_pretend_version(&mut cmd, app); + for key in [PRETEND_VAR.to_string(), pretend_var(app)] { + assert!(cmd.get_envs().any(|(k, v)| k == key.as_str() && v.is_none()), "{} retained {key}", app.id); + } + } + } + + #[cfg(target_os = "linux")] + #[test] + fn flatpak_restart_unsets_persistent_pretend_version_overrides() { + for app in &crate::suite::SUITE { + let cmd = flatpak_restart_command(app, "exec \"$@\"", "123"); + let args: Vec<_> = cmd.get_args().map(|v| v.to_string_lossy().into_owned()).collect(); + let expected = [ + "flatpak".to_string(), + "run".to_string(), + format!("--unset-env={PRETEND_VAR}"), + format!("--unset-env={}", pretend_var(app)), + app.bundle_id(), + ]; + assert!(args.windows(expected.len()).any(|w| w == expected), "{}: {args:?}", app.id); + } + } + /// The helper waits for the old launcher to exit before it starts the new one. #[cfg(unix)] #[test] @@ -1477,7 +1524,7 @@ mod tests { 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(old.id(), &[std::ffi::OsStr::new("touch"), marker.as_os_str()]).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(); diff --git a/crates/update/src/updater.rs b/crates/update/src/updater.rs index 836bf441..63686001 100644 --- a/crates/update/src/updater.rs +++ b/crates/update/src/updater.rs @@ -59,9 +59,11 @@ pub struct Updater { impl Updater { pub fn new(app: &'static App, version: impl Into) -> Self { + let version = version.into(); + let version = self_update::current_version(app, &version).map_or(version, |v| v.to_string()); Self { app, - version: version.into(), + version, source: None, status: Status::Idle, requested: false, @@ -328,6 +330,54 @@ mod tests { Release { tag: "v9.0.0".into(), version: crate::version::parse("9.0.0"), ..Release::default() } } + // Child processes isolate environment overrides from parallel tests without unsafe set_var. + #[test] + fn pretend_versions_trigger_startup_prompts_for_every_app() { + const CHILD: &str = "CRAFT_UPDATE_PRETEND_TEST_APP"; + if let Ok(id) = std::env::var(CHILD) { + let app = crate::suite::by_id(&id).unwrap(); + let mut u = Updater::new(app, "99.0.0"); + assert_eq!(u.version, "0.0.1", "{} ignored its pretend version", app.id); + let version = u.version.clone(); + let repo = app.repo; + u = u.with_source(Arc::new(move || { + crate::offered_release(r#"[{"tag_name":"v1.0.0","name":"Test release","draft":false,"prerelease":false}]"#, repo, &version) + })); + let ctx = egui::Context::default(); + u.check_at_start(&ctx); + settle(&mut u, &ctx); + assert!(u.prompt_open(), "{} did not offer the newer release", app.id); + // The explicit test override also applies to the pre-existing installation guard. + // No assets means the installer stops before any network request or file replacement. + let dir = tempfile::tempdir().unwrap(); + let error = self_update::update( + app, + &self_update::Install::AppImage(dir.path().join("test.AppImage")), + &newer(), + Platform { os: crate::Os::Linux, arch: crate::Arch::X64 }, + &Http::new(crate::TEST_AGENT).unwrap(), + &Paths::under(dir.path()), + &Stop::default(), + &mut |_| {}, + &|| Ok(()), + Some("99.0.0"), + ) + .unwrap_err(); + assert!(error.contains("has no"), "{}: {error}", app.id); + return; + } + for app in &crate::suite::SUITE { + let status = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "updater::tests::pretend_versions_trigger_startup_prompts_for_every_app", "--nocapture"]) + .env(CHILD, app.id) + .env(format!("{}_PRETEND_VERSION", app.id.to_uppercase()), "0.0.1") + .env_remove(self_update::PRETEND_VAR) + .status() + .unwrap(); + assert!(status.success(), "{} pretend-version regression failed", app.id); + } + } + #[test] fn a_requested_check_reports_an_up_to_date_app() { let (ctx, mut u) = (egui::Context::default(), updater(Ok(None))); diff --git a/docs/update-testing.md b/docs/update-testing.md new file mode 100644 index 00000000..1dee183b --- /dev/null +++ b/docs/update-testing.md @@ -0,0 +1,33 @@ +# Testing update prompts + +Every native Craft App accepts an environment variable derived from its app id: +uppercase, hyphens replaced with underscores, followed by `_PRETEND_VERSION`. +Run each command from the corresponding app checkout, with a `craft-update` pin that +includes this fix. These app packages are not part of the `craft-libs` workspace. +For example: + +```sh +LIGHTCRAFT_PRETEND_VERSION=0.0.1 cargo run -p lightcraft +WORDCRAFT_PRETEND_VERSION=0.0.1 cargo run -p wordcraft +PDFCRAFT_PRETEND_VERSION=0.0.1 cargo run -p pdfcraft +``` + +The shared updater uses this version for startup checks, manual checks, the prompt's +running-version label, and installation checks. It still fetches real release metadata; +a network error or an app with no published newer release cannot produce an update prompt. +Use Help → Check for Updates to see errors that the startup check only logs. + +An app-specific setting takes priority over the legacy shared +`ARTCRAFT_LAUNCHER_PRETEND_VERSION` setting. An invalid version falls back to the +compiled package version. Leave the variables unset for normal version detection. +The restart helper clears both settings for the app so a newly installed copy reports +its real version. + +`CRAFT_UPDATE_DEV=1` separately lets a development executable be treated as an installed +copy for testing installation. It is unnecessary to preview a prompt. Download/install +buttons perform a real update; a pretend version does not simulate installation. +The explicit override also applies to the installation version check, allowing tests +to reinstall the current release or replace a newer development build. Leave it unset +to retain the normal protection against installing an older or equal version. + +Web builds have no updater. These settings apply to native builds only.