Auto detect pkg_fmt (#310)

* Refactor: Extract `GhCrateMeta::find_baseline`

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

* Update `Cargo.lock`: Update dep `compact_str`

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

* Fix use of `fetchers`: Set `meta.pkg_fmt` using `fetcher.pkg_fmt()`

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

* Derive `strum_macors::{Display, EnumIter}` for `PkgFmt`

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

* Set typeof field `PkgMeta::pkg_fmt` to be `Option<PkgFmt>`

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

* Optimize `Fetcher::new` to take `&Arc<Data>` instead of `&Data`

To avoid unnecessary `Data::clone` call in `GhCrateMeta`

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

* Optimize `GhCrateMeta::find_baseline`: Avoid unnecessary spawning

for `let Err(_) = url`

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

* Optimize spawning in `GhCrateMeta::find_baseline`

Ret `Option<Url>` instead of `(Url, bool)`

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

* Add new method `target_meta` to trait `Fetcher`

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

* Simplify `resolve_inner` using `Fetcher::target_meta`

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

* Optimize loop in `resolve_inner`: Avoid cloning `PkgOverride`

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

* Impl `PkgMeta::clone_without_overrides`

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

* Optimize `resolve_inner` loop: Use `PkgMeta::clone_without_overrides`

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

* Refactor: Simplify `Context::from_data` impl

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

* Refactor: Extract `launch_baseline_find_tasks`

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

* Refactor: Simplify `<GhCrateMeta as Fetcher>::find`

Instead of launching tasks in an opaque manner in `Self::find_baseline`,
the new design returns an iterator which launches the tasks and thus
have a unified `.await` point for all these tasks.

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

* Add `warn!`ing to report failure of `Context::render_url`

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

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

Cargo.lock

@@ -288,9 +288,9 @@ dependencies = [
 
 [[package]]
 name = "compact_str"
-version = "0.5.2"
+version = "0.6.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5857fc4f27cd0fa2cd7c4a4fa780c0da3d898ae27e66bf6a719343e219e9a559"
+checksum = "bbe793f2e4f8b2b8295ea7dfd141260c0a5c9f6d25c91bde42efed76a47e0bfe"
 dependencies = [
  "castaway",
  "itoa",

crates/lib/src/bins.rs

@@ -42,7 +42,7 @@ impl BinFile {
         // Generate install paths
         // Source path is the download dir + the generated binary path
         let source_file_path = ctx.render(&data.meta.bin_dir)?;
-        let source = if data.meta.pkg_fmt == PkgFmt::Bin {
+        let source = if data.meta.pkg_fmt == Some(PkgFmt::Bin) {
             data.bin_path.clone()
         } else {
             data.bin_path.join(&source_file_path)

crates/lib/src/fetchers.rs

@@ -18,7 +18,7 @@ mod quickinstall;
 #[async_trait::async_trait]
 pub trait Fetcher: Send + Sync {
     /// Create a new fetcher from some data
-    async fn new(client: &Client, data: &Data) -> Arc<Self>
+    async fn new(client: &Client, data: &Arc<Data>) -> Arc<Self>
     where
         Self: Sized;
 
@@ -38,6 +38,9 @@ pub trait Fetcher: Send + Sync {
     /// Return the package format
     fn pkg_fmt(&self) -> PkgFmt;
 
+    /// Return finalized target meta.
+    fn target_meta(&self) -> PkgMeta;
+
     /// A short human-readable name or descriptor for the package source
     fn source_name(&self) -> CompactString;
 

crates/lib/src/fetchers/gh_crate_meta.rs

@@ -5,66 +5,81 @@ use log::{debug, warn};
 use once_cell::sync::OnceCell;
 use reqwest::{Client, Method};
 use serde::Serialize;
+use strum::IntoEnumIterator;
 use tinytemplate::TinyTemplate;
 use url::Url;
 
 use crate::{
     errors::BinstallError,
     helpers::{download::download_and_extract, remote::remote_exists, tasks::AutoAbortJoinHandle},
-    manifests::cargo_toml_binstall::PkgFmt,
+    manifests::cargo_toml_binstall::{PkgFmt, PkgMeta},
 };
 
 use super::Data;
 
 pub struct GhCrateMeta {
     client: Client,
-    data: Data,
+    data: Arc<Data>,
-    url: OnceCell<Url>,
+    resolution: OnceCell<(Url, PkgFmt)>,
+}
+
+type BaselineFindTask = AutoAbortJoinHandle<Result<Option<(Url, PkgFmt)>, BinstallError>>;
+
+impl GhCrateMeta {
+    fn launch_baseline_find_tasks(
+        &self,
+        pkg_fmt: PkgFmt,
+    ) -> impl Iterator<Item = BaselineFindTask> + '_ {
+        // build up list of potential URLs
+        let urls = pkg_fmt.extensions().iter().filter_map(|ext| {
+            let ctx = Context::from_data(&self.data, ext);
+            match ctx.render_url(&self.data.meta.pkg_url) {
+                Ok(url) => Some(url),
+                Err(err) => {
+                    warn!("Failed to render url for {ctx:#?}: {err:#?}");
+                    None
+                }
+            }
+        });
+
+        // go check all potential URLs at once
+        urls.map(move |url| {
+            let client = self.client.clone();
+
+            AutoAbortJoinHandle::spawn(async move {
+                debug!("Checking for package at: '{url}'");
+
+                remote_exists(client, url.clone(), Method::HEAD)
+                    .await
+                    .map(|exists| exists.then_some((url, pkg_fmt)))
+            })
+        })
+    }
 }
 
 #[async_trait::async_trait]
 impl super::Fetcher for GhCrateMeta {
-    async fn new(client: &Client, data: &Data) -> Arc<Self> {
+    async fn new(client: &Client, data: &Arc<Data>) -> Arc<Self> {
         Arc::new(Self {
             client: client.clone(),
             data: data.clone(),
-            url: OnceCell::new(),
+            resolution: OnceCell::new(),
         })
     }
 
     async fn find(&self) -> Result<bool, BinstallError> {
-        // build up list of potential URLs
+        let handles: Vec<_> = if let Some(pkg_fmt) = self.data.meta.pkg_fmt {
-        let urls = self.data.meta.pkg_fmt.extensions().iter().map(|ext| {
+            self.launch_baseline_find_tasks(pkg_fmt).collect()
-            let ctx = Context::from_data(&self.data, ext);
+        } else {
-            ctx.render_url(&self.data.meta.pkg_url)
+            PkgFmt::iter()
-        });
+                .flat_map(|pkg_fmt| self.launch_baseline_find_tasks(pkg_fmt))
+                .collect()
+        };
 
-        // go check all potential URLs at once
+        for handle in handles {
-        let checks = urls
+            if let Some((url, pkg_fmt)) = handle.await?? {
-            .map(|url| {
+                debug!("Winning URL is {url}, with pkg_fmt {pkg_fmt}");
-                let client = self.client.clone();
+                self.resolution.set((url, pkg_fmt)).unwrap(); // find() is called first
-                AutoAbortJoinHandle::spawn(async move {
-                    let url = url?;
-                    debug!("Checking for package at: '{url}'");
-                    remote_exists(client, url.clone(), Method::HEAD)
-                        .await
-                        .map(|exists| (url.clone(), exists))
-                })
-            })
-            .collect::<Vec<_>>();
-
-        // get the first URL that exists
-        for check in checks {
-            let (url, exists) = check.await??;
-            if exists {
-                if url.scheme() != "https" {
-                    warn!(
-                        "URL is not HTTPS! This may become a hard error in the future, tell the upstream!"
-                    );
-                }
-
-                debug!("Winning URL is {url}");
-                self.url.set(url).unwrap(); // find() is called first
                 return Ok(true);
             }
         }
@@ -73,19 +88,25 @@ impl super::Fetcher for GhCrateMeta {
     }
 
     async fn fetch_and_extract(&self, dst: &Path) -> Result<(), BinstallError> {
-        let url = self.url.get().unwrap(); // find() is called first
+        let (url, pkg_fmt) = self.resolution.get().unwrap(); // find() is called first
         debug!("Downloading package from: '{url}'");
-        download_and_extract(&self.client, url, self.pkg_fmt(), dst).await
+        download_and_extract(&self.client, url, *pkg_fmt, dst).await
     }
 
     fn pkg_fmt(&self) -> PkgFmt {
-        self.data.meta.pkg_fmt
+        self.resolution.get().unwrap().1
+    }
+
+    fn target_meta(&self) -> PkgMeta {
+        let mut meta = self.data.meta.clone();
+        meta.pkg_fmt = Some(self.pkg_fmt());
+        meta
     }
 
     fn source_name(&self) -> CompactString {
-        self.url
+        self.resolution
             .get()
-            .map(|url| {
+            .map(|(url, _pkg_fmt)| {
                 if let Some(domain) = url.domain() {
                     domain.to_compact_string()
                 } else if let Some(host) = url.host_str() {
@@ -130,7 +151,7 @@ impl<'c> Context<'c> {
     pub(self) fn from_data(data: &'c Data, archive_format: &'c str) -> Self {
         Self {
             name: &data.name,
-            repo: data.repo.as_ref().map(|s| &s[..]),
+            repo: data.repo.as_deref(),
             target: &data.target,
             version: &data.version,
             format: archive_format,
@@ -271,7 +292,7 @@ mod test {
             pkg_url:
                 "{ repo }/releases/download/v{ version }/{ name }-v{ version }-{ target }.tar.xz"
                     .into(),
-            pkg_fmt: PkgFmt::Txz,
+            pkg_fmt: Some(PkgFmt::Txz),
             ..Default::default()
         };
 
@@ -294,7 +315,7 @@ mod test {
     fn no_archive() {
         let meta = PkgMeta {
             pkg_url: "{ repo }/releases/download/v{ version }/{ name }-v{ version }-{ target }{ binary-ext }".into(),
-            pkg_fmt: PkgFmt::Bin,
+            pkg_fmt: Some(PkgFmt::Bin),
             ..Default::default()
         };
 

crates/lib/src/fetchers/quickinstall.rs

@@ -10,7 +10,7 @@ use url::Url;
 use crate::{
     errors::BinstallError,
     helpers::{download::download_and_extract, remote::remote_exists},
-    manifests::cargo_toml_binstall::PkgFmt,
+    manifests::cargo_toml_binstall::{PkgFmt, PkgMeta},
 };
 
 use super::Data;
@@ -22,11 +22,12 @@ pub struct QuickInstall {
     client: Client,
     package: String,
     target: String,
+    data: Arc<Data>,
 }
 
 #[async_trait::async_trait]
 impl super::Fetcher for QuickInstall {
-    async fn new(client: &Client, data: &Data) -> Arc<Self> {
+    async fn new(client: &Client, data: &Arc<Data>) -> Arc<Self> {
         let crate_name = &data.name;
         let version = &data.version;
         let target = data.target.clone();
@@ -34,6 +35,7 @@ impl super::Fetcher for QuickInstall {
             client: client.clone(),
             package: format!("{crate_name}-{version}-{target}"),
             target,
+            data: data.clone(),
         })
     }
 
@@ -54,6 +56,12 @@ impl super::Fetcher for QuickInstall {
         PkgFmt::Tgz
     }
 
+    fn target_meta(&self) -> PkgMeta {
+        let mut meta = self.data.meta.clone();
+        meta.pkg_fmt = Some(self.pkg_fmt());
+        meta
+    }
+
     fn source_name(&self) -> CompactString {
         CompactString::from("QuickInstall")
     }

crates/lib/src/manifests/cargo_toml_binstall.rs

@@ -37,7 +37,7 @@ pub struct PkgMeta {
     pub pkg_url: String,
 
     /// Format for package downloads
-    pub pkg_fmt: PkgFmt,
+    pub pkg_fmt: Option<PkgFmt>,
 
     /// Path template for binary files in packages
     pub bin_dir: String,
@@ -53,7 +53,7 @@ impl Default for PkgMeta {
     fn default() -> Self {
         Self {
             pkg_url: DEFAULT_PKG_URL.to_string(),
-            pkg_fmt: PkgFmt::default(),
+            pkg_fmt: None,
             bin_dir: DEFAULT_BIN_DIR.to_string(),
             pub_key: None,
             overrides: HashMap::new(),
@@ -62,13 +62,23 @@ impl Default for PkgMeta {
 }
 
 impl PkgMeta {
+    pub fn clone_without_overrides(&self) -> Self {
+        Self {
+            pkg_url: self.pkg_url.clone(),
+            pkg_fmt: self.pkg_fmt,
+            bin_dir: self.bin_dir.clone(),
+            pub_key: self.pub_key.clone(),
+            overrides: HashMap::new(),
+        }
+    }
+
     /// Merge configuration overrides into object
     pub fn merge(&mut self, pkg_override: &PkgOverride) {
         if let Some(o) = &pkg_override.pkg_url {
             self.pkg_url = o.clone();
         }
         if let Some(o) = &pkg_override.pkg_fmt {
-            self.pkg_fmt = *o;
+            self.pkg_fmt = Some(*o);
         }
         if let Some(o) = &pkg_override.bin_dir {
             self.bin_dir = o.clone();

crates/lib/src/manifests/cargo_toml_binstall/package_formats.rs

@@ -1,8 +1,10 @@
 use serde::{Deserialize, Serialize};
-use strum_macros::{Display, EnumString};
+use strum_macros::{Display, EnumIter, EnumString};
 
 /// Binary format enumeration
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Serialize, Deserialize, EnumString)]
+#[derive(
+    Debug, Display, Copy, Clone, Eq, PartialEq, Serialize, Deserialize, EnumString, EnumIter,
+)]
 #[serde(rename_all = "snake_case")]
 pub enum PkgFmt {
     /// Download format is TAR (uncompressed)

crates/lib/src/ops/resolve.rs

@@ -160,7 +160,7 @@ async fn resolve_inner(
         }
     }
 
-    let (mut meta, binaries) = (
+    let (meta, binaries) = (
         package
             .metadata
             .as_ref()
@@ -175,23 +175,23 @@ async fn resolve_inner(
 
     for target in desired_targets {
         debug!("Building metadata for target: {target}");
-        let mut target_meta = meta.clone();
+        let mut target_meta = meta.clone_without_overrides();
 
         // Merge any overrides
-        if let Some(o) = target_meta.overrides.get(target).cloned() {
+        if let Some(o) = meta.overrides.get(target) {
-            target_meta.merge(&o);
+            target_meta.merge(o);
         }
 
         target_meta.merge(&opts.cli_overrides);
         debug!("Found metadata: {target_meta:?}");
 
-        let fetcher_data = Data {
+        let fetcher_data = Arc::new(Data {
             name: package.name.clone(),
             target: target.clone(),
             version: package.version.clone(),
             repo: package.repository.clone(),
             meta: target_meta,
-        };
+        });
 
         fetchers.add(GhCrateMeta::new(&client, &fetcher_data).await);
         fetchers.add(QuickInstall::new(&client, &fetcher_data).await);
@@ -200,11 +200,7 @@ async fn resolve_inner(
     let resolution = match fetchers.first_available().await {
         Some(fetcher) => {
             // Build final metadata
-            let fetcher_target = fetcher.target();
+            let meta = fetcher.target_meta();
-            if let Some(o) = meta.overrides.get(&fetcher_target.to_owned()).cloned() {
-                meta.merge(&o);
-            }
-            meta.merge(&opts.cli_overrides);
 
             // Generate temporary binary path
             let bin_path = temp_dir.join(format!("bin-{}", crate_name.name));