Bootstrap: Check validity of `--target` and `--host` triples before starting a build #123546

Rajveer100 · 2024-04-06T12:06:32Z

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 list of targets.

rustbot · 2024-04-06T12:06:40Z

r? @onur-ozkan

rustbot has assigned @onur-ozkan.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Rajveer100 · 2024-04-06T12:10:29Z

For custom targets, which property do we check for the file specified by passing --target argument?
If I understand correctly, we use this same flag for an existing target too, but instead of a name, we specify the file path?

Also, the build currently fails, since I assumed for my local that the current working directory is the right path, I believe there must exist an environment variable for this rather than using env::current_dir?

onur-ozkan

For custom targets, which property do we check for the file specified by passing --target argument?
If I understand correctly, we use this same flag for an existing target too, but instead of a name, we specify the file path?

RUST_TARGET_PATH environment variable is reserved to be used for the custom targets.

src/bootstrap/src/core/sanity.rs

rustbot · 2024-04-06T18:58:39Z

These commits modify compiler targets.
(See the Target Tier Policy.)

onur-ozkan · 2024-04-06T19:24:31Z

FWIW the current approach is entirely different from what Mark suggested here.

Rajveer100 · 2024-04-06T19:30:11Z

What are your thoughts on this:

That does have the limitation that we're only going to see stage0 targets

onur-ozkan · 2024-04-06T20:06:38Z

We can add them (those not included from the stage0 yet) to a hard-coded list on bootstrap.

Rajveer100 · 2024-04-07T12:21:14Z

Why do few of the tests have target set to A (the tests fail because of this)?

onur-ozkan · 2024-04-08T07:52:31Z

Why do few of the tests have target set to A (the tests fail because of this)?

Those are just an alias for simulating targets to be used for some of the bootstrap tests.

Rajveer100 · 2024-04-08T08:25:42Z

What would be the right approach to fix this, give the actual path rather than the alias?

onur-ozkan · 2024-04-08T08:35:22Z

What would be the right approach to fix this, give the actual path rather than the alias?

Those tests should remain the same; instead, we should find a way to skip the target name sanity check if it was given from the bootstrap tests. We don't need to check target names while testing bootstrap.

Rajveer100 · 2024-04-09T07:11:24Z

In the Build struct, I see few fields like build_stage which I believe would help us?

onur-ozkan · 2024-04-14T06:12:35Z

FWIW the current approach is entirely different from what Mark suggested here.

Unfortunately, that is still the case.

@rustbot author

src/bootstrap/src/core/sanity.rs

…tarting a build Resolves rust-lang#122128

onur-ozkan

Some parts missing in this PR, but this looks good enough for now and I will do the rest in a follow-up PR later as they are not necessarily needed right now.

Thank you for the fixes and patience!

onur-ozkan · 2024-04-27T20:36:46Z

@bors r+

bors · 2024-04-27T20:36:49Z

📌 Commit 09c0768 has been approved by onur-ozkan

It is now in the queue for this repository.

bors · 2024-04-28T04:16:09Z

⌛ Testing commit 09c0768 with merge 6c90ac8...

Rajveer100 · 2024-04-28T04:59:33Z

Thanks for the approval! 🦀

bors · 2024-04-28T06:19:19Z

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

rust-timer · 2024-04-28T07:36:10Z

Finished benchmarking commit (6c90ac8): 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)	-1.5%	[-1.5%, -1.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.5%	[-1.5%, -1.5%]	1

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: 672.137s -> 674.338s (0.33%)
Artifact size: 315.98 MiB -> 315.97 MiB (-0.01%)

bjorn3 · 2024-04-28T12:51:01Z

src/bootstrap/src/core/sanity.rs

+        let target_str = target.to_string();
+
+        let supported_target_list =
+            output(Command::new(&build.config.initial_rustc).args(["--print", "target-list"]));


As I said on the associated issue, asking the bootstrap compiler is not correct when bootstrap doesn't know about a target yet, but the in-tree rustc does. You have to check if the target is listed in compiler/rustc_target/src/spec/mod.rs.

Edit: Missed that this was already discussed in #123546 (comment).

…roalbini handle the targets that are missing in stage0 During sanity checks, we search for target names to determine if they exist in the compiler's built-in target list (`rustc --print target-list`). While a target name may be present in the stage2 compiler, it might not yet be included in stage0. This PR handles that difference. Follow-up of rust-lang#123546

Rollup merge of rust-lang#124461 - onur-ozkan:followup-123546, r=pietroalbini handle the targets that are missing in stage0 During sanity checks, we search for target names to determine if they exist in the compiler's built-in target list (`rustc --print target-list`). While a target name may be present in the stage2 compiler, it might not yet be included in stage0. This PR handles that difference. Follow-up of rust-lang#123546

rustbot assigned onur-ozkan Apr 6, 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 6, 2024

This comment has been minimized.

Sign in to view

onur-ozkan requested changes Apr 6, 2024

View reviewed changes

src/bootstrap/src/core/sanity.rs Outdated Show resolved Hide resolved

src/bootstrap/src/core/sanity.rs Outdated Show resolved Hide resolved

rustbot 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 6, 2024

fbstj reviewed Apr 6, 2024

View reviewed changes

src/bootstrap/src/core/sanity.rs Outdated Show resolved Hide resolved

Rajveer100 force-pushed the branch-for-issue-122128 branch from a80463f to d6618d8 Compare April 6, 2024 18:58

This comment has been minimized.

Sign in to view

Rajveer100 force-pushed the branch-for-issue-122128 branch from d6618d8 to 70e6ffb Compare April 6, 2024 19:05

This comment has been minimized.

Sign in to view

Rajveer100 force-pushed the branch-for-issue-122128 branch from 70e6ffb to 9a868cf Compare April 6, 2024 19:25

This comment has been minimized.

Sign in to view

Rajveer100 force-pushed the branch-for-issue-122128 branch from 9a868cf to c3062d5 Compare April 13, 2024 13:34

Rajveer100 requested a review from onur-ozkan April 13, 2024 17:55

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

Rajveer100 force-pushed the branch-for-issue-122128 branch from 8d62d99 to 59e210a Compare April 25, 2024 08:08

Rajveer100 requested a review from onur-ozkan April 25, 2024 08:08

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

This comment has been minimized.

Sign in to view

Rajveer100 force-pushed the branch-for-issue-122128 branch from 59e210a to ffdb04a Compare April 25, 2024 08:21

onur-ozkan reviewed Apr 27, 2024

View reviewed changes

src/bootstrap/src/core/sanity.rs Show resolved Hide resolved

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 27, 2024

Bootstrap: Check validity of --target and --host triples before s…

09c0768

…tarting a build Resolves rust-lang#122128

Rajveer100 force-pushed the branch-for-issue-122128 branch from ffdb04a to 09c0768 Compare April 27, 2024 18:20

Rajveer100 requested a review from onur-ozkan April 27, 2024 18:21

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

onur-ozkan approved these changes Apr 27, 2024

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

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 28, 2024

bors merged commit 6c90ac8 into rust-lang:master Apr 28, 2024
11 checks passed

rustbot added this to the 1.80.0 milestone Apr 28, 2024

onur-ozkan mentioned this pull request Apr 28, 2024

handle the targets that are missing in stage0 #124461

Merged

bjorn3 reviewed Apr 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bootstrap: Check validity of `--target` and `--host` triples before starting a build #123546

Bootstrap: Check validity of `--target` and `--host` triples before starting a build #123546

Rajveer100 commented Apr 6, 2024

rustbot commented Apr 6, 2024

This comment has been minimized.

Rajveer100 commented Apr 6, 2024 •

edited

Loading

onur-ozkan left a comment

rustbot commented Apr 6, 2024

This comment has been minimized.

This comment has been minimized.

onur-ozkan commented Apr 6, 2024

Rajveer100 commented Apr 6, 2024

This comment has been minimized.

onur-ozkan commented Apr 6, 2024

Rajveer100 commented Apr 7, 2024

onur-ozkan commented Apr 8, 2024

Rajveer100 commented Apr 8, 2024

onur-ozkan commented Apr 8, 2024

Rajveer100 commented Apr 9, 2024

onur-ozkan commented Apr 14, 2024

This comment has been minimized.

onur-ozkan left a comment

onur-ozkan commented Apr 27, 2024

bors commented Apr 27, 2024

bors commented Apr 28, 2024

Rajveer100 commented Apr 28, 2024

bors commented Apr 28, 2024

rust-timer commented Apr 28, 2024

bjorn3 Apr 28, 2024 •

edited

Loading

Bootstrap: Check validity of --target and --host triples before starting a build #123546

Bootstrap: Check validity of --target and --host triples before starting a build #123546

Conversation

Rajveer100 commented Apr 6, 2024

rustbot commented Apr 6, 2024

This comment has been minimized.

Rajveer100 commented Apr 6, 2024 • edited Loading

onur-ozkan left a comment

Choose a reason for hiding this comment

rustbot commented Apr 6, 2024

This comment has been minimized.

This comment has been minimized.

onur-ozkan commented Apr 6, 2024

Rajveer100 commented Apr 6, 2024

This comment has been minimized.

onur-ozkan commented Apr 6, 2024

Rajveer100 commented Apr 7, 2024

onur-ozkan commented Apr 8, 2024

Rajveer100 commented Apr 8, 2024

onur-ozkan commented Apr 8, 2024

Rajveer100 commented Apr 9, 2024

onur-ozkan commented Apr 14, 2024

This comment has been minimized.

onur-ozkan left a comment

Choose a reason for hiding this comment

onur-ozkan commented Apr 27, 2024

bors commented Apr 27, 2024

bors commented Apr 28, 2024

Rajveer100 commented Apr 28, 2024

bors commented Apr 28, 2024

rust-timer commented Apr 28, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

bjorn3 Apr 28, 2024 • edited Loading

Choose a reason for hiding this comment

Bootstrap: Check validity of `--target` and `--host` triples before starting a build #123546

Bootstrap: Check validity of `--target` and `--host` triples before starting a build #123546

Rajveer100 commented Apr 6, 2024 •

edited

Loading

bjorn3 Apr 28, 2024 •

edited

Loading