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.

Sign up for GitHub

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

Jump to bottom

tests: Skip const OOM tests on aarch64-unknown-linux-gnu #136004

Merged
merged 1 commit into from
Jan 31, 2025
Merged

tests: Skip const OOM tests on aarch64-unknown-linux-gnu #136004

merged 1 commit into from
Jan 31, 2025
+8 −14

Conversation

mrkajetanp
Copy link
Contributor

@mrkajetanp mrkajetanp commented Jan 24, 2025 •
edited by Kobzol
Loading

Skip const OOM tests on AArch64 Linux through explicit annotations instead of inside opt-dist.
Intended to avoid confusion in cases like #135952.

Prerequisite for #135960.

r? @Kobzol
cc @workingjubilee

try-job: dist-aarch64-linux

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

Could not assign reviewer from: Kobzol.
User(s) Kobzol are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 24, 2025
@mrkajetanp
Copy link
Contributor Author

r? @workingjubilee maybe

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

Could not assign reviewer from: workingjubilee.
User(s) workingjubilee are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

@jieyouxu
Copy link
Member

jieyouxu commented Jan 24, 2025 •
edited
Loading

cf. #135960 and #135961

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@mrkajetanp mrkajetanp force-pushed the aarch64-skip-large-const-alloc-tests branch from c692b4b to b8d24e2 Compare January 24, 2025 16:58
@Kobzol
Copy link
Contributor

Kobzol commented Jan 24, 2025

Could you remove these two tests from the list of skipped tests of opt-dist? And also please include a WIP commit that comments out DIST_TRY_BUILD, so that we can do a try build to see if it works properly :)

@mrkajetanp
Copy link
Contributor Author

Could you remove these two tests from the list of skipped tests of opt-dist?

I left it out because those are removed in #135960 along with the rest, should I still change it here as well?

@Kobzol
Copy link
Contributor

Kobzol commented Jan 24, 2025

I think that PR is for debugging, but even if not, it would be nice to unblock people running tests locally on x64 ARM. We can unskip the rest of the tests later.

@mrkajetanp mrkajetanp force-pushed the aarch64-skip-large-const-alloc-tests branch from b8d24e2 to dde618b Compare January 24, 2025 20:02
@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

Some changes occurred in src/tools/opt-dist

cc @Kobzol

@rustbot 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 Jan 24, 2025
@mrkajetanp
Copy link
Contributor Author

Sure, there you go

@Kobzol
Copy link
Contributor

Kobzol commented Jan 24, 2025

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 24, 2025
@bors
Auto merge of rust-lang#136004 - mrkajetanp:aarch64-skip-large-const-…
a5b8780 
…alloc-tests, r=<try>

tests: Skip const OOM tests on aarch64-unknown-linux-gnu

Skip const OOM tests on AArch64 Linux through explicit annotations instead of inside opt-dist.
Intended to avoid confusion in cases like rust-lang#135952.

Prerequisite for rust-lang#135960.

 r? `@Kobzol`
cc `@workingjubilee`

try-job: dist-aarch64-linux
@bors
Copy link
Contributor

bors commented Jan 24, 2025

⌛ Trying commit dde618b with merge a5b8780...

@workingjubilee
Copy link
Member

I think that PR is for debugging, but even if not, it would be nice to unblock people running tests locally on x64 ARM. We can unskip the rest of the tests later.

x64 ARM?!?!

@mrkajetanp
Copy link
Contributor Author

x64 ARM

the ultimate architecture

@workingjubilee
Copy link
Member

I guess that'd be like Apple Silicon but with Rosetta 2 in silico?

@Kobzol
Copy link
Contributor

Kobzol commented Jan 24, 2025

😆 Sorry, I'm so used to writing Linux x64 that I somehow mumbled these two words. I meant ARM (Aarch64) Linux, ofc :)

@bors
Copy link
Contributor

bors commented Jan 24, 2025

☀️ Try build successful - checks-actions
Build commit: a5b8780 (a5b87800c9bd87cfb31b63a6a5dc1712a24db7b1)

@mrkajetanp
Copy link
Contributor Author

Should I drop the WIP command then?

@Kobzol
Copy link
Contributor

Kobzol commented Jan 27, 2025

Yes please :)

@workingjubilee
Copy link
Member

@rustbot author

@rustbot 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 Jan 28, 2025
@mrkajetanp mrkajetanp force-pushed the aarch64-skip-large-const-alloc-tests branch from dde618b to cf02667 Compare January 28, 2025 08:44
@mrkajetanp
Copy link
Contributor Author

