Fix artifact matching being sensitive to extensions, add tests

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])
 

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<str>) -> Option<Self> {
-        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<Self> {
+        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<str>) -> Option<Self> {
+        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<ArtifactFormat> {
+        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));
+    }
+}

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.