From aaa67424793ac272fa17a15c6ab6507c6cd8df70 Mon Sep 17 00:00:00 2001 From: VoiceLessQ Date: Sun, 1 Nov 2026 12:54:41 -0200 Subject: [PATCH] fix(update): handle a bundle Flatpak before 1.16.2 won't reinstall Flatpak before 1.16.2 ignores --reinstall for a bundle and fails with " already installed" when that commit is deployed. An app install now uninstalls the user's copy and installs again, without --delete-data, as the launcher does (storytold/craft-launcher#1014). A failure after that says what happened and what to do. A self-update never uninstalls the app it runs from: its download goes right after, so a failed install would leave nothing. That error there means an earlier update landed and never restarted, so it goes on to the restart. Root gets a flatpak command, so tests can record the real calls. --- crates/update/src/install.rs | 252 ++++++++++++++++++++++++++++--- crates/update/src/process.rs | 22 +++ crates/update/src/self_update.rs | 4 +- 3 files changed, 258 insertions(+), 20 deletions(-) diff --git a/crates/update/src/install.rs b/crates/update/src/install.rs index 575966a1..7ae7ed9a 100644 --- a/crates/update/src/install.rs +++ b/crates/update/src/install.rs @@ -119,7 +119,8 @@ pub fn install_at( } #[cfg(target_os = "linux")] PackageKind::Deb | PackageKind::Rpm | PackageKind::ArchPkg | PackageKind::Flatpak => { - linux::install_package(kind, &file, job.stop, &mut *job.phase).map(|_| None) + let id = app.bundle_id(); + linux::install_package(kind, &file, Some(&id), job.stop, &mut *job.phase).map(|_| None) } #[cfg(target_os = "linux")] PackageKind::AppImage => { @@ -770,9 +771,11 @@ fn cancelled(stop: &Stop) -> impl Fn() -> bool + '_ { pub(crate) mod linux { use std::ffi::{OsStr, OsString}; use std::os::unix::fs::PermissionsExt; + use std::time::Duration; use crate::distro; use crate::protocol::CANCELLED; + use crate::version::{self, Version}; use super::{ App, INSTALLER, InstallSource, Installed, JobPhase, PackageKind, Path, PathBuf, Paths, Record, SHARED_CALL, SYSTEM_BIN_DIRS, Stop, cancelled, @@ -792,9 +795,10 @@ pub(crate) mod linux { pub struct Root<'a> { pub queue: &'a crate::queue::Queue, pub run: &'a (dyn Fn(&[&OsStr]) -> Result<(), String> + Sync), + pub flatpak: &'a (dyn Fn() -> std::process::Command + Sync), } - const SYSTEM: Root<'static> = Root { queue: &INSTALLER, run: &run_as_root }; + const SYSTEM: Root<'static> = Root { queue: &INSTALLER, run: &run_as_root, flatpak: &process::flatpak_command_c }; /// Runs `command target` as root. Jobs waiting for the same command go into one call /// (`apt-get install a.deb b.deb`), so the password is asked for once. @@ -962,19 +966,48 @@ pub(crate) mod linux { } } - /// Reinstalls a downloaded `.flatpak` bundle. `system` replaces the system-wide copy through - /// `pkexec`. Per-user stays a normal `flatpak install --user`, which needs no password. - pub(crate) fn install_flatpak_bundle(file: &Path, system: bool, stop: &Stop, phase: &mut dyn FnMut(JobPhase)) -> Result<(), String> { - install_flatpak_bundle_via(&SYSTEM, file, system, stop, phase) + /// Puts a self-update's `.flatpak` bundle in place. `system` replaces the system-wide copy + /// through `pkexec`. Per-user stays a normal `flatpak install --user`, which needs no password. + pub(crate) fn update_flatpak_bundle(file: &Path, id: &str, system: bool, stop: &Stop, phase: &mut dyn FnMut(JobPhase)) -> Result<(), String> { + update_flatpak_bundle_via(&SYSTEM, file, id, system, stop, phase) } - fn install_flatpak_bundle_via(root: &Root<'_>, file: &Path, system: bool, stop: &Stop, phase: &mut dyn FnMut(JobPhase)) -> Result<(), String> { + /// An app never uninstalls itself here: its download goes after this, so a failed install would + /// leave nothing. "Already installed" means an earlier update landed and never restarted. + pub(crate) fn update_flatpak_bundle_via( + root: &Root<'_>, + file: &Path, + id: &str, + system: bool, + stop: &Stop, + phase: &mut dyn FnMut(JobPhase), + ) -> Result<(), String> { + match install_flatpak_bundle_via(root, file, None, system, stop, phase) { + Err(e) if already_installed(&e, id) => { + log::info!("{id} is already installed: only the restart is left"); + Ok(()) + } + result => result, + } + } + + /// Installs a downloaded `.flatpak` bundle. `bundle` is the app id it holds, for a per-user install + /// that may uninstall that app first. + fn install_flatpak_bundle_via( + root: &Root<'_>, + file: &Path, + bundle: Option<&str>, + system: bool, + stop: &Stop, + phase: &mut dyn FnMut(JobPhase), + ) -> Result<(), String> { let install_one = || { log::info!("executing flatpak install --reinstall --bundle for {}", file.display()); let mut cmd = if system { system_bundle_command(file)? } else { - let mut cmd = process::flatpak_command(); + // The C locale keeps the "already installed" error below readable. + let mut cmd = (root.flatpak)(); cmd.args(flatpak_bundle_flags(false)); cmd.arg(file); cmd @@ -985,27 +1018,72 @@ pub(crate) mod linux { } flatpak_bundle_error(&output) }; - root.queue.alone(&cancelled(stop), &mut turn(phase, JobPhase::Installing), install_one).unwrap_or(Err(CANCELLED.into())) + // Inside the queue: an uninstall racing another bundle install can drop its remote. + let install = || { + let result = install_one(); + match bundle { + Some(id) if result.as_ref().is_err_and(|e| already_installed(e, id)) && reinstall_by_hand(root) => { + log::info!("{id} is already installed and this Flatpak can't reinstall a bundle: uninstalling it first"); + // No `--delete-data`: the app's files stay for the next install. + if !(root.flatpak)().args(["uninstall", "--user", "-y", "--noninteractive", id]).status().is_ok_and(|s| s.success()) { + log::warn!("couldn't uninstall {id} to install its bundle again"); + return result; + } + install_one() + .map_err(|e| format!("{e} (It was uninstalled first. Its data is kept, and Install tries again with the same download.)")) + } + _ => result, + } + }; + root.queue.alone(&cancelled(stop), &mut turn(phase, JobPhase::Installing), install).unwrap_or(Err(CANCELLED.into())) } - /// Installs (or upgrades) a downloaded package with the system's own tools. - pub fn install_package(kind: PackageKind, file: &Path, stop: &Stop, phase: &mut dyn FnMut(JobPhase)) -> Result<(), String> { + /// Whether a bundle install of `id` failed because the installation already has the bundle's + /// commit: Flatpak's "already installed", in the C locale. Any other failure leaves the app alone. + pub(crate) fn already_installed(err: &str, id: &str) -> bool { + err.contains(&format!("{id} already installed")) + } + + /// Whether the user's copy has to be uninstalled before its bundle installs again: Flatpak + /// before 1.16.2 ignores `--reinstall` for a bundle. + fn reinstall_by_hand(root: &Root<'_>) -> bool { + let mut cmd = (root.flatpak)(); + cmd.arg("--version"); + process::run_command(cmd, "flatpak", Duration::from_secs(10)).is_ok_and(|v| ignores_bundle_reinstall(&v)) + } + + /// Whether `flatpak --version` names a Flatpak that ignores `--reinstall` for a bundle (flatpak + /// commit 4c5693b8 added it in 1.16.2). + pub(crate) fn ignores_bundle_reinstall(flatpak_version: &str) -> bool { + flatpak_version.split_whitespace().last().and_then(version::parse).is_some_and(|v| v < Version::new(1, 16, 2)) + } + + /// Installs (or upgrades) a downloaded package with the system's own tools. `bundle` is the app + /// id a Flatpak bundle holds. + pub fn install_package(kind: PackageKind, file: &Path, bundle: Option<&str>, stop: &Stop, phase: &mut dyn FnMut(JobPhase)) -> Result<(), String> { if kind == PackageKind::Deb && system_tool("apt-get").is_none() { return Err("this system has no apt-get to install a Debian package with".into()); } if kind == PackageKind::ArchPkg && system_tool("pacman").is_none() { return Err("this system has no pacman to install an Arch package with".into()); } - install_package_via(&SYSTEM, kind, file, stop, phase) + install_package_via(&SYSTEM, kind, file, bundle, stop, phase) } - pub fn install_package_via(root: &Root<'_>, kind: PackageKind, file: &Path, stop: &Stop, phase: &mut dyn FnMut(JobPhase)) -> Result<(), String> { + pub fn install_package_via( + root: &Root<'_>, + kind: PackageKind, + file: &Path, + bundle: Option<&str>, + stop: &Stop, + phase: &mut dyn FnMut(JobPhase), + ) -> Result<(), String> { let f = file.as_os_str(); if let Some(command) = install_command(kind) { return as_root(root, command, f, stop, phase, JobPhase::Installing); } match kind { - PackageKind::Flatpak => install_flatpak_bundle_via(root, file, false, stop, phase), + PackageKind::Flatpak => install_flatpak_bundle_via(root, file, bundle, false, stop, phase), other => Err(format!("{} isn't a system package", other.label())), } } @@ -2482,7 +2560,7 @@ mod tests { ran.lock().unwrap().push(args.iter().map(|a| a.to_string_lossy().into_owned()).collect::>()); root_outcome(Some(code)) }; - let root = linux::Root { queue: &queue, run: &run }; + let root = linux::Root { queue: &queue, run: &run, flatpak: &process::flatpak_command_c }; let (started, has_started) = mpsc::channel(); let (release, released) = mpsc::channel::<()>(); let (waiting, waits) = mpsc::channel(); @@ -2503,7 +2581,7 @@ mod tests { waiting.send(()).unwrap(); } }; - linux::install_package_via(root, PackageKind::Deb, Path::new(file), &Stop::default(), &mut phase) + linux::install_package_via(root, PackageKind::Deb, Path::new(file), None, &Stop::default(), &mut phase) }) }); waits.recv().unwrap(); @@ -2545,13 +2623,151 @@ mod tests { return; } let queue = crate::queue::Queue::new(); - let root = linux::Root { queue: &queue, run: &|_| Ok(()) }; - let result = linux::install_package_via(&root, PackageKind::Flatpak, Path::new("/nonexistent/file.flatpak"), &Stop::default(), &mut |_| ()); + let root = linux::Root { queue: &queue, run: &|_| Ok(()), flatpak: &process::flatpak_command_c }; + let result = + linux::install_package_via(&root, PackageKind::Flatpak, Path::new("/nonexistent/file.flatpak"), None, &Stop::default(), &mut |_| ()); assert!(result.is_err()); let err = result.unwrap_err(); assert!(err.starts_with("Flatpak couldn't install the bundle: "), "unexpected error: {err}"); } + #[cfg(target_os = "linux")] + const FLATPAK_ID: &str = "ai.storyteller.soundcraft"; + #[cfg(target_os = "linux")] + const ALREADY_THERE: &str = "ai.storyteller.soundcraft already installed"; + #[cfg(target_os = "linux")] + const INSTALL_CALL: &str = "install --user -y --noninteractive --reinstall --bundle /x.flatpak"; + + /// Runs `install` against a stand-in `flatpak` that logs its calls and fails its first `failures` + /// installs with `why`. Returns the result and the calls. + #[cfg(target_os = "linux")] + fn with_fake_flatpak( + flatpak_version: &str, + failures: u32, + why: &str, + uninstall_exit: u32, + install: impl FnOnce(&linux::Root<'_>) -> Result<(), String>, + ) -> (Result<(), String>, Vec) { + let dir = tempfile::tempdir().unwrap(); + let (script, calls) = (dir.path().join("flatpak"), dir.path().join("calls")); + let log = calls.display(); + std::fs::write( + &script, + format!( + r#"echo "$*" >> "{log}" +case "$1" in +--version) echo "Flatpak {flatpak_version}" ;; +uninstall) exit {uninstall_exit} ;; +install) + [ "$(grep -c '^install' "{log}")" -gt {failures} ] && exit 0 + echo "error: Failed to install bundle: {why}" >&2 + exit 1 ;; +esac +"# + ), + ) + .unwrap(); + let flatpak = || { + let mut cmd = std::process::Command::new("/bin/sh"); + cmd.arg(&script); + cmd + }; + let queue = crate::queue::Queue::new(); + let root = linux::Root { queue: &queue, run: &|_| Ok(()), flatpak: &flatpak }; + let result = install(&root); + (result, std::fs::read_to_string(calls).unwrap().lines().map(str::to_string).collect()) + } + + #[cfg(target_os = "linux")] + fn install_bundle(root: &linux::Root<'_>, bundle: Option<&str>) -> Result<(), String> { + linux::install_package_via(root, PackageKind::Flatpak, Path::new("/x.flatpak"), bundle, &Stop::default(), &mut |_| {}) + } + + #[cfg(target_os = "linux")] + fn update_bundle(root: &linux::Root<'_>, system: bool) -> Result<(), String> { + linux::update_flatpak_bundle_via(root, Path::new("/x.flatpak"), FLATPAK_ID, system, &Stop::default(), &mut |_| {}) + } + + #[cfg(target_os = "linux")] + #[test] + fn a_bundle_goes_in_again_by_hand_only_when_flatpak_needs_it() { + let uninstall = format!("uninstall --user -y --noninteractive {FLATPAK_ID}"); + let refused = format!("Flatpak couldn't install the bundle: Failed to install bundle: {ALREADY_THERE}"); + let id = Some(FLATPAK_ID); + + // An old Flatpak: the data stays, because nothing else goes on the uninstall's command line. + let (result, calls) = with_fake_flatpak("1.14.6", 1, ALREADY_THERE, 0, |root| install_bundle(root, id)); + assert_eq!(result, Ok(())); + assert_eq!(calls, [INSTALL_CALL, "--version", &uninstall, INSTALL_CALL]); + + // A first install that works, or a call with no bundle id, asks nothing more. + assert_eq!(with_fake_flatpak("1.14.6", 0, ALREADY_THERE, 0, |root| install_bundle(root, id)), (Ok(()), vec![INSTALL_CALL.to_string()])); + let (result, calls) = with_fake_flatpak("1.14.6", 1, ALREADY_THERE, 0, |root| install_bundle(root, None)); + assert_eq!((result, calls), (Err(refused.clone()), vec![INSTALL_CALL.to_string()])); + + // A Flatpak that can reinstall a bundle meant the refusal, so the installed copy stays. + let (result, calls) = with_fake_flatpak("1.16.2", 1, ALREADY_THERE, 0, |root| install_bundle(root, id)); + assert_eq!(result, Err(refused.clone())); + assert_eq!(calls, [INSTALL_CALL, "--version"]); + + // Any other failure leaves it alone too. + let (result, calls) = with_fake_flatpak("1.14.6", 1, "Invalid checksum", 0, |root| install_bundle(root, id)); + assert_eq!(result, Err("Flatpak couldn't install the bundle: Failed to install bundle: Invalid checksum".to_string())); + assert_eq!(calls, [INSTALL_CALL]); + + // A failed uninstall reports the first error, and the app is still there. + let (result, calls) = with_fake_flatpak("1.14.6", 1, ALREADY_THERE, 1, |root| install_bundle(root, id)); + assert_eq!(result, Err(refused.clone())); + assert_eq!(calls, [INSTALL_CALL, "--version", &uninstall]); + + // A failure after the uninstall says what happened and what to do next. + let (result, calls) = with_fake_flatpak("1.14.6", 2, ALREADY_THERE, 0, |root| install_bundle(root, id)); + let error = result.unwrap_err(); + assert!(error.starts_with(&refused) && error.contains("uninstalled first") && error.contains("Install tries again"), "{error}"); + assert_eq!(calls, [INSTALL_CALL, "--version", &uninstall, INSTALL_CALL]); + } + + #[cfg(target_os = "linux")] + #[test] + fn a_self_update_never_uninstalls_the_app_it_runs_from() { + // The update landed earlier and never restarted, so only the restart is left. + let (result, calls) = with_fake_flatpak("1.14.6", 1, ALREADY_THERE, 0, |root| update_bundle(root, false)); + assert_eq!((result, calls), (Ok(()), vec![INSTALL_CALL.to_string()])); + + let (result, calls) = with_fake_flatpak("1.14.6", 0, ALREADY_THERE, 0, |root| update_bundle(root, false)); + assert_eq!((result, calls), (Ok(()), vec![INSTALL_CALL.to_string()])); + + // Anything else is still a failure: a broken bundle, or one holding another app. + for why in ["Invalid checksum", "ai.storyteller.photocraft already installed"] { + let (result, calls) = with_fake_flatpak("1.14.6", 1, why, 0, |root| update_bundle(root, false)); + assert_eq!(result, Err(format!("Flatpak couldn't install the bundle: Failed to install bundle: {why}"))); + assert_eq!(calls, [INSTALL_CALL]); + } + } + + #[cfg(target_os = "linux")] + #[test] + fn only_flatpaks_already_installed_error_counts_as_installed() { + let id = FLATPAK_ID; + let failed = |why: &str| format!("Flatpak couldn't install the bundle: Failed to install bundle {id}: {why}"); + assert!(linux::already_installed(&failed(ALREADY_THERE), id)); + // A bundle Flatpak can't use, or one for another app, keeps the installed copy. + assert!(!linux::already_installed(&failed("Can't change remote during bundle install"), id)); + assert!(!linux::already_installed(&failed("Invalid checksum of length 0 expected 32"), id)); + assert!(!linux::already_installed(&failed("ai.storyteller.photocraft already installed"), id)); + } + + #[cfg(target_os = "linux")] + #[test] + fn only_a_flatpak_before_1_16_2_needs_a_bundle_reinstalled_by_hand() { + assert!(linux::ignores_bundle_reinstall("Flatpak 1.14.6\n")); + assert!(linux::ignores_bundle_reinstall("Flatpak 1.16.1")); + assert!(!linux::ignores_bundle_reinstall("Flatpak 1.16.2")); + assert!(!linux::ignores_bundle_reinstall("Flatpak 1.18.4")); + // An unreadable version leaves the failed install as it is. + assert!(!linux::ignores_bundle_reinstall("")); + } + #[cfg(target_os = "linux")] #[test] fn adds_and_removes_only_its_own_menu_entry_and_icon() { diff --git a/crates/update/src/process.rs b/crates/update/src/process.rs index 4f5c935e..1074cec4 100644 --- a/crates/update/src/process.rs +++ b/crates/update/src/process.rs @@ -142,6 +142,19 @@ pub fn flatpak_command() -> Command { } } +/// [`flatpak_command`] in the C locale, for output this code reads: Flatpak translates its messages. +pub fn flatpak_command_c() -> Command { + if in_sandbox() { + let mut cmd = command("flatpak-spawn"); + cmd.args(["--host", "--env=LC_ALL=C", "flatpak"]); + cmd + } else { + let mut cmd = command("flatpak"); + cmd.env("LC_ALL", "C"); + cmd + } +} + /// Runs a flatpak command and captures its standard output. pub fn capture_flatpak(args: &[&str], timeout: Duration) -> Option { if in_sandbox() { @@ -405,6 +418,15 @@ mod tests { mod run_tests { use super::*; + #[test] + fn flatpak_output_this_code_reads_stays_in_english() { + if in_sandbox() { + return; + } + let cmd = flatpak_command_c(); + assert!(cmd.get_envs().any(|(name, value)| name == "LC_ALL" && value == Some(std::ffi::OsStr::new("C")))); + } + /// A shell command line, for the platform's shell. fn shell(script: &str) -> (&'static str, [&str; 2]) { if cfg!(windows) { ("cmd", ["/C", script]) } else { ("/bin/sh", ["-c", script]) } diff --git a/crates/update/src/self_update.rs b/crates/update/src/self_update.rs index b993aa0f..e1d3afd8 100644 --- a/crates/update/src/self_update.rs +++ b/crates/update/src/self_update.rs @@ -385,7 +385,7 @@ pub fn update( } #[cfg(target_os = "linux")] Install::Package(kind, exe) => { - let result = install::linux::install_package(*kind, &file, stop, phase); + 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()]) @@ -399,7 +399,7 @@ pub fn update( // would install a second one beside it. None => return Err(format!("this copy of {} can't update itself; download the new version instead", app.name)), }; - let result = install::linux::install_flatpak_bundle(&file, system, stop, phase); + 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)