Skip to content

Commit

Permalink
style: improve how CARGO_TARGET_DIR is handled (#10200)
Browse files Browse the repository at this point in the history
Previously cargo themis (builds code) and cargo clippy (checks entire
crate) condended with each other for the target directory lock and most
likely invalidated each other’s cache, at least to some extent.

Keeping target directories between the two separate resolves this
problem altogether.
  • Loading branch information
nagisa authored Nov 17, 2023
1 parent 1b0bfca commit 4eb4d0d
Showing 1 changed file with 29 additions and 23 deletions.
52 changes: 29 additions & 23 deletions test-utils/style/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,37 @@ use std::{
};

/// Add common cargo arguments for tests run by this code.
fn cargo_env(cmd: &mut Command) {
fn cargo_env(cmd: &mut Command, target_dir: Option<&str>) {
// Set the working directory to the project root, rather than using whatever default nextest
// gives us.
let style_root = std::env::var_os("CARGO_MANIFEST_DIR").unwrap_or(OsString::from("./"));
let wp_root: PathBuf = [&style_root, OsStr::new(".."), OsStr::new("..")].into_iter().collect();
cmd.current_dir(&wp_root);

// Use a different target directory to avoid invalidating any cache after tests are run (so
// that running `cargo nextest` twice does not rebuild half of the workspace on the 2nd
// rebuild. Unfortunately cargo itself does not readily expose this information to us, so we
// have to guess a little as to where this directory might end up.
//
// NB: We aren't using a temporary directory proper here in order to *allow* keeping cache
// between individual `clippy` runs and such.
let target_dir: PathBuf =
[wp_root.as_os_str(), OsStr::new("target"), OsStr::new("style")].into_iter().collect();
cmd.env("CARGO_TARGET_DIR", target_dir.as_path());
if let Some(tgt_dir) = target_dir {
// Use a different target directory to avoid invalidating any cache after tests are run (so
// that running `cargo nextest` twice does not rebuild half of the workspace on the 2nd
// rebuild. Unfortunately cargo itself does not readily expose this information to us, so
// we have to guess a little as to where this directory might end up.
//
// NB: We aren't using a temporary directory proper here in order to *allow* keeping cache
// between individual `clippy` runs and such.
let target_dir: PathBuf =
[wp_root.as_os_str(), OsStr::new("target"), OsStr::new(tgt_dir)].into_iter().collect();
cmd.env("CARGO_TARGET_DIR", target_dir.as_path());
}
}

/// Create a cargo command.
///
/// You will want to set `target_dir` to some unique `Some` value whenever there’s a chance that
/// this invocation of `cargo` will build any project code. Setting unique values avoids lock
/// contention and unintentional cache invalidation.
fn cargo(target_dir: Option<&str>) -> Command {
let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
let mut cmd = Command::new(cargo);
cargo_env(&mut cmd, target_dir);
cmd
}

fn ensure_success(mut cmd: std::process::Command) {
Expand All @@ -38,36 +52,28 @@ fn ensure_success(mut cmd: std::process::Command) {

#[test]
fn rustfmt() {
let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
let mut cmd = Command::new(cargo);
cargo_env(&mut cmd);
let mut cmd = cargo(None);
cmd.args(&["fmt", "--", "--check"]);
ensure_success(cmd);
}

#[test]
fn clippy() {
let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
let mut cmd = Command::new(cargo);
cargo_env(&mut cmd);
let mut cmd = cargo(Some("style"));
cmd.args(&["clippy", "--all-targets", "--all-features", "--locked"]);
ensure_success(cmd);
}

#[test]
fn deny() {
let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
let mut cmd = Command::new(cargo);
cargo_env(&mut cmd);
let mut cmd = cargo(None);
cmd.args(&["deny", "--all-features", "--locked", "check", "bans"]);
ensure_success(cmd);
}

#[test]
fn themis() {
let cargo = std::env::var_os("CARGO").unwrap_or(OsString::from("cargo"));
let mut cmd = Command::new(cargo);
cargo_env(&mut cmd);
let mut cmd = cargo(Some("themis"));
cmd.args(&["run", "--locked", "-p", "themis"]);
ensure_success(cmd);
}

0 comments on commit 4eb4d0d

Please sign in to comment.