fix(opt-dist): respect existing config.toml #125473

weihanglo · 2024-05-24T04:14:00Z

This is another step toward making opt-dist work in sandboxed environments. See also #125465.

opt-dist verifies the final built rustc against a subset of rustc test
suite. However it overwrote the pre-existing config.toml ¹,
and that results in ./vendor/ directory removed ².

Instead of overwriting, this patch use --set <config-value> to
override paths to rustc / cargo / llvm-config.

rustbot · 2024-05-24T04:14:08Z

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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

weihanglo · 2024-05-24T04:18:26Z

This is marked as draft because I am still fighting with Nix configurations for LLVM PGO and haven't fully tested it 😬.

Kobzol · 2024-05-24T12:40:33Z

FYI, if you comment out this and do a try build, it will run the post-optimization tests.

Kobzol

r? @Kobzol

Feel free to r=me if a try build with tests passes.

src/tools/opt-dist/src/tests.rs

Kobzol · 2024-05-24T14:27:20Z

@bors try

bors · 2024-05-24T14:28:29Z

⌛ Trying commit 159ccd4 with merge 03cd897...

…ml, r=<try> fix(opt-dist): respect existing config.toml This is another step toward making opt-dist work in sandboxed environments. See also <rust-lang#125465>. opt-dist verifies the final built rustc against a subset of rustc test suite. However it overwrote the pre-existing `config.toml` [^1], and that results in ./vendor/ directory removed [^2]. Instead of overwriting, this patch use `--set <config-value>` to override paths to rustc / cargo / llvm-config. [^1]: https://github.com/rust-lang/rust/blob/606afbb617a2949a4e35c4b0258ff94c980b9451/src/tools/opt-dist/src/tests.rs#L62-L77 [^2]: https://github.com/rust-lang/rust/blob/8679004993f08807289911d9f400f4ac4391d2bc/src/bootstrap/bootstrap.py#L1057

bors · 2024-05-24T16:13:47Z

💔 Test failed - checks-actions

lqd · 2024-05-24T16:32:36Z

merging target-specific config is not currently supported

weihanglo · 2024-05-24T16:43:01Z

I can try to make it work, if desired.

lqd · 2024-05-24T16:49:01Z

to be clear, that's in bootstrap, not in opt-dist

weihanglo · 2024-05-24T17:01:41Z

Yeah I am aware. Cargo also merges target specific configs (in a quirky way 😆). Let me see if bootstrap can do a similar thing in a separate PR.

…onur-ozkan bootstrap: support target specific config overrides Can't find any previous discussion about not supporting this, so I get it done. The motivation of this is from <rust-lang#125473 (comment)>.

…ur-ozkan bootstrap: support target specific config overrides Can't find any previous discussion about not supporting this, so I get it done. The motivation of this is from <rust-lang#125473 (comment)>.

weihanglo · 2024-05-26T00:58:34Z

@bors try

…ml, r=<try> fix(opt-dist): respect existing config.toml This is another step toward making opt-dist work in sandboxed environments. See also <rust-lang#125465>. opt-dist verifies the final built rustc against a subset of rustc test suite. However it overwrote the pre-existing `config.toml` [^1], and that results in ./vendor/ directory removed [^2]. Instead of overwriting, this patch use `--set <config-value>` to override paths to rustc / cargo / llvm-config. [^1]: https://github.com/rust-lang/rust/blob/606afbb617a2949a4e35c4b0258ff94c980b9451/src/tools/opt-dist/src/tests.rs#L62-L77 [^2]: https://github.com/rust-lang/rust/blob/8679004993f08807289911d9f400f4ac4391d2bc/src/bootstrap/bootstrap.py#L1057

bors · 2024-05-26T00:59:45Z

⌛ Trying commit 159ccd4 with merge f94950d...

bors · 2024-05-26T02:48:22Z

