Merge pull request #221 from NobodyXu/optimize

Minor Optimization

src/binstall.rs

@@ -1,5 +1,7 @@
 use std::path::PathBuf;
 
+use crate::{DesiredTargets, PkgOverride};
+
 mod resolve;
 pub use resolve::*;
 
@@ -11,4 +13,6 @@ pub struct Options {
     pub dry_run: bool,
     pub version: Option<String>,
     pub manifest_path: Option<PathBuf>,
+    pub cli_overrides: PkgOverride,
+    pub desired_targets: DesiredTargets,
 }

src/binstall/install.rs

@@ -11,7 +11,6 @@ use crate::{bins, fetchers::Fetcher, *};
 pub async fn install(
     resolution: Resolution,
     opts: Arc<Options>,
-    desired_targets: DesiredTargets,
     jobserver_client: LazyJobserverClient,
 ) -> Result<()> {
     match resolution {
@@ -32,7 +31,7 @@ pub async fn install(
             install_from_package(fetcher, opts, cvs, version, bin_path, bin_files).await
         }
         Resolution::InstallFromSource { package } => {
-            let desired_targets = desired_targets.get().await;
+            let desired_targets = opts.desired_targets.get().await;
             let target = desired_targets
                 .first()
                 .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?;

src/binstall/resolve.rs

@@ -75,11 +75,10 @@ impl Resolution {
 pub async fn resolve(
     opts: Arc<Options>,
     crate_name: CrateName,
-    desired_targets: DesiredTargets,
-    cli_overrides: Arc<PkgOverride>,
     temp_dir: Arc<Path>,
     install_path: Arc<Path>,
     client: Client,
+    crates_io_api_client: crates_io_api::AsyncClient,
 ) -> Result<Resolution> {
     info!("Installing package: '{}'", crate_name);
 
@@ -90,6 +89,7 @@ pub async fn resolve(
         (None, None) => "*".to_string(),
     };
 
+    // Treat 0.1.2 as =0.1.2
     if version
         .chars()
         .next()
@@ -104,7 +104,9 @@ pub async fn resolve(
     // TODO: support git-based fetches (whole repo name rather than just crate name)
     let manifest = match opts.manifest_path.clone() {
         Some(manifest_path) => load_manifest_path(manifest_path.join("Cargo.toml"))?,
-        None => fetch_crate_cratesio(&client, &crate_name.name, &version).await?,
+        None => {
+            fetch_crate_cratesio(&client, &crates_io_api_client, &crate_name.name, &version).await?
+        }
     };
 
     let package = manifest.package.unwrap();
@@ -120,7 +122,7 @@ pub async fn resolve(
 
     let mut fetchers = MultiFetcher::default();
 
-    let desired_targets = desired_targets.get().await;
+    let desired_targets = opts.desired_targets.get().await;
 
     for target in desired_targets {
         debug!("Building metadata for target: {target}");
@@ -131,7 +133,7 @@ pub async fn resolve(
             target_meta.merge(&o);
         }
 
-        target_meta.merge(&cli_overrides);
+        target_meta.merge(&opts.cli_overrides);
         debug!("Found metadata: {target_meta:?}");
 
         let fetcher_data = Data {
@@ -153,7 +155,7 @@ pub async fn resolve(
             if let Some(o) = meta.overrides.get(&fetcher_target.to_owned()).cloned() {
                 meta.merge(&o);
             }
-            meta.merge(&cli_overrides);
+            meta.merge(&opts.cli_overrides);
 
             // Generate temporary binary path
             let bin_path = temp_dir.join(format!("bin-{}", crate_name.name));

src/drivers/crates_io.rs

@@ -1,5 +1,4 @@
 use std::path::PathBuf;
-use std::time::Duration;
 
 use cargo_toml::Manifest;
 use crates_io_api::AsyncClient;
@@ -18,46 +17,25 @@ use visitor::ManifestVisitor;
 /// Fetch a crate Cargo.toml by name and version from crates.io
 pub async fn fetch_crate_cratesio(
     client: &Client,
+    crates_io_api_client: &AsyncClient,
     name: &str,
     version_req: &str,
 ) -> Result<Manifest<Meta>, BinstallError> {
     // Fetch / update index
     debug!("Looking up crate information");
 
-    // Build crates.io api client
-    let api_client = AsyncClient::new(
-        "cargo-binstall (https://github.com/ryankurte/cargo-binstall)",
-        Duration::from_millis(100),
-    )
-    .expect("bug: invalid user agent");
-
     // Fetch online crate information
-    let base_info =
+    let base_info = crates_io_api_client
-        api_client
+        .get_crate(name.as_ref())
-            .get_crate(name.as_ref())
+        .await
-            .await
+        .map_err(|err| BinstallError::CratesIoApi {
-            .map_err(|err| BinstallError::CratesIoApi {
+            crate_name: name.into(),
-                crate_name: name.into(),
+            err,
-                err,
+        })?;
-            })?;
 
     // Locate matching version
-    let version_iter =
+    let version_iter = base_info.versions.iter().filter(|v| !v.yanked);
-        base_info
+    let (version, version_name) = find_version(version_req, version_iter)?;
-            .versions
-            .iter()
-            .filter_map(|v| if !v.yanked { Some(&v.num) } else { None });
-    let version_name = find_version(version_req, version_iter)?;
-
-    // Fetch information for the filtered version
-    let version = base_info
-        .versions
-        .iter()
-        .find(|v| v.num == version_name.to_string())
-        .ok_or_else(|| BinstallError::VersionUnavailable {
-            crate_name: name.into(),
-            v: version_name.clone(),
-        })?;
 
     debug!("Found information for crate version: '{}'", version.num);
 

src/drivers/version.rs

@@ -1,48 +1,56 @@
-use std::collections::BTreeSet;
-
 use log::debug;
-use semver::{Version, VersionReq};
+use semver::VersionReq;
 
 use crate::BinstallError;
 
-pub(super) fn find_version<'a, V: Iterator<Item = &'a String>>(
+pub(super) trait Version {
+    /// Return `None` on error.
+    fn get_version(&self) -> Option<semver::Version>;
+}
+
+impl<T: Version> Version for &T {
+    fn get_version(&self) -> Option<semver::Version> {
+        (*self).get_version()
+    }
+}
+
+impl Version for crates_io_api::Version {
+    fn get_version(&self) -> Option<semver::Version> {
+        // Remove leading `v` for git tags
+        let ver_str = match self.num.strip_prefix('v') {
+            Some(v) => v,
+            None => &self.num,
+        };
+
+        // Parse out version
+        semver::Version::parse(ver_str).ok()
+    }
+}
+
+pub(super) fn find_version<Item: Version, VersionIter: Iterator<Item = Item>>(
     requirement: &str,
-    version_iter: V,
+    version_iter: VersionIter,
-) -> Result<Version, BinstallError> {
+) -> Result<(Item, semver::Version), BinstallError> {
     // Parse version requirement
     let version_req = VersionReq::parse(requirement).map_err(|err| BinstallError::VersionReq {
         req: requirement.into(),
         err,
     })?;
 
-    // Filter for matching versions
+    version_iter
-    let filtered: BTreeSet<_> = version_iter
+        // Filter for matching versions
-        .filter_map(|v| {
+        .filter_map(|item| {
-            // Remove leading `v` for git tags
+            let ver = item.get_version()?;
-            let ver_str = match v.strip_prefix('s') {
-                Some(v) => v,
-                None => v,
-            };
-
-            // Parse out version
-            let ver = Version::parse(ver_str).ok()?;
-            debug!("Version: {:?}", ver);
 
             // Filter by version match
             if version_req.matches(&ver) {
-                Some(ver)
+                debug!("Version: {:?}", ver);
+                Some((item, ver))
             } else {
                 None
             }
         })
-        .collect();
+        // Return highest version
-
+        .max_by_key(|(_item, ver)| ver.clone())
-    debug!("Filtered: {:?}", filtered);
-
-    // Return highest version
-    filtered
-        .iter()
-        .max()
-        .cloned()
         .ok_or(BinstallError::VersionMismatch { req: version_req })
 }

src/fetchers.rs

@@ -47,30 +47,21 @@ pub struct Data {
     pub meta: PkgMeta,
 }
 
+type FetcherJoinHandle = AutoAbortJoinHandle<Result<bool, BinstallError>>;
+
 #[derive(Default)]
-pub struct MultiFetcher {
+pub struct MultiFetcher(Vec<(Arc<dyn Fetcher>, FetcherJoinHandle)>);
-    fetchers: Vec<Arc<dyn Fetcher>>,
-}
 
 impl MultiFetcher {
     pub fn add(&mut self, fetcher: Arc<dyn Fetcher>) {
-        self.fetchers.push(fetcher);
+        self.0.push((
+            fetcher.clone(),
+            AutoAbortJoinHandle::new(tokio::spawn(async move { fetcher.check().await })),
+        ));
     }
 
-    pub async fn first_available(&self) -> Option<Arc<dyn Fetcher>> {
+    pub async fn first_available(self) -> Option<Arc<dyn Fetcher>> {
-        let handles: Vec<_> = self
+        for (fetcher, handle) in self.0 {
-            .fetchers
-            .iter()
-            .cloned()
-            .map(|fetcher| {
-                (
-                    fetcher.clone(),
-                    AutoAbortJoinHandle::new(tokio::spawn(async move { fetcher.check().await })),
-                )
-            })
-            .collect();
-
-        for (fetcher, handle) in handles {
             match handle.await {
                 Ok(Ok(true)) => return Some(fetcher),
                 Ok(Ok(false)) => (),
@@ -83,7 +74,7 @@ impl MultiFetcher {
                 }
                 Err(join_err) => {
                     debug!(
-                        "Error while checking fetcher {}: {}",
+                        "Error while joining the task that checks the fetcher {}: {}",
                         fetcher.source_name(),
                         join_err
                     );

src/helpers.rs

@@ -1,12 +1,14 @@
 use std::fmt::Debug;
 use std::fs;
 use std::io;
+use std::ops;
 use std::path::{Path, PathBuf};
 
 use bytes::Bytes;
 use cargo_toml::Manifest;
 use futures_util::stream::Stream;
 use log::debug;
+use once_cell::sync::OnceCell;
 use reqwest::{tls, Client, ClientBuilder, Method, Response};
 use serde::Serialize;
 use tempfile::NamedTempFile;
@@ -40,6 +42,14 @@ pub use tls_version::TLSVersion;
 mod crate_name;
 pub use crate_name::CrateName;
 
+pub fn cargo_home() -> Result<&'static Path, io::Error> {
+    static CARGO_HOME: OnceCell<PathBuf> = OnceCell::new();
+
+    CARGO_HOME
+        .get_or_try_init(home::cargo_home)
+        .map(ops::Deref::deref)
+}
+
 pub async fn await_task<T>(task: tokio::task::JoinHandle<miette::Result<T>>) -> miette::Result<T> {
     match task.await {
         Ok(res) => res,

src/main.rs

@@ -186,17 +186,28 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
     // `cargo run -- --help` gives ["target/debug/cargo-binstall", "--help"]
     // `cargo binstall --help` gives ["/home/ryan/.cargo/bin/cargo-binstall", "binstall", "--help"]
     let mut args: Vec<OsString> = std::env::args_os().collect();
-    if args.len() > 1 && args[1] == "binstall" {
+    let args = if args.len() > 1 && args[1] == "binstall" {
-        args.remove(1);
+        // Equivalent to
-    }
+        //
+        //     args.remove(1);
+        //
+        // But is O(1)
+        args.swap(0, 1);
+        let mut args = args.into_iter();
+        drop(args.next().unwrap());
+
+        args
+    } else {
+        args.into_iter()
+    };
 
     // Load options
     let mut opts = Options::parse_from(args);
-    let cli_overrides = Arc::new(PkgOverride {
+    let cli_overrides = PkgOverride {
         pkg_url: opts.pkg_url.take(),
         pkg_fmt: opts.pkg_fmt.take(),
         bin_dir: opts.bin_dir.take(),
-    });
+    };
     let crate_names = take(&mut opts.crate_names);
     if crate_names.len() > 1 && opts.manifest_path.is_some() {
         return Err(BinstallError::ManifestPathConflictedWithBatchInstallation.into());
@@ -205,6 +216,13 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
     // Initialize reqwest client
     let client = create_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?;
 
+    // Build crates.io api client
+    let crates_io_api_client = crates_io_api::AsyncClient::new(
+        "cargo-binstall (https://github.com/ryankurte/cargo-binstall)",
+        Duration::from_millis(100),
+    )
+    .expect("bug: invalid user agent");
+
     // Setup logging
     let mut log_config = ConfigBuilder::new();
     log_config.add_filter_ignore("hyper".to_string());
@@ -247,6 +265,8 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
         dry_run: opts.dry_run,
         version: opts.version.take(),
         manifest_path: opts.manifest_path.take(),
+        cli_overrides,
+        desired_targets,
     });
 
     let tasks: Vec<_> = if !opts.dry_run && !opts.no_confirm {
@@ -257,11 +277,10 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
                 tokio::spawn(binstall::resolve(
                     binstall_opts.clone(),
                     crate_name,
-                    desired_targets.clone(),
-                    cli_overrides.clone(),
                     temp_dir_path.clone(),
                     install_path.clone(),
                     client.clone(),
+                    crates_io_api_client.clone(),
                 ))
             })
             .collect();
@@ -281,7 +300,6 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
                 tokio::spawn(binstall::install(
                     resolution,
                     binstall_opts.clone(),
-                    desired_targets.clone(),
                     jobserver_client.clone(),
                 ))
             })
@@ -293,26 +311,23 @@ async fn entry(jobserver_client: LazyJobserverClient) -> Result<()> {
             .map(|crate_name| {
                 let opts = binstall_opts.clone();
                 let temp_dir_path = temp_dir_path.clone();
-                let desired_target = desired_targets.clone();
                 let jobserver_client = jobserver_client.clone();
-                let desired_targets = desired_targets.clone();
                 let client = client.clone();
-                let cli_overrides = cli_overrides.clone();
+                let crates_io_api_client = crates_io_api_client.clone();
                 let install_path = install_path.clone();
 
                 tokio::spawn(async move {
                     let resolution = binstall::resolve(
                         opts.clone(),
                         crate_name,
-                        desired_targets.clone(),
-                        cli_overrides,
                         temp_dir_path,
                         install_path,
                         client,
+                        crates_io_api_client,
                     )
                     .await?;
 
-                    binstall::install(resolution, opts, desired_target, jobserver_client).await
+                    binstall::install(resolution, opts, jobserver_client).await
                 })
             })
             .collect()

src/metafiles/v1.rs

@@ -10,6 +10,7 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
 use super::CrateVersionSource;
+use crate::cargo_home;
 
 #[derive(Clone, Debug, Default, Deserialize, Serialize)]
 pub struct CratesToml {
@@ -18,7 +19,7 @@ pub struct CratesToml {
 
 impl CratesToml {
     pub fn default_path() -> Result<PathBuf, CratesTomlParseError> {
-        Ok(home::cargo_home()?.join(".crates.toml"))
+        Ok(cargo_home()?.join(".crates.toml"))
     }
 
     pub fn load() -> Result<Self, CratesTomlParseError> {

src/metafiles/v2.rs

@@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
 use super::CrateVersionSource;
+use crate::cargo_home;
 
 #[derive(Clone, Debug, Default, Deserialize, Serialize)]
 pub struct Crates2Json {
@@ -39,7 +40,7 @@ pub struct CrateInfo {
 
 impl Crates2Json {
     pub fn default_path() -> Result<PathBuf, Crates2JsonParseError> {
-        Ok(home::cargo_home()?.join(".crates2.json"))
+        Ok(cargo_home()?.join(".crates2.json"))
     }
 
     pub fn load() -> Result<Self, Crates2JsonParseError> {

src/target.rs

@@ -8,18 +8,18 @@ use tokio::sync::OnceCell;
 /// Compiled target triple, used as default for binary fetching
 pub const TARGET: &str = env!("TARGET");
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 enum DesiredTargetsInner {
     AutoDetect(Arc<OnceCell<Vec<String>>>),
-    Initialized(Arc<Vec<String>>),
+    Initialized(Vec<String>),
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct DesiredTargets(DesiredTargetsInner);
 
 impl DesiredTargets {
     fn initialized(targets: Vec<String>) -> Self {
-        Self(DesiredTargetsInner::Initialized(Arc::new(targets)))
+        Self(DesiredTargetsInner::Initialized(targets))
     }
 
     fn auto_detect() -> Self {