From 52f2db4f5797aed4bd56fd66dcf8a56503149d23 Mon Sep 17 00:00:00 2001 From: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> Date: Sun, 4 Aug 2024 12:13:18 +1000 Subject: [PATCH] Add `--maximum-resolution-timeout` (#1862) * Add --maximum-resolution-timeout Fixed #1823 * Display default value for `--maximum-resolution-timeout` in help --- crates/bin/src/args.rs | 9 +++ crates/bin/src/entry.rs | 5 ++ crates/binstalk/Cargo.toml | 1 + crates/binstalk/src/ops.rs | 4 +- crates/binstalk/src/ops/resolve.rs | 105 ++++++++++++++++------------- 5 files changed, 76 insertions(+), 48 deletions(-) diff --git a/crates/bin/src/args.rs b/crates/bin/src/args.rs index cb0aa65a..000abd5f 100644 --- a/crates/bin/src/args.rs +++ b/crates/bin/src/args.rs @@ -187,6 +187,15 @@ pub struct Args { )] pub(crate) no_discover_github_token: bool, + /// Maximum time each resolution (one for each possible target and each strategy), in seconds. + #[clap( + help_heading = "Overrides", + long, + env = "BINSTALL_MAXIMUM_RESOLUTION_TIMEOUT", + default_value_t = NonZeroU16::new(180).unwrap(), + )] + pub(crate) maximum_resolution_timeout: NonZeroU16, + /// This flag is now enabled by default thus a no-op. /// /// By default, Binstall will install a binary as-is in the install path. diff --git a/crates/bin/src/entry.rs b/crates/bin/src/entry.rs index 4a0c446f..5fbf6e84 100644 --- a/crates/bin/src/entry.rs +++ b/crates/bin/src/entry.rs @@ -2,6 +2,7 @@ use std::{ env, fs, path::{Path, PathBuf}, sync::Arc, + time::Duration, }; use binstalk::{ @@ -200,6 +201,10 @@ pub fn install_crates( SignaturePolicy::IfPresent }, disable_telemetry: args.disable_telemetry, + + maximum_resolution_timeout: Duration::from_secs( + args.maximum_resolution_timeout.get().into(), + ), }); // Destruct args before any async function to reduce size of the future diff --git a/crates/binstalk/Cargo.toml b/crates/binstalk/Cargo.toml index 1d44502e..8a07f4a2 100644 --- a/crates/binstalk/Cargo.toml +++ b/crates/binstalk/Cargo.toml @@ -40,6 +40,7 @@ tokio = { version = "1.35.0", features = [ "rt", "process", "sync", + "time", ], default-features = false } tracing = "0.1.39" url = { version = "2.3.1", features = ["serde"] } diff --git a/crates/binstalk/src/ops.rs b/crates/binstalk/src/ops.rs index 57b20c17..8051daaa 100644 --- a/crates/binstalk/src/ops.rs +++ b/crates/binstalk/src/ops.rs @@ -1,6 +1,6 @@ //! Concrete Binstall operations. -use std::{path::PathBuf, sync::Arc}; +use std::{path::PathBuf, sync::Arc, time::Duration}; use semver::VersionReq; @@ -56,4 +56,6 @@ pub struct Options { pub signature_policy: SignaturePolicy, pub disable_telemetry: bool, + + pub maximum_resolution_timeout: Duration, } diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs index 5d27adcf..87a7dbfc 100644 --- a/crates/binstalk/src/ops/resolve.rs +++ b/crates/binstalk/src/ops/resolve.rs @@ -17,7 +17,7 @@ use itertools::Itertools; use leon::Template; use maybe_owned::MaybeOwned; use semver::{Version, VersionReq}; -use tokio::task::spawn_blocking; +use tokio::{task::spawn_blocking, time::timeout}; use tracing::{debug, error, info, instrument, warn}; use url::Url; @@ -182,62 +182,73 @@ async fn resolve_inner( } for fetcher in handles { - match AutoAbortJoinHandle::new(fetcher.clone().find()) - .flattened_join() - .await + match timeout( + opts.maximum_resolution_timeout, + AutoAbortJoinHandle::new(fetcher.clone().find()).flattened_join(), + ) + .await { - Ok(true) => { - // Generate temporary binary path - let bin_path = opts.temp_dir.join(format!( - "bin-{}-{}-{}", - package_info.name, - fetcher.target(), - fetcher.fetcher_name() - )); + Ok(ret) => match ret { + Ok(true) => { + // Generate temporary binary path + let bin_path = opts.temp_dir.join(format!( + "bin-{}-{}-{}", + package_info.name, + fetcher.target(), + fetcher.fetcher_name() + )); - match download_extract_and_verify( - fetcher.as_ref(), - &bin_path, - &package_info, - &opts.install_path, - opts.no_symlinks, - ) - .await - { - Ok(bin_files) => { - if !bin_files.is_empty() { - return Ok(Resolution::Fetch(Box::new(ResolutionFetch { - fetcher, - new_version: package_info.version, - name: package_info.name, - version_req: version_req_str, - source: package_info.source, - bin_files, - }))); - } else { - warn!( - "Error when checking binaries provided by fetcher {}: \ + match download_extract_and_verify( + fetcher.as_ref(), + &bin_path, + &package_info, + &opts.install_path, + opts.no_symlinks, + ) + .await + { + Ok(bin_files) => { + if !bin_files.is_empty() { + return Ok(Resolution::Fetch(Box::new(ResolutionFetch { + fetcher, + new_version: package_info.version, + name: package_info.name, + version_req: version_req_str, + source: package_info.source, + bin_files, + }))); + } else { + warn!( + "Error when checking binaries provided by fetcher {}: \ The fetcher does not provide any optional binary", + fetcher.source_name(), + ); + } + } + Err(err) => { + if let BinstallError::UserAbort = err { + return Err(err); + } + warn!( + "Error while downloading and extracting from fetcher {}: {}", fetcher.source_name(), + err ); } } - Err(err) => { - if let BinstallError::UserAbort = err { - return Err(err); - } - warn!( - "Error while downloading and extracting from fetcher {}: {}", - fetcher.source_name(), - err - ); - } } - } - Ok(false) => (), + Ok(false) => (), + Err(err) => { + warn!( + "Error while checking fetcher {}: {}", + fetcher.source_name(), + err + ); + } + }, Err(err) => { warn!( - "Error while checking fetcher {}: {}", + "Timeout reached while checking fetcher {}: {}", fetcher.source_name(), err );