From 3a4106d414afb9595233beaa09fc14f758b88944 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Mon, 15 Jul 2024 02:02:05 +0200 Subject: [PATCH 1/2] Fix artifact matching being sensitive to extensions, add tests --- lib/sources/artifact.rs | 109 +++++++++++++++++++++++++++++++++------- lib/util/path.rs | 29 +++++++++++ 2 files changed, 120 insertions(+), 18 deletions(-) diff --git a/lib/sources/artifact.rs b/lib/sources/artifact.rs index e6c401f..caeb4bd 100644 --- a/lib/sources/artifact.rs +++ b/lib/sources/artifact.rs @@ -1,4 +1,4 @@ -use std::{fmt, path::Path, str::FromStr}; +use std::{fmt, str::FromStr}; use tracing::instrument; use url::Url; @@ -7,6 +7,7 @@ use crate::{ descriptor::{Descriptor, OS}, result::RokitResult, tool::ToolSpec, + util::path::split_filename_and_extensions, }; use super::{ @@ -80,25 +81,27 @@ impl ArtifactFormat { } } - pub fn from_path_or_url(path_or_url: impl AsRef) -> Option { - let lowercased = path_or_url.as_ref().trim().to_lowercase(); - let extension = Path::new(&lowercased).extension()?.to_str()?; - match extension { - ext if ext.eq_ignore_ascii_case("zip") => Some(Self::Zip), - ext if ext.eq_ignore_ascii_case("tar") => Some(Self::Tar), - ext if ext.eq_ignore_ascii_case("tgz") => Some(Self::TarGz), - ext if ext.eq_ignore_ascii_case("gz") => { - let stem = Path::new(&lowercased).file_stem()?; - let ext2 = Path::new(stem).extension()?; - if ext2.eq_ignore_ascii_case("tar") { - Some(Self::TarGz) - } else { - None - } + #[must_use] + pub fn from_extensions<'a>(extensions: impl AsRef<[&'a str]>) -> Option { + match extensions.as_ref() { + [.., ext] if ext.eq_ignore_ascii_case("zip") => Some(Self::Zip), + [.., ext] if ext.eq_ignore_ascii_case("tar") => Some(Self::Tar), + [.., ext] if ext.eq_ignore_ascii_case("tgz") => Some(Self::TarGz), + [.., ext1, ext2] + if ext1.eq_ignore_ascii_case("tar") && ext2.eq_ignore_ascii_case("gz") => + { + Some(Self::TarGz) } _ => None, } } + + #[must_use] + pub fn from_path_or_url(path_or_url: impl AsRef) -> Option { + let path_or_url = path_or_url.as_ref(); + let (_, extensions) = split_filename_and_extensions(path_or_url); + Self::from_extensions(extensions) + } } impl FromStr for ArtifactFormat { @@ -135,13 +138,14 @@ pub struct Artifact { impl Artifact { pub(crate) fn from_github_release_asset(asset: &Asset, spec: &ToolSpec) -> Self { - let format = ArtifactFormat::from_path_or_url(&asset.name); + let (name, extensions) = split_filename_and_extensions(&asset.name); + let format = ArtifactFormat::from_extensions(extensions); Self { provider: ArtifactProvider::GitHub, format, id: Some(asset.id.to_string()), url: Some(asset.url.clone()), - name: Some(asset.name.clone()), + name: Some(name.to_string()), tool_spec: spec.clone(), } } @@ -280,3 +284,72 @@ impl Artifact { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn format_from_str(s: &str) -> Option { + let (_, extensions) = split_filename_and_extensions(s); + ArtifactFormat::from_extensions(extensions) + } + + #[test] + fn format_from_extensions_valid() { + assert_eq!(format_from_str("file.zip"), Some(ArtifactFormat::Zip)); + assert_eq!(format_from_str("file.tar"), Some(ArtifactFormat::Tar)); + assert_eq!(format_from_str("file.tar.gz"), Some(ArtifactFormat::TarGz)); + assert_eq!( + format_from_str("file.with.many.extensions.tar.gz.zip"), + Some(ArtifactFormat::Zip) + ); + assert_eq!( + format_from_str("file.with.many.extensions.zip.gz.tar"), + Some(ArtifactFormat::Tar) + ); + assert_eq!( + format_from_str("file.with.many.extensions.tar.gz"), + Some(ArtifactFormat::TarGz) + ); + } + + #[test] + fn format_from_extensions_invalid() { + assert_eq!(format_from_str("file-name"), None); + assert_eq!(format_from_str("some/file.exe"), None); + assert_eq!(format_from_str("really.long.file.name"), None); + } + + #[test] + fn format_from_real_tools() { + assert_eq!( + format_from_str("wally-v0.3.2-linux.zip"), + Some(ArtifactFormat::Zip) + ); + assert_eq!( + format_from_str("lune-0.8.6-macos-aarch64.zip"), + Some(ArtifactFormat::Zip) + ); + assert_eq!( + format_from_str("just-1.31.0-aarch64-apple-darwin.tar.gz"), + Some(ArtifactFormat::TarGz) + ); + assert_eq!( + format_from_str("sentry-cli-linux-i686-2.32.1.tgz"), + Some(ArtifactFormat::TarGz) + ); + } + + #[test] + fn format_case_sensitivity() { + assert_eq!(format_from_str("file.ZIP"), Some(ArtifactFormat::Zip)); + assert_eq!(format_from_str("file.zip"), Some(ArtifactFormat::Zip)); + assert_eq!(format_from_str("file.Zip"), Some(ArtifactFormat::Zip)); + assert_eq!(format_from_str("file.tar"), Some(ArtifactFormat::Tar)); + assert_eq!(format_from_str("file.TAR"), Some(ArtifactFormat::Tar)); + assert_eq!(format_from_str("file.Tar"), Some(ArtifactFormat::Tar)); + assert_eq!(format_from_str("file.tar.gz"), Some(ArtifactFormat::TarGz)); + assert_eq!(format_from_str("file.TAR.GZ"), Some(ArtifactFormat::TarGz)); + assert_eq!(format_from_str("file.Tar.Gz"), Some(ArtifactFormat::TarGz)); + } +} diff --git a/lib/util/path.rs b/lib/util/path.rs index 5847da3..beba3a2 100644 --- a/lib/util/path.rs +++ b/lib/util/path.rs @@ -2,6 +2,35 @@ use std::path::{Path, PathBuf}; +/** + Splits a filename into its base name and a list of extensions. + + This is useful for handling files with multiple extensions, such as `file-name.ext1.ext2`. + + # Example + + ```rust ignore + let (name, exts) = split_filename_and_extensions("file-name.ext1.ext2"); + assert_eq!(name, "file-name"); + assert_eq!(exts, vec!["ext1", "ext2"]); + ``` +*/ +pub(crate) fn split_filename_and_extensions(name: &str) -> (&str, Vec<&str>) { + let mut path = Path::new(name); + let mut exts = Vec::new(); + + // Reverse-pop extensions off the path until we reach the + // base name - we will then need to reverse afterwards, too + while let Some(ext) = path.extension() { + exts.push(ext.to_str().expect("input was str")); + path = Path::new(path.file_stem().expect("had an extension")); + } + exts.reverse(); + + let path = path.to_str().expect("input was str"); + (path, exts) +} + /** Cleans up a path and simplifies it for writing to storage or environment variables. From 9f50422714ebe169081ea432453f09b3c6e41a89 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Mon, 15 Jul 2024 02:11:54 +0200 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60b85f4..c90f805 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed artifacts with names such as `toolname-win64.zip` not being detected as compatible on Windows ([#39]) + +[#39]: https://github.com/rojo-rbx/rokit/pull/39 + +## `0.1.5` - July 14th, 2024 + +### Fixed + - Fixed tool specifications failing to parse in `foreman.toml` when using inline tables ([#36]) - Fixed tools not specifying architectures (such as `wally-macos.zip`) failing to install ([#38])