Skip to content

Commit f19004e

Browse files
committed
Don't pass --sysroot twice if SYSROOT is set
This is useful for rust-lang/rust to allow setting a sysroot that's *only* for build scripts, different from the regular sysroot passed in RUSTFLAGS (since cargo doesn't apply RUSTFLAGS to build scripts or proc-macros). That said, the exact motivation is not particularly important: this fixes a regression from 5907e91#r1060215684. Note that only RUSTFLAGS is tested in the new integration test; passing --sysroot through `clippy-driver` never worked as far as I can tell, and no one is using it, so I didn't fix it here.
1 parent 0bca8dd commit f19004e

File tree

2 files changed

+83
-25
lines changed

2 files changed

+83
-25
lines changed

src/driver.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,14 @@ pub fn main() {
256256
LazyLock::force(&ICE_HOOK);
257257
exit(rustc_driver::catch_with_exit_code(move || {
258258
let mut orig_args: Vec<String> = env::args().collect();
259+
let has_sysroot_arg = arg_value(&orig_args, "--sysroot", |_| true).is_some();
259260

260261
let sys_root_env = std::env::var("SYSROOT").ok();
261262
let pass_sysroot_env_if_given = |args: &mut Vec<String>, sys_root_env| {
262263
if let Some(sys_root) = sys_root_env {
263-
args.extend(vec!["--sysroot".into(), sys_root]);
264+
if !has_sysroot_arg {
265+
args.extend(vec!["--sysroot".into(), sys_root]);
266+
}
264267
};
265268
};
266269

tests/integration.rs

Lines changed: 79 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,50 @@
1+
//! To run this test, use `env INTEGRATION=rust-lang/log cargo test --test integration --features integration`
2+
//! You can use a different `INTEGRATION` value to test different repositories.
3+
14
#![cfg(feature = "integration")]
25
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
36
#![warn(rust_2018_idioms, unused_lifetimes)]
47

5-
use std::env;
8+
use std::{env, eprintln};
69
use std::ffi::OsStr;
10+
use std::path::{PathBuf, Path};
711
use std::process::Command;
812

913
#[cfg(not(windows))]
1014
const CARGO_CLIPPY: &str = "cargo-clippy";
1115
#[cfg(windows)]
1216
const CARGO_CLIPPY: &str = "cargo-clippy.exe";
1317

14-
#[cfg_attr(feature = "integration", test)]
15-
fn integration_test() {
16-
let repo_name = env::var("INTEGRATION").expect("`INTEGRATION` var not set");
18+
// NOTE: arguments passed to the returned command will be `clippy-driver` args, not `cargo-clippy` args.
19+
// Use `cargo_args` to pass arguments to cargo-clippy.
20+
fn clippy_command(repo_dir: &Path, cargo_args: &[&str]) -> Command {
21+
let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
22+
let target_dir = option_env!("CARGO_TARGET_DIR").map_or_else(|| root_dir.join("target"), PathBuf::from);
23+
let clippy_binary = target_dir.join(env!("PROFILE")).join(CARGO_CLIPPY);
24+
25+
let mut cargo_clippy = Command::new(clippy_binary);
26+
cargo_clippy
27+
.current_dir(repo_dir)
28+
.env("RUST_BACKTRACE", "full")
29+
.env("CARGO_TARGET_DIR", root_dir.join("target"))
30+
.args([
31+
"clippy",
32+
"--all-targets",
33+
"--all-features",
34+
])
35+
.args(cargo_args)
36+
.args([
37+
"--",
38+
"--cap-lints",
39+
"warn",
40+
"-Wclippy::pedantic",
41+
"-Wclippy::nursery",
42+
]);
43+
cargo_clippy
44+
}
45+
46+
/// Return a directory with a checkout of the repository in `INTEGRATION`.
47+
fn repo_dir(repo_name: &str) -> PathBuf {
1748
let repo_url = format!("https://github.com/{repo_name}");
1849
let crate_name = repo_name
1950
.split('/')
@@ -34,28 +65,19 @@ fn integration_test() {
3465
.expect("unable to run git");
3566
assert!(st.success());
3667

37-
let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
38-
let target_dir = std::path::Path::new(&root_dir).join("target");
39-
let clippy_binary = target_dir.join(env!("PROFILE")).join(CARGO_CLIPPY);
40-
41-
let output = Command::new(clippy_binary)
42-
.current_dir(repo_dir)
43-
.env("RUST_BACKTRACE", "full")
44-
.env("CARGO_TARGET_DIR", target_dir)
45-
.args([
46-
"clippy",
47-
"--all-targets",
48-
"--all-features",
49-
"--",
50-
"--cap-lints",
51-
"warn",
52-
"-Wclippy::pedantic",
53-
"-Wclippy::nursery",
54-
])
55-
.output()
56-
.expect("unable to run clippy");
68+
repo_dir
69+
}
5770

71+
#[cfg_attr(feature = "integration", test)]
72+
fn integration_test() {
73+
let repo_name = env::var("INTEGRATION").expect("`INTEGRATION` var not set");
74+
let repo_dir = repo_dir(&repo_name);
75+
let output = clippy_command(&repo_dir, &[]).output().expect("failed to run clippy");
5876
let stderr = String::from_utf8_lossy(&output.stderr);
77+
if !stderr.is_empty() {
78+
eprintln!("{}", stderr);
79+
}
80+
5981
if let Some(backtrace_start) = stderr.find("error: internal compiler error") {
6082
static BACKTRACE_END_MSG: &str = "end of query stack";
6183
let backtrace_end = stderr[backtrace_start..]
@@ -90,3 +112,36 @@ fn integration_test() {
90112
None => panic!("Process terminated by signal"),
91113
}
92114
}
115+
116+
#[cfg_attr(feature = "integration", test)]
117+
fn test_sysroot() {
118+
let rustc = std::env::var("RUSTC").unwrap_or("rustc".to_string());
119+
let rustc_output = Command::new(rustc).args(["--print", "sysroot"]).output().expect("unable to run rustc");
120+
assert!(rustc_output.status.success());
121+
let sysroot = String::from_utf8(rustc_output.stdout).unwrap();
122+
let sysroot = sysroot.trim_end();
123+
124+
#[track_caller]
125+
fn verify_cmd(cmd: &mut Command) {
126+
// Test that SYSROOT is ignored if `--sysroot` is passed explicitly.
127+
cmd.env("SYSROOT", "/dummy/value/does/not/exist");
128+
// We don't actually care about emitting lints, we only want to verify clippy doesn't give a hard error.
129+
cmd.arg("-Awarnings");
130+
let output = cmd.output().expect("failed to run clippy");
131+
let stderr = String::from_utf8_lossy(&output.stderr);
132+
if !stderr.is_empty() {
133+
panic!("clippy printed an error: {}", stderr);
134+
}
135+
if !output.status.success() {
136+
panic!("clippy exited with an error")
137+
}
138+
}
139+
140+
// This is a fairly small repo; we want to avoid checking out anything heavy twice, so just hard-code it.
141+
let repo_name = "rust-lang/log";
142+
let repo_dir = repo_dir(repo_name);
143+
// Pass the sysroot through RUSTFLAGS.
144+
verify_cmd(clippy_command(&repo_dir, &["--quiet"]).env("RUSTFLAGS", format!("--sysroot={sysroot}")));
145+
// NOTE: we don't test passing the arguments directly to clippy-driver (with `-- --sysroot`) because it breaks for some reason.
146+
// I (@jyn514) haven't taken time to track down the bug because rust-lang/rust uses RUSTFLAGS and nearly no one else uses --sysroot.
147+
}

0 commit comments

Comments
 (0)