diff --git a/Changes.md b/Changes.md index 9fbdb81..4bb40ae 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,9 @@ ## 0.4.0 +- The `ubi` CLI tool now takes an optional `--extract-all` argument. If this is passed, it will only + look for archive files and it will extract the entire contents of an archive it finds. There is + also a new corresponding `UbiBuilder::extract_all` method. Requested by @Entze (Lukas Grassauer). + GH #68. - The `UbiBuilder::install_dir` method now takes `AsRef` instead of `PathBuf`, which should make it more convenient to use. - Previously, `ubi` would create the install directory very early in its process, well before it had diff --git a/README.md b/README.md index 7ba5781..3f03b55 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,17 @@ Options: -e, --exe The name of this project's executable. By default this is the same as the project name, so for houseabsolute/precious we look for precious or precious.exe. When running on Windows the - ".exe" suffix will be added as needed. + ".exe" suffix will be added as needed. You cannot pass + --extract-all when this is set + --extract-all Pass this to tell `ubi` to extract all files from the archive. + By default `ubi` will only extract an executable from an + archive file. But if this is true, it will simply unpack the + archive file in the specified directory. If all of the contents + of the archive file share a top-level directory, that directory + will be removed during unpacking. In other words, if an archive + contains `./project/some-file` and `./project/docs.md`, it will + extract them as `some-file` and `docs.md`. You cannot pass + --exe when this is set. -m, --matching A string that will be matched against the release filename when there are multiple matching files for your OS/arch. For example, there may be multiple releases for an OS/arch that diff --git a/ubi-cli/src/main.rs b/ubi-cli/src/main.rs index 3c94a03..51c292e 100644 --- a/ubi-cli/src/main.rs +++ b/ubi-cli/src/main.rs @@ -57,6 +57,7 @@ async fn main() { const MAX_TERM_WIDTH: usize = 100; +#[allow(clippy::too_many_lines)] fn cmd() -> Command { Command::new("ubi") .version(env!("CARGO_PKG_VERSION")) @@ -98,8 +99,22 @@ fn cmd() -> Command { "The name of this project's executable. By default this is the same as the", " project name, so for houseabsolute/precious we look for precious or", r#" precious.exe. When running on Windows the ".exe" suffix will be added"#, - " as needed.", + " as needed. You cannot pass --extract-all when this is set.", ))) + .arg( + Arg::new("extract-all") + .long("extract-all") + .action(ArgAction::SetTrue) + .help(concat!( + "Pass this to tell `ubi` to extract all files from the archive. By default", + " `ubi` will only extract an executable from an archive file. But if this is", + " true, it will simply unpack the archive file in the specified directory. If", + " all of the contents of the archive file share a top-level directory, that", + " directory will be removed during unpacking. In other words, if an archive", + " contains `./project/some-file` and `./project/docs.md`, it will extract them", + " as `some-file` and `docs.md`. You cannot pass --exe when this is set.", + )), + ) .arg( Arg::new("matching") .long("matching") @@ -196,6 +211,9 @@ fn make_ubi<'a>( if let Some(e) = matches.get_one::("exe") { builder = builder.exe(e); } + if matches.get_flag("extract-all") { + builder = builder.extract_all(); + } if let Some(ft) = matches.get_one::("forge") { builder = builder.forge(ForgeType::from_str(ft)?); } diff --git a/ubi/src/builder.rs b/ubi/src/builder.rs index 55c6d63..3d9b6eb 100644 --- a/ubi/src/builder.rs +++ b/ubi/src/builder.rs @@ -3,7 +3,7 @@ use crate::{ forge::{Forge, ForgeType}, github::GitHub, gitlab::GitLab, - installer::Installer, + installer::{ArchiveInstaller, ExeInstaller, Installer}, picker::AssetPicker, ubi::Ubi, }; @@ -32,6 +32,7 @@ pub struct UbiBuilder<'a> { install_dir: Option, matching: Option<&'a str>, exe: Option<&'a str>, + extract_all: bool, github_token: Option<&'a str>, gitlab_token: Option<&'a str>, platform: Option<&'a Platform>, @@ -100,12 +101,25 @@ impl<'a> UbiBuilder<'a> { /// Set the name of the executable to look for in archive files. By default this is the same as /// the project name, so for `houseabsolute/precious` we look for `precious` or /// `precious.exe`. When running on Windows the ".exe" suffix will be added as needed. + /// + /// You cannot call `extract_all` if you set this. #[must_use] pub fn exe(mut self, exe: &'a str) -> Self { self.exe = Some(exe); self } + /// Call this to tell `ubi` to extract all files from the archive. By default `ubi` will look + /// for an executable in an archive file. But if this is true, it will simply unpack the archive + /// file in the specified directory. + /// + /// You cannot set `exe` when this is true. + #[must_use] + pub fn extract_all(mut self) -> Self { + self.extract_all = true; + self + } + /// Set a GitHub token to use for API requests. If this is not set then this will be taken from /// the `GITHUB_TOKEN` env var if it is set. #[must_use] @@ -175,6 +189,9 @@ impl<'a> UbiBuilder<'a> { if self.url.is_some() && (self.project.is_some() || self.tag.is_some()) { return Err(anyhow!("You cannot set a url with a project or tag")); } + if self.exe.is_some() && self.extract_all { + return Err(anyhow!("You cannot set exe and extract_all")); + } let platform = self.determine_platform()?; @@ -183,20 +200,40 @@ impl<'a> UbiBuilder<'a> { let asset_url = self.url.map(Url::parse).transpose()?; let (project_name, forge_type) = parse_project_name(self.project, asset_url.as_ref(), self.forge.clone())?; - let exe = exe_name(self.exe, &project_name, &platform); + let installer = self.new_installer(&project_name, &platform)?; let forge = self.new_forge(project_name, &forge_type)?; - let install_path = install_path(self.install_dir, &exe)?; let is_musl = self.is_musl.unwrap_or_else(|| platform_is_musl(&platform)); Ok(Ubi::new( forge, asset_url, - AssetPicker::new(self.matching, platform, is_musl), - Installer::new(install_path, exe), + AssetPicker::new(self.matching, platform, is_musl, self.extract_all), + installer, reqwest_client()?, )) } + fn new_installer(&self, project_name: &str, platform: &Platform) -> Result> { + let (install_path, exe) = if self.extract_all { + // We know that this contains a slash because it already went through `parse_project_name` + // successfully. + let project_name = project_name.split('/').last().unwrap(); + ( + install_path(self.install_dir.as_deref(), project_name)?, + None, + ) + } else { + let exe = exe_name(self.exe, project_name, platform); + (install_path(self.install_dir.as_deref(), &exe)?, Some(exe)) + }; + + Ok(if self.extract_all { + Box::new(ArchiveInstaller::new(install_path)) + } else { + Box::new(ExeInstaller::new(install_path, exe.unwrap())) + }) + } + fn new_forge( &self, project_name: String, @@ -283,17 +320,17 @@ fn parse_project_name( )) } -fn install_path(install_dir: Option, exe: &str) -> Result { - let mut path = if let Some(i) = install_dir { - i +fn install_path(install_dir: Option<&Path>, project_name_or_exe: &str) -> Result { + let mut install_dir = if let Some(install_dir) = install_dir { + install_dir.to_path_buf() } else { - let mut i = env::current_dir()?; - i.push("bin"); - i + let mut install_dir = env::current_dir()?; + install_dir.push("bin"); + install_dir }; - path.push(exe); - debug!("install path = {}", path.to_string_lossy()); - Ok(path) + install_dir.push(project_name_or_exe); + debug!("install path = {}", install_dir.to_string_lossy()); + Ok(install_dir) } fn exe_name(exe: Option<&str>, project_name: &str, platform: &Platform) -> String { @@ -303,12 +340,13 @@ fn exe_name(exe: Option<&str>, project_name: &str, platform: &Platform) -> Strin _ => e.to_string(), } } else { - let parts = project_name.split('/').collect::>(); - let e = parts[parts.len() - 1].to_string(); + // We know that this contains a slash because it already went through `parse_project_name` + // successfully. + let e = project_name.split('/').last().unwrap(); if matches!(platform.target_os, OS::Windows) { format!("{e}.exe") } else { - e + e.to_string() } }; debug!("exe name = {name}"); diff --git a/ubi/src/extension.rs b/ubi/src/extension.rs index 093c921..a93daad 100644 --- a/ubi/src/extension.rs +++ b/ubi/src/extension.rs @@ -53,6 +53,23 @@ impl Extension { } } + pub(crate) fn is_archive(&self) -> bool { + match self { + Extension::Bz | Extension::Bz2 | Extension::Exe | Extension::Gz | Extension::Xz => { + false + } + Extension::Tar + | Extension::TarBz + | Extension::TarBz2 + | Extension::TarGz + | Extension::TarXz + | Extension::Tbz + | Extension::Tgz + | Extension::Txz + | Extension::Zip => true, + } + } + pub(crate) fn from_path>(path: S) -> Result> { let path = path.as_ref(); let Some(ext_str) = Path::new(path).extension() else { @@ -146,6 +163,8 @@ mod test { #[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()))] fn from_path(path: &str, expect: Result>) { + crate::test_case::init_logging(); + let ext = Extension::from_path(path); if expect.is_ok() { assert!(ext.is_ok()); diff --git a/ubi/src/installer.rs b/ubi/src/installer.rs index 43b843e..09f7da1 100644 --- a/ubi/src/installer.rs +++ b/ubi/src/installer.rs @@ -5,7 +5,10 @@ use bzip2::read::BzDecoder; use flate2::read::GzDecoder; use log::{debug, info}; use std::{ - fs::{create_dir_all, File}, + collections::HashSet, + ffi::OsString, + fmt::Debug, + fs::{self, create_dir_all, File}, io::{Read, Write}, path::{Path, PathBuf}, }; @@ -17,36 +20,38 @@ use std::fs::{set_permissions, Permissions}; #[cfg(target_family = "unix")] use std::os::unix::fs::PermissionsExt; +pub(crate) trait Installer: Debug { + fn install(&self, download: &Download) -> Result<()>; +} + #[derive(Debug)] -pub(crate) struct Installer { +pub(crate) struct ExeInstaller { install_path: PathBuf, exe: String, } -impl Installer { - pub(crate) fn new(install_path: PathBuf, exe: String) -> Self { - Installer { install_path, exe } - } +#[derive(Debug)] +pub(crate) struct ArchiveInstaller { + install_root: PathBuf, +} - pub(crate) fn install(&self, download: &Download) -> Result<()> { - self.extract_binary(&download.archive_path)?; - self.make_binary_executable()?; - info!("Installed binary into {}", self.install_path.display()); +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()); Ok(()) } +} + +impl ExeInstaller { + pub(crate) fn new(install_path: PathBuf, exe: String) -> Self { + ExeInstaller { install_path, exe } + } - fn extract_binary(&self, downloaded_file: &Path) -> Result<()> { - let filename = downloaded_file - .file_name() - .unwrap_or_else(|| { - panic!( - "downloaded file path {} has no filename!", - downloaded_file.to_string_lossy() - ) - }) - .to_string_lossy(); - match Extension::from_path(filename)? { + fn extract_executable(&self, downloaded_file: &Path) -> Result<()> { + match Extension::from_path(downloaded_file.to_string_lossy())? { Some( Extension::Tar | Extension::TarBz @@ -56,42 +61,18 @@ impl Installer { | Extension::Tbz | Extension::Tgz | Extension::Txz, - ) => self.extract_tarball(downloaded_file), + ) => 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_zip(downloaded_file), + Some(Extension::Zip) => self.extract_executable_from_zip(downloaded_file), Some(Extension::Exe) | None => self.copy_executable(downloaded_file), } } - fn extract_zip(&self, downloaded_file: &Path) -> Result<()> { - debug!("extracting binary from zip file"); - - let mut zip = ZipArchive::new(open_file(downloaded_file)?)?; - for i in 0..zip.len() { - let mut zf = zip.by_index(i)?; - let path = PathBuf::from(zf.name()); - if path.ends_with(&self.exe) { - let mut buffer: Vec = Vec::with_capacity(usize::try_from(zf.size())?); - zf.read_to_end(&mut buffer)?; - self.create_install_dir()?; - return File::create(&self.install_path)? - .write_all(&buffer) - .map_err(Into::into); - } - } - - debug!("could not find any entries named {}", self.exe); - Err(anyhow!( - "could not find any files named {} in the downloaded zip file", - self.exe, - )) - } - - fn extract_tarball(&self, downloaded_file: &Path) -> Result<()> { + fn extract_executable_from_tarball(&self, downloaded_file: &Path) -> Result<()> { debug!( - "extracting binary from tarball at {}", + "extracting executable from tarball at {}", downloaded_file.to_string_lossy(), ); @@ -125,20 +106,44 @@ impl Installer { )) } + fn extract_executable_from_zip(&self, downloaded_file: &Path) -> Result<()> { + debug!("extracting executable from zip file"); + + let mut zip = ZipArchive::new(open_file(downloaded_file)?)?; + for i in 0..zip.len() { + let mut zf = zip.by_index(i)?; + let path = PathBuf::from(zf.name()); + if path.ends_with(&self.exe) { + let mut buffer: Vec = Vec::with_capacity(usize::try_from(zf.size())?); + zf.read_to_end(&mut buffer)?; + self.create_install_dir()?; + return File::create(&self.install_path)? + .write_all(&buffer) + .map_err(Into::into); + } + } + + debug!("could not find any entries named {}", self.exe); + Err(anyhow!( + "could not find any files named {} in the downloaded zip file", + self.exe, + )) + } + fn unbzip(&self, downloaded_file: &Path) -> Result<()> { - debug!("uncompressing binary from bzip file"); + debug!("uncompressing executable from bzip file"); let reader = BzDecoder::new(open_file(downloaded_file)?); self.write_to_install_path(reader) } fn ungzip(&self, downloaded_file: &Path) -> Result<()> { - debug!("uncompressing binary from gzip file"); + debug!("uncompressing executable from gzip file"); let reader = GzDecoder::new(open_file(downloaded_file)?); self.write_to_install_path(reader) } fn unxz(&self, downloaded_file: &Path) -> Result<()> { - debug!("uncompressing binary from xz file"); + debug!("uncompressing executable from xz file"); let reader = XzDecoder::new(open_file(downloaded_file)?); self.write_to_install_path(reader) } @@ -152,7 +157,7 @@ impl Installer { } fn copy_executable(&self, exe_file: &Path) -> Result<()> { - debug!("copying binary to final location"); + debug!("copying executable to final location"); self.create_install_dir()?; std::fs::copy(exe_file, &self.install_path)?; @@ -172,7 +177,7 @@ impl Installer { .with_context(|| format!("could not create a directory at {}", path.display())) } - fn make_binary_executable(&self) -> Result<()> { + fn chmod_executable(&self) -> Result<()> { #[cfg(target_family = "windows")] return Ok(()); @@ -184,6 +189,138 @@ impl Installer { } } +impl Installer for ArchiveInstaller { + fn install(&self, download: &Download) -> Result<()> { + self.extract_entire_archive(&download.archive_path)?; + info!( + "Installed contents of archive file into {}", + self.install_root.display() + ); + + Ok(()) + } +} + +impl ArchiveInstaller { + pub(crate) fn new(install_path: PathBuf) -> Self { + ArchiveInstaller { + install_root: install_path, + } + } + + fn extract_entire_archive(&self, downloaded_file: &Path) -> Result<()> { + match Extension::from_path(downloaded_file.to_string_lossy())? { + Some( + Extension::Tar + | Extension::TarBz + | Extension::TarBz2 + | Extension::TarGz + | Extension::TarXz + | Extension::Tbz + | Extension::Tgz + | Extension::Txz, + ) => self.extract_entire_tarball(downloaded_file)?, + Some(Extension::Zip) => self.extract_entire_zip(downloaded_file)?, + _ => { + return Err(anyhow!( + concat!( + "the downloaded release asset, {}, does not appear to be an", + " archive file so we cannopt extract all of its contents", + ), + downloaded_file.display(), + )) + } + } + + if self.should_move_up_one_dir()? { + Self::move_contents_up_one_dir(&self.install_root)?; + } else { + debug!("extracted archive did not contain a common top-level directory"); + } + + Ok(()) + } + + fn extract_entire_tarball(&self, downloaded_file: &Path) -> Result<()> { + debug!( + "extracting entire tarball at {}", + downloaded_file.to_string_lossy(), + ); + + let mut arch = tar_reader_for(downloaded_file)?; + + arch.unpack(&self.install_root)?; + + Ok(()) + } + + // We do this because some projects use a top-level dir like `project-x86-64-Linux`, which is + // pretty annoying to work with. In this case, it's a lot easier to install this into + // `~/bin/project` so the directory tree ends up with the same structure on all platforms. + fn should_move_up_one_dir(&self) -> Result { + let mut prefixes: HashSet = HashSet::new(); + for entry in fs::read_dir(&self.install_root).with_context(|| { + format!( + "could not read {} after unpacking the tarball into this directory", + self.install_root.display(), + ) + })? { + let full_path = entry + .context("could not get path for tarball entry")? + .path(); + let path = if let Ok(path) = full_path.strip_prefix(&self.install_root) { + path + } else { + &full_path + }; + if let Some(prefix) = path.components().next() { + prefixes.insert(prefix.as_os_str().to_os_string()); + } else { + return Err(anyhow!("directory entry has no path components")); + } + } + + // If all the entries + Ok(prefixes.len() == 1) + } + + fn move_contents_up_one_dir(path: &Path) -> Result<()> { + let mut entries = fs::read_dir(path)?; + let top_level_path = if let Some(dir_entry) = entries.next() { + let dir_entry = dir_entry?; + dir_entry.path() + } else { + return Err(anyhow!("no directory found in path")); + }; + + debug!( + "moving extracted archive contents up one directory from {} to {}", + top_level_path.display(), + path.display(), + ); + + for entry in fs::read_dir(&top_level_path)? { + let entry = entry?; + let target = path.join(entry.file_name()); + fs::rename(entry.path(), target)?; + } + + fs::remove_dir(top_level_path)?; + + Ok(()) + } + + fn extract_entire_zip(&self, downloaded_file: &Path) -> Result<()> { + debug!( + "extracting entire zip file at {}", + downloaded_file.to_string_lossy(), + ); + + let mut zip = ZipArchive::new(open_file(downloaded_file)?)?; + Ok(zip.extract(&self.install_root)?) + } +} + fn tar_reader_for(downloaded_file: &Path) -> Result>> { let file = open_file(downloaded_file)?; @@ -216,28 +353,26 @@ mod tests { use super::*; #[cfg(target_family = "unix")] use std::os::unix::fs::PermissionsExt; - use std::sync::Once; use tempfile::tempdir; use test_case::test_case; + use test_log::test; + + #[test_case("test-data/project.bz")] + #[test_case("test-data/project.bz2")] + #[test_case("test-data/project.exe")] + #[test_case("test-data/project.gz")] + #[test_case("test-data/project.tar")] + #[test_case("test-data/project.tar.bz")] + #[test_case("test-data/project.tar.bz2")] + #[test_case("test-data/project.tar.gz")] + #[test_case("test-data/project.tar.xz")] + #[test_case("test-data/project.xz")] + #[test_case("test-data/project.zip")] + #[test_case("test-data/project")] + fn exe_installer(archive_path: &str) -> Result<()> { + crate::test_case::init_logging(); - #[test_case("test-data/project.bz", "project")] - #[test_case("test-data/project.bz2", "project")] - #[test_case("test-data/project.exe", "project")] - #[test_case("test-data/project.gz", "project")] - #[test_case("test-data/project.tar", "project")] - #[test_case("test-data/project.tar.bz", "project")] - #[test_case("test-data/project.tar.bz2", "project")] - #[test_case("test-data/project.tar.gz", "project")] - #[test_case("test-data/project.tar.xz", "project")] - #[test_case("test-data/project.xz", "project")] - #[test_case("test-data/project.zip", "project")] - #[test_case("test-data/project", "project")] - fn install(archive_path: &str, exe: &str) -> Result<()> { - static INIT_LOGGING: Once = Once::new(); - INIT_LOGGING.call_once(|| { - use env_logger; - let _ = env_logger::builder().is_test(true).try_init(); - }); + let exe = "project"; let td = tempdir()?; let mut path_without_subdir = td.path().to_path_buf(); @@ -246,7 +381,7 @@ mod tests { path_with_subdir.extend(&["subdir", "project"]); for install_path in [path_without_subdir, path_with_subdir] { - let installer = Installer::new(install_path.clone(), exe.to_string()); + let installer = ExeInstaller::new(install_path.clone(), exe.to_string()); installer.install(&Download { // It doesn't matter what we use here. We're not actually going to // put anything in this temp dir. @@ -262,4 +397,79 @@ mod tests { Ok(()) } + + #[test_case("test-data/project.tar")] + #[test_case("test-data/project.tar.bz")] + #[test_case("test-data/project.tar.bz2")] + #[test_case("test-data/project.tar.gz")] + #[test_case("test-data/project.tar.xz")] + #[test_case("test-data/project.zip")] + fn archive_installer(archive_path: &str) -> Result<()> { + crate::test_case::init_logging(); + + let td = tempdir()?; + let mut path_without_subdir = td.path().to_path_buf(); + path_without_subdir.push("project"); + let mut path_with_subdir = td.path().to_path_buf(); + path_with_subdir.extend(&["subdir", "project"]); + + for install_root in [path_without_subdir, path_with_subdir] { + let installer = ArchiveInstaller::new(install_root.clone()); + installer.install(&Download { + // It doesn't matter what we use here. We're not actually going to + // put anything in this temp dir. + _temp_dir: tempdir()?, + archive_path: PathBuf::from(archive_path), + })?; + + assert!(install_root.exists()); + assert!(install_root.is_dir()); + + let bin_dir = install_root.join("bin"); + assert!(bin_dir.exists()); + assert!(bin_dir.is_dir()); + + let exe = bin_dir.join("project"); + assert!(exe.exists()); + assert!(exe.is_file()); + } + + Ok(()) + } + + #[test] + fn archive_installer_no_root_path() -> Result<()> { + let td = tempdir()?; + let mut path_without_subdir = td.path().to_path_buf(); + path_without_subdir.push("project"); + let mut path_with_subdir = td.path().to_path_buf(); + path_with_subdir.extend(&["subdir", "project"]); + + for install_root in [path_without_subdir, path_with_subdir] { + let installer = ArchiveInstaller::new(install_root.clone()); + installer.install(&Download { + // It doesn't matter what we use here. We're not actually going to + // put anything in this temp dir. + _temp_dir: tempdir()?, + archive_path: PathBuf::from("test-data/no-shared-root.tar.gz"), + })?; + + assert!(install_root.exists()); + assert!(install_root.is_dir()); + + let bin_dir = install_root.join("bin"); + assert!(bin_dir.exists()); + assert!(bin_dir.is_dir()); + + let exe = bin_dir.join("project"); + assert!(exe.exists()); + assert!(exe.is_file()); + + let readme = install_root.join("README.md"); + assert!(readme.exists()); + assert!(readme.is_file()); + } + + Ok(()) + } } diff --git a/ubi/src/lib.rs b/ubi/src/lib.rs index 562a9fc..feb386a 100644 --- a/ubi/src/lib.rs +++ b/ubi/src/lib.rs @@ -121,6 +121,8 @@ mod os; mod picker; #[cfg(test)] mod test; +#[cfg(test)] +mod test_case; mod ubi; pub use crate::{builder::UbiBuilder, forge::ForgeType, ubi::Ubi}; diff --git a/ubi/src/picker.rs b/ubi/src/picker.rs index fe13b5b..7653f87 100644 --- a/ubi/src/picker.rs +++ b/ubi/src/picker.rs @@ -20,37 +20,44 @@ pub(crate) struct AssetPicker<'a> { matching: Option<&'a str>, platform: Platform, is_musl: bool, + archive_only: bool, } impl<'a> AssetPicker<'a> { - pub(crate) fn new(matching: Option<&'a str>, platform: Platform, is_musl: bool) -> Self { + pub(crate) fn new( + matching: Option<&'a str>, + platform: Platform, + is_musl: bool, + archive_only: bool, + ) -> Self { Self { matching, platform, is_musl, + archive_only, } } pub(crate) fn pick_asset(&mut self, assets: Vec) -> Result { - debug!("filtering out assets that do not have a valid extension"); - let mut assets: Vec<_> = assets - .into_iter() - .filter(|a| { - if let Err(e) = Extension::from_path(&a.name) { - debug!("skipping asset with invalid extension, `{}`: {e}", a.name); - return false; - } - true - }) - .collect(); + let all_names = assets.iter().map(|a| &a.name).join(", "); + + let mut assets = self.filter_by_extension(assets); + if assets.is_empty() { + let filter = if self.archive_only { + "for archive files (tarball or zip)" + } else { + "for valid extensions" + }; + return Err(anyhow!( + "could not find a release asset after filtering {filter} from {all_names}", + )); + } if assets.len() == 1 { debug!("there is only one asset to pick"); return Ok(assets.remove(0)); } - let all_names = assets.iter().map(|a| &a.name).join(", "); - let mut matches = self.os_matches(assets); if matches.is_empty() { return Err(anyhow!( @@ -84,6 +91,40 @@ impl<'a> AssetPicker<'a> { Ok(picked) } + fn filter_by_extension(&self, assets: Vec) -> Vec { + debug!("filtering out assets that do not have a valid extension"); + assets + .into_iter() + .filter(|a| match Extension::from_path(&a.name) { + Err(e) => { + debug!("skipping asset with invalid extension: {e}"); + false + } + Ok(Some(ext)) => { + debug!("found valid extension, `{}`", ext.extension()); + if self.archive_only { + if ext.is_archive() { + debug!("including this asset because it is an archive file"); + return true; + } + debug!("not including this asset because it is not an archive file"); + false + } else { + true + } + } + Ok(None) => { + debug!("found asset with no extension, `{}`", a.name); + if self.archive_only { + debug!("not including this asset because it is not an archive file"); + return false; + } + true + } + }) + .collect() + } + fn os_matches(&self, assets: Vec) -> Vec { let os_matcher = self.os_matcher(); debug!("matching assets against OS using {}", os_matcher.as_str()); @@ -502,10 +543,8 @@ mod test { matching: Option<&str>, expect_idx: usize, ) -> Result<()> { - // It'd be nice to use `test_log` but that doesn't work with the test-case crate. See - // https://github.com/frondeus/test-case/pull/143. - // - // init_logger(log::LevelFilter::Debug)?; + crate::test_case::init_logging(); + let platform = Platform::find(platform_name) .ok_or(anyhow!("invalid platform"))? .clone(); @@ -513,6 +552,7 @@ mod test { matching, platform, is_musl: platform_name.contains("musl"), + archive_only: false, }; let url = Url::parse("https://example.com")?; @@ -532,6 +572,68 @@ mod test { #[test_case( "x86_64-unknown-linux-gnu", + &["project-Linux-x86_64.tar.gz", "project-Linux-x86_64.gz"], + None, + 0 ; + "picks tarball from multiple matches - tarball first" + )] + #[test_case( + "x86_64-unknown-linux-gnu", + &["project-Linux-x86_64.gz", "project-Linux-x86_64.tar.gz"], + None, + 1 ; + "picks tarball from multiple matches - tarball last" + )] + #[test_case( + "x86_64-unknown-linux-gnu", + &["project-Linux-x86_64.tar.gz", "project-Linux-x86_64.zip"], + None, + 0 ; + "picks tarball over zip - tarball first" + )] + #[test_case( + "x86_64-unknown-linux-gnu", + &["project-Linux-x86_64.zip", "project-Linux-x86_64.tar.gz"], + None, + 1 ; + "picks tarball over zip - tarball last" + )] + fn pick_asset_archive_only( + platform_name: &str, + asset_names: &[&str], + matching: Option<&str>, + expect_idx: usize, + ) -> Result<()> { + crate::test_case::init_logging(); + + let platform = Platform::find(platform_name) + .ok_or(anyhow!("invalid platform"))? + .clone(); + let mut picker = AssetPicker { + matching, + platform, + is_musl: platform_name.contains("musl"), + archive_only: true, + }; + + let url = Url::parse("https://example.com")?; + let assets = asset_names + .iter() + .map(|name| Asset { + name: (*name).to_string(), + url: url.clone(), + }) + .collect::>(); + + let picked_asset = picker.pick_asset(assets)?; + assert_eq!(picked_asset.name, asset_names[expect_idx]); + + Ok(()) + } + + #[test_case( + "x86_64-unknown-linux-gnu", + false, &["project-macOS-arm64.tar.gz", "project-Windows-i686-gnu.tar.gz"], None, "could not find a release asset for this OS (linux) from" ; @@ -539,6 +641,7 @@ mod test { )] #[test_case( "i686-unknown-linux-gnu", + false, &["project-Linux-x86_64-gnu.tar.gz", "project-Windows-i686-gnu.tar.gz"], None, "could not find a release asset for this OS (linux) and architecture (x86) from" ; @@ -546,21 +649,37 @@ mod test { )] #[test_case( "x86_64-unknown-linux-musl", + false, &["project-Linux-x86_64-gnu.tar.gz", "project-Windows-i686-gnu.tar.gz"], None, "could not find a release asset for this OS (linux), architecture (x86_64), and libc (musl) from" ; "x86_64-unknown-linux-musl - only one Linux asset and it is gnu" )] + #[test_case( + "x86_64-unknown-linux-musl", + false, + &["project-Linux-x86_64-gnu.glorp", "project-Linux-x86-64-gnu.asfasf"], + None, + "could not find a release asset after filtering for valid extensions from" ; + "x86_64-unknown-linux-musl - no valid extensions" + )] + #[test_case( + "x86_64-unknown-linux-musl", + true, + &["project-Linux-x86_64-gnu.gz", "project-Linux-x86-64-gnu.bz", "project-Linux-x86-64-gnu"], + None, + "could not find a release asset after filtering for archive files (tarball or zip) from" ; + "x86_64-unknown-linux-musl - no archive files" + )] fn pick_asset_errors( platform_name: &str, + archive_only: bool, asset_names: &[&str], matching: Option<&str>, expect_err: &str, ) -> Result<()> { - // It'd be nice to use `test_log` but that doesn't work with the test-case crate. See - // https://github.com/frondeus/test-case/pull/143. - // - // init_logger(log::LevelFilter::Debug)?; + crate::test_case::init_logging(); + let platform = Platform::find(platform_name) .ok_or(anyhow!("invalid platform"))? .clone(); @@ -568,6 +687,7 @@ mod test { matching, platform, is_musl: platform_name.contains("musl"), + archive_only, }; let url = Url::parse("https://example.com")?; diff --git a/ubi/src/test_case.rs b/ubi/src/test_case.rs new file mode 100644 index 0000000..797390c --- /dev/null +++ b/ubi/src/test_case.rs @@ -0,0 +1,10 @@ +use std::sync::Once; + +// Once https://github.com/d-e-s-o/test-log/issues/35 is fixed we can remove this code. +pub(crate) fn init_logging() { + static INIT_LOGGING: Once = Once::new(); + INIT_LOGGING.call_once(|| { + use env_logger; + let _ = env_logger::builder().is_test(true).try_init(); + }); +} diff --git a/ubi/src/ubi.rs b/ubi/src/ubi.rs index 474c23c..3eda604 100644 --- a/ubi/src/ubi.rs +++ b/ubi/src/ubi.rs @@ -17,7 +17,7 @@ pub struct Ubi<'a> { forge: Box, asset_url: Option, asset_picker: AssetPicker<'a>, - installer: Installer, + installer: Box, reqwest_client: Client, } @@ -41,7 +41,7 @@ impl<'a> Ubi<'a> { forge: Box, asset_url: Option, asset_picker: AssetPicker<'a>, - installer: Installer, + installer: Box, reqwest_client: Client, ) -> Ubi<'a> { Ubi { diff --git a/ubi/test-data/no-shared-root.tar.gz b/ubi/test-data/no-shared-root.tar.gz new file mode 100644 index 0000000..879dba6 Binary files /dev/null and b/ubi/test-data/no-shared-root.tar.gz differ