-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bootstrap: verify that --target
and --host
triples are valid before starting a build
#122128
Comments
@rustbot label -needs-triage |
The target may only exist in the compiler that will be built, so bootstrap would need to parse rust/compiler/rustc_target/src/spec/mod.rs Lines 1401 to 1689 in 4282576
|
@bjorn3 |
Other sanity checks are done in https://github.com/rust-lang/rust/blob/master/src/bootstrap/src/core/sanity.rs |
I believe the checks are being done here: rust/src/bootstrap/src/core/sanity.rs Line 162 in 703dc9c
rust/src/bootstrap/src/core/sanity.rs Line 188 in 703dc9c
Although, I am not quite sure about Since there are limited number of targets, do we just create some constant structure to store this and just map if it does not have any typos? |
I think you found the correct locations for such a check.
You could probably just use regex to extract a target list from the file linked in #122128 (comment), then add a check that the specified hosts and targets all exist in that list. |
This approach is too straightforward; custom targets will fail from this logic. |
Do we have custom target info available early? If not, even a warning that something isn't in the list would be helpful here |
We do. #71009 can be a good ref to this.
|
@onur-ozkan rust/src/librustc_back/target/mod.rs Line 293 in 61aeab4
Do we just use this for the search part? |
That is a 10 year old commit... |
When you say custom targets did you mean when we have some architecture (say arm_64) with certain properties differing from the standard definition (like target-endian, linker, etc.)? Ref: https://github.com/rust-lang/rust/pull/16156/files |
You can indeed do such stuff, but I wouldn't restrict the purpose of "custom-targets" with that. There is this documentation https://docs.rust-embedded.org/embedonomicon/custom-target.html if you want to read more about it. |
As you mentioned, the regex approach wouldn't cover the custom targets that are made by the user in json format, so I currently see tests for this here: https://github.com/rust-lang/cargo/pull/5228/files Could you elaborate the custom target approach, I mean the only thing we have to check if the json file exists at the mentioned user path and whether each of the attributes under it are valid? For the default targets we just check in the static file as mentioned earlier. |
For custom targets I would avoid checking if the attributes are valid. Validating them is a lot more complicated and can give false positives too when an attribute is newly introduced and not available in the bootstrap compiler yet. |
The logic for validating the target name can be as follows:
If none of the above returns true, then either the user forgot to provide the target specification JSON file for the custom target or made a typo in the target name. |
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…tarting a build Resolves rust-lang#122128
…r=onur-ozkan Bootstrap: Check validity of `--target` and `--host` triples before starting a build Resolves rust-lang#122128 As described in the issue, validating the `target` and `host` triples would save a lot of time before actually starting a build. This would also check for custom targets by looking for a valid JSON spec if the specified target does not exist in the [supported](https://github.com/rust-lang/rust/blob/42825768b103c28b10ce0407749acb21d32abeec/compiler/rustc_target/src/spec/mod.rs#L1401-L1689) list of targets.
I ran
./x dist --target mips64-openwrt-linux-musl --host x86_64-unknown-linux-gnu
twice with slight different typos in the--host
argument. Unfortunately, this takes about half an hour to detect (stage2, LLVM builds) and changing the host seems to invalidate some of the build.Bootstrap can't actually query
rustc
for the target list, but it would be great if it could verify the triple string exists somewhere incompiler/rustc_target/src/spec/mod.rs
as a sanity check before kicking off a multistage build.@rustbot label +T-bootstrap +E-easy -needs-triage
The text was updated successfully, but these errors were encountered: