apply some nits and add coverage for downgraded_from

Signed-off-by: onur-ozkan <[email protected]>

Author: onur-ozkan

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

@@ -283,7 +283,7 @@ macro_rules! lint_any {
             }
 
             fn run(self, builder: &Builder<'_>) -> Self::Output {
-                let compiler = builder.compiler(builder.top_stage, builder.config.build, $mode == Mode::ToolRustc);
+                let compiler = builder.compiler(builder.top_stage, builder.config.build, builder.top_stage > 0 && $mode == Mode::ToolRustc);
                 let target = self.target;
 
                 builder.ensure(check::Rustc::new(target, builder).build_kind(Some(Kind::Check)));

src/bootstrap/src/core/build_steps/doc.rs

@@ -953,7 +953,7 @@ macro_rules! tool_doc {
                     // Rustdoc needs the rustc sysroot available to build.
                     // FIXME: is there a way to only ensure `check::Rustc` here? Last time I tried it failed
                     // with strange errors, but only on a full bors test ...
-                    let _ = builder.compiler(compiler.stage, target, false);
+                    let _ = builder.compiler(compiler.stage, target, true);
                 }
 
                 // Build cargo command.

src/bootstrap/src/core/build_steps/tool.rs

@@ -175,7 +175,7 @@ pub fn prepare_tool_cargo(
     extra_features: &[String],
 ) -> CargoCommand {
     // FIXME: remove stage check
-    if mode == Mode::ToolRustc && !compiler.is_downgraded_already() && compiler.stage != 0 {
+    if builder.top_stage > 0 && mode == Mode::ToolRustc && !compiler.is_downgraded_already() {
         // Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
         // we'd have stageN/bin/rustc and stageN/bin/$tool_name be effectively different stage
         // compilers, which isn't what we want.
@@ -617,8 +617,8 @@ impl Step for Rustdoc {
         let compiler = self.compiler;
         let target = compiler.host;
 
-        if compiler.stage == 0 {
-            if !compiler.is_snapshot(builder) && !compiler.is_downgraded_already() {
+        if compiler.stage == 0 && !compiler.is_downgraded_already() {
+            if !compiler.is_snapshot(builder) {
                 panic!("rustdoc in stage 0 must be snapshot rustdoc");
             }
             return builder.initial_rustc.with_file_name(exe("rustdoc", compiler.host));

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

@@ -1220,8 +1220,6 @@ impl<'a> Builder<'a> {
 
         // FIXME: remove stage check
         if stage > 0 && downgrade_for_rustc_tool {
-            assert!(stage > 0, "can't downgrade stage 0 compiler");
-
             self.ensure(compile::Std::new(compiler, host));
             // Similar to `compile::Assemble`, build with the previous stage's compiler. Otherwise
             // we'd have stageN/bin/rustc and stageN/bin/$tool_name be effectively different stage
@@ -1371,8 +1369,12 @@ impl<'a> Builder<'a> {
     }
 
     pub fn rustdoc(&self, mut compiler: Compiler) -> PathBuf {
-        // FIXME: remove stage check
-        if compiler.stage > 0 && !compiler.is_downgraded_already() {
+        if compiler.is_snapshot(self) && !compiler.is_downgraded_already() {
+            return self.initial_rustc.with_file_name(exe("rustdoc", compiler.host));
+        }
+
+        if !compiler.is_downgraded_already() {
+            assert!(compiler.stage > 0, "Can't use stage0 compiler for rustdoc");
             compiler.downgrade();
         }
 

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

@@ -359,11 +359,23 @@ mod defaults {
         assert_eq!(first(cache.all::<tool::ErrorIndex>()), &[tool::ErrorIndex {
             compiler: Compiler::new(0, a)
         }]);
+        assert!(first(cache.all::<tool::Rustdoc>()).is_empty());
+    }
+
+    #[test]
+    fn doc_stage_1() {
+        let mut config = configure("doc", &[TEST_TRIPLE_1], &[TEST_TRIPLE_1]);
+        config.compiler_docs = true;
+        config.stage = 1;
+        config.cmd = Subcommand::Doc { open: false, json: false };
+        let mut cache = run_build(&[], config);
+        let a = TargetSelection::from_user(TEST_TRIPLE_1);
+
         // docs should be built with the beta compiler, not with the stage0 artifacts.
         // recall that rustdoc is off-by-one: `stage` is the compiler rustdoc is _linked_ to,
         // not the one it was built by.
         assert_eq!(first(cache.all::<tool::Rustdoc>()), &[tool::Rustdoc {
-            compiler: Compiler::new(0, a)
+            compiler: Compiler::new(0, a).downgraded_from(1)
         },]);
     }
 }
@@ -396,7 +408,7 @@ mod dist {
         assert_eq!(first(cache.all::<dist::Src>()), &[dist::Src]);
         // Make sure rustdoc is only built once.
         assert_eq!(first(cache.all::<tool::Rustdoc>()), &[tool::Rustdoc {
-            compiler: Compiler::new(1, a).with_stage(1)
+            compiler: Compiler::new(1, a).downgraded_from(2)
         },]);
     }
 
@@ -764,8 +776,8 @@ mod dist {
         // (currently) needed to run "cargo test" on the linkchecker, and
         // should be relatively "free".
         assert_eq!(first(builder.cache.all::<tool::Rustdoc>()), &[
-            tool::Rustdoc { compiler: Compiler::new(0, a) },
-            tool::Rustdoc { compiler: Compiler::new(1, a) },
+            tool::Rustdoc { compiler: Compiler::new(0, a).downgraded_from(1) },
+            tool::Rustdoc { compiler: Compiler::new(1, a).downgraded_from(2) },
         ]);
     }
 }

src/bootstrap/src/lib.rs

@@ -106,8 +106,9 @@ impl std::hash::Hash for Compiler {
 
 impl PartialEq for Compiler {
     fn eq(&self, other: &Self) -> bool {
-        // FIXME: cover `downgraded_from` for test env
         self.stage == other.stage && self.host == other.host
+            // We have coverage for `downgraded_from` in our unit tests
+            && (!cfg!(test) || self.downgraded_from == other.downgraded_from)
     }
 }