Add ToolTarget to bootstrap
#143641
Add ToolTarget to bootstrap
#143641
Conversation
rustbot
added
A-testsuite
rustbot
added
the
S-waiting-on-review
|
#142357 was merged, so undrafting. |
| let (target, min_build_compiler_stage) = match mode { | ||
| ToolTargetBuildMode::Build(target) => { | ||
| assert!(builder.top_stage > 0); | ||
| (target, builder.top_stage - 1) | ||
| } | ||
| ToolTargetBuildMode::Dist(target_compiler) => { | ||
| assert!(target_compiler.stage > 0); | ||
| (target_compiler.host, target_compiler.stage - 1) | ||
| } | ||
| }; | ||
| let compiler = if builder.host_target == target { | ||
| builder.compiler(0, builder.host_target) | ||
| builder.compiler(min_build_compiler_stage, builder.host_target) | ||
| } else { | ||
| builder.compiler(1, builder.host_target) | ||
| builder.compiler(min_build_compiler_stage.max(1), builder.host_target) |
There was a problem hiding this comment.
Question [STAGING 4/2]: I have to admit this logic is a bit confusing to me, especially the stage adjustments and also the stage cap. I need to think about this a bit.
There was a problem hiding this comment.
I might have over-engineered this code. The logic is: if you want to build a stage N tool, or attach a tool to a disted stage N rustc, you need to build the tool with rustc stage N - 1. However, if you're cross-compiling a stage 1 tool, we auto-bump it to a stage 2 tool. That's the logic.
There was a problem hiding this comment.
Tried to clarify it a bit in the latest commit.
| path: "src/tools/wasm-component-ld", | ||
| source_type: SourceType::InTree, | ||
| extra_features: vec![], | ||
| allow_features: "min-specialization", |
There was a problem hiding this comment.
Question: does wasm-component-ld actually need min-specialization? At least, I'm not seeing it used at all.
There was a problem hiding this comment.
It compiled without it. @alexcrichton Do you know if this feature is required for wasm-component-ld?
There was a problem hiding this comment.
No need can definitely be removed. IIRC this was required historically due to the various alignment of other issues but if things work without it then definitely jettison it
There was a problem hiding this comment.
Ah, the historical reason is at tkaitchuck/aHash#238
There was a problem hiding this comment.
Thank you for the context! I removed it, let's see what CI says.
There was a problem hiding this comment.
Wait hold on, that means this is necessary because ahash will try to enable it because of fragile can-enable-feature detection (I think?)
There was a problem hiding this comment.
I was able to build it locally without the feature, IIRC. So maybe the lockfile has been updated to a newer ahash version in the meantime? I remember that ahash eventually gave up with these detections. We'll see what happens on CI.
There was a problem hiding this comment.
I still see min_spec feature detection in ahash master's build.rs, hm
There was a problem hiding this comment.
I did cargo tree on wasm-component-ld and it doesn't return ahash anymore, so maybe that's the reason =D
There was a problem hiding this comment.
Ah yes hashbrown updated to switch away from ahash in 0.15.0 and rustc is on 0.15.4 so the ahash dependency no longer shows up here.
This comment has been minimized.
This comment has been minimized.
Oh, you thought I'm done with refactoring bootstrap tools? Na-ah, think again! After the failure of #143581,
ToolTargetis back with a vengeance. This time, I implemented the test changes and tool cleanups without forcing these tools to be built with the stage0 compiler.There are still some small wins though,
LlvmBitcodeLinkernow starts at stage 1, and not stage 2. Cargo should also be ported to this new mode, but I'm leaving that for a follow-up PR.Hopefully X-th time's the charm 🤞
r? @jieyouxu