fix sysroot issue which appears for ci downloaded rustc

[onur-ozkan]

Currently when compiler is downloaded rather than compiled, sysroot is being ci-rustc-sysroot because of

rust/src/bootstrap/compile.rs

Lines 1125 to 1131 in 7eef946

	let sysroot = if compiler.stage == 0 {
	host_dir.join("stage0-sysroot")
	} else if builder.download_rustc() {
	host_dir.join("ci-rustc-sysroot")
	} else {
	host_dir.join(format!("stage{}", compiler.stage))
	};

this.

And rustdoc is overriding the downloaded one at the end of the process.

With the condition I add, we simply check if the current compiler stage is target build stage, if so use the proper sysroot instead of ci-rustc-sysroot.

Resolves #103206

[rustbot]

r? @jyn514

(rustbot has picked a reviewer for you, use r? to override)

[jyn514]

I don't think this is the correct fix. It breaks x build --stage 1 rustdoc when download-rustc is set, for one - that's rare to want to do, but it should still be supported.

I think a more fundamental issue is that we're using sysroots as both an input and output; ci-rustc-sysroot is an input for building rustdoc and an output for placing the new rustdoc binary. We should be using stage2 as the output instead. But that's likely not a trivial change, since the Sysroot struct doesn't know whether it's being used as an input or output.

[jyn514]

I played around with this a bit more - this new code may or may not be on the right track, but I don't have more time to dedicate to it.

Details

diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs
index f0649577984..39ccefc0bc0 100644
--- a/src/bootstrap/compile.rs
+++ b/src/bootstrap/compile.rs
@@ -1122,13 +1122,18 @@ fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
     fn run(self, builder: &Builder<'_>) -> Interned<PathBuf> {
         let compiler = self.compiler;
         let host_dir = builder.out.join(&compiler.host.triple);
-        let sysroot = if compiler.stage == 0 {
-            host_dir.join("stage0-sysroot")
-        } else if builder.download_rustc() && compiler.stage != builder.top_stage {
-            host_dir.join("ci-rustc-sysroot")
-        } else {
-            host_dir.join(format!("stage{}", compiler.stage))
+
+        let sysroot_dir = |stage| {
+            if stage == 0 {
+                host_dir.join("stage0-sysroot")
+            } else if builder.download_rustc() {
+                host_dir.join("ci-rustc-sysroot")
+            } else {
+                host_dir.join(format!("stage{}", stage))
+            }
         };
+
+        let sysroot = dbg!(sysroot_dir(compiler.stage));
         let _ = fs::remove_dir_all(&sysroot);
         t!(fs::create_dir_all(&sysroot));
 
@@ -1140,8 +1145,15 @@ fn run(self, builder: &Builder<'_>) -> Interned<PathBuf> {
             );
 
             // #102002, cleanup stage1 and stage0-sysroot folders when using download-rustc so people don't use old versions of the toolchain by accident.
-            let _ = fs::remove_dir_all(host_dir.join("stage1"));
-            let _ = fs::remove_dir_all(host_dir.join("stage0-sysroot"));
+            for stage in 0..=2 {
+                if stage != compiler.stage {
+                    let dir = sysroot_dir(stage);
+                    if !dir.ends_with("ci-rustc-sysroot") {
+                        println!("removing {}", dir.display());
+                        let _ = fs::remove_dir_all(dir);
+                    }
+                }
+            }
 
             // Copy the compiler into the correct sysroot.
             let ci_rustc_dir =
diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs
index d3952206947..96a59d7b086 100644
--- a/src/bootstrap/tool.rs
+++ b/src/bootstrap/tool.rs
@@ -518,8 +518,6 @@ fn run(self, builder: &Builder<'_>) -> PathBuf {
         // When using `download-rustc` and a stage0 build_compiler, copying rustc doesn't actually
         // build stage0 libstd (because the libstd in sysroot has the wrong ABI). Explicitly build
         // it.
-        builder.ensure(compile::Std::new(build_compiler, target_compiler.host));
-        builder.ensure(compile::Rustc::new(build_compiler, target_compiler.host));
         // NOTE: this implies that `download-rustc` is pretty useless when compiling with the stage0
         // compiler, since you do just as much work.
         if !builder.config.dry_run && builder.download_rustc() && build_compiler.stage == 0 {
@@ -527,6 +525,8 @@ fn run(self, builder: &Builder<'_>) -> PathBuf {
                 "warning: `download-rustc` does nothing when building stage1 tools; consider using `--stage 2` instead"
             );
         }
+        builder.ensure(compile::Std::new(build_compiler, target_compiler.host));
+        builder.ensure(compile::Rustc::new(build_compiler, target_compiler.host));
 
         // The presence of `target_compiler` ensures that the necessary libraries (codegen backends,
         // compiler libraries, ...) are built. Rustdoc does not require the presence of any
diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs
index 9c08eac4edc..2016960edd7 100644
--- a/src/librustdoc/config.rs
+++ b/src/librustdoc/config.rs
@@ -326,7 +326,7 @@ pub(crate) fn from_matches(
             crate::usage("rustdoc");
             return Err(0);
         } else if matches.opt_present("version") {
-            rustc_driver::version("rustdoc", matches);
+            println!("local");
             return Err(0);
         }

[onur-ozkan]

I played around with this a bit more - this new code may or may not be on the right track, but I don't have more time to dedicate to it.

Will cover it, thanks

[onur-ozkan]

I don't think this is the correct fix. It breaks x build --stage 1 rustdoc when download-rustc is set, for one - that's rare to want to do, but it should still be supported.

Can you clarify breaks. What exactly happens?

[jyn514]

@ozkanonur I don't remember off the top of my head, but I think it was trying to copy a file to a sysroot directory that didn't exist, or something like that. I'd expect you to be able to replicate the error locally.

[onur-ozkan]

@ozkanonur I don't remember off the top of my head, but I think it was trying to copy a file to a sysroot directory that didn't exist, or something like that. I'd expect you to be able to replicate the error locally.

Somehow it doesn't happen on my side.

config

profile = "tools"
changelog-seen = 2
[build]
compiler-docs = true
[rust]
deny-warnings = false
download-rustc = true

terminal output with printing some logs

  ~/devspace/personal/rust-dist/tools  $ ../../rust/x build --stage 1 rustdoc
Building rustbuild
   Compiling bootstrap v0.0.0 (/home/nimda/devspace/personal/rust/src/bootstrap)
    Finished dev [unoptimized] target(s) in 2.47s

sysroot: "/home/nimda/devspace/personal/rust-dist/tools/build/x86_64-unknown-linux-gnu/stage0-sysroot"
compiler.stage: 0
builder.download_rustc() false


sysroot: "/home/nimda/devspace/personal/rust-dist/tools/build/x86_64-unknown-linux-gnu/stage1"
compiler.stage: 1
builder.download_rustc() false


sysroot: "/home/nimda/devspace/personal/rust-dist/tools/build/x86_64-unknown-linux-gnu/stage1"
compiler.stage: 1
builder.download_rustc() true


sysroot: "/home/nimda/devspace/personal/rust-dist/tools/build/x86_64-unknown-linux-gnu/stage0-sysroot"
compiler.stage: 0
builder.download_rustc() true

Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.14s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building stage0 compiler artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.15s
Copying stage0 rustc from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
warning: `download-rustc` does nothing when building stage1 tools; consider using `--stage 2` instead
Building rustdoc for stage1 (x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.13s
Build completed successfully in 0:00:03

[onur-ozkan]

@jyn514 I couldn't repreduce the stage 1 issue you mentioned, but still added this to ensure current sysroots will not be deleted.

ps: highlighted code is updated, builder.top_stage in 0..=builder.top_stage replaced with 2

[jyn514]

Thanks. I am still a little unsure this is the right approach, but it seems strictly better than the current situation.

@bors r+

[bors]

📌 Commit 654a4e8 has been approved by jyn514

It is now in the queue for this repository.

[bors]

🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened.