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.

src/driver.rs

@@ -256,11 +256,14 @@ pub fn main() {
  LazyLock::force(&ICE_HOOK);
  exit(rustc_driver::catch_with_exit_code(move || {
  let mut orig_args: Vec<String> = env::args().collect();
+ let has_sysroot_arg = arg_value(&orig_args, "--sysroot", |_| true).is_some();
 
  let sys_root_env = std::env::var("SYSROOT").ok();
  let pass_sysroot_env_if_given = |args: &mut Vec<String>, sys_root_env| {
  if let Some(sys_root) = sys_root_env {
- args.extend(vec!["--sysroot".into(), sys_root]);
+ if !has_sysroot_arg {
+ args.extend(vec!["--sysroot".into(), sys_root]);
+ }
  };
  };
 

tests/integration.rs

@@ -1,19 +1,42 @@
+//! To run this test, use
+//! `env INTEGRATION=rust-lang/log cargo test --test integration --features=integration`
+//!
+//! You can use a different `INTEGRATION` value to test different repositories.
+
 #![cfg(feature = "integration")]
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 #![warn(rust_2018_idioms, unused_lifetimes)]
 
-use std::env;
 use std::ffi::OsStr;
+use std::path::{Path, PathBuf};
 use std::process::Command;
+use std::{env, eprintln};
 
 #[cfg(not(windows))]
 const CARGO_CLIPPY: &str = "cargo-clippy";
 #[cfg(windows)]
 const CARGO_CLIPPY: &str = "cargo-clippy.exe";
 
-#[cfg_attr(feature = "integration", test)]
-fn integration_test() {
- let repo_name = env::var("INTEGRATION").expect("`INTEGRATION` var not set");
+// NOTE: arguments passed to the returned command will be `clippy-driver` args, not `cargo-clippy`
+// args. Use `cargo_args` to pass arguments to cargo-clippy.
+fn clippy_command(repo_dir: &Path, cargo_args: &[&str]) -> Command {
+ let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+ let target_dir = option_env!("CARGO_TARGET_DIR").map_or_else(|| root_dir.join("target"), PathBuf::from);
+ let clippy_binary = target_dir.join(env!("PROFILE")).join(CARGO_CLIPPY);
+
+ let mut cargo_clippy = Command::new(clippy_binary);
+ cargo_clippy
+ .current_dir(repo_dir)
+ .env("RUST_BACKTRACE", "full")
+ .env("CARGO_TARGET_DIR", root_dir.join("target"))
+ .args(["clippy", "--all-targets", "--all-features"])
+ .args(cargo_args)
+ .args(["--", "--cap-lints", "warn", "-Wclippy::pedantic", "-Wclippy::nursery"]);
+ cargo_clippy
+}
+
+/// Return a directory with a checkout of the repository in `INTEGRATION`.
+fn repo_dir(repo_name: &str) -> PathBuf {
  let repo_url = format!("https:/{repo_name}");
  let crate_name = repo_name
  .split('/')
@@ -34,28 +57,19 @@ fn integration_test() {
  .expect("unable to run git");
  assert!(st.success());
 
- let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
- let target_dir = std::path::Path::new(&root_dir).join("target");
- let clippy_binary = target_dir.join(env!("PROFILE")).join(CARGO_CLIPPY);
-
- let output = Command::new(clippy_binary)
- .current_dir(repo_dir)
- .env("RUST_BACKTRACE", "full")
- .env("CARGO_TARGET_DIR", target_dir)
- .args([
- "clippy",
- "--all-targets",
- "--all-features",
- "--",
- "--cap-lints",
- "warn",
- "-Wclippy::pedantic",
- "-Wclippy::nursery",
- ])
- .output()
- .expect("unable to run clippy");
+ repo_dir
+}
 
+#[cfg_attr(feature = "integration", test)]
+fn integration_test() {
+ let repo_name = env::var("INTEGRATION").expect("`INTEGRATION` var not set");
+ let repo_dir = repo_dir(&repo_name);
+ let output = clippy_command(&repo_dir, &[]).output().expect("failed to run clippy");
  let stderr = String::from_utf8_lossy(&output.stderr);
+ if !stderr.is_empty() {
+ eprintln!("{stderr}");
+ }
+
  if let Some(backtrace_start) = stderr.find("error: internal compiler error") {
  static BACKTRACE_END_MSG: &str = "end of query stack";
  let backtrace_end = stderr[backtrace_start..]
@@ -90,3 +104,39 @@ fn integration_test() {
  None => panic!("Process terminated by signal"),
  }
 }
+
+#[cfg_attr(feature = "integration", test)]
+fn test_sysroot() {
+
+ #[track_caller]
+ fn verify_cmd(cmd: &mut Command) {
+ // Test that SYSROOT is ignored if `--sysroot` is passed explicitly.
+ cmd.env("SYSROOT", "/dummy/value/does/not/exist");
+ // We don't actually care about emitting lints, we only want to verify clippy doesn't give a hard
+ // error.
+ cmd.arg("-Awarnings");
+ let output = cmd.output().expect("failed to run clippy");
+ let stderr = String::from_utf8_lossy(&output.stderr);
+ assert!(stderr.is_empty(), "clippy printed an error: {stderr}");
+ assert!(output.status.success(), "clippy exited with an error");
+ }
+
+ let rustc = std::env::var("RUSTC").unwrap_or("rustc".to_string());
+ let rustc_output = Command::new(rustc)
+ .args(["--print", "sysroot"])
+ .output()
+ .expect("unable to run rustc");
+ assert!(rustc_output.status.success());
+ let sysroot = String::from_utf8(rustc_output.stdout).unwrap();
+ let sysroot = sysroot.trim_end();
+
+ // This is a fairly small repo; we want to avoid checking out anything heavy twice, so just
+ // hard-code it.
+ let repo_name = "rust-lang/log";
+ let repo_dir = repo_dir(repo_name);
+ // Pass the sysroot through RUSTFLAGS.
+ verify_cmd(clippy_command(&repo_dir, &["--quiet"]).env("RUSTFLAGS", format!("--sysroot={sysroot}")));
+ // NOTE: we don't test passing the arguments directly to clippy-driver (with `-- --sysroot`)
+ // because it breaks for some reason. I (@jyn514) haven't taken time to track down the bug
+ // because rust-lang/rust uses RUSTFLAGS and nearly no one else uses --sysroot.
+}