Skip to content

Commit

Permalink
Rollup merge of rust-lang#77351 - jyn514:clippy-sysroot, r=Mark-Simul…
Browse files Browse the repository at this point in the history
…acrum

Fix `x.py clippy`

I don't think this ever worked.

Fixes rust-lang#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`
  • Loading branch information
Dylan-DPC committed Nov 6, 2020
2 parents 4f4ce1c + 8d2fa72 commit 3124e79
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 19 deletions.
46 changes: 42 additions & 4 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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[..]),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
43 changes: 29 additions & 14 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
@@ -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![]
}
}

Expand Down Expand Up @@ -62,7 +77,7 @@ impl Step for Std {
run_cargo(
builder,
cargo,
args(builder.kind),
args(builder),
&libstd_stamp(builder, compiler, target),
vec![],
true,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -165,7 +180,7 @@ impl Step for Rustc {
run_cargo(
builder,
cargo,
args(builder.kind),
args(builder),
&librustc_stamp(builder, compiler, target),
vec![],
true,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -278,7 +293,7 @@ macro_rules! tool_check_step {
run_cargo(
builder,
cargo,
args(builder.kind),
args(builder),
&stamp(builder, compiler, target),
vec![],
true,
Expand Down
6 changes: 5 additions & 1 deletion src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub enum Subcommand {
paths: Vec<PathBuf>,
},
Clippy {
fix: bool,
paths: Vec<PathBuf>,
},
Fix {
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 3124e79

Please sign in to comment.