Skip to content
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

Split x86_64-msvc-ext into two jobs #130072

Merged
merged 3 commits into from
Sep 8, 2024
Merged

Conversation

ChrisDenton
Copy link
Member

@ChrisDenton ChrisDenton commented Sep 7, 2024

This is an attempt to mitigate (but not resolve) the high failure rate of the x86_64-msvc-ext builder. The theory being that doing less makes it less likely to fail. But this may not work as having an extra job that may fail might be worse.

Related to #127883

try-job: x86_64-msvc-ext
try-job: x86_64-msvc-ext2

@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2024

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

@rustbot 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 Sep 7, 2024
@ChrisDenton
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
Split x86_64-msvc-ext into two jobs

This is an attempt to mitigate (but not resolve) the high failure rate of the x86_64-msvc-ext builder. The theory being that doing less makes it less likely to fail. But this may not work as having an extra job that may fail might be worse.

try-job: x86_64-msvc-ext
try-job: x86_64-msvc-ext2
@bors
Copy link
Contributor

bors commented Sep 7, 2024

⌛ Trying commit 7358429 with merge 1cf006f...

@bors
Copy link
Contributor

bors commented Sep 7, 2024

☀️ Try build successful - checks-actions
Build commit: 1cf006f (1cf006f7b3d3c18943d6964dfe168d4b78943d23)

@ChrisDenton
Copy link
Member Author

I'll try this a few times to see what the error rate is like

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
Split x86_64-msvc-ext into two jobs

This is an attempt to mitigate (but not resolve) the high failure rate of the x86_64-msvc-ext builder. The theory being that doing less makes it less likely to fail. But this may not work as having an extra job that may fail might be worse.

try-job: x86_64-msvc-ext
try-job: x86_64-msvc-ext2
@bors
Copy link
Contributor

bors commented Sep 7, 2024

⌛ Trying commit 2f6307d with merge eaeddb8...

@rust-log-analyzer

This comment has been minimized.

@ChrisDenton
Copy link
Member Author

