Improve GhCrateMeta: Detect cases where pkg-fmt is not specified (#757)

* Fix fmt of mod `<GhCrateMeta as Fetcher>::find`
* Add new variant `BinstallError::InvalidPkgFmt`
* Impl new fn `PkgFmt::guess_pkg_format`
* Improve `GhCrateMeta`: Detect cases where `pkg-fmt` is not specified
  
  but `pkg-url` also does not contain format, archive-format or
  archive-suffix which is required for automatically deducing the pkg-fmt.
  
  In these cases, we would call `PkgFmt::guess_pkg_format` to try out best
  to figure out the pkg-fmt, otherwise we just return an error.

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>

crates/binstalk-types/src/cargo_toml_binstall/package_formats.rs

@@ -57,6 +57,38 @@ impl PkgFmt {
             PkgFmt::Zip => &[".zip"],
         }
     }
+
+    /// Given the pkg-url template, guess the possible pkg-fmt.
+    pub fn guess_pkg_format(pkg_url: &str) -> Option<Self> {
+        let mut it = pkg_url.rsplitn(3, '.');
+
+        let guess = match it.next()? {
+            "tar" => Some(PkgFmt::Tar),
+
+            "tbz2" => Some(PkgFmt::Tbz2),
+            "bz2" if it.next() == Some("tar") => Some(PkgFmt::Tbz2),
+
+            "tgz" => Some(PkgFmt::Tgz),
+            "gz" if it.next() == Some("tar") => Some(PkgFmt::Tgz),
+
+            "txz" => Some(PkgFmt::Txz),
+            "xz" if it.next() == Some("tar") => Some(PkgFmt::Txz),
+
+            "tzstd" | "tzst" => Some(PkgFmt::Tzstd),
+            "zst" if it.next() == Some("tar") => Some(PkgFmt::Tzstd),
+
+            "exe" | "bin" => Some(PkgFmt::Bin),
+            "zip" => Some(PkgFmt::Zip),
+
+            _ => None,
+        };
+
+        if it.next().is_some() {
+            guess
+        } else {
+            None
+        }
+    }
 }
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]

crates/binstalk/src/errors.rs

@@ -41,6 +41,16 @@ pub struct CrateContextError {
     err: BinstallError,
 }
 
+#[derive(Debug, Error)]
+#[error("Invalid pkg-url {pkg_url} for {crate_name}@{version} on {target}: {reason}")]
+pub struct InvalidPkgFmtError {
+    pub crate_name: CompactString,
+    pub version: CompactString,
+    pub target: String,
+    pub pkg_url: String,
+    pub reason: &'static str,
+}
+
 /// Error kinds emitted by cargo-binstall.
 #[derive(Error, Diagnostic, Debug)]
 #[non_exhaustive]
@@ -291,6 +301,14 @@ pub enum BinstallError {
     #[diagnostic(severity(error), code(binstall::no_fallback_to_cargo_install))]
     NoFallbackToCargoInstall,
 
+    /// Fallback to `cargo-install` is disabled.
+    ///
+    /// - Code: `binstall::invalid_pkg_fmt`
+    /// - Exit: 95
+    #[error(transparent)]
+    #[diagnostic(severity(error), code(binstall::invalid_pkg_fmt))]
+    InvalidPkgFmt(Box<InvalidPkgFmtError>),
+
     /// A wrapped error providing the context of which crate the error is about.
     #[error(transparent)]
     #[diagnostic(transparent)]
@@ -324,6 +342,7 @@ impl BinstallError {
             InvalidSourceFilePath { .. } => 91,
             EmptySourceFilePath => 92,
             NoFallbackToCargoInstall => 94,
+            InvalidPkgFmt(..) => 95,
             CrateContext(context) => context.err.exit_number(),
         };
 
@@ -428,3 +447,9 @@ impl From<CargoTomlError> for BinstallError {
         BinstallError::CargoManifest(Box::new(e))
     }
 }
+
+impl From<InvalidPkgFmtError> for BinstallError {
+    fn from(e: InvalidPkgFmtError) -> Self {
+        BinstallError::InvalidPkgFmt(Box::new(e))
+    }
+}

crates/binstalk/src/fetchers/gh_crate_meta.rs

@@ -12,7 +12,7 @@ use tracing::{debug, warn};
 use url::Url;
 
 use crate::{
-    errors::BinstallError,
+    errors::{BinstallError, InvalidPkgFmtError},
     helpers::{
         download::Download,
         remote::{Client, Method},
@@ -99,7 +99,34 @@ impl super::Fetcher for GhCrateMeta {
                 None
             };
 
+            let mut pkg_fmt = self.target_data.meta.pkg_fmt;
+
             let pkg_urls = if let Some(pkg_url) = self.target_data.meta.pkg_url.as_deref() {
+                if pkg_fmt.is_none()
+                    && !(pkg_url.contains("format")
+                        || pkg_url.contains("archive-format")
+                        || pkg_url.contains("archive-suffix"))
+                {
+                    // The crate does not specify the pkg-fmt, yet its pkg-url
+                    // template doesn't contains format, archive-format or
+                    // archive-suffix which is required for automatically
+                    // deducing the pkg-fmt.
+                    //
+                    // We will attempt to guess the pkg-fmt there, but this is
+                    // just a best-effort
+                    pkg_fmt = PkgFmt::guess_pkg_format(pkg_url);
+
+                    if pkg_fmt.is_none() {
+                        return Err(InvalidPkgFmtError {
+                            crate_name: self.data.name.clone(),
+                            version: self.data.version.clone(),
+                            target: self.target_data.target.clone(),
+                            pkg_url: pkg_url.to_string(),
+                            reason: "pkg-fmt is not specified, yet pkg-url does not contain format, archive-format or archive-suffix which is required for automatically deducing pkg-fmt",
+                        }
+                        .into());
+                    }
+                }
                 Either::Left(iter::once(Cow::Borrowed(pkg_url)))
             } else if let Some(repo) = repo.as_ref() {
                 if let Some(pkg_urls) =
@@ -108,23 +135,23 @@ impl super::Fetcher for GhCrateMeta {
                     Either::Right(pkg_urls.map(Cow::Owned))
                 } else {
                     warn!(
-                    concat!(
+                        concat!(
-                        "Unknown repository {}, cargo-binstall cannot provide default pkg_url for it.\n",
+                            "Unknown repository {}, cargo-binstall cannot provide default pkg_url for it.\n",
-                        "Please ask the upstream to provide it for target {}."
+                            "Please ask the upstream to provide it for target {}."
-                    ),
+                        ),
-                    repo, self.target_data.target
+                        repo, self.target_data.target
-                );
+                    );
 
                     return Ok(false);
                 }
             } else {
                 warn!(
-                concat!(
+                    concat!(
-                    "Package does not specify repository, cargo-binstall cannot provide default pkg_url for it.\n",
+                        "Package does not specify repository, cargo-binstall cannot provide default pkg_url for it.\n",
-                    "Please ask the upstream to provide it for target {}."
+                        "Please ask the upstream to provide it for target {}."
-                ),
+                    ),
-                self.target_data.target
+                    self.target_data.target
-            );
+                );
 
                 return Ok(false);
             };
@@ -137,7 +164,7 @@ impl super::Fetcher for GhCrateMeta {
             // launch_baseline_find_tasks which moves `this`
             let this = &self;
 
-            let pkg_fmts = if let Some(pkg_fmt) = self.target_data.meta.pkg_fmt {
+            let pkg_fmts = if let Some(pkg_fmt) = pkg_fmt {
                 Either::Left(iter::once(pkg_fmt))
             } else {
                 Either::Right(PkgFmt::iter())