Skip to content

fix sysroot issue which appears for ci downloaded rustc #104076

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 19, 2022

Conversation

onur-ozkan
Copy link
Member

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
Copy link
Collaborator

rustbot commented Nov 6, 2022

r? @jyn514

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

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 6, 2022
@onur-ozkan onur-ozkan force-pushed the fix-ci-rustc-sysroot branch from b0b8196 to 1cf5fe6 Compare November 6, 2022 22:48
@jyn514
Copy link
Member

jyn514 commented Nov 9, 2022

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
Copy link
Member

jyn514 commented Nov 9, 2022

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.

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);
         }

@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 9, 2022
@onur-ozkan
Copy link
Member Author

onur-ozkan commented Nov 9, 2022

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
Copy link
Member Author

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
Copy link
Member

jyn514 commented Nov 12, 2022

@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
Copy link
Member Author

@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

Comment on lines 1148 to 1155
for stage in 0..=builder.top_stage {
if stage != compiler.stage {
let dir = sysroot_dir(stage);
if !dir.ends_with("ci-rustc-sysroot") {
let _ = fs::remove_dir_all(dir);
}
}
}
Copy link
Member Author

@onur-ozkan onur-ozkan Nov 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan onur-ozkan force-pushed the fix-ci-rustc-sysroot branch 2 times, most recently from c6cff01 to 654a4e8 Compare November 13, 2022 17:38
@jyn514
Copy link
Member

jyn514 commented Nov 17, 2022

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

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 17, 2022

📌 Commit 654a4e8 has been approved by jyn514

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 17, 2022

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

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 17, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2022
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#103117 (Use `IsTerminal` in place of `atty`)
 - rust-lang#103969 (Partial support for running UI tests with `download-rustc`)
 - rust-lang#103989 (Fix build of std for thumbv7a-pc-windows-msvc)
 - rust-lang#104076 (fix sysroot issue which appears for ci downloaded rustc)
 - rust-lang#104469 (Make "long type" printing type aware and trim types in E0275)
 - rust-lang#104497 (detect () to avoid redundant <> suggestion for type)
 - rust-lang#104577 (Don't focus on notable trait parent when hiding it)
 - rust-lang#104587 (Update cargo)
 - rust-lang#104593 (Improve spans for RPITIT object-safety errors)
 - rust-lang#104604 (Migrate top buttons style to CSS variables)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d6298d3 into rust-lang:master Nov 19, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 19, 2022
@onur-ozkan onur-ozkan deleted the fix-ci-rustc-sysroot branch November 19, 2022 18:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x.py build no longer places rustc and rustdoc binaries into stage2/ with config profile = "tools"
4 participants