diff --git a/crates/bin/src/args.rs b/crates/bin/src/args.rs index d03b09f4..85f2ca69 100644 --- a/crates/bin/src/args.rs +++ b/crates/bin/src/args.rs @@ -191,6 +191,10 @@ pub struct Args { #[clap(help_heading = "Options", long)] pub(crate) no_cleanup: bool, + /// Continue installing other crates even if one of the crate failed to install. + #[clap(help_heading = "Options", long)] + pub(crate) continue_on_failure: bool, + /// By default, binstall keeps track of the installed packages with metadata files /// stored in the installation root directory. /// diff --git a/crates/bin/src/bin_util.rs b/crates/bin/src/bin_util.rs index d75445f1..df49c3f1 100644 --- a/crates/bin/src/bin_util.rs +++ b/crates/bin/src/bin_util.rs @@ -1,5 +1,4 @@ use std::{ - future::Future, process::{ExitCode, Termination}, time::Duration, }; @@ -48,20 +47,17 @@ impl MainExit { } /// This function would start a tokio multithreading runtime, -/// spawn a new task on it that runs `f()`, then `block_on` it. +/// then `block_on` the task it returns. /// /// It will cancel the future if user requested cancellation /// via signal. -pub fn run_tokio_main(f: Func) -> Result<()> -where - Func: FnOnce() -> Result>, - Fut: Future> + Send + 'static, -{ +pub fn run_tokio_main( + f: impl FnOnce() -> Result>>>, +) -> Result<()> { let rt = Runtime::new().map_err(BinstallError::from)?; let _guard = rt.enter(); - if let Some(fut) = f()? { - let handle = AutoAbortJoinHandle::new(rt.spawn(fut)); + if let Some(handle) = f()? { rt.block_on(cancel_on_user_sig_term(handle))? } else { Ok(()) diff --git a/crates/bin/src/entry.rs b/crates/bin/src/entry.rs index 304bab65..f409a726 100644 --- a/crates/bin/src/entry.rs +++ b/crates/bin/src/entry.rs @@ -1,12 +1,11 @@ use std::{ env, fs, - future::Future, path::{Path, PathBuf}, sync::Arc, }; use binstalk::{ - errors::BinstallError, + errors::{BinstallError, CrateContextError}, fetchers::{Fetcher, GhCrateMeta, QuickInstall, SignaturePolicy}, get_desired_targets, helpers::{ @@ -27,7 +26,7 @@ use binstalk_manifests::{ use file_format::FileFormat; use home::cargo_home; use log::LevelFilter; -use miette::{miette, Result, WrapErr}; +use miette::{miette, Report, Result, WrapErr}; use tokio::task::block_in_place; use tracing::{debug, error, info, warn}; @@ -40,7 +39,7 @@ use crate::{ pub fn install_crates( args: Args, jobserver_client: LazyJobserverClient, -) -> Result>>> { +) -> Result>>> { // Compute Resolvers let mut cargo_install_fallback = false; @@ -217,54 +216,131 @@ pub fn install_crates( }) .collect(); - Ok(Some(async move { - // Collect results - let mut resolution_fetchs = Vec::new(); - let mut resolution_sources = Vec::new(); + Ok(Some(if args.continue_on_failure { + AutoAbortJoinHandle::spawn(async move { + // Collect results + let mut resolution_fetchs = Vec::new(); + let mut resolution_sources = Vec::new(); + let mut errors = Vec::new(); - for task in tasks { - match task.await?? { - Resolution::AlreadyUpToDate => {} - Resolution::Fetch(fetch) => { - fetch.print(&binstall_opts); - resolution_fetchs.push(fetch) - } - Resolution::InstallFromSource(source) => { - source.print(); - resolution_sources.push(source) + for task in tasks { + match task.flattened_join().await { + Ok(Resolution::AlreadyUpToDate) => {} + Ok(Resolution::Fetch(fetch)) => { + fetch.print(&binstall_opts); + resolution_fetchs.push(fetch) + } + Ok(Resolution::InstallFromSource(source)) => { + source.print(); + resolution_sources.push(source) + } + Err(BinstallError::CrateContext(err)) => errors.push(err), + Err(e) => panic!("Expected BinstallError::CrateContext(_), got {}", e), } } - } - if resolution_fetchs.is_empty() && resolution_sources.is_empty() { - debug!("Nothing to do"); - return Ok(()); - } + if resolution_fetchs.is_empty() && resolution_sources.is_empty() { + return if let Some(err) = BinstallError::crate_errors(errors) { + Err(err.into()) + } else { + debug!("Nothing to do"); + Ok(()) + }; + } - // Confirm - if !dry_run && !no_confirm { - confirm().await?; - } + // Confirm + if !dry_run && !no_confirm { + if let Err(abort_err) = confirm().await { + return if let Some(err) = BinstallError::crate_errors(errors) { + Err(Report::new(abort_err).wrap_err(err)) + } else { + Err(abort_err.into()) + }; + } + } - do_install_fetches( - resolution_fetchs, - manifests, - &binstall_opts, - dry_run, - temp_dir, - no_cleanup, - )?; + let manifest_update_res = do_install_fetches_continue_on_failure( + resolution_fetchs, + manifests, + &binstall_opts, + dry_run, + temp_dir, + no_cleanup, + &mut errors, + ); - let tasks: Vec<_> = resolution_sources - .into_iter() - .map(|source| AutoAbortJoinHandle::spawn(source.install(binstall_opts.clone()))) - .collect(); + let tasks: Vec<_> = resolution_sources + .into_iter() + .map(|source| AutoAbortJoinHandle::spawn(source.install(binstall_opts.clone()))) + .collect(); - for task in tasks { - task.await??; - } + for task in tasks { + match task.flattened_join().await { + Ok(_) => (), + Err(BinstallError::CrateContext(err)) => errors.push(err), + Err(e) => panic!("Expected BinstallError::CrateContext(_), got {}", e), + } + } - Ok(()) + match (BinstallError::crate_errors(errors), manifest_update_res) { + (None, Ok(())) => Ok(()), + (None, Err(err)) => Err(err), + (Some(err), Ok(())) => Err(err.into()), + (Some(err), Err(manifest_update_err)) => { + Err(Report::new(err).wrap_err(manifest_update_err)) + } + } + }) + } else { + AutoAbortJoinHandle::spawn(async move { + // Collect results + let mut resolution_fetchs = Vec::new(); + let mut resolution_sources = Vec::new(); + + for task in tasks { + match task.await?? { + Resolution::AlreadyUpToDate => {} + Resolution::Fetch(fetch) => { + fetch.print(&binstall_opts); + resolution_fetchs.push(fetch) + } + Resolution::InstallFromSource(source) => { + source.print(); + resolution_sources.push(source) + } + } + } + + if resolution_fetchs.is_empty() && resolution_sources.is_empty() { + debug!("Nothing to do"); + return Ok(()); + } + + // Confirm + if !dry_run && !no_confirm { + confirm().await?; + } + + do_install_fetches( + resolution_fetchs, + manifests, + &binstall_opts, + dry_run, + temp_dir, + no_cleanup, + )?; + + let tasks: Vec<_> = resolution_sources + .into_iter() + .map(|source| AutoAbortJoinHandle::spawn(source.install(binstall_opts.clone()))) + .collect(); + + for task in tasks { + task.await??; + } + + Ok(()) + }) })) } @@ -449,3 +525,53 @@ fn do_install_fetches( Ok(()) }) } + +#[allow(clippy::vec_box)] +fn do_install_fetches_continue_on_failure( + resolution_fetchs: Vec>, + // Take manifests by value to drop the `FileLock`. + manifests: Option, + binstall_opts: &Options, + dry_run: bool, + temp_dir: tempfile::TempDir, + no_cleanup: bool, + errors: &mut Vec>, +) -> Result<()> { + if resolution_fetchs.is_empty() { + return Ok(()); + } + + if dry_run { + info!("Dry-run: Not proceeding to install fetched binaries"); + return Ok(()); + } + + block_in_place(|| { + let metadata_vec = resolution_fetchs + .into_iter() + .filter_map(|fetch| match fetch.install(binstall_opts) { + Ok(crate_info) => Some(crate_info), + Err(BinstallError::CrateContext(err)) => { + errors.push(err); + None + } + Err(e) => panic!("Expected BinstallError::CrateContext(_), got {}", e), + }) + .collect::>(); + + if let Some(manifests) = manifests { + manifests.update(metadata_vec)?; + } + + if no_cleanup { + // Consume temp_dir without removing it from fs. + let _ = 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/binstalk/src/errors.rs b/crates/binstalk/src/errors.rs index c64525e4..c5afd7a6 100644 --- a/crates/binstalk/src/errors.rs +++ b/crates/binstalk/src/errors.rs @@ -1,5 +1,5 @@ use std::{ - io, + fmt, io, ops, path::PathBuf, process::{ExitCode, ExitStatus, Termination}, }; @@ -9,6 +9,7 @@ use binstalk_downloader::{ }; use binstalk_fetchers::FetchError; use compact_str::CompactString; +use itertools::Itertools; use miette::{Diagnostic, Report}; use target_lexicon::ParseError as TargetTripleParseError; use thiserror::Error; @@ -40,6 +41,90 @@ pub struct CrateContextError { err: BinstallError, } +#[derive(Debug)] +pub struct CrateErrors(Box<[Box]>); + +impl CrateErrors { + fn iter(&self) -> impl Iterator + Clone { + self.0.iter().map(ops::Deref::deref) + } + + fn get_iter_for<'a, T: 'a>( + &'a self, + f: fn(&'a CrateContextError) -> Option, + ) -> Option + 'a> { + let iter = self.iter().filter_map(f); + + if iter.clone().next().is_none() { + None + } else { + Some(iter) + } + } +} + +impl fmt::Display for CrateErrors { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0.iter().format(", "), f) + } +} + +impl std::error::Error for CrateErrors { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.0.first().map(|e| e as _) + } +} + +impl miette::Diagnostic for CrateErrors { + fn code<'a>(&'a self) -> Option> { + Some(Box::new("binstall::many_failure")) + } + + fn severity(&self) -> Option { + self.iter().filter_map(miette::Diagnostic::severity).max() + } + + fn help<'a>(&'a self) -> Option> { + Some(Box::new( + self.get_iter_for(miette::Diagnostic::help)?.format("\n"), + )) + } + + fn url<'a>(&'a self) -> Option> { + Some(Box::new( + self.get_iter_for(miette::Diagnostic::url)?.format("\n"), + )) + } + + fn source_code(&self) -> Option<&dyn miette::SourceCode> { + self.iter().find_map(miette::Diagnostic::source_code) + } + + fn labels(&self) -> Option + '_>> { + let get_iter = || self.iter().filter_map(miette::Diagnostic::labels).flatten(); + + if get_iter().next().is_none() { + None + } else { + Some(Box::new(get_iter())) + } + } + + fn related<'a>(&'a self) -> Option + 'a>> { + Some(Box::new( + self.iter().map(|e| e as _).chain( + self.iter() + .filter_map(miette::Diagnostic::related) + .flatten(), + ), + )) + } + + fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { + self.0.first().map(|err| &**err as _) + } +} + #[derive(Debug, Error)] #[error("Invalid pkg-url {pkg_url} for {crate_name}@{version} on {target}: {reason}")] pub struct InvalidPkgFmtError { @@ -344,6 +429,11 @@ pub enum BinstallError { #[error(transparent)] #[diagnostic(transparent)] CrateContext(Box), + + /// A wrapped error for failures of multiple crates when `--continue-on-failure` is specified. + #[error(transparent)] + #[diagnostic(transparent)] + Errors(CrateErrors), } impl BinstallError { @@ -380,6 +470,7 @@ impl BinstallError { GitError(_) => 98, LoadManifestFromWSError(_) => 99, CrateContext(context) => context.err.exit_number(), + Errors(errors) => (errors.0)[0].err.exit_number(), }; // reserved codes @@ -401,10 +492,27 @@ impl BinstallError { /// Add crate context to the error pub fn crate_context(self, crate_name: impl Into) -> Self { - Self::CrateContext(Box::new(CrateContextError { - err: self, - crate_name: crate_name.into(), - })) + self.crate_context_inner(crate_name.into()) + } + + fn crate_context_inner(self, crate_name: CompactString) -> Self { + match self { + Self::CrateContext(mut crate_context_error) => { + crate_context_error.crate_name = crate_name; + Self::CrateContext(crate_context_error) + } + err => Self::CrateContext(Box::new(CrateContextError { err, crate_name })), + } + } + + pub fn crate_errors(mut errors: Vec>) -> Option { + if errors.is_empty() { + None + } else if errors.len() == 1 { + Some(Self::CrateContext(errors.pop().unwrap())) + } else { + Some(Self::Errors(CrateErrors(errors.into_boxed_slice()))) + } } } diff --git a/crates/binstalk/src/ops/resolve/resolution.rs b/crates/binstalk/src/ops/resolve/resolution.rs index fa06322e..9c149dea 100644 --- a/crates/binstalk/src/ops/resolve/resolution.rs +++ b/crates/binstalk/src/ops/resolve/resolution.rs @@ -51,6 +51,12 @@ impl Resolution { impl ResolutionFetch { pub fn install(self, opts: &Options) -> Result { + let crate_name = self.name.clone(); + self.install_inner(opts) + .map_err(|err| err.crate_context(crate_name)) + } + + fn install_inner(self, opts: &Options) -> Result { type InstallFp = fn(&bins::BinFile) -> Result<(), bins::Error>; let (install_bin, install_link): (InstallFp, InstallFp) = match (opts.no_track, opts.force) @@ -126,6 +132,13 @@ impl ResolutionFetch { impl ResolutionSource { pub async fn install(self, opts: Arc) -> Result<(), BinstallError> { + let crate_name = self.name.clone(); + self.install_inner(opts) + .await + .map_err(|err| err.crate_context(crate_name)) + } + + async fn install_inner(self, opts: Arc) -> Result<(), BinstallError> { let target = if let Some(targets) = opts.desired_targets.get_initialized() { Some(targets.first().ok_or(BinstallError::NoViableTargets)?) } else { @@ -171,7 +184,7 @@ impl ResolutionSource { cmd.arg("--no-track"); } - debug!("Running `{}`", format_cmd(&cmd),); + debug!("Running `{}`", format_cmd(&cmd)); if !opts.dry_run { let mut child = opts diff --git a/e2e-tests/continue-on-failure.sh b/e2e-tests/continue-on-failure.sh new file mode 100755 index 00000000..44080313 --- /dev/null +++ b/e2e-tests/continue-on-failure.sh @@ -0,0 +1,58 @@ +#!/bin/bash + +set -euxo pipefail + +unset CARGO_INSTALL_ROOT + +CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home') +export CARGO_HOME +othertmpdir=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-test') +export PATH="$CARGO_HOME/bin:$othertmpdir/bin:$PATH" + +mkdir -p "$othertmpdir/bin" +# Copy it to bin to test use of env var `CARGO` +cp "./$1" "$othertmpdir/bin/" + + +## Test --continue-on-failure +set +e +cargo binstall --no-confirm --continue-on-failure cargo-watch@8.4.0 non-existent-clippy +exit_code="$?" + +set -e + +if [ "$exit_code" != 76 ]; then + echo "Expected exit code 76, but actual exit code $exit_code" + exit 1 +fi + + +cargo_watch_version="$(cargo watch -V)" +echo "$cargo_watch_version" + +[ "$cargo_watch_version" = "cargo-watch 8.4.0" ] + + +## Test that it is no-op when only one crate is passed +set +e +cargo binstall --no-confirm --continue-on-failure non-existent-clippy +exit_code="$?" + +set -e + +if [ "$exit_code" != 76 ]; then + echo "Expected exit code 76, but actual exit code $exit_code" + exit 1 +fi + +# Test if both crates are invalid +set +e +cargo binstall --no-confirm --continue-on-failure non-existent-clippy non-existent-clippy2 +exit_code="$?" + +set -e + +if [ "$exit_code" != 76 ]; then + echo "Expected exit code 76, but actual exit code $exit_code" + exit 1 +fi diff --git a/e2e-tests/live.sh b/e2e-tests/live.sh index afeb092b..507ac6ae 100755 --- a/e2e-tests/live.sh +++ b/e2e-tests/live.sh @@ -36,7 +36,6 @@ echo "$cargo_release_version" [ "$cargo_release_version" = "cargo-release 0.24.9" ] -cargo-binstall --help >/dev/null cargo binstall --help >/dev/null cargo_binstall_version="$(cargo-binstall -V)" diff --git a/justfile b/justfile index 6ea50afe..361602d7 100644 --- a/justfile +++ b/justfile @@ -248,6 +248,7 @@ e2e-test-no-track: (e2e-test "no-track") e2e-test-git: (e2e-test "git") e2e-test-registries: (e2e-test "registries") e2e-test-signing: (e2e-test "signing") +e2e-test-continue-on-failure: (e2e-test "continue-on-failure") # WinTLS (Windows in CI) does not have TLS 1.3 support [windows] @@ -256,7 +257,7 @@ e2e-test-tls: (e2e-test "tls" "1.2") [macos] e2e-test-tls: (e2e-test "tls" "1.2") (e2e-test "tls" "1.3") -e2e-tests: e2e-test-live e2e-test-manifest-path e2e-test-git e2e-test-other-repos e2e-test-strategies e2e-test-version-syntax e2e-test-upgrade e2e-test-tls e2e-test-self-upgrade-no-symlink e2e-test-uninstall e2e-test-subcrate e2e-test-no-track e2e-test-registries e2e-test-signing +e2e-tests: e2e-test-live e2e-test-manifest-path e2e-test-git e2e-test-other-repos e2e-test-strategies e2e-test-version-syntax e2e-test-upgrade e2e-test-tls e2e-test-self-upgrade-no-symlink e2e-test-uninstall e2e-test-subcrate e2e-test-no-track e2e-test-registries e2e-test-signing e2e-test-continue-on-failure unit-tests: print-env {{cargo-bin}} test {{cargo-build-args}}