Skip to content

Commit 879c564

Browse files
committed
Auto merge of #12999 - ehuss:fix-clippy-wrapper-race, r=epage
Fix clippy-wrapper test race condition. This fixes an issue where if certain tests ran concurrently, they would stomp on each other writing to a global directory. The problem is that `wrapped_clippy_driver` was writing to a global directory without any locking. The solution is to use the existing locking setup we have for supporting similar global tools. This doesn't often show up because the offending tests are usually separated enough alphabetically, but if they happen to run near each other, they would stomp on each other and corrupt the directory.
2 parents 4ac051e + 2eac6f5 commit 879c564

File tree

4 files changed

+36
-27
lines changed

4 files changed

+36
-27
lines changed

crates/cargo-test-support/src/lib.rs

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -523,29 +523,6 @@ pub fn cargo_exe() -> PathBuf {
523523
snapbox::cmd::cargo_bin("cargo")
524524
}
525525

526-
/// A wrapper around `rustc` instead of calling `clippy`.
527-
pub fn wrapped_clippy_driver() -> PathBuf {
528-
let clippy_driver = project()
529-
.at(paths::global_root().join("clippy-driver"))
530-
.file("Cargo.toml", &basic_manifest("clippy-driver", "0.0.1"))
531-
.file(
532-
"src/main.rs",
533-
r#"
534-
fn main() {
535-
let mut args = std::env::args_os();
536-
let _me = args.next().unwrap();
537-
let rustc = args.next().unwrap();
538-
let status = std::process::Command::new(rustc).args(args).status().unwrap();
539-
std::process::exit(status.code().unwrap_or(1));
540-
}
541-
"#,
542-
)
543-
.build();
544-
clippy_driver.cargo("build").run();
545-
546-
clippy_driver.bin("clippy-driver")
547-
}
548-
549526
/// This is the raw output from the process.
550527
///
551528
/// This is similar to `std::process::Output`, however the `status` is

crates/cargo-test-support/src/tools.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use std::sync::OnceLock;
77

88
static ECHO_WRAPPER: OnceLock<Mutex<Option<PathBuf>>> = OnceLock::new();
99
static ECHO: OnceLock<Mutex<Option<PathBuf>>> = OnceLock::new();
10+
static CLIPPY_DRIVER: OnceLock<Mutex<Option<PathBuf>>> = OnceLock::new();
1011

1112
/// Returns the path to an executable that works as a wrapper around rustc.
1213
///
@@ -107,3 +108,34 @@ pub fn echo_subcommand() -> Project {
107108
p.cargo("build").run();
108109
p
109110
}
111+
112+
/// A wrapper around `rustc` instead of calling `clippy`.
113+
pub fn wrapped_clippy_driver() -> PathBuf {
114+
let mut lock = CLIPPY_DRIVER
115+
.get_or_init(|| Default::default())
116+
.lock()
117+
.unwrap();
118+
if let Some(path) = &*lock {
119+
return path.clone();
120+
}
121+
let clippy_driver = project()
122+
.at(paths::global_root().join("clippy-driver"))
123+
.file("Cargo.toml", &basic_manifest("clippy-driver", "0.0.1"))
124+
.file(
125+
"src/main.rs",
126+
r#"
127+
fn main() {
128+
let mut args = std::env::args_os();
129+
let _me = args.next().unwrap();
130+
let rustc = args.next().unwrap();
131+
let status = std::process::Command::new(rustc).args(args).status().unwrap();
132+
std::process::exit(status.code().unwrap_or(1));
133+
}
134+
"#,
135+
)
136+
.build();
137+
clippy_driver.cargo("build").run();
138+
let path = clippy_driver.bin("clippy-driver");
139+
*lock = Some(path.clone());
140+
path
141+
}

tests/testsuite/check.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use crate::messages::raw_rustc_output;
66
use cargo_test_support::install::exe;
77
use cargo_test_support::paths::CargoPathExt;
88
use cargo_test_support::registry::Package;
9+
use cargo_test_support::tools;
910
use cargo_test_support::{basic_bin_manifest, basic_manifest, git, project};
10-
use cargo_test_support::{tools, wrapped_clippy_driver};
1111

1212
#[cargo_test]
1313
fn check_success() {
@@ -1432,7 +1432,7 @@ fn check_fixable_warning_for_clippy() {
14321432

14331433
foo.cargo("check")
14341434
// We can't use `clippy` so we use a `rustc` workspace wrapper instead
1435-
.env("RUSTC_WORKSPACE_WRAPPER", wrapped_clippy_driver())
1435+
.env("RUSTC_WORKSPACE_WRAPPER", tools::wrapped_clippy_driver())
14361436
.with_stderr_contains("[..] (run `cargo clippy --fix --lib -p foo` to apply 1 suggestion)")
14371437
.run();
14381438
}

tests/testsuite/fix.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use cargo_test_support::compare::assert_match_exact;
55
use cargo_test_support::git::{self, init};
66
use cargo_test_support::paths::{self, CargoPathExt};
77
use cargo_test_support::registry::{Dependency, Package};
8+
use cargo_test_support::tools;
89
use cargo_test_support::{basic_manifest, is_nightly, project, Project};
9-
use cargo_test_support::{tools, wrapped_clippy_driver};
1010

1111
#[cargo_test]
1212
fn do_not_fix_broken_builds() {
@@ -193,7 +193,7 @@ fn broken_clippy_fixes_backed_out() {
193193
.env("__CARGO_FIX_YOLO", "1")
194194
.env("RUSTC", p.root().join("foo/target/debug/foo"))
195195
// We can't use `clippy` so we use a `rustc` workspace wrapper instead
196-
.env("RUSTC_WORKSPACE_WRAPPER", wrapped_clippy_driver())
196+
.env("RUSTC_WORKSPACE_WRAPPER", tools::wrapped_clippy_driver())
197197
.with_stderr_contains(
198198
"warning: failed to automatically apply fixes suggested by rustc \
199199
to crate `bar`\n\

0 commit comments

Comments
 (0)