☀️ Try build successful - checks-actions
Build commit: f94950d (f94950d4dfa3b0a971911eb5b99761a21ff4842c)

rustbot · 2024-05-26T03:09:20Z

Some changes occurred in src/tools/opt-dist

cc @Kobzol

weihanglo · 2024-05-26T03:21:33Z

@rust-timer build f94950d

weihanglo · 2024-05-26T03:22:22Z

Oh no forgot to remove the commit

This is another step toward making opt-dist work in sandboxed environments opt-dist verifies the final built rustc against a subset of rustc test suite. However it overwrote the pre-existing `config.toml` [^1], and that results in ./vendor/ directory removed [^2]. Instead of overwriting, this patch use `--set <config-value>` to override paths to rustc / cargo / llvm-config. [^1]: https://github.com/rust-lang/rust/blob/606afbb617a2949a4e35c4b0258ff94c980b9451/src/tools/opt-dist/src/tests.rs#L62-L77 [^2]: https://github.com/rust-lang/rust/blob/8679004993f08807289911d9f400f4ac4391d2bc/src/bootstrap/bootstrap.py#L1057

rust-timer · 2024-05-26T04:36:09Z

Finished benchmarking commit (f94950d): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (secondary -2.4%)

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)	-	-	0
Improvements ✅ (secondary)	-2.4%	[-2.4%, -2.4%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 670.899s -> 672.687s (0.27%)
Artifact size: 315.75 MiB -> 315.61 MiB (-0.04%)

Kobzol · 2024-05-26T07:52:30Z

Thanks for implementing the bootstrap change!

@bors r+ rollup

bors · 2024-05-26T07:52:33Z

📌 Commit c81a40b has been approved by Kobzol

It is now in the queue for this repository.

…toml, r=Kobzol fix(opt-dist): respect existing config.toml This is another step toward making opt-dist work in sandboxed environments. See also <rust-lang#125465>. opt-dist verifies the final built rustc against a subset of rustc test suite. However it overwrote the pre-existing `config.toml` [^1], and that results in ./vendor/ directory removed [^2]. Instead of overwriting, this patch use `--set <config-value>` to override paths to rustc / cargo / llvm-config. [^1]: https://github.com/rust-lang/rust/blob/606afbb617a2949a4e35c4b0258ff94c980b9451/src/tools/opt-dist/src/tests.rs#L62-L77 [^2]: https://github.com/rust-lang/rust/blob/8679004993f08807289911d9f400f4ac4391d2bc/src/bootstrap/bootstrap.py#L1057

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit) - rust-lang#125375 (Create a triagebot ping group for Rust for Linux) - rust-lang#125473 (fix(opt-dist): respect existing config.toml) - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR) - rust-lang#125530 (cleanup dependence of `ExtCtxt` in transcribe when macro expansion) - rust-lang#125544 (Also mention my-self for other check-cfg docs changes) - rust-lang#125559 (Simplify the `unchecked_sh[lr]` ub-checks a bit) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit) - rust-lang#125375 (Create a triagebot ping group for Rust for Linux) - rust-lang#125473 (fix(opt-dist): respect existing config.toml) - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR) - rust-lang#125561 (Stabilize `slice_flatten`) - rust-lang#125571 (f32: use constants instead of reassigning a dummy value as PI) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#125473 - weihanglo:respect-existing-config-toml, r=Kobzol fix(opt-dist): respect existing config.toml This is another step toward making opt-dist work in sandboxed environments. See also <rust-lang#125465>. opt-dist verifies the final built rustc against a subset of rustc test suite. However it overwrote the pre-existing `config.toml` [^1], and that results in ./vendor/ directory removed [^2]. Instead of overwriting, this patch use `--set <config-value>` to override paths to rustc / cargo / llvm-config. [^1]: https://github.com/rust-lang/rust/blob/606afbb617a2949a4e35c4b0258ff94c980b9451/src/tools/opt-dist/src/tests.rs#L62-L77 [^2]: https://github.com/rust-lang/rust/blob/8679004993f08807289911d9f400f4ac4391d2bc/src/bootstrap/bootstrap.py#L1057

