Auto merge of #77351 - jyn514:clippy-sysroot, r=Mark-Simulacrum

Fix `x.py clippy`

I don't think this ever worked.

Fixes #77309. `--fix` support is a work in progress, but works for a very small subset of `libtest`.

This works by using the host `cargo-clippy` driver; it does not use `stage0.txt` at all. To mitigate confusion from this, it gives an error if you don't have `rustc +nightly` as the default rustc in `$PATH`. Additionally, it means that bootstrap can't set `RUSTC`; this makes it no longer possible for clippy to detect the sysroot itself. Instead, bootstrap passes the sysroot to cargo.

r? `@ghost`

Author: bors

Diff for: src/bootstrap/builder.rs

@@ -371,7 +371,7 @@ impl<'a> Builder<'a> {
                 tool::CargoMiri,
                 native::Lld
             ),
-            Kind::Check | Kind::Clippy | Kind::Fix | Kind::Format => describe!(
+            Kind::Check | Kind::Clippy { .. } | Kind::Fix | Kind::Format => describe!(
                 check::Std,
                 check::Rustc,
                 check::Rustdoc,
@@ -539,7 +539,7 @@ impl<'a> Builder<'a> {
         let (kind, paths) = match build.config.cmd {
             Subcommand::Build { ref paths } => (Kind::Build, &paths[..]),
             Subcommand::Check { ref paths, all_targets: _ } => (Kind::Check, &paths[..]),
-            Subcommand::Clippy { ref paths } => (Kind::Clippy, &paths[..]),
+            Subcommand::Clippy { ref paths, .. } => (Kind::Clippy, &paths[..]),
             Subcommand::Fix { ref paths } => (Kind::Fix, &paths[..]),
             Subcommand::Doc { ref paths, .. } => (Kind::Doc, &paths[..]),
             Subcommand::Test { ref paths, .. } => (Kind::Test, &paths[..]),
@@ -849,7 +849,41 @@ impl<'a> Builder<'a> {
                 cargo.args(s.split_whitespace());
             }
             rustflags.env("RUSTFLAGS_BOOTSTRAP");
-            rustflags.arg("--cfg=bootstrap");
+            if cmd == "clippy" {
+                // clippy overwrites sysroot if we pass it to cargo.
+                // Pass it directly to clippy instead.
+                // NOTE: this can't be fixed in clippy because we explicitly don't set `RUSTC`,
+                // so it has no way of knowing the sysroot.
+                rustflags.arg("--sysroot");
+                rustflags.arg(
+                    self.sysroot(compiler)
+                        .as_os_str()
+                        .to_str()
+                        .expect("sysroot must be valid UTF-8"),
+                );
+                // Only run clippy on a very limited subset of crates (in particular, not build scripts).
+                cargo.arg("-Zunstable-options");
+                // Explicitly does *not* set `--cfg=bootstrap`, since we're using a nightly clippy.
+                let host_version = Command::new("rustc").arg("--version").output().map_err(|_| ());
+                let output = host_version.and_then(|output| {
+                    if output.status.success() {
+                        Ok(output)
+                    } else {
+                        Err(())
+                    }
+                }).unwrap_or_else(|_| {
+                    eprintln!(
+                        "error: `x.py clippy` requires a host `rustc` toolchain with the `clippy` component"
+                    );
+                    eprintln!("help: try `rustup component add clippy`");
+                    std::process::exit(1);
+                });
+                if !t!(std::str::from_utf8(&output.stdout)).contains("nightly") {
+                    rustflags.arg("--cfg=bootstrap");
+                }
+            } else {
+                rustflags.arg("--cfg=bootstrap");
+            }
         }
 
         if self.config.rust_new_symbol_mangling {
@@ -974,7 +1008,6 @@ impl<'a> Builder<'a> {
         // src/bootstrap/bin/{rustc.rs,rustdoc.rs}
         cargo
             .env("RUSTBUILD_NATIVE_DIR", self.native_dir(target))
-            .env("RUSTC", self.out.join("bootstrap/debug/rustc"))
             .env("RUSTC_REAL", self.rustc(compiler))
             .env("RUSTC_STAGE", stage.to_string())
             .env("RUSTC_SYSROOT", &sysroot)
@@ -990,6 +1023,11 @@ impl<'a> Builder<'a> {
             )
             .env("RUSTC_ERROR_METADATA_DST", self.extended_error_dir())
             .env("RUSTC_BREAK_ON_ICE", "1");
+        // Clippy support is a hack and uses the default `cargo-clippy` in path.
+        // Don't override RUSTC so that the `cargo-clippy` in path will be run.
+        if cmd != "clippy" {
+            cargo.env("RUSTC", self.out.join("bootstrap/debug/rustc"));
+        }
 
         // Dealing with rpath here is a little special, so let's go into some
         // detail. First off, `-rpath` is a linker option on Unix platforms

Diff for: src/bootstrap/check.rs

@@ -1,26 +1,41 @@
 //! Implementation of compiling the compiler and standard library, in "check"-based modes.
 
+use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step};
 use crate::cache::Interned;
 use crate::compile::{add_to_sysroot, run_cargo, rustc_cargo, rustc_cargo_env, std_cargo};
 use crate::config::TargetSelection;
 use crate::tool::{prepare_tool_cargo, SourceType};
 use crate::INTERNER;
-use crate::{
-    builder::{Builder, Kind, RunConfig, ShouldRun, Step},
-    Subcommand,
-};
-use crate::{Compiler, Mode};
+use crate::{Compiler, Mode, Subcommand};
 use std::path::PathBuf;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct Std {
     pub target: TargetSelection,
 }
 
-fn args(kind: Kind) -> Vec<String> {
-    match kind {
-        Kind::Clippy => vec!["--".to_owned(), "--cap-lints".to_owned(), "warn".to_owned()],
-        _ => Vec::new(),
+/// Returns args for the subcommand itself (not for cargo)
+fn args(builder: &Builder<'_>) -> Vec<String> {
+    fn strings<'a>(arr: &'a [&str]) -> impl Iterator<Item = String> + 'a {
+        arr.iter().copied().map(String::from)
+    }
+
+    if let Subcommand::Clippy { fix, .. } = builder.config.cmd {
+        let mut args = vec![];
+        if fix {
+            #[rustfmt::skip]
+            args.extend(strings(&[
+                "--fix", "-Zunstable-options",
+                // FIXME: currently, `--fix` gives an error while checking tests for libtest,
+                // possibly because libtest is not yet built in the sysroot.
+                // As a workaround, avoid checking tests and benches when passed --fix.
+                "--lib", "--bins", "--examples",
+            ]));
+        }
+        args.extend(strings(&["--", "--cap-lints", "warn"]));
+        args
+    } else {
+        vec![]
     }
 }
 
@@ -62,7 +77,7 @@ impl Step for Std {
         run_cargo(
             builder,
             cargo,
-            args(builder.kind),
+            args(builder),
             &libstd_stamp(builder, compiler, target),
             vec![],
             true,
@@ -104,7 +119,7 @@ impl Step for Std {
             run_cargo(
                 builder,
                 cargo,
-                args(builder.kind),
+                args(builder),
                 &libstd_test_stamp(builder, compiler, target),
                 vec![],
                 true,
@@ -165,7 +180,7 @@ impl Step for Rustc {
         run_cargo(
             builder,
             cargo,
-            args(builder.kind),
+            args(builder),
             &librustc_stamp(builder, compiler, target),
             vec![],
             true,
@@ -220,7 +235,7 @@ impl Step for CodegenBackend {
         run_cargo(
             builder,
             cargo,
-            args(builder.kind),
+            args(builder),
             &codegen_backend_stamp(builder, compiler, target, backend),
             vec![],
             true,
@@ -278,7 +293,7 @@ macro_rules! tool_check_step {
                 run_cargo(
                     builder,
                     cargo,
-                    args(builder.kind),
+                    args(builder),
                     &stamp(builder, compiler, target),
                     vec![],
                     true,

Diff for: src/bootstrap/flags.rs

@@ -55,6 +55,7 @@ pub enum Subcommand {
         paths: Vec<PathBuf>,
     },
     Clippy {
+        fix: bool,
         paths: Vec<PathBuf>,
     },
     Fix {
@@ -273,6 +274,9 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`",
             "bench" => {
                 opts.optmulti("", "test-args", "extra arguments", "ARGS");
             }
+            "clippy" => {
+                opts.optflag("", "fix", "automatically apply lint suggestions");
+            }
             "doc" => {
                 opts.optflag("", "open", "open the docs in a browser");
             }
@@ -513,7 +517,7 @@ Arguments:
             "check" | "c" => {
                 Subcommand::Check { paths, all_targets: matches.opt_present("all-targets") }
             }
-            "clippy" => Subcommand::Clippy { paths },
+            "clippy" => Subcommand::Clippy { paths, fix: matches.opt_present("fix") },
             "fix" => Subcommand::Fix { paths },
             "test" | "t" => Subcommand::Test {
                 paths,