@rustbot ready

@rustbot 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 Jan 28, 2025
@Kobzol
Copy link
Contributor

Kobzol commented Jan 28, 2025

@bors r+

@bors
Copy link
Contributor

bors commented Jan 28, 2025

📌 Commit cf02667 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 Jan 28, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 28, 2025
@matthiaskrgr
Rollup merge of rust-lang#136004 - mrkajetanp:aarch64-skip-large-cons…
6bcb830 
…t-alloc-tests, r=Kobzol

tests: Skip const OOM tests on aarch64-unknown-linux-gnu

Skip const OOM tests on AArch64 Linux through explicit annotations instead of inside opt-dist.
Intended to avoid confusion in cases like rust-lang#135952.

Prerequisite for rust-lang#135960.

 r? `@Kobzol`
cc `@workingjubilee`

try-job: dist-aarch64-linux
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 28, 2025
Closed
@bors
Copy link
Contributor

bors commented Jan 28, 2025

☔ The latest upstream changes (presumably #136185) made this pull request unmergeable. Please resolve the merge conflicts.

@bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 28, 2025
@mrkajetanp
tests: Skip const OOM tests on aarch64-unknown-linux-gnu
b7916fb 
Skip const OOM tests on AArch64 Linux through explicit annotations
instead of inside opt-dist.
Intended to avoid confusion in cases like rust-lang#135952.

Prerequisite for rust-lang#135960.
@mrkajetanp mrkajetanp force-pushed the aarch64-skip-large-const-alloc-tests branch from cf02667 to b7916fb Compare January 29, 2025 16:24
Kobzol
Kobzol reviewed Jan 29, 2025
View reviewed changes
src/tools/opt-dist/src/main.rs
@@ -169,7 +159,7 @@ fn create_environment(args: Args) -> anyhow::Result<(Environment, Vec<String>)>
.shared_llvm(true)
// FIXME: Enable bolt for aarch64 once it's fixed upstream. Broken as of December 2024.
.use_bolt(!is_aarch64)
.skipped_tests(skip_tests)
.skipped_tests(vec![])
Copy link
Contributor

Choose a reason for hiding this comment

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

This call can be completely removed, now that there are no skipped tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, I left it just because there's a separate PR to remove the skipping so I didn't want to trample over it. There's an identical call elsewhere in the file too

@Kobzol
Copy link
Contributor

Kobzol commented Jan 29, 2025

@bors r+

@bors
Copy link
Contributor

bors commented Jan 29, 2025

📌 Commit b7916fb 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 29, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 30, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2025
@bors
Auto merge of rust-lang#136318 - matthiaskrgr:rollup-a159mzo, r=matth…
6c1d960 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#135026 (Cast global variables to default address space)
 - rust-lang#135475 (uefi: Implement path)
 - rust-lang#135852 (Add `AsyncFn*` to `core` prelude)
 - rust-lang#136004 (tests: Skip const OOM tests on aarch64-unknown-linux-gnu)
 - rust-lang#136157 (override build profile for bootstrap tests)
 - rust-lang#136180 (Introduce a wrapper for "typed valtrees" and properly check the type before extracting the value)
 - rust-lang#136256 (Add release notes for 1.84.1)
 - rust-lang#136271 (Remove minor future footgun in `impl Debug for MaybeUninit`)
 - rust-lang#136288 (Improve documentation for file locking)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 47ee068 into rust-lang:master Jan 31, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 31, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2025
@rust-timer
Unrolled build for rust-lang#136004
93586fc 
Rollup merge of rust-lang#136004 - mrkajetanp:aarch64-skip-large-const-alloc-tests, r=Kobzol

tests: Skip const OOM tests on aarch64-unknown-linux-gnu

Skip const OOM tests on AArch64 Linux through explicit annotations instead of inside opt-dist.
Intended to avoid confusion in cases like rust-lang#135952.

Prerequisite for rust-lang#135960.

 r? `@Kobzol`
cc `@workingjubilee`

try-job: dist-aarch64-linux
@mrkajetanp mrkajetanp mentioned this pull request Feb 1, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kobzol Kobzol Kobzol left review comments

Assignees

@workingjubilee workingjubilee

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.
8 participants
@mrkajetanp @rustbot @jieyouxu @rust-log-analyzer @Kobzol @bors @workingjubilee @petrochenkov