From 391218f34bc16c3057a9d53b6d9f9bd1544a4bef Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Mon, 15 Jul 2024 16:17:55 +0200 Subject: [PATCH 1/4] Refactor file extension splitting utility to be more strict --- lib/sources/{artifact.rs => artifact/mod.rs} | 7 +- lib/sources/artifact/util.rs | 88 ++++++++++++++++++++ lib/util/path.rs | 29 ------- 3 files changed, 93 insertions(+), 31 deletions(-) rename lib/sources/{artifact.rs => artifact/mod.rs} (99%) create mode 100644 lib/sources/artifact/util.rs diff --git a/lib/sources/artifact.rs b/lib/sources/artifact/mod.rs similarity index 99% rename from lib/sources/artifact.rs rename to lib/sources/artifact/mod.rs index caeb4bd..a781a2c 100644 --- a/lib/sources/artifact.rs +++ b/lib/sources/artifact/mod.rs @@ -7,7 +7,6 @@ use crate::{ descriptor::{Descriptor, OS}, result::RokitResult, tool::ToolSpec, - util::path::split_filename_and_extensions, }; use super::{ @@ -17,6 +16,10 @@ use super::{ ExtractError, }; +mod util; + +use self::util::split_filename_and_extensions; + /** An artifact provider supported by Rokit. @@ -111,7 +114,7 @@ impl FromStr for ArtifactFormat { match l.as_str() { "zip" => Ok(Self::Zip), "tar" => Ok(Self::Tar), - "tar.gz" => Ok(Self::TarGz), + "tar.gz" | "tgz" => Ok(Self::TarGz), _ => Err(format!("unknown artifact format '{l}'")), } } diff --git a/lib/sources/artifact/util.rs b/lib/sources/artifact/util.rs new file mode 100644 index 0000000..591922a --- /dev/null +++ b/lib/sources/artifact/util.rs @@ -0,0 +1,88 @@ +use std::path::Path; + +const ALLOWED_EXTENSION_NAMES: [&str; 4] = ["zip", "tar", "gz", "tgz"]; +const ALLOWED_EXTENSION_COUNT: usize = 2; + +pub(super) 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() { + let ext = ext.to_str().expect("input was str"); + let stem = path.file_stem().expect("had an extension"); + + if !ALLOWED_EXTENSION_NAMES + .iter() + .any(|e| e.eq_ignore_ascii_case(ext)) + { + break; + } + + exts.push(ext); + path = Path::new(stem); + + if exts.len() >= ALLOWED_EXTENSION_COUNT { + break; + } + } + + exts.reverse(); + + let path = path.to_str().expect("input was str"); + (path, exts) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn split_filename_ext_basic() { + assert_eq!( + split_filename_and_extensions("file.zip"), + ("file", vec!["zip"]) + ); + assert_eq!( + split_filename_and_extensions("file.tar"), + ("file", vec!["tar"]) + ); + assert_eq!( + split_filename_and_extensions("file.tar.gz"), + ("file", vec!["tar", "gz"]) + ); + assert_eq!( + split_filename_and_extensions("file.with.many.extensions.tar.gz.zip"), + ("file.with.many.extensions.tar", vec!["gz", "zip"]) + ); + assert_eq!( + split_filename_and_extensions("file.with.many.extensions.zip.gz.tar"), + ("file.with.many.extensions.zip", vec!["gz", "tar"]) + ); + assert_eq!( + split_filename_and_extensions("file.with.many.extensions.tar.gz"), + ("file.with.many.extensions", vec!["tar", "gz"]) + ); + } + + #[test] + fn split_filename_ext_real_tools() { + assert_eq!( + split_filename_and_extensions("wally-v0.3.2-linux.zip"), + ("wally-v0.3.2-linux", vec!["zip"]) + ); + assert_eq!( + split_filename_and_extensions("lune-0.8.6-macos-aarch64.zip"), + ("lune-0.8.6-macos-aarch64", vec!["zip"]) + ); + assert_eq!( + split_filename_and_extensions("just-1.31.0-aarch64-apple-darwin.tar.gz"), + ("just-1.31.0-aarch64-apple-darwin", vec!["tar", "gz"]) + ); + assert_eq!( + split_filename_and_extensions("sentry-cli-linux-i686-2.32.1.tgz"), + ("sentry-cli-linux-i686-2.32.1", vec!["tgz"]) + ); + } +} diff --git a/lib/util/path.rs b/lib/util/path.rs index beba3a2..5847da3 100644 --- a/lib/util/path.rs +++ b/lib/util/path.rs @@ -2,35 +2,6 @@ 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 435fb9b0f25f9edfc058c782f565b57dbb801a7b Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Mon, 15 Jul 2024 16:19:03 +0200 Subject: [PATCH 2/4] Move provider enum into its own module --- lib/sources/artifact/mod.rs | 45 ++------------------------------ lib/sources/artifact/provider.rs | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 43 deletions(-) create mode 100644 lib/sources/artifact/provider.rs diff --git a/lib/sources/artifact/mod.rs b/lib/sources/artifact/mod.rs index a781a2c..16ba3f6 100644 --- a/lib/sources/artifact/mod.rs +++ b/lib/sources/artifact/mod.rs @@ -16,53 +16,12 @@ use super::{ ExtractError, }; +mod provider; mod util; use self::util::split_filename_and_extensions; -/** - An artifact provider supported by Rokit. - - The default provider is [`ArtifactProvider::GitHub`]. -*/ -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] -pub enum ArtifactProvider { - #[default] - GitHub, -} - -impl ArtifactProvider { - #[must_use] - pub fn as_str(self) -> &'static str { - match self { - Self::GitHub => "github", - } - } - - #[must_use] - pub fn display_name(self) -> &'static str { - match self { - Self::GitHub => "GitHub", - } - } -} - -impl FromStr for ArtifactProvider { - type Err = String; - fn from_str(s: &str) -> Result { - let l = s.trim().to_lowercase(); - match l.as_str() { - "github" => Ok(Self::GitHub), - _ => Err(format!("unknown artifact provider '{l}'")), - } - } -} - -impl fmt::Display for ArtifactProvider { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.as_str().fmt(f) - } -} +pub use self::provider::ArtifactProvider; /** An artifact format supported by Rokit. diff --git a/lib/sources/artifact/provider.rs b/lib/sources/artifact/provider.rs new file mode 100644 index 0000000..b39fe0f --- /dev/null +++ b/lib/sources/artifact/provider.rs @@ -0,0 +1,45 @@ +use std::{fmt, str::FromStr}; + +/** + An artifact provider supported by Rokit. + + The default provider is [`ArtifactProvider::GitHub`]. +*/ +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ArtifactProvider { + #[default] + GitHub, +} + +impl ArtifactProvider { + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::GitHub => "github", + } + } + + #[must_use] + pub fn display_name(self) -> &'static str { + match self { + Self::GitHub => "GitHub", + } + } +} + +impl FromStr for ArtifactProvider { + type Err = String; + fn from_str(s: &str) -> Result { + let l = s.trim().to_lowercase(); + match l.as_str() { + "github" => Ok(Self::GitHub), + _ => Err(format!("unknown artifact provider '{l}'")), + } + } +} + +impl fmt::Display for ArtifactProvider { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_str().fmt(f) + } +} From 347b55cdfea60b0b5def52232adcfa0629a415ca Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Mon, 15 Jul 2024 16:21:12 +0200 Subject: [PATCH 3/4] Move format enum into its own module --- lib/sources/artifact/format.rs | 134 ++++++++++++++++++++++++++++++++ lib/sources/artifact/mod.rs | 135 +-------------------------------- 2 files changed, 136 insertions(+), 133 deletions(-) create mode 100644 lib/sources/artifact/format.rs diff --git a/lib/sources/artifact/format.rs b/lib/sources/artifact/format.rs new file mode 100644 index 0000000..2c7b789 --- /dev/null +++ b/lib/sources/artifact/format.rs @@ -0,0 +1,134 @@ +use std::{fmt, str::FromStr}; + +use super::util::split_filename_and_extensions; + +/** + An artifact format supported by Rokit. +*/ +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ArtifactFormat { + Zip, + Tar, + TarGz, +} + +impl ArtifactFormat { + #[must_use] + pub fn as_str(&self) -> &'static str { + match self { + Self::Zip => "zip", + Self::Tar => "tar", + Self::TarGz => "tar.gz", + } + } + + #[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 { + type Err = String; + fn from_str(s: &str) -> Result { + let l = s.trim().to_lowercase(); + match l.as_str() { + "zip" => Ok(Self::Zip), + "tar" => Ok(Self::Tar), + "tar.gz" | "tgz" => Ok(Self::TarGz), + _ => Err(format!("unknown artifact format '{l}'")), + } + } +} + +impl fmt::Display for ArtifactFormat { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_str().fmt(f) + } +} + +#[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/sources/artifact/mod.rs b/lib/sources/artifact/mod.rs index 16ba3f6..0bbd0b6 100644 --- a/lib/sources/artifact/mod.rs +++ b/lib/sources/artifact/mod.rs @@ -1,5 +1,3 @@ -use std::{fmt, str::FromStr}; - use tracing::instrument; use url::Url; @@ -16,75 +14,15 @@ use super::{ ExtractError, }; +mod format; mod provider; mod util; use self::util::split_filename_and_extensions; +pub use self::format::ArtifactFormat; pub use self::provider::ArtifactProvider; -/** - An artifact format supported by Rokit. -*/ -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub enum ArtifactFormat { - Zip, - Tar, - TarGz, -} - -impl ArtifactFormat { - #[must_use] - pub fn as_str(&self) -> &'static str { - match self { - Self::Zip => "zip", - Self::Tar => "tar", - Self::TarGz => "tar.gz", - } - } - - #[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 { - type Err = String; - fn from_str(s: &str) -> Result { - let l = s.trim().to_lowercase(); - match l.as_str() { - "zip" => Ok(Self::Zip), - "tar" => Ok(Self::Tar), - "tar.gz" | "tgz" => Ok(Self::TarGz), - _ => Err(format!("unknown artifact format '{l}'")), - } - } -} - -impl fmt::Display for ArtifactFormat { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.as_str().fmt(f) - } -} - /** An artifact found by Rokit, to be downloaded and installed. */ @@ -246,72 +184,3 @@ 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)); - } -} From 20cc8fa4c0fb9c43fb48f887bcdd11e339c37bb3 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Mon, 15 Jul 2024 16:39:20 +0200 Subject: [PATCH 4/4] Update changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6421c0..06d0687 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- Fixed artifact names with versions in them suchas `lune-0.8.6-linux-x86_64.zip` no longer installing correctly in Rokit `0.1.6` ([#40]) + +[#40]: https://github.com/rojo-rbx/rokit/pull/40 + ## `0.1.6` - July 15th, 2024 ### Fixed