From b3246e0cb10a6a32e8f652312985d36581f77c19 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 29 Sep 2020 19:46:32 -0400 Subject: [PATCH 1/5] Set the proper sysroot for clippy Clippy does its own runtime detection of the sysroot, which was incorrect in this case (it used the beta sysroot). This overrides the sysroot to use `stage0-sysroot` instead. - Get `x.py clippy` to work on nightly - Give a nice error message if nightly clippy isn't installed --- src/bootstrap/builder.rs | 41 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index dc4243a76d5da..c0578dea1cd5c 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -850,7 +850,40 @@ impl<'a> Builder<'a> { cargo.args(s.split_whitespace()); } rustflags.env("RUSTFLAGS_BOOTSTRAP"); - rustflags.arg("--cfg=bootstrap"); + if cmd == "clippy" { + // clippy overwrites any sysroot we pass on the command line. + // Tell it to use the appropriate sysroot 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(|_| ()); + if let Err(_) = host_version.and_then(|output| { + if output.status.success() + && t!(std::str::from_utf8(&output.stdout)).contains("nightly") + { + Ok(output) + } else { + Err(()) + } + }) { + eprintln!( + "error: `x.py clippy` requires a nightly host `rustc` toolchain with the `clippy` component" + ); + eprintln!("help: try `rustup default nightly`"); + std::process::exit(1); + } + } else { + rustflags.arg("--cfg=bootstrap"); + } } if self.config.rust_new_symbol_mangling { @@ -975,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) @@ -991,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 From 51f8076403b69d803b1feb4624ba86a92417cdeb Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 29 Sep 2020 20:24:14 -0400 Subject: [PATCH 2/5] Add --fix support to `x.py clippy` --- src/bootstrap/builder.rs | 4 ++-- src/bootstrap/check.rs | 28 +++++++++++++++++----------- src/bootstrap/flags.rs | 6 +++++- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index c0578dea1cd5c..a2759b205d75f 100644 --- a/src/bootstrap/builder.rs +++ b/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, @@ -540,7 +540,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[..]), diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 443a490e342d5..c8363bbc7f771 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -7,9 +7,8 @@ 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)] @@ -17,10 +16,17 @@ pub struct Std { pub target: TargetSelection, } -fn args(kind: Kind) -> Vec { - 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 { + if let Subcommand::Clippy { fix, .. } = builder.config.cmd { + let mut args = vec!["--".to_owned(), "--cap-lints".to_owned(), "warn".to_owned()]; + if fix { + args.insert(0, "--fix".to_owned()); + args.insert(0, "-Zunstable-options".to_owned()); + } + args + } else { + vec![] } } @@ -62,7 +68,7 @@ impl Step for Std { run_cargo( builder, cargo, - args(builder.kind), + args(builder), &libstd_stamp(builder, compiler, target), vec![], true, @@ -104,7 +110,7 @@ impl Step for Std { run_cargo( builder, cargo, - args(builder.kind), + args(builder), &libstd_test_stamp(builder, compiler, target), vec![], true, @@ -165,7 +171,7 @@ impl Step for Rustc { run_cargo( builder, cargo, - args(builder.kind), + args(builder), &librustc_stamp(builder, compiler, target), vec![], true, @@ -220,7 +226,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 +284,7 @@ macro_rules! tool_check_step { run_cargo( builder, cargo, - args(builder.kind), + args(builder), &stamp(builder, compiler, target), vec![], true, diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 3834e50e3fa11..dbfcf4df9b4be 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -55,6 +55,7 @@ pub enum Subcommand { paths: Vec, }, Clippy { + fix: bool, paths: Vec, }, Fix { @@ -273,6 +274,9 @@ To learn more about a subcommand, run `./x.py -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, From bdbb54297ab5375f024517873a8b8a60e789b31a Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 10 Oct 2020 00:47:15 -0400 Subject: [PATCH 3/5] x.py fmt --- src/bootstrap/check.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index c8363bbc7f771..b88dca0a9ed34 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -1,13 +1,11 @@ //! 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}, -}; use crate::{Compiler, Mode, Subcommand}; use std::path::PathBuf; From 31ecd2a124ff9c466c9661862aa0a0ad2d48d1a1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 10 Oct 2020 01:11:01 -0400 Subject: [PATCH 4/5] Allow using clippy with either beta or nightly Not 100% sure this will _always_ work, but it works currently. --- src/bootstrap/builder.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index a2759b205d75f..532949e47a17a 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -866,20 +866,22 @@ impl<'a> Builder<'a> { 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(|_| ()); - if let Err(_) = host_version.and_then(|output| { + let output = host_version.and_then(|output| { if output.status.success() - && t!(std::str::from_utf8(&output.stdout)).contains("nightly") { Ok(output) } else { Err(()) } - }) { + }).unwrap_or_else(|_| { eprintln!( - "error: `x.py clippy` requires a nightly host `rustc` toolchain with the `clippy` component" + "error: `x.py clippy` requires a host `rustc` toolchain with the `clippy` component" ); - eprintln!("help: try `rustup default nightly`"); + 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"); From 8d2fa72fc8064e7800e9d2a6512fa7eb302e8d8d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 26 Oct 2020 21:18:28 -0400 Subject: [PATCH 5/5] Get `--fix` working for everything except rustdoc Here's the error for rustdoc: ``` Checking rustdoc artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu) error: no library targets found in package `rustdoc-tool` ``` --- src/bootstrap/builder.rs | 7 +++---- src/bootstrap/check.rs | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 532949e47a17a..011b053f89809 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -851,8 +851,8 @@ impl<'a> Builder<'a> { } rustflags.env("RUSTFLAGS_BOOTSTRAP"); if cmd == "clippy" { - // clippy overwrites any sysroot we pass on the command line. - // Tell it to use the appropriate sysroot instead. + // 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"); @@ -867,8 +867,7 @@ impl<'a> Builder<'a> { // 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() - { + if output.status.success() { Ok(output) } else { Err(()) diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index b88dca0a9ed34..2e3cfc98c8cf2 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -16,12 +16,23 @@ pub struct Std { /// Returns args for the subcommand itself (not for cargo) fn args(builder: &Builder<'_>) -> Vec { + fn strings<'a>(arr: &'a [&str]) -> impl Iterator + 'a { + arr.iter().copied().map(String::from) + } + if let Subcommand::Clippy { fix, .. } = builder.config.cmd { - let mut args = vec!["--".to_owned(), "--cap-lints".to_owned(), "warn".to_owned()]; + let mut args = vec![]; if fix { - args.insert(0, "--fix".to_owned()); - args.insert(0, "-Zunstable-options".to_owned()); + #[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![]