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.

Author: jyn514

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://github.com/{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,42 @@ fn integration_test() {
         None => panic!("Process terminated by signal"),
     }
 }
+
+#[cfg_attr(feature = "integration", test)]
+fn test_sysroot() {
+    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();
+
+    #[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);
+        if !stderr.is_empty() {
+            panic!("clippy printed an error: {}", stderr);
+        }
+        if !output.status.success() {
+            panic!("clippy exited with an error")
+        }
+    }
+
+    // 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.
+}