Simplify LLVM bitcode linker in bootstrap and add tests for it #142357

Kobzol · 2025-06-11T11:15:26Z

This PR tries to simplify the LlvmBitcodeLinker step a little bit, and add tests for it. It also adds tests for LldWrapper.

r? @jieyouxu

jieyouxu

Thanks, some questions/feedback, but looks good overall

jieyouxu · 2025-06-11T11:47:07Z

src/bootstrap/src/core/build_steps/compile.rs

                let bindir_self_contained = builder
                    .sysroot(compiler)
                    .join(format!("lib/rustlib/{}/bin/self-contained", compiler.host));


Suggestion (follow-up, not for this PR): maybe pull out a method to do the "path to sysroot target self-contained" logic.

I actually had a commit for that, but it's super tricky (lol). Because:

There are like 5 places in bootstrap that access the self-contained directory. Some of them need a relative path, some of them absolute, some of them work with a compiler, some don't, it's a mess.

The functions that we have in Builder for getting similar paths (invoked through the Libdir step) actually delete the directory before returning it to you 🤦 I tried to use it here and it broke everything 😂 I need to clean all of this up eventually, and move the directory clearing to a single step, so that you can get a path to something without bootstrap falling over, lol.

Yeah, this is uhhh tricky. I'll open a issue about this follow-up (and that it's tricky).

#142361, not in scope for this PR (keeping this unresolved to make it easier to find).

jieyouxu · 2025-06-11T11:50:17Z

src/bootstrap/src/core/builder/mod.rs

