detect incompatible CI LLVM options more precisely #129788

onur-ozkan · 2024-08-30T17:03:30Z

Previously, the logic here was simply checking whether the option was set in config.toml. This approach was not manageable in our CI runners as we set so many options in config.toml. In reality, those values are not incompatible since they are usually the same value used to generate the CI llvm. Now, the new logic compares the configuration values with the values used to generate the CI llvm, so we get more precise results and make the process more manageable.

Fixes #129153

rustbot · 2024-08-30T17:03:38Z

r? @albertlarsan68

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

rustbot · 2024-08-30T17:03:40Z

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Kobzol

Ooooh, I have wanted this for so long! Great job ❤️

Left two nits.

src/bootstrap/src/core/download.rs

src/bootstrap/src/core/config/config.rs

Kobzol · 2024-09-04T10:57:09Z

Thank you!

@bors r+ rollup=never

bors · 2024-09-04T10:57:12Z

📌 Commit 36588d2 has been approved by Kobzol

It is now in the queue for this repository.

…-llvm, r=Kobzol detect incompatible CI LLVM options more precisely Previously, the logic here was simply checking whether the option was set in `config.toml`. This approach was not manageable in our CI runners as we set so many options in config.toml. In reality, those values are not incompatible since they are usually the same value used to generate the CI llvm. Now, the new logic compares the configuration values with the values used to generate the CI llvm, so we get more precise results and make the process more manageable. Fixes rust-lang#129153

bors · 2024-09-08T13:48:21Z

⌛ Testing commit 36588d2 with merge 8ecc4c2...

bors · 2024-09-08T13:52:56Z

💔 Test failed - checks-actions

Previously, the logic here was simply checking whether the option was set in `config.toml`. This approach was not manageable in our CI runners as we set so many options in config.toml. In reality, those values are not incompatible since they are usually the same value used to generate the CI llvm. Now, the new logic compares the configuration values with the values used to generate the CI llvm, so we get more precise results and make the process more manageable. Signed-off-by: onur-ozkan <work@onurozkan.dev>

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2024-09-08T15:33:10Z

Rebased.

@bors r=Kobzol

bors · 2024-09-08T15:33:12Z

📌 Commit 7b8cbe4 has been approved by Kobzol

It is now in the queue for this repository.

bors · 2024-09-08T21:56:19Z

⌛ Testing commit 7b8cbe4 with merge b7d9051...

…-llvm, r=Kobzol detect incompatible CI LLVM options more precisely Previously, the logic here was simply checking whether the option was set in `config.toml`. This approach was not manageable in our CI runners as we set so many options in config.toml. In reality, those values are not incompatible since they are usually the same value used to generate the CI llvm. Now, the new logic compares the configuration values with the values used to generate the CI llvm, so we get more precise results and make the process more manageable. Fixes rust-lang#129153

rust-log-analyzer · 2024-09-08T22:45:43Z

The job x86_64-msvc-ext2 failed! Check out the build log: (web) (plain)

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

[RUSTC-TIMING] miri test:false 4.433
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`

Caused by:
  Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
  local time: Sun, Sep  8, 2024 10:45:29 PM
  network time: Sun, 08 Sep 2024 22:45:29 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

bors · 2024-09-08T22:46:25Z

💔 Test failed - checks-actions

onur-ozkan · 2024-09-09T04:06:08Z

@bors retry

bors · 2024-09-09T06:59:46Z

⌛ Testing commit 7b8cbe4 with merge 263a3ae...

bors · 2024-09-09T09:23:25Z

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing 263a3ae to master...

rust-timer · 2024-09-09T10:40:10Z

Finished benchmarking commit (263a3ae): 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 (secondary 0.8%)

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

Cycles

Results (primary -3.3%)

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

Binary size

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

Bootstrap: 754.903s -> 757.632s (0.36%)
Artifact size: 341.24 MiB -> 341.17 MiB (-0.02%)

rustbot assigned albertlarsan68 Aug 30, 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 Aug 30, 2024

onur-ozkan changed the title ~~Incompatibility check for llvm~~ detect incompatible CI LLVM options more precisely Aug 30, 2024

onur-ozkan force-pushed the incompatibility-check-for-llvm branch from 087c935 to c882838 Compare August 30, 2024 17:08

This comment has been minimized.

Sign in to view

Kobzol reviewed Sep 3, 2024

View reviewed changes

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

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

onur-ozkan force-pushed the incompatibility-check-for-llvm branch from 9cc647a to 36588d2 Compare September 4, 2024 10:39

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 Sep 4, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 8, 2024

This comment has been minimized.

Sign in to view

onur-ozkan added 6 commits September 8, 2024 17:51

remove check_ci_llvm usage

23df3a9

Signed-off-by: onur-ozkan <work@onurozkan.dev>

print incompatible options even if we don't download

ea34bb0

Signed-off-by: onur-ozkan <work@onurozkan.dev>

fix llvm ThinLTO behaviour

018ed9a

Signed-off-by: onur-ozkan <work@onurozkan.dev>

use Config::get_builder_toml for ci-rustc config parsing

13e16a9

Signed-off-by: onur-ozkan <work@onurozkan.dev>

handle dry-run mode in Config::get_builder_toml

7b8cbe4

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the incompatibility-check-for-llvm branch from 36588d2 to 7b8cbe4 Compare September 8, 2024 14:57

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 8, 2024

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 8, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 8, 2024

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 Sep 9, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 9, 2024

bors merged commit 263a3ae into rust-lang:master Sep 9, 2024
7 checks passed

rustbot added this to the 1.83.0 milestone Sep 9, 2024

onur-ozkan deleted the incompatibility-check-for-llvm branch September 9, 2024 09:58

RalfJung mentioned this pull request Sep 9, 2024

All x.py commands fail in bootstrap: cannot find file "ci-llvm/builder-config" #130144

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

detect incompatible CI LLVM options more precisely #129788

detect incompatible CI LLVM options more precisely #129788

onur-ozkan commented Aug 30, 2024

rustbot commented Aug 30, 2024

rustbot commented Aug 30, 2024

This comment has been minimized.

Kobzol left a comment

Kobzol commented Sep 4, 2024

bors commented Sep 4, 2024

bors commented Sep 8, 2024

bors commented Sep 8, 2024

This comment has been minimized.

onur-ozkan commented Sep 8, 2024

bors commented Sep 8, 2024

bors commented Sep 8, 2024

rust-log-analyzer commented Sep 8, 2024

bors commented Sep 8, 2024

onur-ozkan commented Sep 9, 2024

bors commented Sep 9, 2024

bors commented Sep 9, 2024

rust-timer commented Sep 9, 2024

detect incompatible CI LLVM options more precisely #129788

detect incompatible CI LLVM options more precisely #129788

Conversation

onur-ozkan commented Aug 30, 2024

rustbot commented Aug 30, 2024

rustbot commented Aug 30, 2024

This comment has been minimized.

Kobzol left a comment

Choose a reason for hiding this comment

Kobzol commented Sep 4, 2024

bors commented Sep 4, 2024

bors commented Sep 8, 2024

bors commented Sep 8, 2024

This comment has been minimized.

onur-ozkan commented Sep 8, 2024

bors commented Sep 8, 2024

bors commented Sep 8, 2024

rust-log-analyzer commented Sep 8, 2024

bors commented Sep 8, 2024

onur-ozkan commented Sep 9, 2024

bors commented Sep 9, 2024

bors commented Sep 9, 2024

rust-timer commented Sep 9, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size