From b099d39112efb50b36c65ffefa22b13a92247f20 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 26 Mar 2022 18:08:11 +0800 Subject: [PATCH 1/2] test: external subcommand inherits jobserver --- tests/testsuite/jobserver.rs | 117 +++++++++++++++++++++++------------ 1 file changed, 78 insertions(+), 39 deletions(-) diff --git a/tests/testsuite/jobserver.rs b/tests/testsuite/jobserver.rs index bcb1fdf95a4..76c0d46181e 100644 --- a/tests/testsuite/jobserver.rs +++ b/tests/testsuite/jobserver.rs @@ -4,50 +4,50 @@ use std::net::TcpListener; use std::process::Command; use std::thread; +use cargo_test_support::install::{assert_has_installed_exe, cargo_home}; use cargo_test_support::{cargo_exe, project}; -#[cargo_test] -fn jobserver_exists() { - let p = project() - .file( - "build.rs", - r#" - use std::env; +const EXE_CONTENT: &str = r#" +use std::env; - fn main() { - let var = env::var("CARGO_MAKEFLAGS").unwrap(); - let arg = var.split(' ') - .find(|p| p.starts_with("--jobserver")) - .unwrap(); - let val = &arg[arg.find('=').unwrap() + 1..]; - validate(val); - } +fn main() { + let var = env::var("CARGO_MAKEFLAGS").unwrap(); + let arg = var.split(' ') + .find(|p| p.starts_with("--jobserver")) + .unwrap(); + let val = &arg[arg.find('=').unwrap() + 1..]; + validate(val); +} - #[cfg(unix)] - fn validate(s: &str) { - use std::fs::File; - use std::io::*; - use std::os::unix::prelude::*; - - let fds = s.split(',').collect::>(); - println!("{}", s); - assert_eq!(fds.len(), 2); - unsafe { - let mut read = File::from_raw_fd(fds[0].parse().unwrap()); - let mut write = File::from_raw_fd(fds[1].parse().unwrap()); - - let mut buf = [0]; - assert_eq!(read.read(&mut buf).unwrap(), 1); - assert_eq!(write.write(&buf).unwrap(), 1); - } - } +#[cfg(unix)] +fn validate(s: &str) { + use std::fs::File; + use std::io::*; + use std::os::unix::prelude::*; + + let fds = s.split(',').collect::>(); + println!("{}", s); + assert_eq!(fds.len(), 2); + unsafe { + let mut read = File::from_raw_fd(fds[0].parse().unwrap()); + let mut write = File::from_raw_fd(fds[1].parse().unwrap()); + + let mut buf = [0]; + assert_eq!(read.read(&mut buf).unwrap(), 1); + assert_eq!(write.write(&buf).unwrap(), 1); + } +} - #[cfg(windows)] - fn validate(_: &str) { - // a little too complicated for a test... - } - "#, - ) +#[cfg(windows)] +fn validate(_: &str) { + // a little too complicated for a test... +} +"#; + +#[cargo_test] +fn jobserver_exists() { + let p = project() + .file("build.rs", EXE_CONTENT) .file("src/lib.rs", "") .build(); @@ -57,6 +57,45 @@ fn jobserver_exists() { p.cargo("build -j2").run(); } +#[cargo_test] +fn external_subcommand_inherits_jobserver() { + let make = if cfg!(windows) { + "mingw32-make" + } else { + "make" + }; + if Command::new(make).arg("--version").output().is_err() { + return; + } + + let name = "cargo-jobserver-check"; + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "{name}" + version = "0.0.1" + "# + ), + ) + .file("src/main.rs", EXE_CONTENT) + .file( + "Makefile", + "\ +all: +\t+$(CARGO) jobserver-check +", + ) + .build(); + + p.cargo("install --path .").run(); + assert_has_installed_exe(cargo_home(), name); + + p.process(make).env("CARGO", cargo_exe()).arg("-j2").run(); +} + #[cargo_test] fn makes_jobserver_used() { let make = if cfg!(windows) { From c1b90b97280360457dee682c69e40ae42100fb49 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 23 Mar 2022 14:54:30 +0800 Subject: [PATCH 2/2] Support inheriting jobserver fd for external subcommands If cargo detects the existence of a jobserver, cargo should pass the jobserver down to the external subcommand. Here are the reasons: 1. The existence of jobserver implies the user "expects" the amount of job is under control. However, before this commit, external subcommands cannnot benefit from the global view of the jobserver. 2. `cargo-clippy` as an external subcommand migth also love to respect the jobserver protocol. 3. There are several well-known external subcommands calling "cargo" interally (cargo-fuzz, cargo-tarpaulin, etc.) Caveats: Job without special prefix `+` might still be considered as a sub-make and would inherit the jobserver, though I don't see it as an issue. According to GNU Make Manual "13.1.1 POSIX Jobserver Interaction" [^1], if `--jobserver-auth` option is available in `MAKEFLAGS` but the file descriptors are closed, it means that the calling `make` didn't consider our tool awas a recursive `make` invocation. I make an assumption that if those fds are still open, we are happy to use those jobserver tokens. [^1]: https://www.gnu.org/software/make/manual/make.html#POSIX-Jobserver --- src/bin/cargo/main.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs index b605d911fe0..22c41a1b935 100644 --- a/src/bin/cargo/main.rs +++ b/src/bin/cargo/main.rs @@ -174,11 +174,12 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[&str]) -> Cli }; let cargo_exe = config.cargo_exe()?; - let err = match ProcessBuilder::new(&command) - .env(cargo::CARGO_ENV, cargo_exe) - .args(args) - .exec_replace() - { + let mut cmd = ProcessBuilder::new(&command); + cmd.env(cargo::CARGO_ENV, cargo_exe).args(args); + if let Some(client) = config.jobserver_from_env() { + cmd.inherit_jobserver(client); + } + let err = match cmd.exec_replace() { Ok(()) => return Ok(()), Err(e) => e, };