+    /// Return the lowest stage compiler that can compile code for the given `target`.
+    pub fn compiler_for_target(&self, target: TargetSelection) -> Compiler {


Discussion: maybe tool_compiler_for_target? I ask because this should also say something like "it must not be used to build stuff that depends on staged rustc/std", right?

I don't think there's any requirement like this. It's really just "give me a compiler that can build code for target T". If stage0 could cross-compile for any target, we could use it for this always.

We actually have a bunch of situations like this in bootstrap already, but it was always done sort of implicitly. Here I wanted to make it explicit.

In other words, I think that this could be useful also for other things than just tools. But right now it's only for a single tool, ofc, so happy to rename if you want.

Hm right. The reason I asked is that

this could be useful also for other things than just tools

is probably true, just that the "other things" must not use this method if they do "depend on staged compiler/std" (be it through rustc_private or directly or somehow).

For now, I think it's safer to name this as sth like tool_compiler_for_target to make it clear that you should not use it for sth beyond its purpose (because we also have Builder::{compiler,compiler_for}), and then consider its naming if we do use it for not-just-tools, but it's not really a blocking concern.

src/bootstrap/src/core/builder/mod.rs

jieyouxu · 2025-06-11T11:54:01Z

src/bootstrap/src/core/builder/tests.rs

+/// Check that during a non-cross-compiling stage 2 build, we only compile rustc host tools
+/// (such as llvm-bitcode-linker) only once.
+#[test]
+fn llvm_bitcode_linker_compile_once() {


Question: should we have another test that does check the cross-compiling case, that we do build a stage 1 (rustc, std) pair that can produce artifacts for target, and that llvm-bitcode-linker is produced by that pair for the target?

Sure, happy to add it. Although from my local experiments, I'm almost sure that bootstrap is actually broken for these situations somehow. Like, ./x build --host i686-unknown-linux-gnu, which is like the simplest cross-compilation scenario on x64 Linux, fails for me on master.

And you would be right 🎡

jieyouxu · 2025-06-11T11:55:15Z

@rustbot author

rustbot · 2025-06-11T11:55:19Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

bors · 2025-06-12T08:12:06Z

☔ The latest upstream changes (presumably #142392) made this pull request unmergeable. Please resolve the merge conflicts.

…=jieyouxu Add initial version of snapshot tests to bootstrap When making any changes to bootstrap (steps), it is very difficult to realize how does it affect various common bootstrap commands, and if everything still works as we expect it to. We are far away from having actual end-to-end tests, but what we could at least do is have a way of testing what steps does bootstrap execute in dry run mode. Now, we already have something like this in `src/bootstrap/src/core/builder/tests.rs`, however that is quite limited, because it only checks executed steps for a specific impl of `Step` and it does not consider step order. Recently, when working on what I thought was one of the simplest possible step untanglings in bootstrap (rust-lang#142357), I ran into errors in tests that were quite hard to debug. Partly also because the current staging test diffs are multiline and use `Debug` output, so it's quite difficult for me to make sense of them. In this PR, I introduce `insta`, which allows writing snapshot tests in a very simple way. With it, I want to allow writing tests that will clearly show us what is going on during bootstrap execution, and then write golden tests for `build/check/test` stage `0/1/2` for compiler/std/tools etc., to make sure that we don't regress something, and also to help with [#t-infra/bootstrap > Proposal to cleanup stages and steps after the redesign](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Proposal.20to.20cleanup.20stages.20and.20steps.20after.20the.20redesign/with/523488806), to help avoid a situation where we would (again) have to make a flurry of staging changes because of unexpected consequences. In the snapshot tests, we currently render the build of rustc, std and LLVM. Currently I render the executed steps using downcasting, which is not super pretty, but it allows us to make the test rendering localized in one place, and it's IMO enough for now. I implemented only a single test using the new machinery. Maybe if you take a look at it, you will understand why 😆 Bootstrap currently does some peculiar things, such as running a stage 0 std step (even though stage 0 std no longer exists) and running the Rustc stage 0 -> 1 step twice, once with a single crates, once with all rustc crates. So I think that even with this single step, there will be a bunch of things to fix in the near future... The way we currently prepare the Config test fixtures is far from ideal, this is something I think `@Shourya742` could work on as a part of their GSoC project (remove as much command execution from Config construction as possible, actually run bootstrap on a temporary directory instead of running it on the rustc checkout, create a Builder-like API for creating the Config test fixtures). r? `@jieyouxu`

…=jieyouxu Add initial version of snapshot tests to bootstrap When making any changes to bootstrap (steps), it is very difficult to realize how does it affect various common bootstrap commands, and if everything still works as we expect it to. We are far away from having actual end-to-end tests, but what we could at least do is have a way of testing what steps does bootstrap execute in dry run mode. Now, we already have something like this in `src/bootstrap/src/core/builder/tests.rs`, however that is quite limited, because it only checks executed steps for a specific impl of `Step` and it does not consider step order. Recently, when working on what I thought was one of the simplest possible step untanglings in bootstrap (rust-lang#142357), I ran into errors in tests that were quite hard to debug. Partly also because the current staging test diffs are multiline and use `Debug` output, so it's quite difficult for me to make sense of them. In this PR, I introduce `insta`, which allows writing snapshot tests in a very simple way. With it, I want to allow writing tests that will clearly show us what is going on during bootstrap execution, and then write golden tests for `build/check/test` stage `0/1/2` for compiler/std/tools etc., to make sure that we don't regress something, and also to help with [#t-infra/bootstrap > Proposal to cleanup stages and steps after the redesign](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Proposal.20to.20cleanup.20stages.20and.20steps.20after.20the.20redesign/with/523488806), to help avoid a situation where we would (again) have to make a flurry of staging changes because of unexpected consequences. In the snapshot tests, we currently render the build of rustc, std and LLVM. Currently I render the executed steps using downcasting, which is not super pretty, but it allows us to make the test rendering localized in one place, and it's IMO enough for now. I implemented only a single test using the new machinery. Maybe if you take a look at it, you will understand why 😆 Bootstrap currently does some peculiar things, such as running a stage 0 std step (even though stage 0 std no longer exists) and running the Rustc stage 0 -> 1 step twice, once with a single crates, once with all rustc crates. So I think that even with this single step, there will be a bunch of things to fix in the near future... The way we currently prepare the Config test fixtures is far from ideal, this is something I think ``@Shourya742`` could work on as a part of their GSoC project (remove as much command execution from Config construction as possible, actually run bootstrap on a temporary directory instead of running it on the rustc checkout, create a Builder-like API for creating the Config test fixtures). r? ``@jieyouxu``

Rollup merge of #142431 - Kobzol:bootstrap-snapshot-tests, r=jieyouxu Add initial version of snapshot tests to bootstrap When making any changes to bootstrap (steps), it is very difficult to realize how does it affect various common bootstrap commands, and if everything still works as we expect it to. We are far away from having actual end-to-end tests, but what we could at least do is have a way of testing what steps does bootstrap execute in dry run mode. Now, we already have something like this in `src/bootstrap/src/core/builder/tests.rs`, however that is quite limited, because it only checks executed steps for a specific impl of `Step` and it does not consider step order. Recently, when working on what I thought was one of the simplest possible step untanglings in bootstrap (#142357), I ran into errors in tests that were quite hard to debug. Partly also because the current staging test diffs are multiline and use `Debug` output, so it's quite difficult for me to make sense of them. In this PR, I introduce `insta`, which allows writing snapshot tests in a very simple way. With it, I want to allow writing tests that will clearly show us what is going on during bootstrap execution, and then write golden tests for `build/check/test` stage `0/1/2` for compiler/std/tools etc., to make sure that we don't regress something, and also to help with [#t-infra/bootstrap > Proposal to cleanup stages and steps after the redesign](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Proposal.20to.20cleanup.20stages.20and.20steps.20after.20the.20redesign/with/523488806), to help avoid a situation where we would (again) have to make a flurry of staging changes because of unexpected consequences. In the snapshot tests, we currently render the build of rustc, std and LLVM. Currently I render the executed steps using downcasting, which is not super pretty, but it allows us to make the test rendering localized in one place, and it's IMO enough for now. I implemented only a single test using the new machinery. Maybe if you take a look at it, you will understand why 😆 Bootstrap currently does some peculiar things, such as running a stage 0 std step (even though stage 0 std no longer exists) and running the Rustc stage 0 -> 1 step twice, once with a single crates, once with all rustc crates. So I think that even with this single step, there will be a bunch of things to fix in the near future... The way we currently prepare the Config test fixtures is far from ideal, this is something I think ``@Shourya742`` could work on as a part of their GSoC project (remove as much command execution from Config construction as possible, actually run bootstrap on a temporary directory instead of running it on the rustc checkout, create a Builder-like API for creating the Config test fixtures). r? ``@jieyouxu``

Add initial version of snapshot tests to bootstrap When making any changes to bootstrap (steps), it is very difficult to realize how does it affect various common bootstrap commands, and if everything still works as we expect it to. We are far away from having actual end-to-end tests, but what we could at least do is have a way of testing what steps does bootstrap execute in dry run mode. Now, we already have something like this in `src/bootstrap/src/core/builder/tests.rs`, however that is quite limited, because it only checks executed steps for a specific impl of `Step` and it does not consider step order. Recently, when working on what I thought was one of the simplest possible step untanglings in bootstrap (rust-lang/rust#142357), I ran into errors in tests that were quite hard to debug. Partly also because the current staging test diffs are multiline and use `Debug` output, so it's quite difficult for me to make sense of them. In this PR, I introduce `insta`, which allows writing snapshot tests in a very simple way. With it, I want to allow writing tests that will clearly show us what is going on during bootstrap execution, and then write golden tests for `build/check/test` stage `0/1/2` for compiler/std/tools etc., to make sure that we don't regress something, and also to help with [#t-infra/bootstrap > Proposal to cleanup stages and steps after the redesign](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Proposal.20to.20cleanup.20stages.20and.20steps.20after.20the.20redesign/with/523488806), to help avoid a situation where we would (again) have to make a flurry of staging changes because of unexpected consequences. In the snapshot tests, we currently render the build of rustc, std and LLVM. Currently I render the executed steps using downcasting, which is not super pretty, but it allows us to make the test rendering localized in one place, and it's IMO enough for now. I implemented only a single test using the new machinery. Maybe if you take a look at it, you will understand why 😆 Bootstrap currently does some peculiar things, such as running a stage 0 std step (even though stage 0 std no longer exists) and running the Rustc stage 0 -> 1 step twice, once with a single crates, once with all rustc crates. So I think that even with this single step, there will be a bunch of things to fix in the near future... The way we currently prepare the Config test fixtures is far from ideal, this is something I think ``@Shourya742`` could work on as a part of their GSoC project (remove as much command execution from Config construction as possible, actually run bootstrap on a temporary directory instead of running it on the rustc checkout, create a Builder-like API for creating the Config test fixtures). r? ``@jieyouxu``

It wasn't used anywhere.

That step should be responsible for building the tool, not performing side-effects. Also, only copy the tool to the `self-contained` directory, not to the `rustlib/<target>/bin` directory.

Kobzol · 2025-07-08T09:50:08Z

I nerfed this PR to only add tests and do some light cleanup, without the stage changes.

src/bootstrap/src/core/builder/tests.rs

jieyouxu · 2025-07-08T11:26:34Z

What? Why is my review "outdated" 😓

jieyouxu · 2025-07-08T13:20:58Z

Thanks, you can r=me after PR CI is green.

Kobzol · 2025-07-08T14:35:32Z

@bors r=jieyouxu

bors · 2025-07-08T14:35:35Z

📌 Commit 961bac0 has been approved by jieyouxu

It is now in the queue for this repository.

Rollup of 9 pull requests Successful merges: - #142357 (Simplify LLVM bitcode linker in bootstrap and add tests for it) - #143177 (Remove false label when `self` resolve failure does not relate to macro) - #143339 (Respect endianness correctly in CheckEnums test suite) - #143426 (clippy fix: indentation) - #143475 (tests: Use `cfg_target_has_reliable_f16_f128` in `conv-bits-runtime-const`) - #143499 (Don't call `predicates_of` on a dummy obligation cause's body id) - #143520 (Fix perf regression caused by tracing) - #143532 (More carefully consider span context when suggesting remove `&mut`) - #143606 (configure.py: Write last key in each section) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #142357 - Kobzol:simplify-llvm-bitcode-linker, r=jieyouxu Simplify LLVM bitcode linker in bootstrap and add tests for it This PR tries to simplify the `LlvmBitcodeLinker` step a little bit, and add tests for it. It also adds tests for `LldWrapper`. r? `@jieyouxu`

Rollup of 9 pull requests Successful merges: - rust-lang/rust#142357 (Simplify LLVM bitcode linker in bootstrap and add tests for it) - rust-lang/rust#143177 (Remove false label when `self` resolve failure does not relate to macro) - rust-lang/rust#143339 (Respect endianness correctly in CheckEnums test suite) - rust-lang/rust#143426 (clippy fix: indentation) - rust-lang/rust#143475 (tests: Use `cfg_target_has_reliable_f16_f128` in `conv-bits-runtime-const`) - rust-lang/rust#143499 (Don't call `predicates_of` on a dummy obligation cause's body id) - rust-lang/rust#143520 (Fix perf regression caused by tracing) - rust-lang/rust#143532 (More carefully consider span context when suggesting remove `&mut`) - rust-lang/rust#143606 (configure.py: Write last key in each section) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned jieyouxu Jun 11, 2025

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 Jun 11, 2025

jieyouxu reviewed Jun 11, 2025

View reviewed changes

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2025

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 11, 2025

jieyouxu mentioned this pull request Jun 11, 2025

Cleanup: make the self-contained path within sysroot calculation more principled #142361

Open

This comment has been minimized.

Sign in to view

Kobzol mentioned this pull request Jun 12, 2025

Add initial version of snapshot tests to bootstrap #142431

Merged

Kobzol marked this pull request as draft June 16, 2025 08:41

Kobzol added 4 commits July 8, 2025 11:39

Remove extra_features from LlvmBitcodeLinker

754d46e

It wasn't used anywhere.

Remove sysroot copy from LlvmBitcodeLinker step

c33f908

That step should be responsible for building the tool, not performing side-effects. Also, only copy the tool to the `self-contained` directory, not to the `rustlib/<target>/bin` directory.

Update llvm-bitcode-linker tests

fd37722

Also test LldWrapper and remove llvm-config override from tests

b14323a

Kobzol force-pushed the simplify-llvm-bitcode-linker branch from 769f984 to b14323a Compare July 8, 2025 09:49

Kobzol changed the title ~~Simplify LLVM bitcode linker in bootstrap~~ Simplify LLVM bitcode linker in bootstrap and add tests for it Jul 8, 2025

Kobzol marked this pull request as ready for review July 8, 2025 09:49

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 8, 2025

jieyouxu mentioned this pull request Jul 8, 2025

Edge case: draft PR marked as S-waiting-on-author receives S-waiting-on-review on undraft in addition instead of flip rust-lang/triagebot#2102

Closed

jieyouxu reviewed Jul 8, 2025

View reviewed changes

src/bootstrap/src/core/builder/tests.rs Outdated Show resolved Hide resolved

Add cross-compilation tool test

961bac0

Kobzol removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 8, 2025

jieyouxu approved these changes Jul 8, 2025

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 Jul 8, 2025

Kobzol mentioned this pull request Jul 8, 2025

Add ToolTarget to bootstrap #143641

Open

tgross35 mentioned this pull request Jul 9, 2025

Rollup of 9 pull requests #143667

Merged

bors merged commit d069a7c into rust-lang:master Jul 9, 2025
11 checks passed

rustbot added this to the 1.90.0 milestone Jul 9, 2025

Kobzol deleted the simplify-llvm-bitcode-linker branch July 9, 2025 09:45

		/// Return the lowest stage compiler that can compile code for the given `target`.
		pub fn compiler_for_target(&self, target: TargetSelection) -> Compiler {

Simplify LLVM bitcode linker in bootstrap and add tests for it #142357

Simplify LLVM bitcode linker in bootstrap and add tests for it #142357

Uh oh!

Conversation

Kobzol commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jieyouxu left a comment

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

Kobzol Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

Kobzol Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jieyouxu Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

Kobzol Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu commented Jun 11, 2025

Uh oh!

rustbot commented Jun 11, 2025

Uh oh!

This comment has been minimized.

bors commented Jun 12, 2025

Uh oh!

Kobzol commented Jul 8, 2025

Uh oh!

Uh oh!

jieyouxu commented Jul 8, 2025

Uh oh!

jieyouxu commented Jul 8, 2025

Uh oh!

Kobzol commented Jul 8, 2025

Uh oh!

bors commented Jul 8, 2025

Uh oh!

Uh oh!

Uh oh!

Kobzol commented Jun 11, 2025 •

edited

Loading

Kobzol Jun 11, 2025 •

edited

Loading

jieyouxu Jun 11, 2025 •

edited

Loading

Kobzol Jun 11, 2025 •

edited

Loading

jieyouxu Jun 11, 2025 •

edited

Loading