From 50b6e62164e69b61c1a96a1fe5bf0dd5a82186f4 Mon Sep 17 00:00:00 2001
From: Jiahao XU <Jiahao_XU@outlook.com>
Date: Sat, 19 Nov 2022 18:00:27 +1100
Subject: [PATCH] Minor refactor and optimization (#543)
* Avoid potential panicking in `args::parse` by using `Vec::get` instead of indexing
* Refactor: Simplify `opts::{resolve, install}` API
Many parameters can be shared and put into `opts::Options` intead and
that would also avoid a few `Arc<Path>`.
* Optimize `get_install_path`: Avoid cloning `install_path`
* Optimize `LazyJobserverClient`: Un`Arc` & remove `Clone` impl
to avoid additional boxing
* Optimize `find_version`: Avoid cloning `semver::Version`
* Optimize `GhCrateMeta::launch_baseline_find_tasks`
return `impl Iterator<Item = impl Future<Output = ...>>`
instead of `impl Iterator<Item = AutoAbortJoinHandle<...>>`
to avoid unnecessary spawning.
Each task spawned has to be boxed and then polled by tokio runtime.
They might also be moved.
While they increase parallelism, spawning these futures does not justify
the costs because:
- Each `Future` only calls `remote_exists`
- Each `remote_exists` call send requests to the same domain, which is
likely to share the same http2 connection.
Since the conn is shared anyway, spawning does not speedup anything
but merely add communication overhead.
- Plus the tokio runtime spawning cost
* Optimize `install_crates`: Destruct `Args` before any `.await` point
to reduce size of the future
* Refactor `logging`: Replace param `arg` with `log_level` & `json_output`
to avoid dep on `Args`
* Add dep strum & strum_macros to crates/bin
* Derive `strum_macros::EnumCount` for `Strategy`
* Optimize strategies parsing in `install_crates`
* Fix panic in `install_crates` when `Compile` is not the last strategy specified
* Optimize: Take `Vec<Self>` instead of slice in `CrateName::dedup`
* Refactor: Extract new fn `compute_resolvers`
* Refactor: Extract new fn `compute_paths_and_load_manifests`
* Refactor: Extract new fn `filter_out_installed_crates`
* Reorder `install_crates`: Only run target detection if args are valid
and there are some crates to be installed.
* Optimize `filter_out_installed_crates`: Avoid allocation
by returning an `Iterator`
* Fix user_agent of `remote::Client`: Let user specify it
* Refactor: Replace `UIThread` with `ui::confirm`
which is much simpler.
Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
---
Cargo.lock | 2 +
crates/bin/Cargo.toml | 2 +
crates/bin/src/args.rs | 5 +-
crates/bin/src/entry.rs | 394 ++++++++++--------
crates/bin/src/install_path.rs | 15 +-
crates/bin/src/logging.rs | 8 +-
crates/bin/src/main.rs | 2 +-
crates/bin/src/ui.rs | 114 ++---
crates/binstalk-downloader/src/remote.rs | 45 +-
crates/binstalk/src/drivers/version.rs | 2 +-
crates/binstalk/src/fetchers/gh_crate_meta.rs | 13 +-
.../binstalk/src/helpers/jobserver_client.rs | 7 +-
crates/binstalk/src/ops.rs | 13 +-
crates/binstalk/src/ops/install.rs | 5 +-
crates/binstalk/src/ops/resolve.rs | 39 +-
crates/binstalk/src/ops/resolve/crate_name.rs | 7 +-
16 files changed, 325 insertions(+), 348 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index 8f56eacd..07f46c2e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -295,6 +295,8 @@ dependencies = [
"mimalloc",
"once_cell",
"semver",
+ "strum",
+ "strum_macros",
"supports-color",
"tempfile",
"tokio",
diff --git a/crates/bin/Cargo.toml b/crates/bin/Cargo.toml
index d5ea20d2..546017b5 100644
--- a/crates/bin/Cargo.toml
+++ b/crates/bin/Cargo.toml
@@ -32,6 +32,8 @@ miette = "5.4.1"
mimalloc = { version = "0.1.32", default-features = false, optional = true }
once_cell = "1.16.0"
semver = "1.0.14"
+strum = "0.24.1"
+strum_macros = "0.24.3"
supports-color = "1.3.1"
tempfile = "3.3.0"
tokio = { version = "1.21.2", features = ["rt-multi-thread"], default-features = false }
diff --git a/crates/bin/src/args.rs b/crates/bin/src/args.rs
index 571ba0f8..0265c974 100644
--- a/crates/bin/src/args.rs
+++ b/crates/bin/src/args.rs
@@ -15,6 +15,7 @@ use binstalk::{
use clap::{Parser, ValueEnum};
use log::LevelFilter;
use semver::VersionReq;
+use strum_macros::EnumCount;
#[derive(Debug, Parser)]
#[clap(
@@ -296,7 +297,7 @@ impl Default for RateLimit {
}
/// Strategy for installing the package
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, ValueEnum)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, ValueEnum, EnumCount)]
pub enum Strategy {
/// Attempt to download official pre-built artifacts using
/// information provided in `Cargo.toml`.
@@ -312,7 +313,7 @@ pub fn parse() -> Result<Args, BinstallError> {
// `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();
- let args = if args.len() > 1 && args[1] == "binstall" {
+ let args = if args.get(1).map(|arg| arg == "binstall").unwrap_or_default() {
// Equivalent to
//
// args.remove(1);
diff --git a/crates/bin/src/entry.rs b/crates/bin/src/entry.rs
index 624d82b1..e41efdbd 100644
--- a/crates/bin/src/entry.rs
+++ b/crates/bin/src/entry.rs
@@ -1,4 +1,4 @@
-use std::{fs, mem, path::Path, sync::Arc, time::Duration};
+use std::{fs, path::PathBuf, sync::Arc, time::Duration};
use binstalk::{
errors::BinstallError,
@@ -8,27 +8,197 @@ use binstalk::{
ops::{
self,
resolve::{CrateName, Resolution, VersionReqExt},
+ Resolver,
},
};
use binstalk_manifests::{
binstall_crates_v1::Records, cargo_crates_v1::CratesToml, cargo_toml_binstall::PkgOverride,
};
+use crates_io_api::AsyncClient as CratesIoApiClient;
use log::LevelFilter;
use miette::{miette, Result, WrapErr};
+use strum::EnumCount;
use tokio::task::block_in_place;
use tracing::{debug, error, info, warn};
use crate::{
args::{Args, Strategy},
install_path,
- ui::UIThread,
+ ui::confirm,
};
-pub async fn install_crates(mut args: Args, jobserver_client: LazyJobserverClient) -> Result<()> {
+pub async fn install_crates(args: Args, jobserver_client: LazyJobserverClient) -> Result<()> {
+ // Compute strategies
+ let (resolvers, cargo_install_fallback) =
+ compute_resolvers(args.strategies, args.disable_strategies)?;
+
+ // Compute paths
+ let (install_path, cargo_roots, metadata, temp_dir) =
+ compute_paths_and_load_manifests(args.roots, args.install_path)?;
+
+ // Remove installed crates
+ let mut crate_names =
+ filter_out_installed_crates(args.crate_names, args.force, metadata.as_ref()).peekable();
+
+ if crate_names.peek().is_none() {
+ debug!("Nothing to do");
+ return Ok(());
+ }
+
+ // Launch target detection
+ let desired_targets = get_desired_targets(args.targets);
+
+ // Computer cli_overrides
+ let cli_overrides = PkgOverride {
+ pkg_url: args.pkg_url,
+ pkg_fmt: args.pkg_fmt,
+ bin_dir: args.bin_dir,
+ };
+
+ // Initialize reqwest client
+ let rate_limit = args.rate_limit;
+
+ let client = Client::new(
+ concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION")),
+ args.min_tls_version.map(|v| v.into()),
+ Duration::from_millis(rate_limit.duration.get()),
+ rate_limit.request_count,
+ )
+ .map_err(BinstallError::from)?;
+
+ // Build crates.io api client
+ let crates_io_api_client =
+ CratesIoApiClient::with_http_client(client.get_inner().clone(), Duration::from_millis(100));
+
+ // Create binstall_opts
+ let binstall_opts = Arc::new(ops::Options {
+ no_symlinks: args.no_symlinks,
+ dry_run: args.dry_run,
+ force: args.force,
+ quiet: args.log_level == LevelFilter::Off,
+
+ version_req: args.version_req,
+ manifest_path: args.manifest_path,
+ cli_overrides,
+
+ desired_targets,
+ resolvers,
+ cargo_install_fallback,
+
+ temp_dir: temp_dir.path().to_owned(),
+ install_path,
+ client,
+ crates_io_api_client,
+ jobserver_client,
+ });
+
+ // Destruct args before any async function to reduce size of the future
+ let dry_run = args.dry_run;
+ let no_confirm = args.no_confirm;
+ let no_cleanup = args.no_cleanup;
+
+ let tasks: Vec<_> = if !dry_run && !no_confirm {
+ // Resolve crates
+ let tasks: Vec<_> = crate_names
+ .map(|(crate_name, current_version)| {
+ AutoAbortJoinHandle::spawn(ops::resolve::resolve(
+ binstall_opts.clone(),
+ crate_name,
+ current_version,
+ ))
+ })
+ .collect();
+
+ // Confirm
+ let mut resolutions = Vec::with_capacity(tasks.len());
+ for task in tasks {
+ match task.await?? {
+ Resolution::AlreadyUpToDate => {}
+ res => resolutions.push(res),
+ }
+ }
+
+ if resolutions.is_empty() {
+ debug!("Nothing to do");
+ return Ok(());
+ }
+
+ confirm().await?;
+
+ // Install
+ resolutions
+ .into_iter()
+ .map(|resolution| {
+ AutoAbortJoinHandle::spawn(ops::install::install(resolution, binstall_opts.clone()))
+ })
+ .collect()
+ } else {
+ // Resolve crates and install without confirmation
+ crate_names
+ .map(|(crate_name, current_version)| {
+ let opts = binstall_opts.clone();
+
+ AutoAbortJoinHandle::spawn(async move {
+ let resolution =
+ ops::resolve::resolve(opts.clone(), crate_name, current_version).await?;
+
+ ops::install::install(resolution, opts).await
+ })
+ })
+ .collect()
+ };
+
+ let mut metadata_vec = Vec::with_capacity(tasks.len());
+ for task in tasks {
+ if let Some(metadata) = task.await?? {
+ metadata_vec.push(metadata);
+ }
+ }
+
+ block_in_place(|| {
+ if let Some(mut records) = metadata {
+ // The cargo manifest path is already created when loading
+ // metadata.
+
+ debug!("Writing .crates.toml");
+ CratesToml::append_to_path(cargo_roots.join(".crates.toml"), metadata_vec.iter())?;
+
+ debug!("Writing binstall/crates-v1.json");
+ for metadata in metadata_vec {
+ records.replace(metadata);
+ }
+ records.overwrite()?;
+ }
+
+ if 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}");
+ });
+ }
+
+ Ok(())
+ })
+}
+
+/// Return (resolvers, cargo_install_fallback)
+fn compute_resolvers(
+ input_strategies: Vec<Strategy>,
+ mut disable_strategies: Vec<Strategy>,
+) -> Result<(Vec<Resolver>, bool), BinstallError> {
+ // Compute strategies
let mut strategies = vec![];
// Remove duplicate strategies
- for strategy in mem::take(&mut args.strategies) {
+ for strategy in input_strategies {
+ if strategies.len() == Strategy::COUNT {
+ // All variants of Strategy is present in strategies,
+ // there is no need to continue since all the remaining
+ // args.strategies must be present in stratetgies.
+ break;
+ }
if !strategies.contains(&strategy) {
strategies.push(strategy);
}
@@ -43,9 +213,12 @@ pub async fn install_crates(mut args: Args, jobserver_client: LazyJobserverClien
];
}
- let disable_strategies = mem::take(&mut args.disable_strategies);
-
let mut strategies: Vec<Strategy> = if !disable_strategies.is_empty() {
+ // Since order doesn't matter, we can sort it and remove all duplicates
+ // to speedup checking.
+ disable_strategies.sort_unstable();
+ disable_strategies.dedup();
+
strategies
.into_iter()
.filter(|strategy| !disable_strategies.contains(strategy))
@@ -55,7 +228,7 @@ pub async fn install_crates(mut args: Args, jobserver_client: LazyJobserverClien
};
if strategies.is_empty() {
- return Err(BinstallError::InvalidStrategies(&"No strategy is provided").into());
+ return Err(BinstallError::InvalidStrategies(&"No strategy is provided"));
}
let cargo_install_fallback = *strategies.last().unwrap() == Strategy::Compile;
@@ -64,54 +237,35 @@ pub async fn install_crates(mut args: Args, jobserver_client: LazyJobserverClien
strategies.pop().unwrap();
}
- let resolvers: Vec<_> = strategies
+ let resolvers = strategies
.into_iter()
.map(|strategy| match strategy {
- Strategy::CrateMetaData => GhCrateMeta::new,
- Strategy::QuickInstall => QuickInstall::new,
- Strategy::Compile => unreachable!(),
+ Strategy::CrateMetaData => Ok(GhCrateMeta::new as Resolver),
+ Strategy::QuickInstall => Ok(QuickInstall::new as Resolver),
+ Strategy::Compile => Err(BinstallError::InvalidStrategies(
+ &"Compile strategy must be the last one",
+ )),
})
- .collect();
+ .collect::<Result<Vec<_>, BinstallError>>()?;
- let cli_overrides = PkgOverride {
- pkg_url: args.pkg_url.take(),
- pkg_fmt: args.pkg_fmt.take(),
- bin_dir: args.bin_dir.take(),
- };
+ Ok((resolvers, cargo_install_fallback))
+}
- // Launch target detection
- let desired_targets = get_desired_targets(args.targets.take());
-
- let rate_limit = args.rate_limit;
-
- // Initialize reqwest client
- let client = Client::new(
- args.min_tls_version.map(|v| v.into()),
- Duration::from_millis(rate_limit.duration.get()),
- rate_limit.request_count,
- )
- .map_err(BinstallError::from)?;
-
- // Build crates.io api client
- let crates_io_api_client = crates_io_api::AsyncClient::with_http_client(
- client.get_inner().clone(),
- Duration::from_millis(100),
- );
-
- // Initialize UI thread
- let mut uithread = UIThread::new(!args.no_confirm);
-
- let (install_path, cargo_roots, metadata, temp_dir) = block_in_place(|| -> Result<_> {
+/// Return (install_path, cargo_roots, metadata, temp_dir)
+fn compute_paths_and_load_manifests(
+ roots: Option<PathBuf>,
+ install_path: Option<PathBuf>,
+) -> Result<(PathBuf, PathBuf, Option<Records>, tempfile::TempDir)> {
+ block_in_place(|| {
// Compute cargo_roots
- let cargo_roots =
- install_path::get_cargo_roots_path(args.roots.take()).ok_or_else(|| {
- error!("No viable cargo roots path found of specified, try `--roots`");
- miette!("No cargo roots path found or specified")
- })?;
+ let cargo_roots = install_path::get_cargo_roots_path(roots).ok_or_else(|| {
+ error!("No viable cargo roots path found of specified, try `--roots`");
+ miette!("No cargo roots path found or specified")
+ })?;
// Compute install directory
let (install_path, custom_install_path) =
- install_path::get_install_path(args.install_path.as_deref(), Some(&cargo_roots));
+ install_path::get_install_path(install_path, Some(&cargo_roots));
let install_path = install_path.ok_or_else(|| {
error!("No viable install path found of specified, try `--install-path`");
miette!("No install path found or specified")
@@ -143,14 +297,20 @@ pub async fn install_crates(mut args: Args, jobserver_client: LazyJobserverClien
.wrap_err("Creating a temporary directory failed.")?;
Ok((install_path, cargo_roots, metadata, temp_dir))
- })?;
+ })
+}
- // Remove installed crates
- let crate_names = CrateName::dedup(&args.crate_names)
- .filter_map(|crate_name| {
+/// Return vec of (crate_name, current_version)
+fn filter_out_installed_crates(
+ crate_names: Vec<CrateName>,
+ force: bool,
+ metadata: Option<&Records>,
+) -> impl Iterator<Item = (CrateName, Option<semver::Version>)> + '_ {
+ CrateName::dedup(crate_names)
+ .filter_map(move |crate_name| {
match (
- args.force,
- metadata.as_ref().and_then(|records| records.get(&crate_name.name)),
+ force,
+ metadata.and_then(|records| records.get(&crate_name.name)),
&crate_name.version_req,
) {
(false, Some(metadata), Some(version_req))
@@ -173,134 +333,4 @@ pub async fn install_crates(mut args: Args, jobserver_client: LazyJobserverClien
_ => Some((crate_name, None)),
}
})
- .collect::<Vec<_>>();
-
- if crate_names.is_empty() {
- debug!("Nothing to do");
- return Ok(());
- }
-
- let temp_dir_path: Arc<Path> = Arc::from(temp_dir.path());
-
- // Create binstall_opts
- let binstall_opts = Arc::new(ops::Options {
- no_symlinks: args.no_symlinks,
- dry_run: args.dry_run,
- force: args.force,
- version_req: args.version_req.take(),
- manifest_path: args.manifest_path.take(),
- cli_overrides,
- desired_targets,
- quiet: args.log_level == LevelFilter::Off,
- resolvers,
- cargo_install_fallback,
- });
-
- let tasks: Vec<_> = if !args.dry_run && !args.no_confirm {
- // Resolve crates
- let tasks: Vec<_> = crate_names
- .into_iter()
- .map(|(crate_name, current_version)| {
- AutoAbortJoinHandle::spawn(ops::resolve::resolve(
- binstall_opts.clone(),
- crate_name,
- current_version,
- temp_dir_path.clone(),
- install_path.clone(),
- client.clone(),
- crates_io_api_client.clone(),
- ))
- })
- .collect();
-
- // Confirm
- let mut resolutions = Vec::with_capacity(tasks.len());
- for task in tasks {
- match task.await?? {
- Resolution::AlreadyUpToDate => {}
- res => resolutions.push(res),
- }
- }
-
- if resolutions.is_empty() {
- debug!("Nothing to do");
- return Ok(());
- }
-
- uithread.confirm().await?;
-
- // Install
- resolutions
- .into_iter()
- .map(|resolution| {
- AutoAbortJoinHandle::spawn(ops::install::install(
- resolution,
- binstall_opts.clone(),
- jobserver_client.clone(),
- ))
- })
- .collect()
- } else {
- // Resolve crates and install without confirmation
- crate_names
- .into_iter()
- .map(|(crate_name, current_version)| {
- let opts = binstall_opts.clone();
- let temp_dir_path = temp_dir_path.clone();
- let jobserver_client = jobserver_client.clone();
- let client = client.clone();
- let crates_io_api_client = crates_io_api_client.clone();
- let install_path = install_path.clone();
-
- AutoAbortJoinHandle::spawn(async move {
- let resolution = ops::resolve::resolve(
- opts.clone(),
- crate_name,
- current_version,
- temp_dir_path,
- install_path,
- client,
- crates_io_api_client,
- )
- .await?;
-
- ops::install::install(resolution, opts, jobserver_client).await
- })
- })
- .collect()
- };
-
- let mut metadata_vec = Vec::with_capacity(tasks.len());
- for task in tasks {
- if let Some(metadata) = task.await?? {
- metadata_vec.push(metadata);
- }
- }
-
- block_in_place(|| {
- if let Some(mut records) = metadata {
- // The cargo manifest path is already created when loading
- // metadata.
-
- debug!("Writing .crates.toml");
- CratesToml::append_to_path(cargo_roots.join(".crates.toml"), metadata_vec.iter())?;
-
- debug!("Writing binstall/crates-v1.json");
- for metadata in metadata_vec {
- records.replace(metadata);
- }
- records.overwrite()?;
- }
-
- if args.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}");
- });
- }
-
- Ok(())
- })
}
diff --git a/crates/bin/src/install_path.rs b/crates/bin/src/install_path.rs
index 042fee29..71da8317 100644
--- a/crates/bin/src/install_path.rs
+++ b/crates/bin/src/install_path.rs
@@ -1,7 +1,6 @@
use std::{
env::var_os,
path::{Path, PathBuf},
- sync::Arc,
};
use binstalk::home::cargo_home;
@@ -31,18 +30,18 @@ pub fn get_cargo_roots_path(cargo_roots: Option<PathBuf>) -> Option<PathBuf> {
/// roughly follows <https://doc.rust-lang.org/cargo/commands/cargo-install.html#description>
///
/// Return (install_path, is_custom_install_path)
-pub fn get_install_path<P: AsRef<Path>>(
- install_path: Option<P>,
- cargo_roots: Option<P>,
-) -> (Option<Arc<Path>>, bool) {
+pub fn get_install_path(
+ install_path: Option<PathBuf>,
+ cargo_roots: Option<impl AsRef<Path>>,
+) -> (Option<PathBuf>, bool) {
// Command line override first first
if let Some(p) = install_path {
- return (Some(Arc::from(p.as_ref())), true);
+ return (Some(p), true);
}
// Then cargo_roots
if let Some(p) = cargo_roots {
- return (Some(Arc::from(p.as_ref().join("bin"))), false);
+ return (Some(p.as_ref().join("bin")), false);
}
// Local executable dir if no cargo is found
@@ -52,5 +51,5 @@ pub fn get_install_path<P: AsRef<Path>>(
debug!("Fallback to {}", d.display());
}
- (dir.map(Arc::from), true)
+ (dir, true)
}
diff --git a/crates/bin/src/logging.rs b/crates/bin/src/logging.rs
index 29869361..4001d773 100644
--- a/crates/bin/src/logging.rs
+++ b/crates/bin/src/logging.rs
@@ -13,8 +13,6 @@ use tracing_core::{identify_callsite, metadata::Kind, subscriber::Subscriber};
use tracing_log::AsTrace;
use tracing_subscriber::{filter::targets::Targets, fmt::fmt, layer::SubscriberExt};
-use crate::args::Args;
-
// Shamelessly taken from tracing-log
struct Fields {
@@ -131,9 +129,9 @@ impl Log for Logger {
fn flush(&self) {}
}
-pub fn logging(args: &Args) {
+pub fn logging(log_level: LevelFilter, json_output: bool) {
// Calculate log_level
- let log_level = min(args.log_level, STATIC_MAX_LEVEL);
+ let log_level = min(log_level, STATIC_MAX_LEVEL);
let allowed_targets =
(log_level != LevelFilter::Trace).then_some(["binstalk", "cargo_binstall"]);
@@ -145,7 +143,7 @@ pub fn logging(args: &Args) {
let log_level = log_level.as_trace();
let subscriber_builder = fmt().with_max_level(log_level);
- let subscriber: Box<dyn Subscriber + Send + Sync> = if args.json_output {
+ let subscriber: Box<dyn Subscriber + Send + Sync> = if json_output {
Box::new(subscriber_builder.json().finish())
} else {
// Disable time, target, file, line_num, thread name/ids to make the
diff --git a/crates/bin/src/main.rs b/crates/bin/src/main.rs
index b839f65a..9940a933 100644
--- a/crates/bin/src/main.rs
+++ b/crates/bin/src/main.rs
@@ -27,7 +27,7 @@ fn main() -> MainExit {
println!("{}", env!("CARGO_PKG_VERSION"));
MainExit::Success(None)
} else {
- logging(&args);
+ logging(args.log_level, args.json_output);
let start = Instant::now();
diff --git a/crates/bin/src/ui.rs b/crates/bin/src/ui.rs
index 19667e0c..461ef5d1 100644
--- a/crates/bin/src/ui.rs
+++ b/crates/bin/src/ui.rs
@@ -4,95 +4,45 @@ use std::{
};
use binstalk::errors::BinstallError;
-use tokio::sync::mpsc;
+use tokio::sync::oneshot;
-#[derive(Debug)]
-struct UIThreadInner {
- /// Request for confirmation
- request_tx: mpsc::Sender<()>,
+pub async fn confirm() -> Result<(), BinstallError> {
+ let (tx, rx) = oneshot::channel();
- /// Confirmation
- confirm_rx: mpsc::Receiver<Result<(), BinstallError>>,
-}
+ thread::spawn(move || {
+ // This task should be the only one able to
+ // access stdin
+ let mut stdin = io::stdin().lock();
+ let mut input = String::with_capacity(16);
-impl UIThreadInner {
- fn new() -> Self {
- let (request_tx, mut request_rx) = mpsc::channel(1);
- let (confirm_tx, confirm_rx) = mpsc::channel(10);
+ let res = loop {
+ {
+ let mut stdout = io::stdout().lock();
- thread::spawn(move || {
- // This task should be the only one able to
- // access stdin
- let mut stdin = io::stdin().lock();
- let mut input = String::with_capacity(16);
-
- loop {
- if request_rx.blocking_recv().is_none() {
- break;
- }
-
- let res = loop {
- {
- let mut stdout = io::stdout().lock();
-
- writeln!(&mut stdout, "Do you wish to continue? yes/[no]").unwrap();
- write!(&mut stdout, "? ").unwrap();
- stdout.flush().unwrap();
- }
-
- input.clear();
- stdin.read_line(&mut input).unwrap();
-
- match input.as_str().trim() {
- "yes" | "y" | "YES" | "Y" => break Ok(()),
- "no" | "n" | "NO" | "N" | "" => break Err(BinstallError::UserAbort),
- _ => continue,
- }
- };
-
- confirm_tx
- .blocking_send(res)
- .expect("entry exits when confirming request");
+ write!(&mut stdout, "Do you wish to continue? yes/[no]\n? ").unwrap();
+ stdout.flush().unwrap();
}
- });
- Self {
- request_tx,
- confirm_rx,
- }
- }
+ stdin.read_line(&mut input).unwrap();
- async fn confirm(&mut self) -> Result<(), BinstallError> {
- self.request_tx
- .send(())
- .await
- .map_err(|_| BinstallError::UserAbort)?;
+ match input.as_str().trim() {
+ "yes" | "y" | "YES" | "Y" => break true,
+ "no" | "n" | "NO" | "N" | "" => break false,
+ _ => {
+ input.clear();
+ continue;
+ }
+ }
+ };
- self.confirm_rx
- .recv()
- .await
- .unwrap_or(Err(BinstallError::UserAbort))
- }
-}
-
-#[derive(Debug)]
-pub struct UIThread(Option<UIThreadInner>);
-
-impl UIThread {
- /// * `enable` - `true` to enable confirmation, `false` to disable it.
- pub fn new(enable: bool) -> Self {
- Self(if enable {
- Some(UIThreadInner::new())
- } else {
- None
- })
- }
-
- pub async fn confirm(&mut self) -> Result<(), BinstallError> {
- if let Some(inner) = self.0.as_mut() {
- inner.confirm().await
- } else {
- Ok(())
- }
+ // The main thread might be terminated by signal and thus cancelled
+ // the confirmation.
+ tx.send(res).ok();
+ });
+
+ if rx.await.unwrap() {
+ Ok(())
+ } else {
+ Err(BinstallError::UserAbort)
}
}
diff --git a/crates/binstalk-downloader/src/remote.rs b/crates/binstalk-downloader/src/remote.rs
index 86313afb..228d31a1 100644
--- a/crates/binstalk-downloader/src/remote.rs
+++ b/crates/binstalk-downloader/src/remote.rs
@@ -1,5 +1,4 @@
use std::{
- env,
num::NonZeroU64,
sync::Arc,
time::{Duration, SystemTime},
@@ -52,32 +51,40 @@ impl Client {
/// * `num_request` - maximum number of requests to be processed for
/// each `per` duration.
pub fn new(
+ user_agent: impl AsRef<str>,
min_tls: Option<tls::Version>,
per: Duration,
num_request: NonZeroU64,
) -> Result<Self, Error> {
- const USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"));
+ fn inner(
+ user_agent: &str,
+ min_tls: Option<tls::Version>,
+ per: Duration,
+ num_request: NonZeroU64,
+ ) -> Result<Client, Error> {
+ let mut builder = reqwest::ClientBuilder::new()
+ .user_agent(user_agent)
+ .https_only(true)
+ .min_tls_version(tls::Version::TLS_1_2)
+ .tcp_nodelay(false);
- let mut builder = reqwest::ClientBuilder::new()
- .user_agent(USER_AGENT)
- .https_only(true)
- .min_tls_version(tls::Version::TLS_1_2)
- .tcp_nodelay(false);
+ if let Some(ver) = min_tls {
+ builder = builder.min_tls_version(ver);
+ }
- if let Some(ver) = min_tls {
- builder = builder.min_tls_version(ver);
+ let client = builder.build()?;
+
+ Ok(Client {
+ client: client.clone(),
+ rate_limit: Arc::new(Mutex::new(
+ ServiceBuilder::new()
+ .rate_limit(num_request.get(), per)
+ .service(client),
+ )),
+ })
}
- let client = builder.build()?;
-
- Ok(Self {
- client: client.clone(),
- rate_limit: Arc::new(Mutex::new(
- ServiceBuilder::new()
- .rate_limit(num_request.get(), per)
- .service(client),
- )),
- })
+ inner(user_agent.as_ref(), min_tls, per, num_request)
}
/// Return inner reqwest client.
diff --git a/crates/binstalk/src/drivers/version.rs b/crates/binstalk/src/drivers/version.rs
index 3861d360..3c6abaed 100644
--- a/crates/binstalk/src/drivers/version.rs
+++ b/crates/binstalk/src/drivers/version.rs
@@ -45,7 +45,7 @@ pub(super) fn find_version<Item: Version, VersionIter: Iterator<Item = Item>>(
}
})
// Return highest version
- .max_by_key(|(_item, ver)| ver.clone())
+ .max_by(|(_item_x, ver_x), (_item_y, ver_y)| ver_x.cmp(ver_y))
.ok_or(BinstallError::VersionMismatch {
req: version_req.clone(),
})
diff --git a/crates/binstalk/src/fetchers/gh_crate_meta.rs b/crates/binstalk/src/fetchers/gh_crate_meta.rs
index 8df5680b..473fa2db 100644
--- a/crates/binstalk/src/fetchers/gh_crate_meta.rs
+++ b/crates/binstalk/src/fetchers/gh_crate_meta.rs
@@ -1,4 +1,4 @@
-use std::{path::Path, sync::Arc};
+use std::{future::Future, path::Path, sync::Arc};
use compact_str::{CompactString, ToCompactString};
use futures_util::stream::{FuturesUnordered, StreamExt};
@@ -15,7 +15,6 @@ use crate::{
download::Download,
remote::{Client, Method},
signal::wait_on_cancellation_signal,
- tasks::AutoAbortJoinHandle,
},
manifests::cargo_toml_binstall::{PkgFmt, PkgMeta},
};
@@ -31,7 +30,7 @@ pub struct GhCrateMeta {
resolution: OnceCell<(Url, PkgFmt)>,
}
-type BaselineFindTask = AutoAbortJoinHandle<Result<Option<(Url, PkgFmt)>, BinstallError>>;
+type FindTaskRes = Result<Option<(Url, PkgFmt)>, BinstallError>;
impl GhCrateMeta {
fn launch_baseline_find_tasks<'a>(
@@ -39,7 +38,7 @@ impl GhCrateMeta {
pkg_fmt: PkgFmt,
pkg_url: &'a str,
repo: Option<&'a str>,
- ) -> impl Iterator<Item = BaselineFindTask> + 'a {
+ ) -> impl Iterator<Item = impl Future<Output = FindTaskRes> + 'a> + 'a {
// build up list of potential URLs
let urls = pkg_fmt.extensions().iter().filter_map(move |ext| {
let ctx = Context::from_data_with_repo(&self.data, ext, repo);
@@ -56,13 +55,13 @@ impl GhCrateMeta {
urls.map(move |url| {
let client = self.client.clone();
- AutoAbortJoinHandle::spawn(async move {
+ async move {
debug!("Checking for package at: '{url}'");
Ok((client.remote_exists(url.clone(), Method::HEAD).await?
|| client.remote_exists(url.clone(), Method::GET).await?)
.then_some((url, pkg_fmt)))
- })
+ }
})
}
}
@@ -134,7 +133,7 @@ impl super::Fetcher for GhCrateMeta {
};
while let Some(res) = handles.next().await {
- if let Some((url, pkg_fmt)) = res?? {
+ if let Some((url, pkg_fmt)) = res? {
debug!("Winning URL is {url}, with pkg_fmt {pkg_fmt}");
self.resolution.set((url, pkg_fmt)).unwrap(); // find() is called first
return Ok(true);
diff --git a/crates/binstalk/src/helpers/jobserver_client.rs b/crates/binstalk/src/helpers/jobserver_client.rs
index ca65fcd4..5b625d9d 100644
--- a/crates/binstalk/src/helpers/jobserver_client.rs
+++ b/crates/binstalk/src/helpers/jobserver_client.rs
@@ -1,12 +1,11 @@
-use std::{num::NonZeroUsize, sync::Arc, thread::available_parallelism};
+use std::{num::NonZeroUsize, thread::available_parallelism};
use jobslot::Client;
use tokio::sync::OnceCell;
use crate::errors::BinstallError;
-#[derive(Clone)]
-pub struct LazyJobserverClient(Arc<OnceCell<Client>>);
+pub struct LazyJobserverClient(OnceCell<Client>);
impl LazyJobserverClient {
/// This must be called at the start of the program since
@@ -19,7 +18,7 @@ impl LazyJobserverClient {
// It doesn't do anything that is actually unsafe, like
// dereferencing pointer.
let opt = unsafe { Client::from_env() };
- Self(Arc::new(OnceCell::new_with(opt)))
+ Self(OnceCell::new_with(opt))
}
pub async fn get(&self) -> Result<&Client, BinstallError> {
diff --git a/crates/binstalk/src/ops.rs b/crates/binstalk/src/ops.rs
index 805cd4d1..b4d26a44 100644
--- a/crates/binstalk/src/ops.rs
+++ b/crates/binstalk/src/ops.rs
@@ -2,11 +2,12 @@
use std::{path::PathBuf, sync::Arc};
+use crates_io_api::AsyncClient as CratesIoApiClient;
use semver::VersionReq;
use crate::{
fetchers::{Data, Fetcher},
- helpers::remote::Client,
+ helpers::{jobserver_client::LazyJobserverClient, remote::Client},
manifests::cargo_toml_binstall::PkgOverride,
DesiredTargets,
};
@@ -20,11 +21,19 @@ pub struct Options {
pub no_symlinks: bool,
pub dry_run: bool,
pub force: bool,
+ pub quiet: bool,
+
pub version_req: Option<VersionReq>,
pub manifest_path: Option<PathBuf>,
pub cli_overrides: PkgOverride,
+
pub desired_targets: DesiredTargets,
- pub quiet: bool,
pub resolvers: Vec<Resolver>,
pub cargo_install_fallback: bool,
+
+ pub temp_dir: PathBuf,
+ pub install_path: PathBuf,
+ pub client: Client,
+ pub crates_io_api_client: CratesIoApiClient,
+ pub jobserver_client: LazyJobserverClient,
}
diff --git a/crates/binstalk/src/ops/install.rs b/crates/binstalk/src/ops/install.rs
index a5c95ee0..cb4c504d 100644
--- a/crates/binstalk/src/ops/install.rs
+++ b/crates/binstalk/src/ops/install.rs
@@ -16,7 +16,6 @@ use crate::{
pub async fn install(
resolution: Resolution,
opts: Arc<Options>,
- jobserver_client: LazyJobserverClient,
) -> Result<Option<CrateInfo>, BinstallError> {
match resolution {
Resolution::AlreadyUpToDate => Ok(None),
@@ -51,7 +50,7 @@ pub async fn install(
&name,
&version,
target,
- jobserver_client,
+ &opts.jobserver_client,
opts.quiet,
opts.force,
)
@@ -99,7 +98,7 @@ async fn install_from_source(
name: &str,
version: &str,
target: &str,
- lazy_jobserver_client: LazyJobserverClient,
+ lazy_jobserver_client: &LazyJobserverClient,
quiet: bool,
force: bool,
) -> Result<(), BinstallError> {
diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs
index 7999ae96..45d736f7 100644
--- a/crates/binstalk/src/ops/resolve.rs
+++ b/crates/binstalk/src/ops/resolve.rs
@@ -8,6 +8,7 @@ use std::{
use cargo_toml::Manifest;
use compact_str::{CompactString, ToCompactString};
+use crates_io_api::AsyncClient as CratesIoApiClient;
use itertools::Itertools;
use semver::{Version, VersionReq};
use tokio::task::block_in_place;
@@ -94,23 +95,11 @@ pub async fn resolve(
opts: Arc<Options>,
crate_name: CrateName,
curr_version: Option<Version>,
- temp_dir: Arc<Path>,
- install_path: Arc<Path>,
- client: Client,
- crates_io_api_client: crates_io_api::AsyncClient,
) -> Result<Resolution, BinstallError> {
let crate_name_name = crate_name.name.clone();
- let resolution = resolve_inner(
- &opts,
- crate_name,
- curr_version,
- temp_dir,
- install_path,
- client,
- crates_io_api_client,
- )
- .await
- .map_err(|err| err.crate_context(crate_name_name))?;
+ let resolution = resolve_inner(&opts, crate_name, curr_version)
+ .await
+ .map_err(|err| err.crate_context(crate_name_name))?;
resolution.print(&opts);
@@ -121,10 +110,6 @@ async fn resolve_inner(
opts: &Options,
crate_name: CrateName,
curr_version: Option<Version>,
- temp_dir: Arc<Path>,
- install_path: Arc<Path>,
- client: Client,
- crates_io_api_client: crates_io_api::AsyncClient,
) -> Result<Resolution, BinstallError> {
info!("Resolving package: '{}'", crate_name);
@@ -141,8 +126,8 @@ async fn resolve_inner(
crate_name.name,
curr_version,
version_req,
- client.clone(),
- crates_io_api_client).await?
+ opts.client.clone(),
+ &opts.crates_io_api_client).await?
else {
return Ok(Resolution::AlreadyUpToDate)
};
@@ -175,7 +160,7 @@ async fn resolve_inner(
})
.cartesian_product(resolvers)
.map(|(fetcher_data, f)| {
- let fetcher = f(&client, &fetcher_data);
+ let fetcher = f(&opts.client, &fetcher_data);
(
fetcher.clone(),
AutoAbortJoinHandle::spawn(async move { fetcher.find().await }),
@@ -187,7 +172,7 @@ async fn resolve_inner(
match handle.flattened_join().await {
Ok(true) => {
// Generate temporary binary path
- let bin_path = temp_dir.join(format!(
+ let bin_path = opts.temp_dir.join(format!(
"bin-{}-{}-{}",
package_info.name,
fetcher.target(),
@@ -198,7 +183,7 @@ async fn resolve_inner(
fetcher.as_ref(),
&bin_path,
&package_info,
- &install_path,
+ &opts.install_path,
opts.no_symlinks,
)
.await
@@ -407,14 +392,12 @@ impl PackageInfo {
curr_version: Option<Version>,
version_req: VersionReq,
client: Client,
- crates_io_api_client: crates_io_api::AsyncClient,
+ crates_io_api_client: &CratesIoApiClient,
) -> Result<Option<Self>, BinstallError> {
// Fetch crate via crates.io, git, or use a local manifest path
let manifest = match opts.manifest_path.as_ref() {
Some(manifest_path) => load_manifest_path(manifest_path)?,
- None => {
- fetch_crate_cratesio(client, &crates_io_api_client, &name, &version_req).await?
- }
+ None => fetch_crate_cratesio(client, crates_io_api_client, &name, &version_req).await?,
};
let Some(mut package) = manifest.package else {
diff --git a/crates/binstalk/src/ops/resolve/crate_name.rs b/crates/binstalk/src/ops/resolve/crate_name.rs
index 398f9793..775c1f33 100644
--- a/crates/binstalk/src/ops/resolve/crate_name.rs
+++ b/crates/binstalk/src/ops/resolve/crate_name.rs
@@ -43,8 +43,7 @@ impl FromStr for CrateName {
}
impl CrateName {
- pub fn dedup(crate_names: &[Self]) -> impl Iterator<Item = Self> {
- let mut crate_names = crate_names.to_vec();
+ pub fn dedup(mut crate_names: Vec<Self>) -> impl Iterator<Item = Self> {
crate_names.sort_by(|x, y| x.name.cmp(&y.name));
crate_names.into_iter().coalesce(|previous, current| {
if previous.name == current.name {
@@ -62,7 +61,7 @@ mod tests {
macro_rules! assert_dedup {
([ $( ( $input_name:expr, $input_version:expr ) ),* ], [ $( ( $output_name:expr, $output_version:expr ) ),* ]) => {
- let input_crate_names = [$( CrateName {
+ let input_crate_names = vec![$( CrateName {
name: $input_name.into(),
version_req: Some($input_version.parse().unwrap())
}, )*];
@@ -72,7 +71,7 @@ mod tests {
}, )*];
output_crate_names.sort_by(|x, y| x.name.cmp(&y.name));
- let crate_names: Vec<_> = CrateName::dedup(&input_crate_names).collect();
+ let crate_names: Vec<_> = CrateName::dedup(input_crate_names).collect();
assert_eq!(crate_names, output_crate_names);
};
}