Add cargo-clippy sysroot test

Whne SYSROOT is defined, clippy-driver will insert a --sysroot argument
when calling rustc. However, when a sysroot argument is already defined,
e.g. through RUSTFLAGS=--sysroot=... the `cargo clippy` call would
error. This tests that the sysroot argument is only passed once and that
SYSROOT is ignored in this case.

.github/driver.sh

@@ -17,6 +17,13 @@ test "$sysroot" = $desired_sysroot
 sysroot=$(SYSROOT=$desired_sysroot ./target/debug/clippy-driver --print sysroot)
 test "$sysroot" = $desired_sysroot
 
+# Check that the --sysroot argument is only passed once (SYSROOT is ignored)
+(
+    cd rustc_tools_util
+    touch src/lib.rs
+    SYSROOT=/tmp RUSTFLAGS="--sysroot=$(rustc --print sysroot)" ../target/debug/cargo-clippy clippy --verbose
+)
+
 # Make sure this isn't set - clippy-driver should cope without it
 unset CARGO_MANIFEST_DIR
 

tests/integration.rs

@@ -1,42 +1,28 @@
-//! To run this test, use
+//! This test is meant to only be run in CI. To run it locally use:
+//!
 //! `env INTEGRATION=rust-lang/log cargo test --test integration --features=integration`
 //!
 //! You can use a different `INTEGRATION` value to test different repositories.
+//!
+//! This test will clone the specified repository and run Clippy on it. The test succeeds, if
+//! Clippy doesn't produce an ICE. Lint warnings are ignored by this test.
 
 #![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";
 
-// NOTE: arguments passed to the returned command will be `clippy-driver` args, not `cargo-clippy`
+#[cfg_attr(feature = "integration", test)]
-// args. Use `cargo_args` to pass arguments to cargo-clippy.
+fn integration_test() {
-fn clippy_command(repo_dir: &Path, cargo_args: &[&str]) -> Command {
+    let repo_name = env::var("INTEGRATION").expect("`INTEGRATION` var not set");
-    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('/')
@@ -57,19 +43,28 @@ fn repo_dir(repo_name: &str) -> PathBuf {
         .expect("unable to run git");
     assert!(st.success());
 
-    repo_dir
+    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");
 
-#[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..]
@@ -104,38 +99,3 @@ 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.
-}