Repository navigation
Add Linux support - #5
Conversation
Portable ships as {app_name}.tar.gz extracted next to the current executable, the
same shape as the Windows zip. The installer ships as {app_name}.AppImage and
self-updates by replacing the running AppImage (located via the APPIMAGE env var
the AppImage runtime sets) since there's no separate install step. Both relaunch
through a detached shell script that waits for this process to exit, mirroring
the Windows PowerShell flow.
trypsynth
left a comment
There was a problem hiding this comment.
Thanks for this! Overall it looks good. The module wiring, shell quoting, and asset names all look right. A few things below.
|
|
||
| fn installer_script(pid: u32, new_appimage: &str, current_appimage: &str) -> String { | ||
| format!( | ||
| "{wait}; chmod +x {new_q} && mv -f {new_q} {cur_q} && nohup {cur_q} >/dev/null 2>&1 &", |
There was a problem hiding this comment.
Because the steps are chained with &&, if any step fails the app never relaunches. For example, if the AppImage lives somewhere the user can't write (like /opt/App.AppImage), mv -f fails after the app has already quit, and nothing starts it again. The script output goes nowhere in a normal desktop launch, so to the user the app just closes and stays closed.
The same applies to the tar.gz script below (a bad archive or full disk also leaves the .tar.gz behind). The Windows scripts use ; so the relaunch always happens. Could we do the same here, so the old version at least comes back up if the update fails?
There was a problem hiding this comment.
Good catch, fixed in 30f2724: both scripts now use ; between steps instead of &&, so a failed chmod/mv/tar/rm still relaunches the existing app (old AppImage, or the exe that's still on disk) instead of leaving it closed. Added tests asserting neither script contains && anymore.
| // file. `current_exe` would instead resolve to the temporary FUSE mount the runtime | ||
| // extracts itself to, which disappears when the AppImage exits. | ||
| let current_appimage = | ||
| env::var("APPIMAGE").map_err(|_| t("Not running from an AppImage; cannot self-update."))?; |
There was a problem hiding this comment.
This APPIMAGE check only runs in install(), which is after the full download. If the app is set to InstallKind::Installer but isn't running as an AppImage (launched from an extracted folder, or during development), the user waits for the whole download and only then gets the error. The downloaded file is also past its temp guard by then, so it stays in /tmp forever.
Could this check happen before the download starts, for example in download_dir?
There was a problem hiding this comment.
Fixed in 30f2724: the APPIMAGE check now happens in download_dir, which runs before the download starts, so a non-AppImage build configured with InstallKind::Installer fails immediately instead of after downloading (and the file no longer gets orphaned in /tmp past the temp guard).
| #[cfg(target_os = "linux")] | ||
| pub fn download_dir(config: &UpdaterConfig) -> Result<PathBuf, UpdateError> { | ||
| match config.install_kind { | ||
| InstallKind::Installer => Ok(env::temp_dir()), |
There was a problem hiding this comment.
Minor, and unlikely to matter in practice, so fine to leave for a later fix: /tmp is shared between all users on Linux, and the download uses a fixed name (/tmp/{app}.AppImage.tmp, then /tmp/{app}.AppImage). If another local user creates a file with that name first, updates fail for everyone else because of the sticky bit (and protected_regular on many distros). A per-user folder like $XDG_CACHE_HOME would avoid it.
Use ; instead of && between the shell script's steps so a failed chmod/mv/tar/rm still relaunches the existing app instead of leaving it closed with no explanation, matching the Windows scripts. Also move the APPIMAGE env var check into download_dir so InstallKind::Installer on a non-AppImage build fails before spending a download on an update it can never apply, rather than after.
CI's ubuntu-latest job failed clippy: doc comments only compiled under cfg(target_os = "linux") referenced "AppImage" without backticks, which clippy::doc_markdown flags as looking like a bare identifier. Only the Linux check job hits this, since these doc comments aren't compiled at all on Windows/macOS. Wrap the remaining mentions in backticks.
The Installer flow downloaded to the shared system temp directory under
a fixed, predictable name. On a multi-user Linux box another local user
could create that path first, and the sticky bit (plus protected_regular
on many distros) would then break updates for everyone else. Download to
$XDG_CACHE_HOME/{app_name} instead, falling back to $HOME/.cache, which
is private to the running user.
Summary
{app_name}.tar.gz, extracted next to the current executable (same placement as the Windows zip){app_name}.AppImage; since AppImage has no separate install step, the update replaces the running AppImage file in place (located via theAPPIMAGEenv var the AppImage runtime sets) and relaunches it