Link rustc tools into the correct sysroot by Noratrieb · Pull Request #123384 · rust-lang/rust

Noratrieb · 2024-04-02T19:10:26Z

When copying tool binaries, we were linking them into the sysroot of the compiler that built the binaries. This makes no sense, the binaries are for the next sysroot. So when the stage0 compiler builds clippy, this clippy belongs into stage1, and when the stage1 compiler builds clippy, this clippy belongs into stage2.
This puts it right next to the librustc_driver it actually links against.
Additionally, we ensure(Assemble) of this librustc_driver such that the tool will be working as expected.
To run the tool manually, we still need to set LD_LIBRARY_PATH, but now with this, setting the rpath to $ORIGIN/../lib (like the rustc and rustdoc binaries) should be possible as future work now.

Rustdoc, with its special treatment, was already getting the correct behavior.

See https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Stage.20for.20copying.20tools.20into.20the.20sysroot for a bit more context.
fixes this comment but not the issue #119946 (comment)

r? onur-ozkan cc @RalfJung

Noratrieb · 2024-04-02T19:11:03Z

src/bootstrap/src/core/build_steps/tool.rs


-                if (false $(|| !$add_bins_to_sysroot.is_empty())?) && $sel.compiler.stage > 0 {
-                    let bindir = $builder.sysroot($sel.compiler).join("bin");
+                if (false $(|| !$add_bins_to_sysroot.is_empty())?) {


Now that we put it into the correct stage, there's nothing wrong with x build clippy --stage 0 anymore, and we can put that into the sysroot too.

This conflicts with the idea of ToolStd and ToolRustc

rust/src/bootstrap/src/lib.rs

Lines 257 to 266 in 029cb1b

/// Build a tool which uses the locally built std, placing output in the

/// "stageN-tools" directory. Its usage is quite rare, mainly used by

/// compiletest which needs libtest.

ToolStd,

/// Build a tool which uses the locally built rustc and the target std,

/// placing the output in the "stageN-tools" directory. This is used for

/// anything that needs a fully functional rustc, such as rustdoc, clippy,

/// cargo, rls, rustfmt, miri, etc.

ToolRustc,

I don't quite understand what you mean with this. The two ToolStd built here don't have any bins added to the sysroot anyways, so this doesn't affect them. As for the ToolRustc, this change was targetted specifically at them.

Noratrieb · 2024-04-02T19:11:25Z

src/bootstrap/src/core/build_steps/tool.rs

-    Clippy, "src/tools/clippy", "clippy-driver", stable=true, add_bins_to_sysroot = ["clippy-driver", "cargo-clippy"];
-    Miri, "src/tools/miri", "miri", stable=false, add_bins_to_sysroot = ["miri"];
-    CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=true, add_bins_to_sysroot = ["cargo-miri"];
+    Cargofmt, "src/tools/rustfmt", "cargo-fmt", mode=Mode::ToolRustc, stable=true;


I've also made this a bit more obvious as to which tools are what, looking for the tool_std=true was a bit annoying.

I agree that tool_std seems a bit awkward, but this macro shouldn't accept any modes other than ToolRustc and ToolStd.

I'm gonna revert this change to keep the PR minimal as it's complicated enough as-is, but will keep that in mind if I do this change in the future.

Noratrieb · 2024-04-02T19:36:55Z

I tested clippy manually, it works like a charm: LD_LIBRARY_PATH="build/host/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" build/host/stage1/bin/clippy-driver uwu.rs lints uwu.rs, finds the standard library and is overall a great experience (modulo having to set LD_LIBRARY_PATH).

RalfJung · 2024-04-02T21:05:44Z

So what will ./x.py build miri --stage 0 do with this PR?

Because for rustdoc that is kind of broken, it doesn't actually do anything.

Noratrieb · 2024-04-03T05:07:16Z

So what will ./x.py build miri --stage 0 do with this PR?

Because for rustdoc that is kind of broken, it doesn't actually do anything.

It will build a Miri linked against stage1 rustc and put it into stage1. Rustdoc is off by one here, the equivalent rustdoc invocation is using --stage 1.

RalfJung · 2024-04-03T06:48:20Z

Okay, nice. :)
Does that also mean that this line

rust/src/bootstrap/src/core/builder.rs

Line 1260 in 8938f88

add_dylib_path(self.rustc_lib_paths(run_compiler), &mut cmd);

can now use add_rustc_lib_path without breaking things on Windows? If the binary is in the same folder as its libraries, that should be the case.

Noratrieb · 2024-05-08T21:31:52Z

linking this because it's relevant thanks jyn for finding it https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Stage.20numbering.20for.20tools

When copying tool binaries, we were linking them into the sysroot of the compiler that built the binaries. This makes no sense, the binaries are for the next sysroot. So when the stage0 compiler builds clippy, this clippy belongs into stage1, and when the stage1 compiler builds clippy, this clippy belongs into stage2. This puts it right next to the librustc_driver it actually links against. Additionally, we `ensure(Assemble)` of this librustc_driver such that the tool will be working as expected. To run the tool manually, we still need to set LD_LIBRARY_PATH, but now with this, setting the rpath to `$ORIGIN/../lib` (like the `rustc` and `rustdoc` binaries) should be possible as future work now. Rustdoc, with its special treatment, was already getting the correct behavior.

rust-log-analyzer · 2024-06-07T20:29:59Z

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#16 exporting to docker image format
#16 sending tarball 29.2s done
#16 DONE 33.8s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
  Downloaded boml v0.3.1
   Compiling boml v0.3.1
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished `release` profile [optimized] target(s) in 3.50s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --mini-tests --std-tests`
Using system GCC
[BUILD] example
[AOT] mini_core_hello_world
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/mini_core_hello_world
abc

alex-semenyuk · 2024-09-15T06:32:54Z

@Noratrieb
Ping from triage. Could you please post your status on this PR? This PR has not been updated in more than 15 days.
It has merge conflicts and failed CI

Noratrieb · 2024-09-15T08:42:34Z

i haven't abandoned it but I need to find the time to work on it

Noratrieb · 2025-02-23T10:35:58Z

#137215 did this!

rustbot assigned onur-ozkan Apr 2, 2024

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 Apr 2, 2024

Noratrieb commented Apr 2, 2024

View reviewed changes

Noratrieb force-pushed the back-where-you-belong branch 2 times, most recently from da31765 to 53db4e2 Compare April 2, 2024 19:36

This comment has been minimized.

Sign in to view

onur-ozkan added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2024

Noratrieb force-pushed the back-where-you-belong branch from 53db4e2 to 2b66f46 Compare June 6, 2024 19:49

This comment has been minimized.

Sign in to view

Noratrieb force-pushed the back-where-you-belong branch from 87bbdc4 to ec20e7a Compare June 7, 2024 19:18

Noratrieb force-pushed the back-where-you-belong branch from ec20e7a to 48d1b9b Compare June 7, 2024 19:47

Noratrieb changed the title ~~Link rustc/std tools into the correct sysroot~~ Link rustc tools into the correct sysroot Jun 7, 2024

alex-semenyuk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 4, 2024

Noratrieb mentioned this pull request Feb 23, 2025

tools no longer find librustc_driver #137469

Closed

Noratrieb closed this Feb 23, 2025

	/// Build a tool which uses the locally built std, placing output in the
	/// "stageN-tools" directory. Its usage is quite rare, mainly used by
	/// compiletest which needs libtest.
	ToolStd,

	/// Build a tool which uses the locally built rustc and the target std,
	/// placing the output in the "stageN-tools" directory. This is used for
	/// anything that needs a fully functional rustc, such as rustdoc, clippy,
	/// cargo, rls, rustfmt, miri, etc.
	ToolRustc,

Uh oh!

Conversation

Noratrieb commented Apr 2, 2024

Uh oh!

Noratrieb Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

onur-ozkan Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

Noratrieb Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

Noratrieb Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

onur-ozkan Apr 2, 2024

Choose a reason for hiding this comment

Uh oh!

Noratrieb Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

Noratrieb commented Apr 2, 2024

Uh oh!

This comment has been minimized.

RalfJung commented Apr 2, 2024

Uh oh!

Noratrieb commented Apr 3, 2024

Uh oh!

RalfJung commented Apr 3, 2024

Uh oh!

Noratrieb commented May 8, 2024

Uh oh!

This comment has been minimized.

rust-log-analyzer commented Jun 7, 2024

Uh oh!

alex-semenyuk commented Sep 15, 2024

Uh oh!

Noratrieb commented Sep 15, 2024

Uh oh!

Noratrieb commented Feb 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants