move dummy commit logic into x86_64-gnu-llvm-18 #131531

onur-ozkan · 2024-10-11T06:45:35Z

Doing the dummy commit logic in a runner that uses CI-LLVM breaks in merge CI. This should be properly fixed by #131358 (see the actual problem). Since we can also fix it by moving the dummy commit into the runner where we use in-tree LLVM, so why not do that as well?

rustbot · 2024-10-11T06:45:43Z

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

onur-ozkan · 2024-10-11T06:51:43Z

There are too many PRs assigned to Mark.

r? Kobzol (feel free to "r" it again)

Kobzol · 2024-10-11T07:12:56Z

This whole test setup (including creating the commit) should ideally be done within bootstrap :( But I realize that it would be quite hard to do with our basic testing setup.

I don't understand why the specific runner is important for the test though. Shouldn't we write the test in a way that the tested Config does not depend on external state (ok, except for git), and can be executed on any CI runner?

onur-ozkan · 2024-10-11T07:18:22Z

I don't understand why the specific runner is important for the test though. Shouldn't we write the test in a way that the tested Config does not depend on external state (ok, except for git), and can be executed on any CI runner?

It’s not related to how we write tests. When test is run the external config is already ignored. But when we invoke x, it starts with the config parsing logic and that’s where the error comes from. To avoid this we can run that particular test directly with cargo (if it exists in the runner) without x, but I’m not sure if that’s any better than the current approach (just changing the runner without requiring cargo to be installed).

onur-ozkan · 2024-10-11T07:20:32Z

This whole test setup (including creating the commit) should ideally be done within bootstrap :( But I realize that it would be quite hard to do with our basic testing setup.

I think this shouldn't be done in bootstrap, otherwise the test will always think "there is a change on the compiler".

Kobzol · 2024-10-11T07:23:26Z

I see. So the real issue is that we run tests through x and/or that we do Config sanity checks even when we run bootstrap unit tests, which we could in theory change in the future. In any case, I agree that nothing that I talked about here is a hotfix, so let's just go with this for now.

I think this shouldn't be done in bootstrap, otherwise it will always think "there is a change on the compiler".

Well ideally, the test should literally clone/copy rustc into a separate folder, setup config, do a test commit and assert that it works :) But our testing infrastructure (and bootstrap itself) is of course currently far away from something like this.

You can r=me once PR CI is green.

onur-ozkan · 2024-10-11T07:28:52Z

Well ideally, the test should literally clone/copy rustc into a separate folder, setup config, do a test commit and assert that it works :) But our testing infrastructure (and bootstrap itself) is of course currently far away from something like this.

I meant the test will always consider that there are changes on the compiler and it will not cover the other way, but yeah that can be handled in the test logic too.

onur-ozkan · 2024-10-11T07:50:47Z

I just tried creating two tests (one with commit logic and one without), but then I realized that the commit logic won't work in runners where READ_ONLY_SRC=0 isn't set. At this point, it gets worse — I don't think we should set READ_ONLY_SRC=0 in some runners just to run some code in bootsrap. Current approach is straightforward, we set READ_ONLY_SRC=0 because we want to push a dummy commit in the runner script.

You can r=me once PR CI is green.

@bors r=Kobzol

bors · 2024-10-11T07:50:50Z

📌 Commit c25d732 has been approved by Kobzol

It is now in the queue for this repository.

…bzol move dummy commit logic into x86_64-gnu-llvm-18 Doing the dummy commit logic in a runner that uses CI-LLVM breaks in merge CI. This should be properly fixed by rust-lang#131358 (see the [actual problem](rust-lang#131448 (comment))). Since we can also fix it by moving the dummy commit into the runner where we use in-tree LLVM, so why not do that as well?

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#131464 (Update wasm-component-ld to 0.5.10) - rust-lang#131498 (Consider outermost const-anon in `non_local_def` lint) - rust-lang#131512 (Fixing rustDoc for LayoutError.) - rust-lang#131529 (rustdoc-json-types: fix typo in comment) - rust-lang#131531 (move dummy commit logic into x86_64-gnu-llvm-18) Failed merges: - rust-lang#131476 (coverage: Include the highest counter ID seen in `.cov-map` dumps) r? `@ghost` `@rustbot` modify labels: rollup

