From 9e1f873bb52a07c0f69307396aa59ca4f588dbb8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 16:01:52 +1000 Subject: [PATCH 01/33] `derive(Clone)` for `DesiredTargets` Signed-off-by: Jiahao XU --- src/target.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/target.rs b/src/target.rs index 8dfb75b6..fdca606f 100644 --- a/src/target.rs +++ b/src/target.rs @@ -8,13 +8,13 @@ use tokio::sync::OnceCell; /// Compiled target triple, used as default for binary fetching pub const TARGET: &str = env!("TARGET"); -#[derive(Debug)] +#[derive(Debug, Clone)] enum DesiredTargetsInner { AutoDetect(Arc>>), Initialized(Vec), } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct DesiredTargets(DesiredTargetsInner); impl DesiredTargets { From 409f31f0bf5d5dd0e7c734834a5a37734af26cd0 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 16:07:47 +1000 Subject: [PATCH 02/33] `derive(Clone)` for `CrateName` Signed-off-by: Jiahao XU --- src/helpers/crate_name.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers/crate_name.rs b/src/helpers/crate_name.rs index b7eb1077..58ef59c0 100644 --- a/src/helpers/crate_name.rs +++ b/src/helpers/crate_name.rs @@ -2,7 +2,7 @@ use std::convert::Infallible; use std::fmt; use std::str::FromStr; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct CrateName { pub name: String, pub version: Option, From d6db552db1643ea2dd9cb987cdd979e97b98ef82 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 16:16:50 +1000 Subject: [PATCH 03/33] Refactor `main.rs`: Extract new fn `resolve` prepare for the new feature batch installation. Signed-off-by: Jiahao XU --- src/main.rs | 204 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 132 insertions(+), 72 deletions(-) diff --git a/src/main.rs b/src/main.rs index b5a8af31..56800774 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,6 +3,7 @@ use std::{ ffi::OsString, path::{Path, PathBuf}, process::{ExitCode, Termination}, + sync::Arc, time::{Duration, Instant}, }; @@ -10,6 +11,7 @@ use cargo_toml::{Package, Product}; use clap::Parser; use log::{debug, error, info, warn, LevelFilter}; use miette::{miette, IntoDiagnostic, Result, WrapErr}; +use reqwest::Client; use simplelog::{ColorChoice, ConfigBuilder, TermLogger, TerminalMode}; use tempfile::TempDir; use tokio::{ @@ -197,11 +199,12 @@ async fn entry() -> Result<()> { // Load options let mut opts = Options::parse_from(args); - let cli_overrides = PkgOverride { + let cli_overrides = Arc::new(PkgOverride { pkg_url: opts.pkg_url.take(), pkg_fmt: opts.pkg_fmt.take(), bin_dir: opts.bin_dir.take(), - }; + }); + let opts = Arc::new(opts); // Initialize reqwest client let client = create_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?; @@ -236,9 +239,119 @@ async fn entry() -> Result<()> { .map_err(BinstallError::from) .wrap_err("Creating a temporary directory failed.")?; - info!("Installing package: '{}'", opts.crate_name); + let resolution = resolve( + opts.clone(), + opts.crate_name.clone(), + desired_targets.clone(), + cli_overrides.clone(), + temp_dir.path().to_path_buf(), + install_path.clone(), + client.clone(), + ) + .await?; - let mut version = match (&opts.crate_name.version, &opts.version) { + match resolution { + Resolution::Fetch { + fetcher, + package, + version, + bin_path, + bin_files, + } => { + let fetcher_target = fetcher.target(); + // Prompt user for confirmation + debug!( + "Found a binary install source: {} ({fetcher_target})", + fetcher.source_name() + ); + + if fetcher.is_third_party() { + warn!( + "The package will be downloaded from third-party source {}", + fetcher.source_name() + ); + } else { + info!( + "The package will be downloaded from {}", + fetcher.source_name() + ); + } + + info!("This will install the following binaries:"); + for file in &bin_files { + info!(" - {}", file.preview_bin()); + } + + if !opts.no_symlinks { + info!("And create (or update) the following symlinks:"); + for file in &bin_files { + info!(" - {}", file.preview_link()); + } + } + + if !opts.dry_run { + uithread.confirm().await?; + } + + install_from_package( + fetcher.as_ref(), + opts, + package, + temp_dir, + version, + &bin_path, + &bin_files, + ) + .await + } + Resolution::InstallFromSource { package } => { + if !opts.no_cleanup { + temp_dir.close().unwrap_or_else(|err| { + warn!("Failed to clean up some resources: {err}"); + }); + } + + let desired_targets = desired_targets.get().await; + let target = desired_targets + .first() + .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?; + + // Prompt user for source install + warn!("The package will be installed from source (with cargo)",); + if !opts.dry_run { + uithread.confirm().await?; + } + + install_from_source(opts, package, target).await + } + } +} + +enum Resolution { + Fetch { + fetcher: Arc, + package: Package, + version: String, + bin_path: PathBuf, + bin_files: Vec, + }, + InstallFromSource { + package: Package, + }, +} + +async fn resolve( + opts: Arc, + crate_name: CrateName, + desired_targets: DesiredTargets, + cli_overrides: Arc, + temp_dir: PathBuf, + install_path: PathBuf, + client: Client, +) -> Result { + info!("Installing package: '{}'", crate_name); + + let mut version = match (&crate_name.version, &opts.version) { (Some(version), None) => version.to_string(), (None, Some(version)) => version.to_string(), (Some(_), Some(_)) => Err(BinstallError::DuplicateVersionReq)?, @@ -259,7 +372,7 @@ async fn entry() -> Result<()> { // 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, &opts.crate_name.name, &version).await?, + None => fetch_crate_cratesio(&client, &crate_name.name, &version).await?, }; let package = manifest.package.unwrap(); @@ -311,9 +424,7 @@ async fn entry() -> Result<()> { meta.merge(&cli_overrides); // Generate temporary binary path - let bin_path = temp_dir - .path() - .join(format!("bin-{}", opts.crate_name.name)); + let bin_path = temp_dir.join(format!("bin-{}", crate_name.name)); debug!("Using temporary binary path: {}", bin_path.display()); let bin_files = collect_bin_files( @@ -325,70 +436,15 @@ async fn entry() -> Result<()> { install_path, )?; - // Prompt user for confirmation - debug!( - "Found a binary install source: {} ({fetcher_target})", - fetcher.source_name() - ); - - if fetcher.is_third_party() { - warn!( - "The package will be downloaded from third-party source {}", - fetcher.source_name() - ); - } else { - info!( - "The package will be downloaded from {}", - fetcher.source_name() - ); - } - - info!("This will install the following binaries:"); - for file in &bin_files { - info!(" - {}", file.preview_bin()); - } - - if !opts.no_symlinks { - info!("And create (or update) the following symlinks:"); - for file in &bin_files { - info!(" - {}", file.preview_link()); - } - } - - if !opts.dry_run { - uithread.confirm().await?; - } - - install_from_package( - fetcher.as_ref(), - opts, + Ok(Resolution::Fetch { + fetcher, package, - temp_dir, version, - &bin_path, - &bin_files, - ) - .await - } - None => { - if !opts.no_cleanup { - temp_dir.close().unwrap_or_else(|err| { - warn!("Failed to clean up some resources: {err}"); - }); - } - - let target = desired_targets - .first() - .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?; - - // Prompt user for source install - warn!("The package will be installed from source (with cargo)",); - if !opts.dry_run { - uithread.confirm().await?; - } - - install_from_source(opts, package, target).await + bin_path, + bin_files, + }) } + None => Ok(Resolution::InstallFromSource { package }), } } @@ -437,7 +493,7 @@ fn collect_bin_files( async fn install_from_package( fetcher: &dyn Fetcher, - opts: Options, + opts: Arc, package: Package, temp_dir: TempDir, version: String, @@ -481,7 +537,7 @@ async fn install_from_package( } let cvs = metafiles::CrateVersionSource { - name: opts.crate_name.name, + name: opts.crate_name.name.clone(), version: package.version.parse().into_diagnostic()?, source: metafiles::Source::Registry( url::Url::parse("https://github.com/rust-lang/crates.io-index").unwrap(), @@ -539,7 +595,11 @@ async fn install_from_package( }) } -async fn install_from_source(opts: Options, package: Package, target: &str) -> Result<()> { +async fn install_from_source( + opts: Arc, + package: Package, + target: &str, +) -> Result<()> { if opts.dry_run { info!( "Dry-run: running `cargo install {} --version {} --target {target}`", From 730f7d6c15825f07f6e79a46e200525a27f65ea5 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 16:21:03 +1000 Subject: [PATCH 04/33] Refactor `main.rs`: Simplify `install_from_source` Rm arg `opts` Signed-off-by: Jiahao XU --- src/main.rs | 75 +++++++++++++++++++++++++---------------------------- 1 file changed, 35 insertions(+), 40 deletions(-) diff --git a/src/main.rs b/src/main.rs index 56800774..55ba5b42 100644 --- a/src/main.rs +++ b/src/main.rs @@ -320,9 +320,16 @@ async fn entry() -> Result<()> { warn!("The package will be installed from source (with cargo)",); if !opts.dry_run { uithread.confirm().await?; - } - install_from_source(opts, package, target).await + install_from_source(package, target).await + } else { + info!( + "Dry-run: running `cargo install {} --version {} --target {target}`", + package.name, package.version + ); + + Ok(()) + } } } } @@ -595,45 +602,33 @@ async fn install_from_package( }) } -async fn install_from_source( - opts: Arc, - package: Package, - target: &str, -) -> Result<()> { - if opts.dry_run { - info!( - "Dry-run: running `cargo install {} --version {} --target {target}`", - package.name, package.version - ); +async fn install_from_source(package: Package, target: &str) -> Result<()> { + debug!( + "Running `cargo install {} --version {} --target {target}`", + package.name, package.version + ); + let mut child = Command::new("cargo") + .arg("install") + .arg(package.name) + .arg("--version") + .arg(package.version) + .arg("--target") + .arg(target) + .spawn() + .into_diagnostic() + .wrap_err("Spawning cargo install failed.")?; + debug!("Spawned command pid={:?}", child.id()); + + let status = child + .wait() + .await + .into_diagnostic() + .wrap_err("Running cargo install failed.")?; + if status.success() { + info!("Cargo finished successfully"); Ok(()) } else { - debug!( - "Running `cargo install {} --version {} --target {target}`", - package.name, package.version - ); - let mut child = Command::new("cargo") - .arg("install") - .arg(package.name) - .arg("--version") - .arg(package.version) - .arg("--target") - .arg(target) - .spawn() - .into_diagnostic() - .wrap_err("Spawning cargo install failed.")?; - debug!("Spawned command pid={:?}", child.id()); - - let status = child - .wait() - .await - .into_diagnostic() - .wrap_err("Running cargo install failed.")?; - if status.success() { - info!("Cargo finished successfully"); - Ok(()) - } else { - error!("Cargo errored! {status:?}"); - Err(miette!("Cargo install error")) - } + error!("Cargo errored! {status:?}"); + Err(miette!("Cargo install error")) } } From 5e7aab7373686268ca52a8e75f119ea7cd49bdb9 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 16:46:21 +1000 Subject: [PATCH 05/33] Impl `helpers::await_task`: Handle `JoinError` Signed-off-by: Jiahao XU --- src/helpers.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/helpers.rs b/src/helpers.rs index 254119b8..2732f1e7 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -37,6 +37,11 @@ pub use tls_version::TLSVersion; mod crate_name; pub use crate_name::CrateName; +pub async fn await_task(task: tokio::task::JoinHandle) -> miette::Result { + task.await + .map_err(|join_err| miette::miette!("Task failed to join: {}", join_err)) +} + /// Load binstall metadata from the crate `Cargo.toml` at the provided path pub fn load_manifest_path>( manifest_path: P, From 7f11b74f5ef0354d2aa8773e3e5046274cccc8b6 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 16:46:51 +1000 Subject: [PATCH 06/33] Support new feature batch installation! Signed-off-by: Jiahao XU --- src/main.rs | 225 ++++++++++++++++++++++++++++------------------------ 1 file changed, 123 insertions(+), 102 deletions(-) diff --git a/src/main.rs b/src/main.rs index 55ba5b42..33f750b2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,7 +1,8 @@ use std::{ collections::BTreeSet, ffi::OsString, - path::{Path, PathBuf}, + mem::take, + path::PathBuf, process::{ExitCode, Termination}, sync::Arc, time::{Duration, Instant}, @@ -37,7 +38,7 @@ struct Options { /// /// This must be a crates.io package name. #[clap(value_name = "crate")] - crate_name: CrateName, + crate_names: Vec, /// Semver filter to select the package version to install. /// @@ -204,6 +205,7 @@ async fn entry() -> Result<()> { pkg_fmt: opts.pkg_fmt.take(), bin_dir: opts.bin_dir.take(), }); + let crate_names = take(&mut opts.crate_names); let opts = Arc::new(opts); // Initialize reqwest client @@ -239,105 +241,129 @@ async fn entry() -> Result<()> { .map_err(BinstallError::from) .wrap_err("Creating a temporary directory failed.")?; - let resolution = resolve( - opts.clone(), - opts.crate_name.clone(), - desired_targets.clone(), - cli_overrides.clone(), - temp_dir.path().to_path_buf(), - install_path.clone(), - client.clone(), - ) - .await?; + let tasks: Vec<_> = crate_names + .into_iter() + .map(|crate_name| { + tokio::spawn(resolve( + opts.clone(), + crate_name, + desired_targets.clone(), + cli_overrides.clone(), + temp_dir.path().to_path_buf(), + install_path.clone(), + client.clone(), + )) + }) + .collect(); - match resolution { - Resolution::Fetch { - fetcher, - package, - version, - bin_path, - bin_files, - } => { - let fetcher_target = fetcher.target(); - // Prompt user for confirmation - debug!( - "Found a binary install source: {} ({fetcher_target})", - fetcher.source_name() - ); + let mut resolutions = Vec::with_capacity(tasks.len()); + for task in tasks { + resolutions.push(await_task(task).await??); + } - if fetcher.is_third_party() { - warn!( - "The package will be downloaded from third-party source {}", + for resolution in &resolutions { + match resolution { + Resolution::Fetch { + fetcher, bin_files, .. + } => { + let fetcher_target = fetcher.target(); + // Prompt user for confirmation + debug!( + "Found a binary install source: {} ({fetcher_target})", fetcher.source_name() ); - } else { - info!( - "The package will be downloaded from {}", - fetcher.source_name() - ); - } - info!("This will install the following binaries:"); - for file in &bin_files { - info!(" - {}", file.preview_bin()); - } + if fetcher.is_third_party() { + warn!( + "The package will be downloaded from third-party source {}", + fetcher.source_name() + ); + } else { + info!( + "The package will be downloaded from {}", + fetcher.source_name() + ); + } - if !opts.no_symlinks { - info!("And create (or update) the following symlinks:"); - for file in &bin_files { - info!(" - {}", file.preview_link()); + info!("This will install the following binaries:"); + for file in bin_files { + info!(" - {}", file.preview_bin()); + } + + if !opts.no_symlinks { + info!("And create (or update) the following symlinks:"); + for file in bin_files { + info!(" - {}", file.preview_link()); + } } } - - if !opts.dry_run { - uithread.confirm().await?; - } - - install_from_package( - fetcher.as_ref(), - opts, - package, - temp_dir, - version, - &bin_path, - &bin_files, - ) - .await - } - Resolution::InstallFromSource { package } => { - if !opts.no_cleanup { - temp_dir.close().unwrap_or_else(|err| { - warn!("Failed to clean up some resources: {err}"); - }); - } - - let desired_targets = desired_targets.get().await; - let target = desired_targets - .first() - .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?; - - // Prompt user for source install - warn!("The package will be installed from source (with cargo)",); - if !opts.dry_run { - uithread.confirm().await?; - - install_from_source(package, target).await - } else { - info!( - "Dry-run: running `cargo install {} --version {} --target {target}`", - package.name, package.version - ); - - Ok(()) + Resolution::InstallFromSource { .. } => { + warn!("The package will be installed from source (with cargo)",) } } } + + if !opts.dry_run { + uithread.confirm().await?; + } + + let desired_targets = desired_targets.get().await; + let target = desired_targets + .first() + .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?; + + let tasks: Vec<_> = resolutions + .into_iter() + .map(|resolution| match resolution { + Resolution::Fetch { + fetcher, + package, + crate_name, + version, + bin_path, + bin_files, + } => tokio::spawn(install_from_package( + fetcher, + opts.clone(), + package, + crate_name, + temp_dir.path().to_path_buf(), + version, + bin_path, + bin_files, + )), + Resolution::InstallFromSource { package } => { + if !opts.dry_run { + tokio::spawn(install_from_source(package, target.clone())) + } else { + info!( + "Dry-run: running `cargo install {} --version {} --target {target}`", + package.name, package.version + ); + tokio::spawn(async { Ok(()) }) + } + } + }) + .collect(); + + for task in tasks { + await_task(task).await??; + } + + if !opts.no_cleanup { + temp_dir.close().unwrap_or_else(|err| { + warn!("Failed to clean up some resources: {err}"); + }); + } + + Ok(()) } enum Resolution { Fetch { fetcher: Arc, package: Package, + crate_name: CrateName, version: String, bin_path: PathBuf, bin_files: Vec, @@ -446,6 +472,7 @@ async fn resolve( Ok(Resolution::Fetch { fetcher, package, + crate_name, version, bin_path, bin_files, @@ -498,20 +525,22 @@ fn collect_bin_files( Ok(bin_files) } +#[allow(unused, clippy::too_many_arguments)] async fn install_from_package( - fetcher: &dyn Fetcher, + fetcher: Arc, opts: Arc, package: Package, - temp_dir: TempDir, + crate_name: CrateName, + temp_dir: PathBuf, version: String, - bin_path: &Path, - bin_files: &[bins::BinFile], + bin_path: PathBuf, + bin_files: Vec, ) -> Result<()> { // Download package if opts.dry_run { info!("Dry run, not downloading package"); } else { - fetcher.fetch_and_extract(bin_path).await?; + fetcher.fetch_and_extract(&bin_path).await?; } #[cfg(incomplete)] @@ -528,7 +557,7 @@ async fn install_from_package( debug!("Fetching signature file: {sig_url}"); // Download signature file - let sig_path = temp_dir.path().join(format!("{pkg_name}.sig")); + let sig_path = temp_dir.join(format!("{pkg_name}.sig")); download(&sig_url, &sig_path).await?; // TODO: do the signature check @@ -544,7 +573,7 @@ async fn install_from_package( } let cvs = metafiles::CrateVersionSource { - name: opts.crate_name.name.clone(), + name: crate_name.name.clone(), version: package.version.parse().into_diagnostic()?, source: metafiles::Source::Registry( url::Url::parse("https://github.com/rust-lang/crates.io-index").unwrap(), @@ -553,25 +582,17 @@ async fn install_from_package( info!("Installing binaries..."); block_in_place(|| { - for file in bin_files { + for file in &bin_files { file.install_bin()?; } // Generate symlinks if !opts.no_symlinks { - for file in bin_files { + for file in &bin_files { file.install_link()?; } } - if opts.no_cleanup { - let _ = temp_dir.into_path(); - } else { - temp_dir.close().unwrap_or_else(|err| { - warn!("Failed to clean up some resources: {err}"); - }); - } - let bins: BTreeSet = bin_files.iter().map(|bin| bin.base_name.clone()).collect(); { @@ -602,7 +623,7 @@ async fn install_from_package( }) } -async fn install_from_source(package: Package, target: &str) -> Result<()> { +async fn install_from_source(package: Package, target: String) -> Result<()> { debug!( "Running `cargo install {} --version {} --target {target}`", package.name, package.version From 40a872dbe3d49715d1547ae69ce1fa8e2d0b10cb Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 16:54:45 +1000 Subject: [PATCH 07/33] Avoid `Box::clone` for `targets` Signed-off-by: Jiahao XU --- src/main.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main.rs b/src/main.rs index 33f750b2..d74c4fb4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -308,9 +308,12 @@ async fn entry() -> Result<()> { } let desired_targets = desired_targets.get().await; - let target = desired_targets - .first() - .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?; + let target = Arc::from( + desired_targets + .first() + .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))? + .as_str(), + ); let tasks: Vec<_> = resolutions .into_iter() @@ -334,7 +337,7 @@ async fn entry() -> Result<()> { )), Resolution::InstallFromSource { package } => { if !opts.dry_run { - tokio::spawn(install_from_source(package, target.clone())) + tokio::spawn(install_from_source(package, Arc::clone(&target))) } else { info!( "Dry-run: running `cargo install {} --version {} --target {target}`", @@ -623,7 +626,7 @@ async fn install_from_package( }) } -async fn install_from_source(package: Package, target: String) -> Result<()> { +async fn install_from_source(package: Package, target: Arc) -> Result<()> { debug!( "Running `cargo install {} --version {} --target {target}`", package.name, package.version @@ -634,7 +637,7 @@ async fn install_from_source(package: Package, target: String) -> Result<( .arg("--version") .arg(package.version) .arg("--target") - .arg(target) + .arg(&*target) .spawn() .into_diagnostic() .wrap_err("Spawning cargo install failed.")?; From d514219ee405ed292083c385743b7de7a394bd68 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 16:58:04 +1000 Subject: [PATCH 08/33] Refactor `main.rs`: Extract new fn `install` Signed-off-by: Jiahao XU --- src/main.rs | 71 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/src/main.rs b/src/main.rs index d74c4fb4..de2f5d39 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,7 +2,7 @@ use std::{ collections::BTreeSet, ffi::OsString, mem::take, - path::PathBuf, + path::{Path, PathBuf}, process::{ExitCode, Termination}, sync::Arc, time::{Duration, Instant}, @@ -317,36 +317,7 @@ async fn entry() -> Result<()> { let tasks: Vec<_> = resolutions .into_iter() - .map(|resolution| match resolution { - Resolution::Fetch { - fetcher, - package, - crate_name, - version, - bin_path, - bin_files, - } => tokio::spawn(install_from_package( - fetcher, - opts.clone(), - package, - crate_name, - temp_dir.path().to_path_buf(), - version, - bin_path, - bin_files, - )), - Resolution::InstallFromSource { package } => { - if !opts.dry_run { - tokio::spawn(install_from_source(package, Arc::clone(&target))) - } else { - info!( - "Dry-run: running `cargo install {} --version {} --target {target}`", - package.name, package.version - ); - tokio::spawn(async { Ok(()) }) - } - } - }) + .map(|resolution| install(resolution, &opts, temp_dir.path(), &target)) .collect(); for task in tasks { @@ -528,6 +499,44 @@ fn collect_bin_files( Ok(bin_files) } +fn install( + resolution: Resolution, + opts: &Arc, + temp_dir: &Path, + target: &Arc, +) -> tokio::task::JoinHandle> { + match resolution { + Resolution::Fetch { + fetcher, + package, + crate_name, + version, + bin_path, + bin_files, + } => tokio::spawn(install_from_package( + fetcher, + opts.clone(), + package, + crate_name, + temp_dir.to_path_buf(), + version, + bin_path, + bin_files, + )), + Resolution::InstallFromSource { package } => { + if !opts.dry_run { + tokio::spawn(install_from_source(package, Arc::clone(target))) + } else { + info!( + "Dry-run: running `cargo install {} --version {} --target {target}`", + package.name, package.version + ); + tokio::spawn(async { Ok(()) }) + } + } + } +} + #[allow(unused, clippy::too_many_arguments)] async fn install_from_package( fetcher: Arc, From 8ca85382afe349d7ebdb4e4e7370bfa454c0542f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 17:00:18 +1000 Subject: [PATCH 09/33] Refactor: Avoid repeated heap alloc of `temp_dir` Signed-off-by: Jiahao XU --- src/main.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index de2f5d39..f813467c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -315,9 +315,11 @@ async fn entry() -> Result<()> { .as_str(), ); + let temp_dir_path = Arc::from(temp_dir.path()); + let tasks: Vec<_> = resolutions .into_iter() - .map(|resolution| install(resolution, &opts, temp_dir.path(), &target)) + .map(|resolution| install(resolution, &opts, &temp_dir_path, &target)) .collect(); for task in tasks { @@ -502,7 +504,7 @@ fn collect_bin_files( fn install( resolution: Resolution, opts: &Arc, - temp_dir: &Path, + temp_dir: &Arc, target: &Arc, ) -> tokio::task::JoinHandle> { match resolution { @@ -518,7 +520,7 @@ fn install( opts.clone(), package, crate_name, - temp_dir.to_path_buf(), + Arc::clone(temp_dir), version, bin_path, bin_files, @@ -543,7 +545,7 @@ async fn install_from_package( opts: Arc, package: Package, crate_name: CrateName, - temp_dir: PathBuf, + temp_dir: Arc, version: String, bin_path: PathBuf, bin_files: Vec, From 90059c11cf1ce62e13235fb21f515120414b846b Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 17:09:29 +1000 Subject: [PATCH 10/33] Optimize: Launch `install` immediately if confirmation is not required Signed-off-by: Jiahao XU --- src/main.rs | 161 +++++++++++++++++++++++++++++----------------------- 1 file changed, 89 insertions(+), 72 deletions(-) diff --git a/src/main.rs b/src/main.rs index f813467c..ceada477 100644 --- a/src/main.rs +++ b/src/main.rs @@ -256,57 +256,6 @@ async fn entry() -> Result<()> { }) .collect(); - let mut resolutions = Vec::with_capacity(tasks.len()); - for task in tasks { - resolutions.push(await_task(task).await??); - } - - for resolution in &resolutions { - match resolution { - Resolution::Fetch { - fetcher, bin_files, .. - } => { - let fetcher_target = fetcher.target(); - // Prompt user for confirmation - debug!( - "Found a binary install source: {} ({fetcher_target})", - fetcher.source_name() - ); - - if fetcher.is_third_party() { - warn!( - "The package will be downloaded from third-party source {}", - fetcher.source_name() - ); - } else { - info!( - "The package will be downloaded from {}", - fetcher.source_name() - ); - } - - info!("This will install the following binaries:"); - for file in bin_files { - info!(" - {}", file.preview_bin()); - } - - if !opts.no_symlinks { - info!("And create (or update) the following symlinks:"); - for file in bin_files { - info!(" - {}", file.preview_link()); - } - } - } - Resolution::InstallFromSource { .. } => { - warn!("The package will be installed from source (with cargo)",) - } - } - } - - if !opts.dry_run { - uithread.confirm().await?; - } - let desired_targets = desired_targets.get().await; let target = Arc::from( desired_targets @@ -317,10 +266,82 @@ async fn entry() -> Result<()> { let temp_dir_path = Arc::from(temp_dir.path()); - let tasks: Vec<_> = resolutions - .into_iter() - .map(|resolution| install(resolution, &opts, &temp_dir_path, &target)) - .collect(); + let tasks: Vec<_> = if !opts.dry_run && !opts.no_confirm { + let mut resolutions = Vec::with_capacity(tasks.len()); + for task in tasks { + resolutions.push(await_task(task).await??); + } + + for resolution in &resolutions { + match resolution { + Resolution::Fetch { + fetcher, bin_files, .. + } => { + let fetcher_target = fetcher.target(); + // Prompt user for confirmation + debug!( + "Found a binary install source: {} ({fetcher_target})", + fetcher.source_name() + ); + + if fetcher.is_third_party() { + warn!( + "The package will be downloaded from third-party source {}", + fetcher.source_name() + ); + } else { + info!( + "The package will be downloaded from {}", + fetcher.source_name() + ); + } + + info!("This will install the following binaries:"); + for file in bin_files { + info!(" - {}", file.preview_bin()); + } + + if !opts.no_symlinks { + info!("And create (or update) the following symlinks:"); + for file in bin_files { + info!(" - {}", file.preview_link()); + } + } + } + Resolution::InstallFromSource { .. } => { + warn!("The package will be installed from source (with cargo)",) + } + } + } + + uithread.confirm().await?; + + resolutions + .into_iter() + .map(|resolution| { + tokio::spawn(install( + resolution, + Arc::clone(&opts), + Arc::clone(&temp_dir_path), + Arc::clone(&target), + )) + }) + .collect() + } else { + tasks + .into_iter() + .map(|task| { + let opts = Arc::clone(&opts); + let temp_dir_path = Arc::clone(&temp_dir_path); + let target = Arc::clone(&target); + + tokio::spawn(async move { + let resolution = await_task(task).await??; + install(resolution, opts, temp_dir_path, target).await + }) + }) + .collect() + }; for task in tasks { await_task(task).await??; @@ -501,12 +522,12 @@ fn collect_bin_files( Ok(bin_files) } -fn install( +async fn install( resolution: Resolution, - opts: &Arc, - temp_dir: &Arc, - target: &Arc, -) -> tokio::task::JoinHandle> { + opts: Arc, + temp_dir: Arc, + target: Arc, +) -> Result<()> { match resolution { Resolution::Fetch { fetcher, @@ -515,25 +536,21 @@ fn install( version, bin_path, bin_files, - } => tokio::spawn(install_from_package( - fetcher, - opts.clone(), - package, - crate_name, - Arc::clone(temp_dir), - version, - bin_path, - bin_files, - )), + } => { + install_from_package( + fetcher, opts, package, crate_name, temp_dir, version, bin_path, bin_files, + ) + .await + } Resolution::InstallFromSource { package } => { if !opts.dry_run { - tokio::spawn(install_from_source(package, Arc::clone(target))) + install_from_source(package, target).await } else { info!( "Dry-run: running `cargo install {} --version {} --target {target}`", package.name, package.version ); - tokio::spawn(async { Ok(()) }) + Ok(()) } } } From f0fb7da99b5b4e5c66cb107efac0371895e108e0 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 17:11:27 +1000 Subject: [PATCH 11/33] Refactor `main.rs`: Extract fn `Resolution::print` Signed-off-by: Jiahao XU --- src/main.rs | 83 ++++++++++++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/src/main.rs b/src/main.rs index ceada477..98573bc2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -273,45 +273,7 @@ async fn entry() -> Result<()> { } for resolution in &resolutions { - match resolution { - Resolution::Fetch { - fetcher, bin_files, .. - } => { - let fetcher_target = fetcher.target(); - // Prompt user for confirmation - debug!( - "Found a binary install source: {} ({fetcher_target})", - fetcher.source_name() - ); - - if fetcher.is_third_party() { - warn!( - "The package will be downloaded from third-party source {}", - fetcher.source_name() - ); - } else { - info!( - "The package will be downloaded from {}", - fetcher.source_name() - ); - } - - info!("This will install the following binaries:"); - for file in bin_files { - info!(" - {}", file.preview_bin()); - } - - if !opts.no_symlinks { - info!("And create (or update) the following symlinks:"); - for file in bin_files { - info!(" - {}", file.preview_link()); - } - } - } - Resolution::InstallFromSource { .. } => { - warn!("The package will be installed from source (with cargo)",) - } - } + resolution.print(&opts); } uithread.confirm().await?; @@ -369,6 +331,49 @@ enum Resolution { package: Package, }, } +impl Resolution { + fn print(&self, opts: &Options) { + match self { + Resolution::Fetch { + fetcher, bin_files, .. + } => { + let fetcher_target = fetcher.target(); + // Prompt user for confirmation + debug!( + "Found a binary install source: {} ({fetcher_target})", + fetcher.source_name() + ); + + if fetcher.is_third_party() { + warn!( + "The package will be downloaded from third-party source {}", + fetcher.source_name() + ); + } else { + info!( + "The package will be downloaded from {}", + fetcher.source_name() + ); + } + + info!("This will install the following binaries:"); + for file in bin_files { + info!(" - {}", file.preview_bin()); + } + + if !opts.no_symlinks { + info!("And create (or update) the following symlinks:"); + for file in bin_files { + info!(" - {}", file.preview_link()); + } + } + } + Resolution::InstallFromSource { .. } => { + warn!("The package will be installed from source (with cargo)",) + } + } + } +} async fn resolve( opts: Arc, From c66d8154eb89c7dd26778bd834a9c0739682d4df Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 17:13:06 +1000 Subject: [PATCH 12/33] Print out `resolution` in optimized path where confirmation isn't required. Signed-off-by: Jiahao XU --- src/main.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main.rs b/src/main.rs index 98573bc2..d3f28c11 100644 --- a/src/main.rs +++ b/src/main.rs @@ -299,6 +299,9 @@ async fn entry() -> Result<()> { tokio::spawn(async move { let resolution = await_task(task).await??; + + resolution.print(&opts); + install(resolution, opts, temp_dir_path, target).await }) }) From bcb46cd73692e86ad07e918f2ea3dd9da79a5d19 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 17:17:56 +1000 Subject: [PATCH 13/33] Optimize `main.rs`: Avoid frequent `Box::clone` Signed-off-by: Jiahao XU --- src/main.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/main.rs b/src/main.rs index d3f28c11..54582586 100644 --- a/src/main.rs +++ b/src/main.rs @@ -230,10 +230,12 @@ async fn entry() -> Result<()> { let desired_targets = get_desired_targets(&opts.targets); // Compute install directory - let install_path = get_install_path(opts.install_path.as_deref()).ok_or_else(|| { - error!("No viable install path found of specified, try `--install-path`"); - miette!("No install path found or specified") - })?; + let install_path: Arc = Arc::from( + get_install_path(opts.install_path.as_deref()).ok_or_else(|| { + error!("No viable install path found of specified, try `--install-path`"); + miette!("No install path found or specified") + })?, + ); debug!("Using install path: {}", install_path.display()); // Create a temporary directory for downloads etc. @@ -241,6 +243,8 @@ async fn entry() -> Result<()> { .map_err(BinstallError::from) .wrap_err("Creating a temporary directory failed.")?; + let temp_dir_path: Arc = Arc::from(temp_dir.path()); + let tasks: Vec<_> = crate_names .into_iter() .map(|crate_name| { @@ -249,7 +253,7 @@ async fn entry() -> Result<()> { crate_name, desired_targets.clone(), cli_overrides.clone(), - temp_dir.path().to_path_buf(), + temp_dir_path.clone(), install_path.clone(), client.clone(), )) @@ -264,8 +268,6 @@ async fn entry() -> Result<()> { .as_str(), ); - let temp_dir_path = Arc::from(temp_dir.path()); - let tasks: Vec<_> = if !opts.dry_run && !opts.no_confirm { let mut resolutions = Vec::with_capacity(tasks.len()); for task in tasks { @@ -383,8 +385,8 @@ async fn resolve( crate_name: CrateName, desired_targets: DesiredTargets, cli_overrides: Arc, - temp_dir: PathBuf, - install_path: PathBuf, + temp_dir: Arc, + install_path: Arc, client: Client, ) -> Result { info!("Installing package: '{}'", crate_name); @@ -471,7 +473,7 @@ async fn resolve( meta, binaries, bin_path.clone(), - install_path, + install_path.to_path_buf(), )?; Ok(Resolution::Fetch { From 119192f8eea1d1fae801d8c3761bb1172ee55edf Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 17:21:35 +1000 Subject: [PATCH 14/33] Refactor `main.rs`: Print resolution in `resolve` This simplified `entry`. Signed-off-by: Jiahao XU --- src/main.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/main.rs b/src/main.rs index 54582586..738985ae 100644 --- a/src/main.rs +++ b/src/main.rs @@ -274,10 +274,6 @@ async fn entry() -> Result<()> { resolutions.push(await_task(task).await??); } - for resolution in &resolutions { - resolution.print(&opts); - } - uithread.confirm().await?; resolutions @@ -301,9 +297,6 @@ async fn entry() -> Result<()> { tokio::spawn(async move { let resolution = await_task(task).await??; - - resolution.print(&opts); - install(resolution, opts, temp_dir_path, target).await }) }) @@ -454,7 +447,7 @@ async fn resolve( fetchers.add(QuickInstall::new(&client, &fetcher_data).await); } - match fetchers.first_available().await { + let resolution = match fetchers.first_available().await { Some(fetcher) => { // Build final metadata let fetcher_target = fetcher.target(); @@ -476,17 +469,21 @@ async fn resolve( install_path.to_path_buf(), )?; - Ok(Resolution::Fetch { + Resolution::Fetch { fetcher, package, crate_name, version, bin_path, bin_files, - }) + } } - None => Ok(Resolution::InstallFromSource { package }), - } + None => Resolution::InstallFromSource { package }, + }; + + resolution.print(&opts); + + Ok(resolution) } fn collect_bin_files( From 79ec12264700685d63ca8c055417c807ad58d44f Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 17:25:35 +1000 Subject: [PATCH 15/33] Refactor `entry`: Avoid `Arc::clone` Signed-off-by: Jiahao XU --- src/main.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main.rs b/src/main.rs index 738985ae..a324108a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -261,7 +261,7 @@ async fn entry() -> Result<()> { .collect(); let desired_targets = desired_targets.get().await; - let target = Arc::from( + let target: Arc = Arc::from( desired_targets .first() .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))? @@ -281,9 +281,9 @@ async fn entry() -> Result<()> { .map(|resolution| { tokio::spawn(install( resolution, - Arc::clone(&opts), - Arc::clone(&temp_dir_path), - Arc::clone(&target), + opts.clone(), + temp_dir_path.clone(), + target.clone(), )) }) .collect() @@ -291,9 +291,9 @@ async fn entry() -> Result<()> { tasks .into_iter() .map(|task| { - let opts = Arc::clone(&opts); - let temp_dir_path = Arc::clone(&temp_dir_path); - let target = Arc::clone(&target); + let opts = opts.clone(); + let temp_dir_path = temp_dir_path.clone(); + let target = target.clone(); tokio::spawn(async move { let resolution = await_task(task).await??; From 9552e0e8eddaec767b3cf61e54c0c5e502995311 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 17:31:29 +1000 Subject: [PATCH 16/33] Add comment to `entry` to improve readbility Signed-off-by: Jiahao XU --- src/main.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main.rs b/src/main.rs index a324108a..7ed9199b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -225,8 +225,10 @@ async fn entry() -> Result<()> { ) .unwrap(); + // Initialize UI thread let mut uithread = UIThread::new(!opts.no_confirm); + // Launch target detection let desired_targets = get_desired_targets(&opts.targets); // Compute install directory @@ -245,6 +247,7 @@ async fn entry() -> Result<()> { let temp_dir_path: Arc = Arc::from(temp_dir.path()); + // Resolve crates let tasks: Vec<_> = crate_names .into_iter() .map(|crate_name| { @@ -269,6 +272,7 @@ async fn entry() -> Result<()> { ); let tasks: Vec<_> = if !opts.dry_run && !opts.no_confirm { + // Confirm let mut resolutions = Vec::with_capacity(tasks.len()); for task in tasks { resolutions.push(await_task(task).await??); @@ -276,6 +280,7 @@ async fn entry() -> Result<()> { uithread.confirm().await?; + // Install resolutions .into_iter() .map(|resolution| { @@ -288,6 +293,7 @@ async fn entry() -> Result<()> { }) .collect() } else { + // Install without confirm tasks .into_iter() .map(|task| { From 8cc085b1b6371f8ff031a1b2e521ccd21136fd96 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 20:46:44 +1000 Subject: [PATCH 17/33] Add new dep jobserver v0.1.24 Signed-off-by: Jiahao XU --- Cargo.lock | 1 + Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 34d280f4..0ab1f24e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -144,6 +144,7 @@ dependencies = [ "futures-util", "guess_host_triple", "home", + "jobserver", "log", "miette", "mimalloc", diff --git a/Cargo.toml b/Cargo.toml index 334e39d0..7fa926ff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ dirs = "4.0.0" flate2 = { version = "1.0.24", features = ["zlib-ng"], default-features = false } futures-util = { version = "0.3.21", default-features = false } home = "0.5.3" +jobserver = "0.1.24" log = "0.4.14" miette = { version = "5.1.1", features = ["fancy-no-backtrace"] } mimalloc = { version = "0.1.29", default-features = false, optional = true } From c67c59b3ca577e0da02b9f38dce852eefca01eff Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 21:04:36 +1000 Subject: [PATCH 18/33] Impl new fn `helpers::create_jobserver_client` Signed-off-by: Jiahao XU --- src/helpers.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/helpers.rs b/src/helpers.rs index 2732f1e7..c442e478 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -1,7 +1,9 @@ use std::fmt::Debug; use std::fs; use std::io; +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; +use std::thread::available_parallelism; use bytes::Bytes; use cargo_toml::Manifest; @@ -42,6 +44,20 @@ pub async fn await_task(task: tokio::task::JoinHandle) -> miette::Result Result { + use jobserver::Client; + + // Safety: + // + // Client::from_env is unsafe because from_raw_fd is unsafe. + if let Some(client) = unsafe { Client::from_env() } { + Ok(client) + } else { + let ncore = available_parallelism().map(NonZeroUsize::get).unwrap_or(1); + Ok(Client::new(ncore)?) + } +} + /// Load binstall metadata from the crate `Cargo.toml` at the provided path pub fn load_manifest_path>( manifest_path: P, From fb0a6a551405810f502e806b59f396a9b7dda4aa Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 18 Jul 2022 21:12:58 +1000 Subject: [PATCH 19/33] Use jobserver to limit parallism of `cargo-install` Since we execute multiple `cargo-install` concurrently, we must use jobserver to limit the parallism so that they won't spawn too much processes/threads to overload the system. Signed-off-by: Jiahao XU --- src/main.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index 7ed9199b..ae327319 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,6 +3,7 @@ use std::{ ffi::OsString, mem::take, path::{Path, PathBuf}, + process, process::{ExitCode, Termination}, sync::Arc, time::{Duration, Instant}, @@ -208,6 +209,9 @@ async fn entry() -> Result<()> { let crate_names = take(&mut opts.crate_names); let opts = Arc::new(opts); + // Create jobserver client + let jobserver_client = create_jobserver_client()?; + // Initialize reqwest client let client = create_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?; @@ -289,6 +293,7 @@ async fn entry() -> Result<()> { opts.clone(), temp_dir_path.clone(), target.clone(), + jobserver_client.clone(), )) }) .collect() @@ -300,10 +305,11 @@ async fn entry() -> Result<()> { let opts = opts.clone(); let temp_dir_path = temp_dir_path.clone(); let target = target.clone(); + let jobserver_client = jobserver_client.clone(); tokio::spawn(async move { let resolution = await_task(task).await??; - install(resolution, opts, temp_dir_path, target).await + install(resolution, opts, temp_dir_path, target, jobserver_client).await }) }) .collect() @@ -540,6 +546,7 @@ async fn install( opts: Arc, temp_dir: Arc, target: Arc, + jobserver_client: jobserver::Client, ) -> Result<()> { match resolution { Resolution::Fetch { @@ -557,7 +564,7 @@ async fn install( } Resolution::InstallFromSource { package } => { if !opts.dry_run { - install_from_source(package, target).await + install_from_source(package, target, jobserver_client).await } else { info!( "Dry-run: running `cargo install {} --version {} --target {target}`", @@ -667,12 +674,19 @@ async fn install_from_package( }) } -async fn install_from_source(package: Package, target: Arc) -> Result<()> { +async fn install_from_source( + package: Package, + target: Arc, + jobserver_client: jobserver::Client, +) -> Result<()> { debug!( "Running `cargo install {} --version {} --target {target}`", package.name, package.version ); - let mut child = Command::new("cargo") + let mut command = process::Command::new("cargo"); + jobserver_client.configure(&mut command); + + let mut child = Command::from(command) .arg("install") .arg(package.name) .arg("--version") From 072253ebaede3ea8e2c9522ad03d20762ca731ff Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 00:32:56 +1000 Subject: [PATCH 20/33] Improve comment in `create_jobserver_client` Signed-off-by: Jiahao XU --- src/helpers.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/helpers.rs b/src/helpers.rs index c442e478..3e714c45 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -50,6 +50,8 @@ pub fn create_jobserver_client() -> Result { // Safety: // // Client::from_env is unsafe because from_raw_fd is unsafe. + // It doesn't do anything that is actually unsafe, like + // dereferencing pointer. if let Some(client) = unsafe { Client::from_env() } { Ok(client) } else { From b223990bb1fde7e8fcf6c716eaaba80ea55bb30e Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 00:35:48 +1000 Subject: [PATCH 21/33] Simplify `helpers::await_task` API Signed-off-by: Jiahao XU --- src/helpers.rs | 8 +++++--- src/main.rs | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 3e714c45..5ad0e7d6 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -39,9 +39,11 @@ pub use tls_version::TLSVersion; mod crate_name; pub use crate_name::CrateName; -pub async fn await_task(task: tokio::task::JoinHandle) -> miette::Result { - task.await - .map_err(|join_err| miette::miette!("Task failed to join: {}", join_err)) +pub async fn await_task(task: tokio::task::JoinHandle>) -> miette::Result { + match task.await { + Ok(res) => res, + Err(join_err) => Err(miette::miette!("Task failed to join: {}", join_err)), + } } pub fn create_jobserver_client() -> Result { diff --git a/src/main.rs b/src/main.rs index ae327319..62071031 100644 --- a/src/main.rs +++ b/src/main.rs @@ -279,7 +279,7 @@ async fn entry() -> Result<()> { // Confirm let mut resolutions = Vec::with_capacity(tasks.len()); for task in tasks { - resolutions.push(await_task(task).await??); + resolutions.push(await_task(task).await?); } uithread.confirm().await?; @@ -308,7 +308,7 @@ async fn entry() -> Result<()> { let jobserver_client = jobserver_client.clone(); tokio::spawn(async move { - let resolution = await_task(task).await??; + let resolution = await_task(task).await?; install(resolution, opts, temp_dir_path, target, jobserver_client).await }) }) @@ -316,7 +316,7 @@ async fn entry() -> Result<()> { }; for task in tasks { - await_task(task).await??; + await_task(task).await?; } if !opts.no_cleanup { From c6281d8ea0e7203d3ea2b1f22bad27562bbcb171 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 00:39:27 +1000 Subject: [PATCH 22/33] Fix `opts.no_cleanup` behavior Signed-off-by: Jiahao XU --- src/main.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 62071031..d5d3c0d2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -319,7 +319,10 @@ async fn entry() -> Result<()> { await_task(task).await?; } - if !opts.no_cleanup { + if opts.no_cleanup { + // Consume temp_dir without removing it from fs. + temp_dir.into_path(); + } else { temp_dir.close().unwrap_or_else(|err| { warn!("Failed to clean up some resources: {err}"); }); From b026462018d905aedd5399e2b22438d8c3c4da89 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 00:43:17 +1000 Subject: [PATCH 23/33] Refactor: Simplify struct `Resolution` Signed-off-by: Jiahao XU --- src/main.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main.rs b/src/main.rs index d5d3c0d2..9302f17c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -335,7 +335,7 @@ enum Resolution { Fetch { fetcher: Arc, package: Package, - crate_name: CrateName, + name: String, version: String, bin_path: PathBuf, bin_files: Vec, @@ -487,7 +487,7 @@ async fn resolve( Resolution::Fetch { fetcher, package, - crate_name, + name: crate_name.name, version, bin_path, bin_files, @@ -555,13 +555,13 @@ async fn install( Resolution::Fetch { fetcher, package, - crate_name, + name, version, bin_path, bin_files, } => { install_from_package( - fetcher, opts, package, crate_name, temp_dir, version, bin_path, bin_files, + fetcher, opts, package, name, temp_dir, version, bin_path, bin_files, ) .await } @@ -584,7 +584,7 @@ async fn install_from_package( fetcher: Arc, opts: Arc, package: Package, - crate_name: CrateName, + name: String, temp_dir: Arc, version: String, bin_path: PathBuf, @@ -627,7 +627,7 @@ async fn install_from_package( } let cvs = metafiles::CrateVersionSource { - name: crate_name.name.clone(), + name: name.clone(), version: package.version.parse().into_diagnostic()?, source: metafiles::Source::Registry( url::Url::parse("https://github.com/rust-lang/crates.io-index").unwrap(), From d58f340a45fcd167343460994717b01a420bb1d4 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 00:45:15 +1000 Subject: [PATCH 24/33] Test batch installtion in `run_tests_unix.sh` Signed-off-by: Jiahao XU --- ci-scripts/run_tests_unix.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ci-scripts/run_tests_unix.sh b/ci-scripts/run_tests_unix.sh index 9c3ca0b7..f07f89f7 100755 --- a/ci-scripts/run_tests_unix.sh +++ b/ci-scripts/run_tests_unix.sh @@ -6,9 +6,8 @@ bins="cargo-deb cargo-llvm-cov cargo-binstall" test_bins="cargo-deb cargo-llvm-cov" # Install binaries using cargo-binstall -for bin in $bins; do - "./$1" binstall --log-level debug --no-confirm "$bin" -done +# shellcheck disable=SC2086 +"./$1" binstall --log-level debug --no-confirm $bins # Test that the installed binaries can be run for bin in $test_bins; do From 758dab7d4f6990a72d9f5794e3d6ca7989174848 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 11:59:10 +1000 Subject: [PATCH 25/33] Optimize `DesiredTargets`: Avoid mem alloc on `clone` Signed-off-by: Jiahao XU --- src/target.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/target.rs b/src/target.rs index fdca606f..c6bf8dd9 100644 --- a/src/target.rs +++ b/src/target.rs @@ -11,7 +11,7 @@ pub const TARGET: &str = env!("TARGET"); #[derive(Debug, Clone)] enum DesiredTargetsInner { AutoDetect(Arc>>), - Initialized(Vec), + Initialized(Arc>), } #[derive(Debug, Clone)] @@ -19,7 +19,7 @@ pub struct DesiredTargets(DesiredTargetsInner); impl DesiredTargets { fn initialized(targets: Vec) -> Self { - Self(DesiredTargetsInner::Initialized(targets)) + Self(DesiredTargetsInner::Initialized(Arc::new(targets))) } fn auto_detect() -> Self { From de7ecad32c5b2843fa78f3b5f920207963b2b6d7 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 12:02:09 +1000 Subject: [PATCH 26/33] Optimize: Avoid creation of `Arc` for target in `entry` Signed-off-by: Jiahao XU --- src/main.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/main.rs b/src/main.rs index 9302f17c..1493729a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -267,14 +267,6 @@ async fn entry() -> Result<()> { }) .collect(); - let desired_targets = desired_targets.get().await; - let target: Arc = Arc::from( - desired_targets - .first() - .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))? - .as_str(), - ); - let tasks: Vec<_> = if !opts.dry_run && !opts.no_confirm { // Confirm let mut resolutions = Vec::with_capacity(tasks.len()); @@ -292,7 +284,7 @@ async fn entry() -> Result<()> { resolution, opts.clone(), temp_dir_path.clone(), - target.clone(), + desired_targets.clone(), jobserver_client.clone(), )) }) @@ -304,12 +296,19 @@ async fn entry() -> Result<()> { .map(|task| { let opts = opts.clone(); let temp_dir_path = temp_dir_path.clone(); - let target = target.clone(); + let desired_target = desired_targets.clone(); let jobserver_client = jobserver_client.clone(); tokio::spawn(async move { let resolution = await_task(task).await?; - install(resolution, opts, temp_dir_path, target, jobserver_client).await + install( + resolution, + opts, + temp_dir_path, + desired_target, + jobserver_client, + ) + .await }) }) .collect() @@ -548,7 +547,7 @@ async fn install( resolution: Resolution, opts: Arc, temp_dir: Arc, - target: Arc, + desired_targets: DesiredTargets, jobserver_client: jobserver::Client, ) -> Result<()> { match resolution { @@ -566,6 +565,11 @@ async fn install( .await } Resolution::InstallFromSource { package } => { + let desired_targets = desired_targets.get().await; + let target = desired_targets + .first() + .ok_or_else(|| miette!("No viable targets found, try with `--targets`"))?; + if !opts.dry_run { install_from_source(package, target, jobserver_client).await } else { @@ -679,7 +683,7 @@ async fn install_from_package( async fn install_from_source( package: Package, - target: Arc, + target: &str, jobserver_client: jobserver::Client, ) -> Result<()> { debug!( From 2bf7640729fe525a536acea2a28754106203ad7a Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 12:06:16 +1000 Subject: [PATCH 27/33] Optimize: Avoid double `spawn` if `no_confirm` Signed-off-by: Jiahao XU --- src/main.rs | 54 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/src/main.rs b/src/main.rs index 1493729a..d2cf94b6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -251,23 +251,23 @@ async fn entry() -> Result<()> { let temp_dir_path: Arc = Arc::from(temp_dir.path()); - // Resolve crates - let tasks: Vec<_> = crate_names - .into_iter() - .map(|crate_name| { - tokio::spawn(resolve( - opts.clone(), - crate_name, - desired_targets.clone(), - cli_overrides.clone(), - temp_dir_path.clone(), - install_path.clone(), - client.clone(), - )) - }) - .collect(); - let tasks: Vec<_> = if !opts.dry_run && !opts.no_confirm { + // Resolve crates + let tasks: Vec<_> = crate_names + .into_iter() + .map(|crate_name| { + tokio::spawn(resolve( + opts.clone(), + crate_name, + desired_targets.clone(), + cli_overrides.clone(), + temp_dir_path.clone(), + install_path.clone(), + client.clone(), + )) + }) + .collect(); + // Confirm let mut resolutions = Vec::with_capacity(tasks.len()); for task in tasks { @@ -290,17 +290,31 @@ async fn entry() -> Result<()> { }) .collect() } else { - // Install without confirm - tasks + // Resolve crates and install without confirmation + crate_names .into_iter() - .map(|task| { + .map(|crate_name| { let opts = 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 install_path = install_path.clone(); tokio::spawn(async move { - let resolution = await_task(task).await?; + let resolution = resolve( + opts.clone(), + crate_name, + desired_targets.clone(), + cli_overrides, + temp_dir_path.clone(), + install_path, + client, + ) + .await?; + install( resolution, opts, From 67ca36a0b55d1b04ca078faecc7b4a6a52706ed6 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 19 Jul 2022 12:15:00 +1000 Subject: [PATCH 28/33] Fix `jobserver_client`: Create it as early as possible Signed-off-by: Jiahao XU --- src/main.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index d2cf94b6..cea320ce 100644 --- a/src/main.rs +++ b/src/main.rs @@ -171,10 +171,16 @@ impl Termination for MainExit { } fn main() -> MainExit { + // Create jobserver client + let jobserver_client = match create_jobserver_client() { + Ok(jobserver_client) => jobserver_client, + Err(binstall_err) => return MainExit::Error(binstall_err), + }; + let start = Instant::now(); let rt = Runtime::new().unwrap(); - let handle = rt.spawn(entry()); + let handle = rt.spawn(entry(jobserver_client)); let result = rt.block_on(handle); drop(rt); @@ -190,7 +196,7 @@ fn main() -> MainExit { }) } -async fn entry() -> Result<()> { +async fn entry(jobserver_client: jobserver::Client) -> Result<()> { // Filter extraneous arg when invoked by cargo // `cargo run -- --help` gives ["target/debug/cargo-binstall", "--help"] // `cargo binstall --help` gives ["/home/ryan/.cargo/bin/cargo-binstall", "binstall", "--help"] @@ -209,9 +215,6 @@ async fn entry() -> Result<()> { let crate_names = take(&mut opts.crate_names); let opts = Arc::new(opts); - // Create jobserver client - let jobserver_client = create_jobserver_client()?; - // Initialize reqwest client let client = create_reqwest_client(opts.secure, opts.min_tls_version.map(|v| v.into()))?; From 3961dbb84a34cea827b1eb40839e95df23d1fe16 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 20 Jul 2022 16:43:02 +1000 Subject: [PATCH 29/33] Add new dep `once_cell` v1.13.0 Signed-off-by: Jiahao XU --- Cargo.lock | 5 +++-- Cargo.toml | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0ab1f24e..f181cfcd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -148,6 +148,7 @@ dependencies = [ "log", "miette", "mimalloc", + "once_cell", "reqwest", "scopeguard", "semver", @@ -952,9 +953,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.12.0" +version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7709cef83f0c1f58f666e746a08b21e0085f7440fa6a29cc194d68aac97a4225" +checksum = "18a6dbe30758c9f83eb00cbea4ac95966305f5a7772f3f42ebfc7fc7eddbd8e1" [[package]] name = "os_str_bytes" diff --git a/Cargo.toml b/Cargo.toml index 7fa926ff..1a4699a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ jobserver = "0.1.24" log = "0.4.14" miette = { version = "5.1.1", features = ["fancy-no-backtrace"] } mimalloc = { version = "0.1.29", default-features = false, optional = true } +once_cell = "1.13.0" reqwest = { version = "0.11.11", features = ["rustls-tls", "stream"], default-features = false } scopeguard = "1.1.0" semver = "1.0.12" From 5bdffd9178805899c8d06a6d060778f32d8abec7 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 20 Jul 2022 16:48:34 +1000 Subject: [PATCH 30/33] Refactor: Impl `Source::cratesio_registry` Makes initializing `metafiles::CrateVersionSource` more readable and improves performance since the parsing is now cached. Signed-off-by: Jiahao XU --- src/main.rs | 4 +--- src/metafiles/cvs.rs | 10 ++++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index cea320ce..9ed8f67f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -650,9 +650,7 @@ async fn install_from_package( let cvs = metafiles::CrateVersionSource { name: name.clone(), version: package.version.parse().into_diagnostic()?, - source: metafiles::Source::Registry( - url::Url::parse("https://github.com/rust-lang/crates.io-index").unwrap(), - ), + source: metafiles::Source::cratesio_registry(), }; info!("Installing binaries..."); diff --git a/src/metafiles/cvs.rs b/src/metafiles/cvs.rs index a111d02f..2b8f2214 100644 --- a/src/metafiles/cvs.rs +++ b/src/metafiles/cvs.rs @@ -1,6 +1,7 @@ use std::{fmt, str::FromStr}; use miette::Diagnostic; +use once_cell::sync::Lazy; use semver::Version; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use thiserror::Error; @@ -20,6 +21,15 @@ pub enum Source { Registry(Url), } +impl Source { + pub fn cratesio_registry() -> Source { + static CRATESIO: Lazy Url> = + Lazy::new(|| url::Url::parse("https://github.com/rust-lang/crates.io-index").unwrap()); + + Self::Registry(CRATESIO.clone()) + } +} + impl FromStr for CrateVersionSource { type Err = CvsParseError; fn from_str(s: &str) -> Result { From 1ebd4bdb75f268034d1c6a4e7529846a104158a8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 20 Jul 2022 16:58:12 +1000 Subject: [PATCH 31/33] Refactor: Reduce params of `install_from_package` Signed-off-by: Jiahao XU --- src/main.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/main.rs b/src/main.rs index 9ed8f67f..24d2f411 100644 --- a/src/main.rs +++ b/src/main.rs @@ -576,10 +576,13 @@ async fn install( bin_path, bin_files, } => { - install_from_package( - fetcher, opts, package, name, temp_dir, version, bin_path, bin_files, - ) - .await + let cvs = metafiles::CrateVersionSource { + name, + version: package.version.parse().into_diagnostic()?, + source: metafiles::Source::cratesio_registry(), + }; + + install_from_package(fetcher, opts, cvs, temp_dir, version, bin_path, bin_files).await } Resolution::InstallFromSource { package } => { let desired_targets = desired_targets.get().await; @@ -604,8 +607,7 @@ async fn install( async fn install_from_package( fetcher: Arc, opts: Arc, - package: Package, - name: String, + cvs: metafiles::CrateVersionSource, temp_dir: Arc, version: String, bin_path: PathBuf, @@ -647,12 +649,6 @@ async fn install_from_package( return Ok(()); } - let cvs = metafiles::CrateVersionSource { - name: name.clone(), - version: package.version.parse().into_diagnostic()?, - source: metafiles::Source::cratesio_registry(), - }; - info!("Installing binaries..."); block_in_place(|| { for file in &bin_files { From 1eedae1ee2ee575b9e1eee0bd0c6cfd591d2c677 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 20 Jul 2022 16:59:23 +1000 Subject: [PATCH 32/33] Rm unused param `temp_dir` from `install_from_package` Signed-off-by: Jiahao XU --- src/main.rs | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/main.rs b/src/main.rs index 24d2f411..2441f57a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -286,7 +286,6 @@ async fn entry(jobserver_client: jobserver::Client) -> Result<()> { tokio::spawn(install( resolution, opts.clone(), - temp_dir_path.clone(), desired_targets.clone(), jobserver_client.clone(), )) @@ -312,20 +311,13 @@ async fn entry(jobserver_client: jobserver::Client) -> Result<()> { crate_name, desired_targets.clone(), cli_overrides, - temp_dir_path.clone(), + temp_dir_path, install_path, client, ) .await?; - install( - resolution, - opts, - temp_dir_path, - desired_target, - jobserver_client, - ) - .await + install(resolution, opts, desired_target, jobserver_client).await }) }) .collect() @@ -563,7 +555,6 @@ fn collect_bin_files( async fn install( resolution: Resolution, opts: Arc, - temp_dir: Arc, desired_targets: DesiredTargets, jobserver_client: jobserver::Client, ) -> Result<()> { @@ -582,7 +573,7 @@ async fn install( source: metafiles::Source::cratesio_registry(), }; - install_from_package(fetcher, opts, cvs, temp_dir, version, bin_path, bin_files).await + install_from_package(fetcher, opts, cvs, version, bin_path, bin_files).await } Resolution::InstallFromSource { package } => { let desired_targets = desired_targets.get().await; @@ -603,12 +594,10 @@ async fn install( } } -#[allow(unused, clippy::too_many_arguments)] async fn install_from_package( fetcher: Arc, opts: Arc, cvs: metafiles::CrateVersionSource, - temp_dir: Arc, version: String, bin_path: PathBuf, bin_files: Vec, From 3b82e9e37595d6a7f93ea7ffaabeeda66f7a00d7 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 20 Jul 2022 17:02:20 +1000 Subject: [PATCH 33/33] Rm unnecessary `clone` in `install_from_package` Signed-off-by: Jiahao XU --- src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 2441f57a..e017db8d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -651,7 +651,7 @@ async fn install_from_package( } } - let bins: BTreeSet = bin_files.iter().map(|bin| bin.base_name.clone()).collect(); + let bins: BTreeSet = bin_files.into_iter().map(|bin| bin.base_name).collect(); { debug!("Writing .crates.toml"); @@ -664,7 +664,7 @@ async fn install_from_package( debug!("Writing .crates2.json"); let mut c2 = metafiles::v2::Crates2Json::load().unwrap_or_default(); c2.insert( - cvs.clone(), + cvs, metafiles::v2::CrateInfo { version_req: Some(version), bins,