Fix Clippy staging for compiler

Author: Kobzol

src/bootstrap/src/core/build_steps/check.rs

@@ -30,10 +30,6 @@ pub struct Std {
 
 impl Std {
     const CRATE_OR_DEPS: &[&str] = &["sysroot", "coretests", "alloctests"];
-
-    pub fn new(build_compiler: Compiler, target: TargetSelection) -> Self {
-        Self { build_compiler, target, crates: vec![] }
-    }
 }
 
 impl Step for Std {
@@ -232,7 +228,7 @@ impl Step for Rustc {
 }
 
 /// Prepares a compiler that will check something with the given `mode`.
-fn prepare_compiler_for_check(
+pub fn prepare_compiler_for_check(
     builder: &Builder<'_>,
     target: TargetSelection,
     mode: Mode,

src/bootstrap/src/core/build_steps/clippy.rs

@@ -1,14 +1,27 @@
 //! Implementation of running clippy on the compiler, standard library and various tools.
+//!
+//! This serves a double purpose:
+//! - The first is to run Clippy itself on in-tree code, in order to test and dogfood it.
+//! - The second is to actually lint the in-tree codebase on CI, with a hard-coded set of rules,
+//!   which is performed by the `x clippy ci` command.
+//!
+//! In order to prepare a build compiler for running clippy, use the
+//! `check::prepare_compiler_for_check` function. That prepares a compiler and a standard library
+//! for running Clippy. The second part (actually building Clippy) is performed inside
+//! [Builder::cargo_clippy_cmd]. It would be nice if this was more explicit, and we actually had
+//! to pass a prebuilt Clippy from the outside when running `cargo clippy`, but that would be
+//! (as usual) a massive undertaking/refactoring.
 
 use super::check;
 use super::compile::{run_cargo, rustc_cargo, std_cargo};
 use super::tool::{RustcPrivateCompilers, SourceType, prepare_tool_cargo};
 use crate::builder::{Builder, ShouldRun};
+use crate::core::build_steps::check::prepare_compiler_for_check;
 use crate::core::build_steps::compile::std_crates_for_run_make;
 use crate::core::builder;
 use crate::core::builder::{Alias, Kind, RunConfig, Step, StepMetadata, crate_description};
 use crate::utils::build_stamp::{self, BuildStamp};
-use crate::{Mode, Subcommand, TargetSelection};
+use crate::{Compiler, Mode, Subcommand, TargetSelection};
 
 /// Disable the most spammy clippy lints
 const IGNORED_RULES_FOR_STD_AND_RUSTC: &[&str] = &[
@@ -179,14 +192,35 @@ impl Step for Std {
     }
 }
 
+/// Lints the compiler.
+///
+/// This will build Clippy with the `build_compiler` and use it to lint
+/// in-tree rustc.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Rustc {
-    pub target: TargetSelection,
+    build_compiler: Compiler,
+    target: TargetSelection,
     config: LintConfig,
     /// Whether to lint only a subset of crates.
     crates: Vec<String>,
 }
 
+impl Rustc {
+    fn new(
+        builder: &Builder<'_>,
+        target: TargetSelection,
+        config: LintConfig,
+        crates: Vec<String>,
+    ) -> Self {
+        Self {
+            build_compiler: prepare_compiler_for_check(builder, target, Mode::Rustc),
+            target,
+            config,
+            crates,
+        }
+    }
+}
+
 impl Step for Rustc {
     type Output = ();
     const ONLY_HOSTS: bool = true;
@@ -197,43 +231,26 @@ impl Step for Rustc {
     }
 
     fn make_run(run: RunConfig<'_>) {
+        let builder = run.builder;
         let crates = run.make_run_crates(Alias::Compiler);
         let config = LintConfig::new(run.builder);
-        run.builder.ensure(Rustc { target: run.target, config, crates });
+        run.builder.ensure(Rustc::new(builder, run.target, config, crates));
     }
 
-    /// Lints the compiler.
-    ///
-    /// This will lint the compiler for a particular stage of the build using
-    /// the `compiler` targeting the `target` architecture.
     fn run(self, builder: &Builder<'_>) {
-        let compiler = builder.compiler(builder.top_stage, builder.config.host_target);
+        let build_compiler = self.build_compiler;
         let target = self.target;
 
-        if !builder.download_rustc() {
-            if compiler.stage != 0 {
-                // If we're not in stage 0, then we won't have a std from the beta
-                // compiler around. That means we need to make sure there's one in
-                // the sysroot for the compiler to find. Otherwise, we're going to
-                // fail when building crates that need to generate code (e.g., build
-                // scripts and their dependencies).
-                builder.std(compiler, compiler.host);
-                builder.std(compiler, target);
-            } else {
-                builder.ensure(check::Std::new(compiler, target));
-            }
-        }
-
         let mut cargo = builder::Cargo::new(
             builder,
-            compiler,
+            build_compiler,
             Mode::Rustc,
             SourceType::InTree,
             target,
             Kind::Clippy,
         );
 
-        rustc_cargo(builder, &mut cargo, target, &compiler, &self.crates);
+        rustc_cargo(builder, &mut cargo, target, &self.build_compiler, &self.crates);
 
         // Explicitly pass -p for all compiler crates -- this will force cargo
         // to also lint the tests/benches/examples for these crates, rather
@@ -249,15 +266,15 @@ impl Step for Rustc {
             builder,
             cargo,
             lint_args(builder, &self.config, IGNORED_RULES_FOR_STD_AND_RUSTC),
-            &build_stamp::librustc_stamp(builder, compiler, target),
+            &build_stamp::librustc_stamp(builder, build_compiler, target),
             vec![],
             true,
             false,
         );
     }
 
     fn metadata(&self) -> Option<StepMetadata> {
-        Some(StepMetadata::clippy("rustc", self.target))
+        Some(StepMetadata::clippy("rustc", self.target).built_by(self.build_compiler))
     }
 }
 
@@ -515,11 +532,12 @@ impl Step for CI {
             ],
             forbid: vec![],
         };
-        builder.ensure(Rustc {
-            target: self.target,
-            config: self.config.merge(&compiler_clippy_cfg),
-            crates: vec![],
-        });
+        builder.ensure(Rustc::new(
+            builder,
+            self.target,
+            self.config.merge(&compiler_clippy_cfg),
+            vec![],
+        ));
 
         let rustc_codegen_gcc = LintConfig {
             allow: vec![],

src/bootstrap/src/core/builder/mod.rs

@@ -1556,8 +1556,10 @@ You have to build a stage1 compiler for `{}` first, and then use it to build a s
         self.ensure(tool::Rustdoc { target_compiler })
     }
 
-    pub fn cargo_clippy_cmd(&self, run_compiler: Compiler) -> BootstrapCommand {
-        if run_compiler.stage == 0 {
+    /// Create a Cargo command for running Clippy.
+    /// The used Clippy is (or in the case of stage 0, already was) built using `build_compiler`.
+    pub fn cargo_clippy_cmd(&self, build_compiler: Compiler) -> BootstrapCommand {
+        if build_compiler.stage == 0 {
             let cargo_clippy = self
                 .config
                 .initial_cargo_clippy
@@ -1569,15 +1571,16 @@ You have to build a stage1 compiler for `{}` first, and then use it to build a s
             return cmd;
         }
 
-        // FIXME: double check that `run_compiler`'s stage is what we want to use
-        let compilers =
-            RustcPrivateCompilers::new(self, run_compiler.stage, self.build.host_target);
-        assert_eq!(run_compiler, compilers.target_compiler());
+        let compilers = RustcPrivateCompilers::from_build_compiler(
+            self,
+            build_compiler,
+            self.build.host_target,
+        );
 
         let _ = self.ensure(tool::Clippy::from_compilers(compilers));
         let cargo_clippy = self.ensure(tool::CargoClippy::from_compilers(compilers));
         let mut dylib_path = helpers::dylib_path();
-        dylib_path.insert(0, self.sysroot(run_compiler).join("lib"));
+        dylib_path.insert(0, self.sysroot(build_compiler).join("lib"));
 
         let mut cmd = command(cargo_clippy.tool_path);
         cmd.env(helpers::dylib_path_var(), env::join_paths(&dylib_path).unwrap());

src/bootstrap/src/core/builder/tests.rs

@@ -2076,12 +2076,13 @@ mod snapshot {
         [build] llvm <host>
         [build] rustc 0 <host> -> rustc 1 <host>
         [check] rustc 1 <host> -> rustc 2 <host>
-        [build] rustc 0 <host> -> clippy-driver 1 <host>
-        [build] rustc 0 <host> -> cargo-clippy 1 <host>
+        [build] rustc 1 <host> -> std 1 <host>
+        [build] rustc 1 <host> -> rustc 2 <host>
+        [build] rustc 1 <host> -> clippy-driver 2 <host>
+        [build] rustc 1 <host> -> cargo-clippy 2 <host>
         [clippy] bootstrap <host>
         [clippy] std <host>
-        [build] rustc 1 <host> -> std 1 <host>
-        [clippy] rustc <host>
+        [clippy] rustc 0 <host> -> rustc 1 <host>
         [clippy] rustc 0 <host> -> rustc_codegen_gcc 1 <host>
         ");
     }
@@ -2099,17 +2100,30 @@ mod snapshot {
         [build] rustc 1 <host> -> std 1 <host>
         [build] rustc 1 <host> -> rustc 2 <host>
         [check] rustc 2 <host> -> rustc 3 <host>
-        [build] rustc 1 <host> -> clippy-driver 2 <host>
-        [build] rustc 1 <host> -> cargo-clippy 2 <host>
+        [build] rustc 2 <host> -> std 2 <host>
+        [build] rustc 2 <host> -> rustc 3 <host>
+        [build] rustc 2 <host> -> clippy-driver 3 <host>
+        [build] rustc 2 <host> -> cargo-clippy 3 <host>
         [clippy] bootstrap <host>
         [clippy] std <host>
-        [build] rustc 2 <host> -> std 2 <host>
-        [clippy] rustc <host>
-        [build] rustc 0 <host> -> clippy-driver 1 <host>
-        [build] rustc 0 <host> -> cargo-clippy 1 <host>
+        [build] rustc 1 <host> -> clippy-driver 2 <host>
+        [build] rustc 1 <host> -> cargo-clippy 2 <host>
+        [clippy] rustc 1 <host> -> rustc 2 <host>
         [clippy] rustc 1 <host> -> rustc_codegen_gcc 2 <host>
         ");
     }
+
+    #[test]
+    fn clippy_compiler() {
+        let ctx = TestCtx::new();
+        insta::assert_snapshot!(
+            ctx.config("clippy")
+                .path("compiler")
+                .render_steps(), @r"
+        [build] llvm <host>
+        [clippy] rustc 0 <host> -> rustc 1 <host>
+        ");
+    }
 }
 
 struct ExecutedSteps {

src/bootstrap/src/utils/build_stamp.rs

@@ -146,13 +146,13 @@ pub fn libstd_stamp(
 }
 
 /// Cargo's output path for librustc in a given stage, compiled by a particular
-/// compiler for the specified target.
+/// `build_compiler` for the specified target.
 pub fn librustc_stamp(
     builder: &Builder<'_>,
-    compiler: Compiler,
+    build_compiler: Compiler,
     target: TargetSelection,
 ) -> BuildStamp {
-    BuildStamp::new(&builder.cargo_out(compiler, Mode::Rustc, target)).with_prefix("librustc")
+    BuildStamp::new(&builder.cargo_out(build_compiler, Mode::Rustc, target)).with_prefix("librustc")
 }
 
 /// Computes a hash representing the state of a repository/submodule and additional input.