move dummy commit logic into x86_64-gnu-llvm-18 Doing the dummy commit logic in a runner that uses CI-LLVM breaks in merge CI. This should be properly fixed by rust-lang#131358 (see the [actual problem](rust-lang#131448 (comment))). Since we can also fix it by moving the dummy commit into the runner where we use in-tree LLVM, so why not do that as well?

bors · 2024-10-11T10:18:57Z

⌛ Testing commit c25d732 with merge e114788...

bors · 2024-10-11T10:27:01Z

💔 Test failed - checks-actions

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

onur-ozkan · 2024-10-11T13:01:10Z

Just realized x86_64-gnu-llvm.sh is used from llvm-19 runner too. https://github.com/rust-lang/rust/compare/c25d732a3aa7b0c7fd1fcf6968535ff6291d104c..b72dbd56caa4b5919bb1fc92fd915c9e9b4e952c should make it work this time.

@bors r=Kobzol

bors · 2024-10-11T13:01:12Z

📌 Commit b72dbd5 has been approved by Kobzol

It is now in the queue for this repository.

onur-ozkan · 2024-10-11T14:36:22Z

From #131448 (comment):

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle, if CI ever allows this PR to land.

@rustbot label +beta-accepted

So let's prioritize this a bit.

@bors p=1

move dummy commit logic into x86_64-gnu-llvm-18 Doing the dummy commit logic in a runner that uses CI-LLVM breaks in merge CI. This should be properly fixed by rust-lang#131358 (see the [actual problem](rust-lang#131448 (comment))). Since we can also fix it by moving the dummy commit into the runner where we use in-tree LLVM, so why not do that as well?

bors · 2024-10-11T18:58:43Z

⌛ Testing commit b72dbd5 with merge 1822539...

rust-log-analyzer · 2024-10-11T19:25:56Z

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

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

400272 ./src/tools/enzyme/enzyme
384700 ./src/tools/enzyme/enzyme/benchmarks
383788 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode
358000 ./src/llvm-project/clang/test
281240 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode/gmm
281188 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode/gmm/data
244624 ./.git/modules/src/gcc
234824 ./.git/modules/src/llvm-project/objects
234816 ./.git/modules/src/llvm-project/objects/pack
229684 ./.git/modules/src/gcc/objects
229684 ./.git/modules/src/gcc/objects
229676 ./.git/modules/src/gcc/objects/pack
196288 ./src/tools/rustc-perf
192232 ./src/tools/rustc-perf/collector
184180 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode/gmm/data/10k
181632 ./src/tools/rustc-perf/collector/compile-benchmarks
167268 ./tests
147688 ./src/gcc/gcc/testsuite/gcc.target
140644 ./src/llvm-project/clang/test/CodeGen/RISCV
---
62016 ./src/gcc/gcc/ada
59244 ./src/llvm-project/libcxx/test/std
58244 ./src/gcc/libgo
58192 ./src/gcc/gcc/po
55680 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode/gmm/data/1k
53720 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode/nn
53720 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode/nn
53676 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode/nn/data
52656 ./src/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy
51924 ./src/llvm-project/clang/lib
51408 ./src/llvm-project/clang-tools-extra
51024 ./src/llvm-project/compiler-rt
---
43708 ./src/llvm-project/llvm/test/Analysis/CostModel
43296 ./src/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded
43064 ./src/llvm-project/clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded
42964 ./src/doc
41316 ./src/tools/enzyme/enzyme/benchmarks/ReverseMode/gmm/data/2.5M
38376 ./src/llvm-project/lldb/test
37952 ./src/llvm-project/libc
37848 ./.git/objects
37840 ./.git/objects/pack
---
[RUSTC-TIMING] build_script_build test:false 0.626
   Compiling rustc-std-workspace-core v1.99.0 (/checkout/library/rustc-std-workspace-core)
[RUSTC-TIMING] rustc_std_workspace_core test:false 0.026

Session terminated, killing shell...::group::Clock drift check
  local time: Fri Oct 11 19:25:35 UTC 2024
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.
   Compiling cfg-if v1.0.0
   Compiling adler v1.0.2
[RUSTC-TIMING] cfg_if test:false 0.064
   Compiling rustc-demangle v0.1.24

