From 68ba84d9001e4ad3307aebbcbbbe213d99b1e12b Mon Sep 17 00:00:00 2001 From: Philip Metzger Date: Wed, 8 Nov 2023 20:15:16 +0100 Subject: [PATCH] run: Fix up various things. Adress the post-merge comments from #2486, which found a doc comment inaccurate and to not blindly ignore `-j 0` which would've worked until now. I've also reduced the default `jobs` size to one, as it's user-visible configuration which determines how many processes should run. Thanks to @necauqua the controversial `unsafe` usage was already removed. I've omitted to change `revisions` from an Vec to a RevisonArg for the moment, as I will keep working on the file anyway. --- cli/src/commands/run.rs | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/cli/src/commands/run.rs b/cli/src/commands/run.rs index cd343d490a8..7baf6b057fa 100644 --- a/cli/src/commands/run.rs +++ b/cli/src/commands/run.rs @@ -14,8 +14,6 @@ //! This file contains the internal implementation of `run`. -use std::num::NonZeroUsize; - use crate::cli_util::{ resolve_multiple_nonempty_revsets, user_error, CommandError, CommandHelper, RevisionArg, }; @@ -39,8 +37,6 @@ pub struct RunArgs { /// The command to run across all selected revisions. shell_command: String, /// The revisions to change. - /// Multiple revsets are accepted and the work will be done on a - /// intersection of them. #[arg(long, short, default_value = "@")] revisions: Vec, /// A no-op option to match the interface of `git rebase -x`. @@ -55,16 +51,18 @@ pub fn cmd_run(ui: &mut Ui, command: &CommandHelper, args: &RunArgs) -> Result<( let workspace_command = command.workspace_helper(ui)?; let _resolved_commits = resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command, ui)?; - let _jobs = if let Some(_jobs) = args.jobs { - _jobs - } else { - // Use all available cores - - // SAFETY: - // We use a internal constant of 4 threads, if it fails - let available = - std::thread::available_parallelism().unwrap_or(NonZeroUsize::new(4).unwrap()); - available.into() - }; + // Jobs are resolved in this order: + // 1. Commandline argument iff > 0. + // 2. the amount of cores available. + // 3. a single job, if all of the above fails. + let _jobs = match args.jobs { + Some(0) => return Err(user_error("must pass atleast one job")), + Some(jobs) => Some(jobs), + None => std::thread::available_parallelism() + .ok() + .and_then(|t| t.try_into().ok()), + } + // Fallback to a single user-visible job. + .unwrap_or(1usize); Err(user_error("This is a stub, do not use")) }