Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Copy stage0 binaries into stage0-sysroot #101691

Closed
jyn514 opened this issue Sep 11, 2022 · 19 comments · Fixed by #101711, #107956 or #113341
Closed

Copy stage0 binaries into stage0-sysroot #101691

jyn514 opened this issue Sep 11, 2022 · 19 comments · Fixed by #101711, #107956 or #113341
Assignees
@KittyBorgX
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Sep 11, 2022 •
edited
Loading

Currently, the beta binaries are only present in build/stage0/bin; build/stage0-sysroot has only the standard library, not any binaries. That causes two issues:

  1. rustup toolchain link doesn't work with stage0-sysroot, meaning there's no way to change the standard library and use those changes at runtime without doing a full stage 1 build
  2. x dist --stage 0 doesn't work. This is very rare to want to do, but can be useful when modifying the dist process itself, to avoid long compile times.
thread 'main' panicked at 'could not read dir "/home/dawn/projects/rust/build/x86_64-unknown-linux-gnu/stage0-sysroot/bin": Os { code: 2, kind: NotFound, message: "No such file or directory" }', lib.rs:1573:25

Copying the binaries into stage0-sysroot is an easy way to fix both problems at once.

Mentoring instructions: Add a builder.copy call in impl Step for Sysroot

rust/src/bootstrap/compile.rs

Line 1146 in 6f0c4a6

INTERNER.intern_path(sysroot)
which copies the whole bin directory from build/stage0.
@jyn514 jyn514 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Sep 11, 2022
@jyn514
Copy link
Member Author

jyn514 commented Sep 11, 2022

We should also consider renaming stage0-sysroot to something else; the name is very confusing.

@chenyukang
Copy link
Member

@rustbot claim

@chenyukang chenyukang mentioned this issue Sep 12, 2022
Merged
@bjorn3
Copy link
Member

bjorn3 commented Sep 12, 2022

We should also consider renaming stage0-sysroot to something else; the name is very confusing.

The name is correct though. It is a directory you can pass in as --sysroot argument to rustc.

@jyn514
Copy link
Member Author

jyn514 commented Sep 12, 2022

@bjorn3 so is stage0. I'm not saying the name is wrong, just that it's ambiguous and not intuitive.

Honestly I would rather rename all the stageN directories to stageN-sysroot and change stage0-sysroot to stage0-sysroot-with-libstd or something.

@bjorn3
Copy link
Member

bjorn3 commented Sep 12, 2022

stage0-sysroot-with-libstd is confusing too as stage0 already has a libstd. Just one from the bootstrap compiler rather than compiled from the current source.

@jyn514
Copy link
Member Author

jyn514 commented Sep 12, 2022

Ok, then maybe stage0-sysroot-with-recompiled-libstd 🤷 nearly anything would be less confusing than just appending -sysroot IMO

@bjorn3
Copy link
Member

bjorn3 commented Sep 12, 2022

Maybe rename stage0 to bootstrap and stage0-sysroot to stage0, matching stage1-...?

@jyn514
Copy link
Member Author

jyn514 commented Sep 12, 2022

bootstrap is confusing with the artifacts for src/bootstrap, but maybe bootstrap-sysroot would work, keeping stage0-sysroot as is and renaming stage1 to stage1-sysroot

chenyukang added a commit to chenyukang/rust that referenced this issue Sep 13, 2022
@chenyukang
fix rust-lang#101691: copy stage0 binaries into stage0-sysroot
7a7c0d1
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 16, 2022
@bors
Auto merge of rust-lang#101711 - chenyukang:fix-101691, r=jyn514
0ee5a1a 
Copy stage0 binaries into stage0-sysroot

Fixes rust-lang#101691
@bors bors closed this as completed in 32f8eb2 Sep 16, 2022
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this issue Sep 17, 2022
@Mark-Simulacrum
Revert "fix rust-lang#101691: copy stage0 binaries into stage0-sysroot"
f0c78ee 
This reverts commit 32f8eb2.
@jyn514
Copy link
Member Author

jyn514 commented Sep 18, 2022

Reverted in #101942

@jyn514 jyn514 reopened this Sep 18, 2022
@jyn514 jyn514 mentioned this issue Sep 18, 2022
Open
@the8472
Copy link
Member

the8472 commented Oct 3, 2022

This is very rare to want to do

I actually wanted to recommend something like this as an alternative std benchmarking workflow. Currently I have to suggest a stage1 build.

@jyn514
Copy link
Member Author

jyn514 commented Oct 3, 2022

@the8472 I meant dist --stage 0 is very rare. I agree wanting to use a stage 0 toolchain with just the libstd changes is common :)

