Add support for the .AppImage extension

autarch · autarch · commit 1dc8a0c82ec1 · 2025-01-18T15:52:29.000-06:00
This also updates extension handling in a few other ways:

* We only consider assets with `.exe` extensions on Windows.
* We only consider assets with `.AppImage` extensions on Linux.
* We make sure to preserve `.exe`, `.pytz`, and `.AppImage` extensions when installing assets with
  these extensions.
diff --git a/Changes.md b/Changes.md
@@ -6,6 +6,11 @@
   includes things like a version number of platform information. Based on discussion in #86.
 - Added support for release artifacts with a `.pyz` extension. These are zip files containing Python
   code, and they can be directly executed.
+- Added support for release artifacts with a `.AppImage` extension. These will only be picked when
+  running Linux. Requested by @saulh (Saul Reynolds-Haertle). GH #86.
+- Fixed a bug where `ubi` would consider an asset with `.exe` extension on non-Windows platforms. In
+  practice, this would probably only have been an issue for projects with exactly one release
+  artifact, where that artifact had a `.exe` extension.
 - The `--extract-all` CLI option added in the previous release did not have any description in the
   help output. This has been fixed.
 
diff --git a/ubi-cli/tests/ubi.rs b/ubi-cli/tests/ubi.rs
@@ -492,6 +492,16 @@ fn integration_tests() -> Result<()> {
         make_exe_pathbuf(&["bin", "glab"]),
     )?;
 
+    #[cfg(target_os = "linux")]
+    {
+        run_test(
+            td.path(),
+            ubi.as_ref(),
+            &["--project", "hzeller/timg", "--tag", "v1.6.1"],
+            make_exe_pathbuf(&["bin", "timg.AppImage"]),
+        )?;
+    }
+
     Ok(())
 }
 
diff --git a/ubi/src/extension.rs b/ubi/src/extension.rs
@@ -4,19 +4,24 @@ use anyhow::Result;
 use itertools::Itertools;
 use lazy_regex::{regex, Lazy};
 use log::debug;
+use platforms::{Platform, OS};
 use regex::Regex;
-use std::{ffi::OsStr, path::Path};
+use std::{
+    ffi::OsStr,
+    path::{Path, PathBuf},
+};
 use strum::{EnumIter, IntoEnumIterator};
 use thiserror::Error;
 
 #[derive(Debug, Error)]
 pub(crate) enum ExtensionError {
-    #[error("{path:} has unknown extension {ext:}")]
-    UnknownExtension { path: String, ext: String },
+    #[error("{} has unknown extension {ext:}", path.display())]
+    UnknownExtension { path: PathBuf, ext: String },
 }
 
 #[derive(Debug, EnumIter, PartialEq, Eq)]
 pub(crate) enum Extension {
+    AppImage,
     Bz,
     Bz2,
     Exe,
@@ -37,6 +42,7 @@ pub(crate) enum Extension {
 impl Extension {
     pub(crate) fn extension(&self) -> &'static str {
         match self {
+            Extension::AppImage => ".AppImage",
             Extension::Bz => ".bz",
             Extension::Bz2 => ".bz2",
             Extension::Exe => ".exe",
@@ -55,9 +61,14 @@ impl Extension {
         }
     }
 
+    pub(crate) fn extension_without_dot(&self) -> &str {
+        self.extension().strip_prefix('.').unwrap()
+    }
+
     pub(crate) fn is_archive(&self) -> bool {
         match self {
-            Extension::Bz
+            Extension::AppImage
+            | Extension::Bz
             | Extension::Bz2
             | Extension::Exe
             | Extension::Gz
@@ -75,41 +86,71 @@ impl Extension {
         }
     }
 
-    pub(crate) fn from_path<S: AsRef<str>>(path: S) -> Result<Option<Extension>> {
-        let path = path.as_ref();
-        let Some(ext_str) = Path::new(path).extension() else {
+    pub(crate) fn should_preserve_extension_on_install(&self) -> bool {
+        match self {
+            Extension::AppImage | Extension::Exe | Extension::Pyz => true,
+            Extension::Xz
+            | Extension::Gz
+            | Extension::Bz
+            | Extension::Bz2
+            | Extension::Tar
+            | Extension::TarBz
+            | Extension::TarBz2
+            | Extension::TarGz
+            | Extension::TarXz
+            | Extension::Tbz
+            | Extension::Tgz
+            | Extension::Txz
+            | Extension::Zip => false,
+        }
+    }
+
+    pub(crate) fn matches_platform(&self, platform: &Platform) -> bool {
+        match self {
+            Extension::AppImage => platform.target_os == OS::Linux,
+            Extension::Exe => platform.target_os == OS::Windows,
+            _ => true,
+        }
+    }
+
+    pub(crate) fn from_path(path: &Path) -> Result<Option<Extension>> {
+        let Some(ext_str_from_path) = path.extension() else {
             return Ok(None);
         };
+        let path_str = path.to_string_lossy();
 
         // We need to try the longest extensions first so that ".tar.gz" matches before ".gz" and so
         // on for other compression formats.
         if let Some(ext) = Extension::iter()
             .sorted_by(|a, b| Ord::cmp(&a.extension().len(), &b.extension().len()))
             .rev()
-            .find(|e| path.ends_with(e.extension()))
+            // This is intentionally using a string comparison instead of looking at
+            // path.extension(). That's because the `.extension()` method returns `"bz"` for paths
+            // like "foo.tar.bz", instead of "tar.bz".
+            .find(|e| path_str.ends_with(e.extension()))
         {
             return Ok(Some(ext));
         }
 
-        if extension_is_part_of_version(path, ext_str) {
-            debug!("the extension {ext_str:?} is part of the version, ignoring");
+        if extension_is_part_of_version(path, ext_str_from_path) {
+            debug!("the extension {ext_str_from_path:?} is part of the version, ignoring");
             return Ok(None);
         }
 
-        if extension_is_platform(ext_str) {
-            debug!("the extension {ext_str:?} is a platform name, ignoring");
+        if extension_is_platform(ext_str_from_path) {
+            debug!("the extension {ext_str_from_path:?} is a platform name, ignoring");
             return Ok(None);
         }
 
         Err(ExtensionError::UnknownExtension {
-            path: path.to_string(),
-            ext: ext_str.to_string_lossy().to_string(),
+            path: path.to_path_buf(),
+            ext: ext_str_from_path.to_string_lossy().to_string(),
         }
         .into())
     }
 }
 
-fn extension_is_part_of_version(path: &str, ext_str: &OsStr) -> bool {
+fn extension_is_part_of_version(path: &Path, ext_str: &OsStr) -> bool {
     let ext_str = ext_str.to_string_lossy().to_string();
 
     let version_number_ext_re = regex!(r"^[0-9]+");
@@ -119,7 +160,9 @@ fn extension_is_part_of_version(path: &str, ext_str: &OsStr) -> bool {
 
     // This matches something like "foo_3.2.1_linux_amd64" and captures "1_linux_amd64".
     let version_number_re = regex!(r"[0-9]+\.([0-9]+[^.]*)$");
-    let Some(caps) = version_number_re.captures(path) else {
+    let Some(caps) = version_number_re.captures(path.to_str().expect(
+        "this path came from a UTF-8 string originally so it should always convert back to one",
+    )) else {
         return false;
     };
     let Some(dot_num) = caps.get(1) else {
@@ -149,7 +192,9 @@ fn extension_is_platform(ext_str: &OsStr) -> bool {
 mod test {
     use super::*;
     use test_case::test_case;
+    use test_log::test;
 
+    #[test_case("foo.AppImage", Ok(Some(Extension::AppImage)))]
     #[test_case("foo.bz", Ok(Some(Extension::Bz)))]
     #[test_case("foo.bz2", Ok(Some(Extension::Bz2)))]
     #[test_case("foo.exe", Ok(Some(Extension::Exe)))]
@@ -166,11 +211,11 @@ mod test {
     #[test_case("foo_3.9.1.linux.amd64", Ok(None))]
     #[test_case("i386-linux-ghcup-0.1.30.0", Ok(None))]
     #[test_case("i386-linux-ghcup-0.1.30.0-linux_amd64", Ok(None))]
-    #[test_case("foo.bar", Err(ExtensionError::UnknownExtension { path: "foo.bar".to_string(), ext: "bar".to_string() }.into()))]
+    #[test_case("foo.bar", Err(ExtensionError::UnknownExtension { path: PathBuf::from("foo.bar"), ext: "bar".to_string() }.into()))]
     fn from_path(path: &str, expect: Result<Option<Extension>>) {
         crate::test_case::init_logging();
 
-        let ext = Extension::from_path(path);
+        let ext = Extension::from_path(Path::new(path));
         if expect.is_ok() {
             assert!(ext.is_ok());
             assert_eq!(ext.unwrap(), expect.unwrap());
@@ -181,4 +226,37 @@ mod test {
             );
         }
     }
+
+    #[test]
+    fn matches_platform() -> Result<()> {
+        let freebsd = Platform::find("x86_64-unknown-freebsd").unwrap().clone();
+        let linux = Platform::find("x86_64-unknown-linux-gnu").unwrap().clone();
+        let macos = Platform::find("aarch64-apple-darwin").unwrap().clone();
+        let windows = Platform::find("x86_64-pc-windows-msvc").unwrap().clone();
+
+        let ext = Extension::from_path(Path::new("foo.exe"))?.unwrap();
+        assert!(
+            ext.matches_platform(&windows),
+            "foo.exe is valid on {windows}"
+        );
+        for p in [&freebsd, &linux, &macos] {
+            assert!(!ext.matches_platform(p), "foo.exe is not valid on {p}");
+        }
+
+        let ext = Extension::from_path(Path::new("foo.AppImage"))?.unwrap();
+        assert!(
+            ext.matches_platform(&linux),
+            "foo.exe is valid on {windows}"
+        );
+        for p in [&freebsd, &macos, &windows] {
+            assert!(!ext.matches_platform(p), "foo.AppImage is not valid on {p}");
+        }
+
+        let ext = Extension::from_path(Path::new("foo.tar.gz"))?.unwrap();
+        for p in [&freebsd, &linux, &macos, &windows] {
+            assert!(ext.matches_platform(p), "foo.tar.gz is valid on {p}");
+        }
+
+        Ok(())
+    }
 }
diff --git a/ubi/src/installer.rs b/ubi/src/installer.rs
@@ -40,8 +40,8 @@ impl ExeInstaller {
         ExeInstaller { install_path, exe }
     }
 
-    fn extract_executable(&self, downloaded_file: &Path) -> Result<()> {
-        match Extension::from_path(downloaded_file.to_string_lossy())? {
+    fn extract_executable(&self, downloaded_file: &Path) -> Result<Option<PathBuf>> {
+        match Extension::from_path(downloaded_file)? {
             Some(
                 Extension::Tar
                 | Extension::TarBz
@@ -51,12 +51,29 @@ impl ExeInstaller {
                 | Extension::Tbz
                 | Extension::Tgz
                 | Extension::Txz,
-            ) => self.extract_executable_from_tarball(downloaded_file),
-            Some(Extension::Bz | Extension::Bz2) => self.unbzip(downloaded_file),
-            Some(Extension::Gz) => self.ungzip(downloaded_file),
-            Some(Extension::Xz) => self.unxz(downloaded_file),
-            Some(Extension::Zip) => self.extract_executable_from_zip(downloaded_file),
-            Some(Extension::Exe | Extension::Pyz) | None => self.copy_executable(downloaded_file),
+            ) => {
+                self.extract_executable_from_tarball(downloaded_file)?;
+                Ok(None)
+            }
+            Some(Extension::Bz | Extension::Bz2) => {
+                self.unbzip(downloaded_file)?;
+                Ok(None)
+            }
+            Some(Extension::Gz) => {
+                self.ungzip(downloaded_file)?;
+                Ok(None)
+            }
+            Some(Extension::Xz) => {
+                self.unxz(downloaded_file)?;
+                Ok(None)
+            }
+            Some(Extension::Zip) => {
+                self.extract_executable_from_zip(downloaded_file)?;
+                Ok(None)
+            }
+            Some(Extension::AppImage | Extension::Exe | Extension::Pyz) | None => {
+                self.copy_executable(downloaded_file)
+            }
         }
     }
 
@@ -146,12 +163,24 @@ impl ExeInstaller {
         Ok(())
     }
 
-    fn copy_executable(&self, exe_file: &Path) -> Result<()> {
+    fn copy_executable(&self, exe_file: &Path) -> Result<Option<PathBuf>> {
         debug!("copying executable to final location");
         self.create_install_dir()?;
-        std::fs::copy(exe_file, &self.install_path)?;
 
-        Ok(())
+        let mut install_path = self.install_path.clone();
+        if let Some(ext) = Extension::from_path(exe_file)? {
+            if ext.should_preserve_extension_on_install() {
+                debug!("preserving the {} extension on install", ext.extension());
+                install_path.set_extension(ext.extension_without_dot());
+            }
+        }
+        std::fs::copy(exe_file, &install_path).context(format!(
+            "error copying file from {} to {}",
+            exe_file.display(),
+            install_path.display()
+        ))?;
+
+        Ok(Some(install_path))
     }
 
     fn create_install_dir(&self) -> Result<()> {
@@ -167,12 +196,12 @@ impl ExeInstaller {
             .with_context(|| format!("could not create a directory at {}", path.display()))
     }
 
-    fn chmod_executable(&self) -> Result<()> {
+    fn chmod_executable(exe: &Path) -> Result<()> {
         #[cfg(target_family = "windows")]
         return Ok(());
 
         #[cfg(target_family = "unix")]
-        match set_permissions(&self.install_path, Permissions::from_mode(0o755)) {
+        match set_permissions(exe, Permissions::from_mode(0o755)) {
             Ok(()) => Ok(()),
             Err(e) => Err(anyhow::Error::new(e)),
         }
@@ -181,9 +210,10 @@ impl ExeInstaller {
 
 impl Installer for ExeInstaller {
     fn install(&self, download: &Download) -> Result<()> {
-        self.extract_executable(&download.archive_path)?;
-        self.chmod_executable()?;
-        info!("Installed executable into {}", self.install_path.display());
+        let exe = self.extract_executable(&download.archive_path)?;
+        let real_exe = exe.as_deref().unwrap_or(&self.install_path);
+        Self::chmod_executable(real_exe)?;
+        info!("Installed executable into {}", real_exe.display());
 
         Ok(())
     }
@@ -197,7 +227,7 @@ impl ArchiveInstaller {
     }
 
     fn extract_entire_archive(&self, downloaded_file: &Path) -> Result<()> {
-        match Extension::from_path(downloaded_file.to_string_lossy())? {
+        match Extension::from_path(downloaded_file)? {
             Some(
                 Extension::Tar
                 | Extension::TarBz
@@ -370,6 +400,7 @@ mod tests {
     use test_case::test_case;
     use test_log::test;
 
+    #[test_case("test-data/project.AppImage")]
     #[test_case("test-data/project.bz")]
     #[test_case("test-data/project.bz2")]
     #[test_case("test-data/project.exe")]
@@ -394,6 +425,14 @@ mod tests {
         let mut path_with_subdir = td.path().to_path_buf();
         path_with_subdir.extend(&["subdir", "project"]);
 
+        // The installer special-cases this extension and preserves it for the installed file.
+        if let Some(ext) = Extension::from_path(Path::new(archive_path))? {
+            if ext.should_preserve_extension_on_install() {
+                path_without_subdir.set_extension(ext.extension_without_dot());
+                path_with_subdir.set_extension(ext.extension_without_dot());
+            }
+        }
+
         for install_path in [path_without_subdir, path_with_subdir] {
             let installer = ExeInstaller::new(install_path.clone(), exe.to_string());
             installer.install(&Download {
diff --git a/ubi/src/lib.rs b/ubi/src/lib.rs
@@ -32,8 +32,9 @@
 //! with the same name as the project. In either case, the file will be installed with the name it
 //! has in the archive file.
 //!
-//! If the release is in the form of a bare executable or a compressed executable, then the installed
-//! executable will use the name of the project instead.
+//! If the release is in the form of a bare executable or a compressed executable, then the
+//! installed executable will use the name of the project instead. For files with a `.exe`, `.pyz`
+//! or `.AppImage`, the installed executable will be `$project_name.$extension`.
 //!
 //! This is a bit inconsistent, but it's how `ubi` has behaved since it was created, and I find this
 //! to be the sanest behavior. Some projects, for example `rust-analyzer`, provide releases as
diff --git a/ubi/src/picker.rs b/ubi/src/picker.rs
diff --git a/ubi/test-data/project.AppImage b/ubi/test-data/project.AppImage
