Copy stage0 binaries into stage0-sysroot by chenyukang · Pull Request #101711 · rust-lang/rust

chenyukang · 2022-09-12T06:33:42Z

rust-highfive · 2022-09-12T06:33:45Z

(rust-highfive has picked a reviewer for you, use r? to override)

chenyukang · 2022-09-12T06:34:51Z

jyn514

This looks good :) have you tested locally that it fixes the two problems I mentioned in the issue? The rustup one should be easy to test (same way as for the stage 1 sysroot).

jyn514 · 2022-09-12T14:38:40Z

src/bootstrap/compile.rs

Oh, please also leave a comment explaining why we're special casing stage 0.

jyn514 · 2022-09-13T13:45:39Z

src/bootstrap/compile.rs

+            t!(fs::create_dir_all(&sysroot_bin_dir));
+            builder.cp_r(&stage0_bin_dir, &sysroot_bin_dir);
+
+            // copy all *.so files from stage0/lib to stage0-sysroot/lib


Are you sure this is correct? We shouldn't be copying any artifacts at all for libstd, I don't remember whether we distribute it as an .so or not.

jyn514 · 2022-09-13T15:49:58Z

@bors r+

Thanks! Can you also make a PR to rustc-dev-guide mentioning you can use this for a rustup toolchain, under "suggested workflows"?

bors · 2022-09-13T15:50:01Z

📌 Commit 32f8eb2 has been approved by jyn514

It is now in the queue for this repository.

bors · 2022-09-16T00:02:49Z

⌛ Testing commit 32f8eb2 with merge 0ee5a1a...

bors · 2022-09-16T02:43:46Z

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 0ee5a1a to master...

rust-timer · 2022-09-16T04:09:58Z

Finished benchmarking commit (0ee5a1a): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Warning ⚠: The following benchmark(s) failed to build:

rustc

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	3.7%	[3.2%, 4.3%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.7%	[3.2%, 4.3%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.1%	[2.8%, 3.2%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

Revert "Copy stage0 binaries into stage0-sysroot" This reverts PR rust-lang#101711. The PR broke the rustc/bootstrap benchmark on rustc-perf, I believe due to the assumption that the stage0 directory exists. Fixing that by just skipping this logic might be reasonable, but I think there's a larger discussion to be had around the right behavior when we don't have a single bin/ directory (when rustc= and cargo= are specified in config.toml). I think it's potentially reasonable to put those binaries (cargo, rustc, rustfmt?) into the bin directory, but for now just want to get us back to a healthy state. r? `@jyn514` (but would appreciate review from others as this is just a direct revert).

…crum Copy stage0 `rustc` binaries to `stage0-sysroot` This is basically a revival of rust-lang#101711 and rust-lang#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#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.

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.

rust-highfive assigned Mark-Simulacrum Sep 12, 2022

rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 12, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 12, 2022

chenyukang changed the title ~~opy stage0 binaries into stage0-sysroot~~ Copy stage0 binaries into stage0-sysroot Sep 12, 2022

rust-highfive assigned jyn514 and unassigned Mark-Simulacrum Sep 12, 2022

jyn514 reviewed Sep 12, 2022

View reviewed changes

src/bootstrap/compile.rs Outdated

Copy link

Member

jyn514 Sep 12, 2022

Choose a reason for hiding this comment

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

Oh, please also leave a comment explaining why we're special casing stage 0.

chenyukang force-pushed the fix-101691 branch 3 times, most recently from 4817a0f to 7a7c0d1 Compare September 13, 2022 04:39

jyn514 reviewed Sep 13, 2022

View reviewed changes

fix rust-lang#101691: copy stage0 binaries into stage0-sysroot

32f8eb2

chenyukang force-pushed the fix-101691 branch from 7a7c0d1 to 32f8eb2 Compare September 13, 2022 14:59

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 Sep 13, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 16, 2022

bors merged commit 0ee5a1a into rust-lang:master Sep 16, 2022

rustbot added this to the 1.65.0 milestone Sep 16, 2022

Mark-Simulacrum mentioned this pull request Sep 17, 2022

Revert "Copy stage0 binaries into stage0-sysroot" #101942

Merged

jyn514 mentioned this pull request Feb 7, 2023

Copy stage0 binaries into stage0-sysroot #101691

Closed

KittyBorgX mentioned this pull request Feb 13, 2023

Copy bin/* and lib/*.dylib files to stage0-sysroot #107956

Merged

Kobzol mentioned this pull request Jul 4, 2023

Copy stage0 rustc binaries to stage0-sysroot #113341

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Copy stage0 binaries into stage0-sysroot#101711

Copy stage0 binaries into stage0-sysroot#101711
bors merged 1 commit intorust-lang:masterfrom
chenyukang:fix-101691

chenyukang commented Sep 12, 2022 •

edited

Loading

Uh oh!

rust-highfive commented Sep 12, 2022

Uh oh!

chenyukang commented Sep 12, 2022

Uh oh!

jyn514 left a comment

Uh oh!

jyn514 Sep 12, 2022

Uh oh!

jyn514 Sep 13, 2022

Uh oh!

jyn514 commented Sep 13, 2022

Uh oh!

bors commented Sep 13, 2022

Uh oh!

bors commented Sep 16, 2022

Uh oh!

bors commented Sep 16, 2022

Uh oh!

rust-timer commented Sep 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

chenyukang commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Sep 12, 2022

Uh oh!

chenyukang commented Sep 12, 2022

Uh oh!

jyn514 left a comment

Choose a reason for hiding this comment

Uh oh!

jyn514 Sep 12, 2022

Choose a reason for hiding this comment

Uh oh!

jyn514 Sep 13, 2022

Choose a reason for hiding this comment

Uh oh!

jyn514 commented Sep 13, 2022

Uh oh!

bors commented Sep 13, 2022

Uh oh!

bors commented Sep 16, 2022

Uh oh!

bors commented Sep 16, 2022

Uh oh!

rust-timer commented Sep 16, 2022

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

chenyukang commented Sep 12, 2022 •

edited

Loading