Skip to content

Commit

Permalink
feat: inherit jobserver from env for all kinds of runner
Browse files Browse the repository at this point in the history
External subcommands are already able to inherit the jobserver from
env since #10511. However, users reported that they've expected
`cargo run` to behave the same as external subcommands.

A popular example "cargo-xtask" pattern is used as scripting to run
arbitrary tasks. Users may want to invoke `cargo run` from Make and
expect some parallelism. This PR provides such an ability to the
general `target.<...>.runner`, which affects `cargo test`,
`cargo bench`, and `cargo run`.

Note that this PR doesn't create any jobserver client if there is no
existing jobserver from the environment. Nor `-j`/`--jobs` would create
a new client. Reasons for this decision:

* There might be crates don't want the jobserver to pollute their
  file descriptors, although they might be rare
* Creating a jobsever driver with the new FIFO named pipe style is not
  yet supported as of `jobserver@0.1.26`. Once we can create a named
  pipe-based jobserver, it will be less risky and inheritance by
  default can be implemented.
  • Loading branch information
weihanglo committed Jan 19, 2024
1 parent ac1dc0b commit 511beaa
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 11 deletions.
9 changes: 8 additions & 1 deletion src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ impl<'cfg> Compilation<'cfg> {
kind: CompileKind,
pkg: &Package,
script_meta: Option<Metadata>,
jobserver_client: Option<&jobserver::Client>,
) -> CargoResult<ProcessBuilder> {
let builder = if let Some((runner, args)) = self.target_runner(kind) {
let mut builder = ProcessBuilder::new(runner);
Expand All @@ -257,7 +258,13 @@ impl<'cfg> Compilation<'cfg> {
} else {
ProcessBuilder::new(cmd)
};
self.fill_env(builder, pkg, script_meta, kind, false)
let mut builder = self.fill_env(builder, pkg, script_meta, kind, false)?;

if let Some(client) = jobserver_client {
builder.inherit_jobserver(client);
}

Ok(builder)
}

/// Prepares a new process with an appropriate environment to run against
Expand Down
8 changes: 7 additions & 1 deletion src/cargo/ops/cargo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,13 @@ pub fn run(
Err(_) => path.to_path_buf(),
};
let pkg = bins[0].0;
let mut process = compile.target_process(exe, unit.kind, pkg, *script_meta)?;
let mut process = compile.target_process(
exe,
unit.kind,
pkg,
*script_meta,
config.jobserver_from_env(),
)?;

// Sets the working directory of the child process to the current working
// directory of the parent process.
Expand Down
8 changes: 7 additions & 1 deletion src/cargo/ops/cargo_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,13 @@ fn cmd_builds(
),
};

let mut cmd = compilation.target_process(path, unit.kind, &unit.pkg, *script_meta)?;
let mut cmd = compilation.target_process(
path,
unit.kind,
&unit.pkg,
*script_meta,
config.jobserver_from_env(),
)?;
cmd.args(test_args);
if unit.target.harness() && config.shell().verbosity() == Verbosity::Quiet {
cmd.arg("--quiet");
Expand Down
8 changes: 0 additions & 8 deletions tests/testsuite/jobserver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,33 +192,25 @@ test-runner:
.env("CARGO", cargo_exe())
.arg("run")
.arg("-j2")
.with_status(2)
.with_stderr_contains("[..]no jobserver from env[..]")
.run();
p.process(make)
.env("PATH", path)
.env("CARGO", cargo_exe())
.arg("run-runner")
.arg("-j2")
.with_status(2)
.with_stderr_contains("[..]this is a runner[..]")
.with_stderr_contains("[..]no jobserver from env[..]")
.run();
p.process(make)
.env("CARGO", cargo_exe())
.arg("test")
.arg("-j2")
.with_status(2)
.with_stdout_contains("[..]no jobserver from env[..]")
.run();
p.process(make)
.env("PATH", path)
.env("CARGO", cargo_exe())
.arg("test-runner")
.arg("-j2")
.with_status(2)
.with_stderr_contains("[..]this is a runner[..]")
.with_stdout_contains("[..]no jobserver from env[..]")
.run();

// but not from `-j` flag
Expand Down

0 comments on commit 511beaa

Please sign in to comment.