Oops, I messed up the bash syntax. Fixed.

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
Updating files:  99% (48857/49350)
Updating files:  99% (49248/49350)
Updating files: 100% (49350/49350)
Updating files: 100% (49350/49350), done.
branch 'try' set up to track 'origin/try'.
Switched to a new branch 'try'
[command]"C:\Program Files\Git\bin\git.exe" log -1 --format='%H'
'eaeddb817ad6824cbd60df156b0673eb3308334a'
##[group]Run src/ci/scripts/setup-environment.sh
src/ci/scripts/setup-environment.sh
---
file:.git/config remote.origin.url=https://github.com/rust-lang-ci/rust
file:.git/config remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
file:.git/config gc.auto=0
file:.git/config http.https://github.com/.extraheader=AUTHORIZATION: basic ***
file:.git/config branch.try.remote=origin
file:.git/config branch.try.merge=refs/heads/try
file:.git/config submodule.library/backtrace.url=https://github.com/rust-lang/backtrace-rs.git
file:.git/config submodule.library/stdarch.active=true
file:.git/config submodule.library/stdarch.url=https://github.com/rust-lang/stdarch.git
file:.git/config submodule.src/doc/book.active=true
---
##[endgroup]
[TIMING] core::build_steps::test::CargoMiri { target: x86_64-pc-windows-msvc } -- 44.538
Build completed successfully in 0:14:22
+ case $HOST_TARGET in
src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh: line 78: syntax error near unexpected token `fi'
  network time: Sat, 07 Sep 2024 17:35:44 GMT
##[error]Process completed with exit code 2.
Post job cleanup.
[command]"C:\Program Files\Git\bin\git.exe" version

@ChrisDenton
Copy link
Member Author

That one didn't count because of the messed up bash but at least msvc-ext2 passed! Let's try again

@bors try

@bors
Copy link
Contributor

bors commented Sep 7, 2024

⌛ Trying commit 0d94e6b with merge ce47cec...

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
Split x86_64-msvc-ext into two jobs

This is an attempt to mitigate (but not resolve) the high failure rate of the x86_64-msvc-ext builder. The theory being that doing less makes it less likely to fail. But this may not work as having an extra job that may fail might be worse.

try-job: x86_64-msvc-ext
try-job: x86_64-msvc-ext2
@bors
Copy link
Contributor

bors commented Sep 7, 2024

☀️ Try build successful - checks-actions
Build commit: ce47cec (ce47cec155133d0bd252f96eb341405b2a1da2ca)

@ChrisDenton
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
Split x86_64-msvc-ext into two jobs

This is an attempt to mitigate (but not resolve) the high failure rate of the x86_64-msvc-ext builder. The theory being that doing less makes it less likely to fail. But this may not work as having an extra job that may fail might be worse.

try-job: x86_64-msvc-ext
try-job: x86_64-msvc-ext2
@bors
Copy link
Contributor

bors commented Sep 7, 2024

⌛ Trying commit 0d94e6b with merge 714f76a...

@bors
Copy link
Contributor

bors commented Sep 7, 2024

☀️ Try build successful - checks-actions
Build commit: 714f76a (714f76a81ae99067b955aac39c9f3e0f9d45c100)

@ChrisDenton
Copy link
Member Author

Succes again. I would have expected an access denied error by now so this does seem to be an improvement. I can keep running try jobs but my question to the reviewer is if adding a new job would be acceptable (hopefully as a temporary measure). I'd reiterate that failures are still likely but hopefully much rarer.

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2024
Split x86_64-msvc-ext into two jobs

This is an attempt to mitigate (but not resolve) the high failure rate of the x86_64-msvc-ext builder. The theory being that doing less makes it less likely to fail. But this may not work as having an extra job that may fail might be worse.

try-job: x86_64-msvc-ext
try-job: x86_64-msvc-ext2
@bors
Copy link
Contributor

bors commented Sep 7, 2024

⌛ Trying commit 0d94e6b with merge 347a9d1...

@bors
Copy link
Contributor

bors commented Sep 7, 2024

☀️ Try build successful - checks-actions
Build commit: 347a9d1 (347a9d102cd5ffb10d6e556e3bef1d242b11d8f6)

@bors
Copy link
Contributor

bors commented Sep 8, 2024

⌛ Testing commit 0d94e6b with merge 535f25e...

@bors
Copy link
Contributor

bors commented Sep 8, 2024

💔 Test failed - checks-actions

@bors bors 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 Sep 8, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Sep 8, 2024

Ok, let's try again.

@bors r+

@bors
Copy link
Contributor

bors commented Sep 8, 2024

📌 Commit 0d94e6b has been approved by Kobzol

It is now in the queue for this repository.

@bors 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 8, 2024
@workingjubilee
Copy link
Member

@bors p=1

@workingjubilee
Copy link
Member

...it's already p=1, right.

@bors
Copy link
Contributor

bors commented Sep 8, 2024

⌛ Testing commit 0d94e6b with merge 8f93a10...

@bors
Copy link
Contributor

bors commented Sep 8, 2024

☀️ Test successful - checks-actions
Approved by: Kobzol
Pushing 8f93a10 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 8, 2024
@bors bors merged commit 8f93a10 into rust-lang:master Sep 8, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 8, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8f93a10): 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 -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% [-2.9%, -2.3%] 2
All ❌✅ (primary) - - 0

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: 756.515s -> 756.659s (0.02%)
Artifact size: 341.16 MiB -> 341.09 MiB (-0.02%)

@ChrisDenton ChrisDenton deleted the split-ci branch September 8, 2024 13:04
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 2, 2024
…try>

Revert MSVC CI changes

- rust-lang#130151
- rust-lang#130072

try-job: x86_64-msvc
try-job: x86_64-msvc-ext
@tgross35 tgross35 mentioned this pull request Oct 12, 2024
@cuviper cuviper added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 12, 2024
@cuviper cuviper modified the milestones: 1.83.0, 1.82.0 Oct 12, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2024
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448
- Split x86_64-msvc-ext into two jobs rust-lang#130072
- Use a small runner for msvc-ext2 job rust-lang#130151

r? ghost
@@ -58,8 +58,9 @@ case $HOST_TARGET in
# Strangely, Linux targets do not work here. cargo always says
# "error: cannot produce cdylib for ... as the target ... does not support these crate types".
# Only run "pass" tests, which is quite a bit faster.
python3 "$X_PY" test --stage 2 src/tools/miri --target aarch64-apple-darwin --test-args pass
python3 "$X_PY" test --stage 2 src/tools/miri --target i686-pc-windows-gnu --test-args pass
#FIXME: Re-enable this once CI issues are fixed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an issue that tracks this? Would be good to reference it here so that it's easy to check when the FIXME can be resolved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, would have been nice to ping @rust-lang/miri here; this change could mean a breakage on those targets only gets detected by Miri CI, and it would have been very confusing to figure out what happened when we are sure that Rust CI already covers these targets.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I found the issue: #127883

Added it to the PR description, too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I messed something up, we should still be testing everything that we were testing. It's just being tested in a separate job (x86_64-msvc-ext2).

I only intended this to be very temporary but it seems the issues is persisting longer than I'd hoped.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, now I see, this got moved to src/ci/github-actions/jobs.yml... which is not where our tools testing logic usually lives so I missed that.

jhpratt added a commit to jhpratt/rust that referenced this pull request Oct 18, 2024
…youxu

checktools.sh: add link to issue for more context about disabled Miri tests

Adds some context for the changes made in rust-lang#130072.
jhpratt added a commit to jhpratt/rust that referenced this pull request Oct 18, 2024
…isDenton

checktools.sh: add link to issue for more context about disabled Miri tests

Adds some context for the changes made in rust-lang#130072.
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Oct 18, 2024
…isDenton

checktools.sh: add link to issue for more context about disabled Miri tests

Adds some context for the changes made in rust-lang#130072.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 18, 2024
Rollup merge of rust-lang#131877 - RalfJung:checktools-comment, r=ChrisDenton

checktools.sh: add link to issue for more context about disabled Miri tests

Adds some context for the changes made in rust-lang#130072.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants