Skip to content

Commit

Permalink
Add support for the .AppImage extension
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
autarch committed Jan 18, 2025
1 parent 3b02ab6 commit ee80d78
Show file tree
Hide file tree
Showing 7 changed files with 202 additions and 39 deletions.
5 changes: 5 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
10 changes: 10 additions & 0 deletions ubi-cli/tests/ubi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,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(())
}

Expand Down
114 changes: 96 additions & 18 deletions ubi/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
Expand All @@ -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
Expand All @@ -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]+");
Expand All @@ -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 {
Expand Down Expand Up @@ -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)))]
Expand All @@ -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());
Expand All @@ -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(())
}
}
73 changes: 56 additions & 17 deletions ubi/src/installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}
}

Expand Down Expand Up @@ -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<()> {
Expand All @@ -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)),
}
Expand All @@ -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(())
}
Expand All @@ -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
Expand Down Expand Up @@ -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")]
Expand All @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions ubi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit ee80d78

Please sign in to comment.