cuviper pushed a commit to cuviper/rust that referenced this issue Oct 4, 2022
@Mark-Simulacrum @cuviper
Revert "fix rust-lang#101691: copy stage0 binaries into stage0-sysroot"
6504453 
This reverts commit 32f8eb2.

(cherry picked from commit f0c78ee)
@jyn514
Copy link
Member Author

jyn514 commented Nov 5, 2022

@chenyukang do you know when you'll have a chance to follow up here? If you're not planning to follow up, can you please release your assignment on the issue?

@jyn514 jyn514 added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Nov 5, 2022
@chenyukang
Copy link
Member

@chenyukang do you know when you'll have a chance to follow up here? If you're not planning to follow up, can you please release your assignment on the issue?

Sorry, I didn't follow this item recently.
Ok, release it now.

@chenyukang chenyukang removed their assignment Nov 5, 2022
@KittyBorgX
Copy link
Member

KittyBorgX commented Feb 7, 2023 •
edited
Loading

@rustbot claim
I would like to take this issue up [with a bit of mentoring :)] if that's alright. It's my first time contributing!

@jyn514
Copy link
Member Author

jyn514 commented Feb 7, 2023 •
edited
Loading

@KittyBorgX thanks for looking at it :) the first step is to cherry-pick the changes in #101711 to the latest master commit. Then you'll need to fix the bug Mark mentioned in #101942, which I believe has some more details in https://rust-lang.zulipchat.com/#narrow/stream/247081-t-compiler.2Fperformance/topic/rustc.20benchmark. I think it's enough to copy the user-provided rustc over to stage0-sysroot.

Feel free to ask questions here or in https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap.

@KittyBorgX KittyBorgX mentioned this issue Feb 12, 2023
Merged
@est31 est31 mentioned this issue Feb 12, 2023
Open
@bors bors closed this as completed in 9956766 Feb 17, 2023
@the8472 the8472 mentioned this issue Feb 18, 2023
Merged
@jyn514 jyn514 mentioned this issue Feb 19, 2023
Open
@workingjubilee
Copy link
Member

It was mentioned that #108302 reverted this so I am reopening this unless someone can point me to the, er, revert of the revert?

@KittyBorgX
Copy link
Member

KittyBorgX commented Mar 28, 2023 •
edited
Loading

Useful Links and Points for anyone who wants to work on this:
Initial PR : #101711
Revert of the Initial PR : #101942
Attempt 2 : #107956
Revert of attempt 2 : #108302

If I am to understand correctly, the revert of the revert was rejected mainly because rust perf (https://perf.rust-lang.org/status.html) broke. For anyone working on this issue again, here's a point to note:

Just copying the entire parent bin to stage0-sysroot will not work as pointed out by bjorn3 in zulip as, suppose the rustc is installed in /usr/bin/rustc and you try and copy the underlying parent dir which would be /usr/bin. That time, stage0-sysroot/bin will get all the binaries in /usr/bin. This will however work when the installation is in something like : ~/.rustup/toolchains/stable-aarch64-apple-darwin/bin

@KittyBorgX KittyBorgX removed their assignment Mar 28, 2023
@jyn514
Copy link
Member Author

jyn514 commented Apr 13, 2023

My suggestion would be to re-land #107956 almost as-is, except with this feature disabled when a custom build.rustc is set.

@KittyBorgX
Copy link
Member

@rustbot claim
I'll give this another shot then ❤️

@jyn514 jyn514 mentioned this issue Apr 20, 2023
Merged
This was referenced Jul 4, 2023
Closed
Merged
@bors bors closed this as completed in 1065d87 Jul 9, 2023
RalfJung pushed a commit to RalfJung/miri that referenced this issue Jul 10, 2023
@bors
Auto merge of #113341 - Kobzol:stage0-sysroot, r=Mark-Simulacrum
d282f51 
Copy stage0 `rustc` binaries to `stage0-sysroot`

This is basically a revival of rust-lang/rust#101711 and rust-lang/rust#107956, with an added check that the full sysroot will only be created if the original rustc comes from `stage0/bin`.

What is/should be tested:

- [x] `rustup toolchain link stage0` (new libstd is used correctly)
- [x]  `python3 x.py fmt dist --stage 0`
- [x] Custom rustc/cargo in `config.toml` (in this case this logic is ignored)
- [x]  Perfbot (try perf run has succeeded)
- [x] Real use case (rust-lang/backtrace-rs#542)

(Hopefully) fixes: rust-lang/rust#101691

This is not the "end all, be all" solution to this problem, but as long as it resolves the basic use-case, and doesn't break perfbot, I say ship it. This code will probably be nuked anyway Soon™ because of the stage redesign.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@KittyBorgX KittyBorgX

Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
6 participants
@chenyukang @the8472 @bjorn3 @jyn514 @workingjubilee @KittyBorgX