bootstrap: support target specific config overrides Can't find any previous discussion about not supporting this, so I get it done. The motivation of this is from <rust-lang/rust#125473 (comment)>.

rustbot assigned Mark-Simulacrum May 24, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 24, 2024

weihanglo force-pushed the respect-existing-config-toml branch from 191ba65 to 395024a Compare May 24, 2024 04:22

Kobzol reviewed May 24, 2024

View reviewed changes

src/tools/opt-dist/src/tests.rs Outdated Show resolved Hide resolved

rustbot assigned Kobzol and unassigned Mark-Simulacrum May 24, 2024

weihanglo force-pushed the respect-existing-config-toml branch from 395024a to 159ccd4 Compare May 24, 2024 14:21

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels May 24, 2024

This comment has been minimized.

Sign in to view

bors 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 May 24, 2024

weihanglo mentioned this pull request May 24, 2024

bootstrap: support target specific config overrides #125515

Merged

weihanglo marked this pull request as ready for review May 26, 2024 03:09

This comment has been minimized.

Sign in to view

weihanglo force-pushed the respect-existing-config-toml branch from 159ccd4 to c81a40b Compare May 26, 2024 03:41

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 26, 2024

matthiaskrgr mentioned this pull request May 26, 2024

Rollup of 7 pull requests #125569

Closed

matthiaskrgr mentioned this pull request May 26, 2024

Rollup of 6 pull requests #125574

Merged

bors merged commit c7a1765 into rust-lang:master May 26, 2024
6 checks passed

rustbot added this to the 1.80.0 milestone May 26, 2024

weihanglo deleted the respect-existing-config-toml branch May 26, 2024 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(opt-dist): respect existing config.toml #125473

fix(opt-dist): respect existing config.toml #125473

weihanglo commented May 24, 2024 •

edited

Loading

rustbot commented May 24, 2024

weihanglo commented May 24, 2024

Kobzol commented May 24, 2024

Kobzol left a comment

Kobzol commented May 24, 2024

bors commented May 24, 2024

This comment has been minimized.

bors commented May 24, 2024

lqd commented May 24, 2024

weihanglo commented May 24, 2024

lqd commented May 24, 2024

weihanglo commented May 24, 2024 •

edited

Loading

weihanglo commented May 26, 2024

bors commented May 26, 2024

bors commented May 26, 2024

rustbot commented May 26, 2024

weihanglo commented May 26, 2024

This comment has been minimized.

weihanglo commented May 26, 2024

rust-timer commented May 26, 2024

Kobzol commented May 26, 2024

bors commented May 26, 2024

fix(opt-dist): respect existing config.toml #125473

fix(opt-dist): respect existing config.toml #125473

Conversation

weihanglo commented May 24, 2024 • edited Loading

Footnotes

rustbot commented May 24, 2024

weihanglo commented May 24, 2024

Kobzol commented May 24, 2024

Kobzol left a comment

Choose a reason for hiding this comment

Kobzol commented May 24, 2024

bors commented May 24, 2024

This comment has been minimized.

bors commented May 24, 2024

lqd commented May 24, 2024

weihanglo commented May 24, 2024

lqd commented May 24, 2024

weihanglo commented May 24, 2024 • edited Loading

weihanglo commented May 26, 2024

bors commented May 26, 2024

bors commented May 26, 2024

rustbot commented May 26, 2024

weihanglo commented May 26, 2024

This comment has been minimized.

weihanglo commented May 26, 2024

rust-timer commented May 26, 2024

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Kobzol commented May 26, 2024

bors commented May 26, 2024

weihanglo commented May 24, 2024 •

edited

Loading

weihanglo commented May 24, 2024 •

edited

Loading