From 07cc897a4ffd0743f26c43a210089dedfc27fac2 Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Mon, 1 Mar 2021 17:05:22 +0100 Subject: [PATCH] Added support for negative --jobs parameter, counting backwards from max CPUs. --- src/cargo/core/compiler/build_config.rs | 10 +++++++--- src/cargo/core/compiler/context/mod.rs | 6 +++--- src/cargo/core/compiler/timings.rs | 2 +- src/cargo/ops/cargo_package.rs | 2 +- src/cargo/ops/registry.rs | 2 +- src/cargo/util/command_prelude.rs | 17 +++++++++++++--- src/cargo/util/config/mod.rs | 2 +- tests/testsuite/bad_config.rs | 26 ------------------------- tests/testsuite/build.rs | 20 ++++++++++++++----- 9 files changed, 43 insertions(+), 44 deletions(-) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index d0f5431a47e..d2c7398a08c 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -50,14 +50,14 @@ impl BuildConfig { /// * `target.$target.libfoo.metadata` pub fn new( config: &Config, - jobs: Option, + jobs: Option, requested_targets: &[String], mode: CompileMode, ) -> CargoResult { let cfg = config.build_config()?; let requested_kinds = CompileKind::from_requested_targets(config, requested_targets)?; if jobs == Some(0) { - anyhow::bail!("jobs must be at least 1") + anyhow::bail!("jobs must not be zero") } if jobs.is_some() && config.jobserver_from_env().is_some() { config.shell().warn( @@ -66,7 +66,11 @@ impl BuildConfig { its environment, ignoring the `-j` parameter", )?; } - let jobs = jobs.or(cfg.jobs).unwrap_or(::num_cpus::get() as u32); + let jobs = match jobs.or(cfg.jobs) { + None => ::num_cpus::get() as u32, + Some(j) if j < 0 => (::num_cpus::get() as i32 + j).max(1) as u32, + Some(j) => j as u32, + }; Ok(BuildConfig { requested_kinds, diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index cb1dbb6240b..dfb4ccf8e91 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -95,8 +95,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> { let jobserver = match bcx.config.jobserver_from_env() { Some(c) => c.clone(), None => { - let client = Client::new(bcx.build_config.jobs as usize) - .chain_err(|| "failed to create jobserver")?; + let client = + Client::new(bcx.jobs() as usize).chain_err(|| "failed to create jobserver")?; client.acquire_raw()?; client } @@ -558,7 +558,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } pub fn new_jobserver(&mut self) -> CargoResult { - let tokens = self.bcx.build_config.jobs as usize; + let tokens = self.bcx.jobs() as usize; let client = Client::new(tokens).chain_err(|| "failed to create jobserver")?; // Drain the client fully diff --git a/src/cargo/core/compiler/timings.rs b/src/cargo/core/compiler/timings.rs index a206023a99e..4891526f20d 100644 --- a/src/cargo/core/compiler/timings.rs +++ b/src/cargo/core/compiler/timings.rs @@ -461,7 +461,7 @@ impl<'cfg> Timings<'cfg> { self.total_dirty, self.total_fresh + self.total_dirty, max_concurrency, - bcx.build_config.jobs, + bcx.jobs(), num_cpus::get(), self.start_str, total_time, diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index a07f9fe072d..8aadc6ec214 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -27,7 +27,7 @@ pub struct PackageOpts<'cfg> { pub check_metadata: bool, pub allow_dirty: bool, pub verify: bool, - pub jobs: Option, + pub jobs: Option, pub targets: Vec, pub features: Vec, pub all_features: bool, diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 7032ae13090..68e5479dc60 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -47,7 +47,7 @@ pub struct PublishOpts<'cfg> { pub index: Option, pub verify: bool, pub allow_dirty: bool, - pub jobs: Option, + pub jobs: Option, pub targets: Vec, pub dry_run: bool, pub registry: Option, diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 3d7268caac5..c72743aa4a0 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -67,7 +67,8 @@ pub trait AppExt: Sized { self._arg( opt("jobs", "Number of parallel jobs, defaults to # of CPUs") .short("j") - .value_name("N"), + .value_name("N") + .allow_hyphen_values(true), ) } @@ -287,6 +288,16 @@ pub trait ArgMatchesExt { Ok(arg) } + fn value_of_i32(&self, name: &str) -> CargoResult> { + let arg = match self._value_of(name) { + None => None, + Some(arg) => Some(arg.parse::().map_err(|_| { + clap::Error::value_validation_auto(format!("could not parse `{}` as a number", arg)) + })?), + }; + Ok(arg) + } + /// Returns value of the `name` command-line argument as an absolute path fn value_of_path(&self, name: &str, config: &Config) -> Option { self._value_of(name).map(|path| config.cwd().join(path)) @@ -320,8 +331,8 @@ pub trait ArgMatchesExt { Ok(ws) } - fn jobs(&self) -> CargoResult> { - self.value_of_u32("jobs") + fn jobs(&self) -> CargoResult> { + self.value_of_i32("jobs") } fn targets(&self) -> Vec { diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index ecf21d1e810..73e4bc857d3 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -1874,7 +1874,7 @@ pub struct CargoBuildConfig { pub target_dir: Option, pub incremental: Option, pub target: Option, - pub jobs: Option, + pub jobs: Option, pub rustflags: Option, pub rustdocflags: Option, pub rustc_wrapper: Option, diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index e7a6b01c0e5..a8101d63909 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -138,32 +138,6 @@ Caused by: .run(); } -#[cargo_test] -fn bad_cargo_config_jobs() { - let p = project() - .file("src/lib.rs", "") - .file( - ".cargo/config", - r#" - [build] - jobs = -1 - "#, - ) - .build(); - p.cargo("build -v") - .with_status(101) - .with_stderr( - "\ -[ERROR] error in [..].cargo/config: \ -could not load config key `build.jobs` - -Caused by: - invalid value: integer `-1`, expected u32 -", - ) - .run(); -} - #[cargo_test] fn invalid_global_config() { let p = project() diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 35feb0fcdd6..9f9e25dad08 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -4719,6 +4719,18 @@ fn good_cargo_config_jobs() { p.cargo("build -v").run(); } +#[cargo_test] +fn good_jobs() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/foo.rs", &main_file(r#""i am foo""#, &[])) + .build(); + + p.cargo("build --jobs 1").run(); + + p.cargo("build --jobs -1").run(); +} + #[cargo_test] fn invalid_jobs() { let p = project() @@ -4726,11 +4738,9 @@ fn invalid_jobs() { .file("src/foo.rs", &main_file(r#""i am foo""#, &[])) .build(); - p.cargo("build --jobs -1") - .with_status(1) - .with_stderr_contains( - "error: Found argument '-1' which wasn't expected, or isn't valid in this context", - ) + p.cargo("build --jobs 0") + .with_status(101) + .with_stderr_contains("error: jobs must not be zero") .run(); p.cargo("build --jobs over9000")