Copy stage0 binaries into stage0-sysroot #101711

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

@@ -1143,6 +1143,12 @@ impl Step for Sysroot {
            }
        }

+        if compiler.stage == 0 {


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

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

Copy stage0 binaries into stage0-sysroot #101711

Copy stage0 binaries into stage0-sysroot #101711

chenyukang commented Sep 12, 2022 •

edited

Loading

rust-highfive commented Sep 12, 2022

chenyukang commented Sep 12, 2022

jyn514 left a comment

jyn514 Sep 12, 2022

jyn514 Sep 13, 2022

jyn514 commented Sep 13, 2022

bors commented Sep 13, 2022

bors commented Sep 16, 2022

bors commented Sep 16, 2022

rust-timer commented Sep 16, 2022

Copy stage0 binaries into stage0-sysroot #101711

Copy stage0 binaries into stage0-sysroot #101711

Conversation

chenyukang commented Sep 12, 2022 • edited Loading

rust-highfive commented Sep 12, 2022

chenyukang commented Sep 12, 2022

jyn514 left a comment

Choose a reason for hiding this comment

jyn514 Sep 12, 2022

Choose a reason for hiding this comment

jyn514 Sep 13, 2022

Choose a reason for hiding this comment

jyn514 commented Sep 13, 2022

bors commented Sep 13, 2022

bors commented Sep 16, 2022

bors commented Sep 16, 2022

rust-timer commented Sep 16, 2022

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

chenyukang commented Sep 12, 2022 •

edited

Loading