bors · 2024-10-11T19:26:26Z

💔 Test failed - checks-actions

onur-ozkan · 2024-10-11T19:35:16Z

...killed.
Error: The operation was canceled.

I have no idea what happened, but it doesn't seem related to what this PR does.

onur-ozkan · 2024-10-11T19:36:38Z

@bors retry

bors · 2024-10-11T21:29:30Z

⌛ Testing commit b72dbd5 with merge 1bc403d...

bors · 2024-10-11T23:56:35Z

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing 1bc403d to master...

rust-timer · 2024-10-12T01:13:40Z

Finished benchmarking commit (1bc403d): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[1.8%, 2.9%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -0.2%)

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)	1.2%	[0.8%, 1.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-3.0%, -3.0%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -2.6%)

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.6%	[-3.2%, -2.0%]	7
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 780.968s -> 782.608s (0.21%)
Artifact size: 332.05 MiB -> 332.11 MiB (0.02%)

rustbot assigned Mark-Simulacrum Oct 11, 2024

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Oct 11, 2024

onur-ozkan force-pushed the move-dummy-commit branch 2 times, most recently from e4ffcf0 to 9c8d9df Compare October 11, 2024 06:50

rustbot assigned Kobzol and unassigned Mark-Simulacrum Oct 11, 2024

This comment has been minimized.

Sign in to view

onur-ozkan force-pushed the move-dummy-commit branch from 9c8d9df to c25d732 Compare October 11, 2024 06:59

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 Oct 11, 2024

onur-ozkan mentioned this pull request Oct 11, 2024

Update LLVM submodule #131448

Merged

Zalathar mentioned this pull request Oct 11, 2024

Rollup of 5 pull requests #131536

Closed

matthiaskrgr mentioned this pull request Oct 11, 2024

Rollup of 5 pull requests #131539

Closed

This comment has been minimized.

Sign in to view

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 Oct 11, 2024

move dummy commit logic into x86_64-gnu-llvm-18

b72dbd5

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

onur-ozkan force-pushed the move-dummy-commit branch from c25d732 to b72dbd5 Compare October 11, 2024 12:31

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 Oct 11, 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 Oct 11, 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 Oct 11, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 11, 2024

bors merged commit 1bc403d into rust-lang:master Oct 11, 2024
7 checks passed

rustbot added this to the 1.83.0 milestone Oct 11, 2024

onur-ozkan deleted the move-dummy-commit branch October 12, 2024 03:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move dummy commit logic into x86_64-gnu-llvm-18 #131531

move dummy commit logic into x86_64-gnu-llvm-18 #131531

onur-ozkan commented Oct 11, 2024

rustbot commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

This comment has been minimized.

Kobzol commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024 •

edited

Loading

Kobzol commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024 •

edited

Loading

bors commented Oct 11, 2024

bors commented Oct 11, 2024

This comment has been minimized.

bors commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

bors commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

bors commented Oct 11, 2024

rust-log-analyzer commented Oct 11, 2024

bors commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024 •

edited

Loading

onur-ozkan commented Oct 11, 2024

bors commented Oct 11, 2024

bors commented Oct 11, 2024

rust-timer commented Oct 12, 2024

move dummy commit logic into x86_64-gnu-llvm-18 #131531

move dummy commit logic into x86_64-gnu-llvm-18 #131531

Conversation

onur-ozkan commented Oct 11, 2024

rustbot commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

This comment has been minimized.

Kobzol commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024 • edited Loading

Kobzol commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024 • edited Loading

bors commented Oct 11, 2024

bors commented Oct 11, 2024

This comment has been minimized.

bors commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

bors commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024

bors commented Oct 11, 2024

rust-log-analyzer commented Oct 11, 2024

bors commented Oct 11, 2024

onur-ozkan commented Oct 11, 2024 • edited Loading

onur-ozkan commented Oct 11, 2024

bors commented Oct 11, 2024

bors commented Oct 11, 2024

rust-timer commented Oct 12, 2024

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

onur-ozkan commented Oct 11, 2024 •

edited

Loading

onur-ozkan commented Oct 11, 2024 •

edited

Loading

onur-ozkan commented Oct 11, 2024 •

edited

Loading