bootstrap: copy self-contained linking components to `stage0-sysroot` by lqd · Pull Request #116276 · rust-lang/rust

lqd · 2023-09-29T21:02:39Z

I hit this issue while trying to bootstrap using a rustc where rust-lld is used by default: this was the cause of the failure to profile rustc-perf's bootstrap benchmark in #113382.

stage0-sysroot currently only has libs and self-contained objects, not the other self-contained linking components yet. Most notably, it does not contain the linker and wrappers that we build, and that rustup distributes.

If you try to bootstrap using the bootstrap compiler's rust-lld, it will fail to link std at stage0 because rust-lld and the gcc-ld wrappers, will not be found in stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/bin.

This PR copies the bin directory next to the lib directory when rust.lld is enabled in the config (though maybe it could be done unconditionally, the fact that we need it to link does not necessarily mean that we'd want to build and provide it at stage1).

cc @Kobzol who also encountered this issue while using lld during bootstrap.

rustbot · 2023-09-29T21:02:48Z

r? @onur-ozkan

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

Kobzol · 2023-09-29T21:41:47Z

I think that it should be done unconditionally. We should copy the things required by the snapshot compiler to stage0-sysroot, so that it can Do its thing™. Especially if it will eventually use rust-lld by default, because in that case rust-lld needs to be there, so that rustup link stage0 ... continues to work.

rust.lld = true specifies whether lld should appear in stage1/2/N compiler's sysroot, which is an orthogonal decision, IMO.

onur-ozkan · 2023-09-30T12:20:37Z

Doing it unconditionally seems better in my opinion too. r=me with doing it unconditionally.

lqd · 2023-09-30T20:37:45Z

I've removed the rust.lld check, making the copy unconditional.

@bors r=onur-ozkan

bors · 2023-09-30T20:37:47Z

📌 Commit 5fde111 has been approved by onur-ozkan

It is now in the queue for this repository.

bors · 2023-10-01T05:49:46Z

⌛ Testing commit 5fde111 with merge 2685d3e...

bootstrap: copy self-contained linking components to `stage0-sysroot` I hit this issue while trying to bootstrap using a rustc where `rust-lld` is used by default: this was the cause of the failure to profile rustc-perf's bootstrap benchmark in rust-lang#113382. `stage0-sysroot` currently only has libs and self-contained objects, not the other self-contained linking components yet. Most notably, it does not contain the linker and wrappers that we build, and that rustup distributes. If you try to bootstrap using the bootstrap compiler's `rust-lld`, it will fail to link std at stage0 because `rust-lld` and the `gcc-ld` wrappers, will not be found in `stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/bin`. This PR copies the `bin` directory next to the `lib` directory when `rust.lld` is enabled in the config (though maybe it could be done unconditionally, the fact that we need it to link does not necessarily mean that we'd want to build and provide it at stage1). cc `@Kobzol` who also encountered this issue while using lld during bootstrap.

bors · 2023-10-01T05:55:39Z

💔 Test failed - checks-actions

otherwise bootstrap will fail to link the stdlib on a target using the self-contained linker: rust-lld will not be found since it's currently not in the stage0-sysroot.

lqd · 2023-10-01T10:59:41Z

Evidently the snapshot sysroot libdir is "bin" on windows and not "lib", pointing to a non-existent path to rust-lld.
I've pushed an update locating the source sysroot bin folder from the root of the snapshot sysroot, and avoid this bin/lib difference.

i.e. not using rustc_snapshot_libdir since that is "bin" on windows, and not the needed rustc_snapshot_sysroot() + "/lib": the path to rust-lld is always lib/rustlib/host/bin.

onur-ozkan · 2023-10-02T08:59:25Z

@bors r+

bors · 2023-10-02T08:59:27Z

📌 Commit 82d23a6 has been approved by onur-ozkan

It is now in the queue for this repository.

bors · 2023-10-02T09:41:45Z

⌛ Testing commit 82d23a6 with merge b0889cb...

bors · 2023-10-02T11:28:18Z

☀️ Test successful - checks-actions
Approved by: onur-ozkan
Pushing b0889cb to master...

rust-timer · 2023-10-02T14:01:32Z

Finished benchmarking commit (b0889cb): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.5%	[-4.5%, -4.5%]	1
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 626.682s -> 628.193s (0.24%)
Artifact size: 273.33 MiB -> 273.37 MiB (0.02%)

tmandry · 2023-10-02T23:12:20Z

Our stage0 compiler doesn't include rust-lld because we don't use it to link. Shouldn't use-lld tell you whether lld is needed or not?

The unconditional copy causes the build to fail. ~~Can this be reverted?~~ EDIT: #116365 should fix this

lqd · 2023-10-03T00:04:28Z

Shouldn't use-lld tell you whether lld is needed or not?

Not really no, use-lld uses system lld (unless on msvc maybe).

tmandry · 2023-10-03T00:10:51Z

That's not what I interpret config.example.toml to mean:

# Indicates whether LLD will be used to link Rust crates during bootstrap on
# supported platforms. The LLD from the bootstrap distribution will be used
# and not the LLD compiled during the bootstrap.

"LLD from the bootstrap distribution" sounds pretty clearly like stage0. Is the comment wrong?

lqd · 2023-10-03T00:16:44Z

Is the comment wrong?

That seems to be the case: the most recent look at lld usage in bootstrap, #116278, describes this as being maybe true only on msvc.

tmandry · 2023-10-03T00:30:46Z

Gah, that's annoying. Glad we have people cleaning this up now though!

Kobzol · 2023-10-03T05:23:15Z

I guess that the copy should be unconditional of the contents of config.toml, but conditional of the actual presence of rust-lld in the stage0 compiler.

lqd · 2023-10-03T06:43:39Z

Agreed. That’s what I suggested in the new PR.

bootstrap: make copying linker binaries conditional The change in rust-lang#116276 breaks bootstrapping if you don't use `lld` for linking with your stage0 compiler. Making this copy conditional should be enough to fix it.

Rollup merge of rust-lang#116365 - P1n3appl3:master, r=onur-ozkan bootstrap: make copying linker binaries conditional The change in rust-lang#116276 breaks bootstrapping if you don't use `lld` for linking with your stage0 compiler. Making this copy conditional should be enough to fix it.

rustbot assigned onur-ozkan Sep 29, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Sep 29, 2023

Kobzol mentioned this pull request Sep 29, 2023
Generalize LLD usage in bootstrap #116278
Merged

lqd force-pushed the lld-sysroot branch from 935bc19 to 5fde111Compare September 30, 2023 20:34

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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 30, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 1, 2023

This comment has been minimized.
Sign in to view

bootstrap: copy self-contained linking components to stage0-sysroot
82d23a6
otherwise bootstrap will fail to link the stdlib on a target using the self-contained linker: rust-lld will not be found since it's currently not in the stage0-sysroot.

lqd force-pushed the lld-sysroot branch from 5fde111 to 82d23a6Compare October 1, 2023 10:35

onur-ozkan approved these changes Oct 2, 2023
View reviewed changes

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 2, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 2, 2023

bors merged commit b0889cb into rust-lang:masterOct 2, 2023

rustbot added this to the 1.75.0 milestone Oct 2, 2023

lqd deleted the lld-sysroot branch October 2, 2023 11:33

P1n3appl3 mentioned this pull request Oct 2, 2023
bootstrap: make copying linker binaries conditional #116365
Merged

pvdrz mentioned this pull request Oct 4, 2023
Fix divergence from upstream masterferrocene/ferrocene#23
Merged

Uh oh!

Uh oh!

Conversation

lqd commented Sep 29, 2023

Uh oh!

rustbot commented Sep 29, 2023

Uh oh!

Kobzol commented Sep 29, 2023

Uh oh!

onur-ozkan commented Sep 30, 2023

Uh oh!

lqd commented Sep 30, 2023

Uh oh!

bors commented Sep 30, 2023

Uh oh!

bors commented Oct 1, 2023

Uh oh!

bors commented Oct 1, 2023

Uh oh!

This comment has been minimized.

lqd commented Oct 1, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

onur-ozkan commented Oct 2, 2023

Uh oh!

bors commented Oct 2, 2023

Uh oh!

bors commented Oct 2, 2023

Uh oh!

bors commented Oct 2, 2023

Uh oh!

rust-timer commented Oct 2, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

tmandry commented Oct 2, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lqd commented Oct 3, 2023

Uh oh!

tmandry commented Oct 3, 2023

Uh oh!

lqd commented Oct 3, 2023

Uh oh!

tmandry commented Oct 3, 2023

Uh oh!

Kobzol commented Oct 3, 2023

Uh oh!

lqd commented Oct 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

lqd commented Oct 1, 2023•
edited
Loading

tmandry commented Oct 2, 2023•
edited
Loading