-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Migrate dump-ice-to-disk
and panic-abort-eh_frame
run-make
tests to rmake
#127523
Conversation
This PR modifies cc @jieyouxu |
@bors delegate+ (for try jobs only) |
✌️ @Oneirical, you can now approve this pull request! If @jieyouxu told you to " |
@bors try |
Migrate `dump-ice-to-disk` and `panic-abort-eh_frame` `run-make` tests to rmake Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). Please try: try-job: aarch64-apple try-job: x86_64-msvc
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
Migrate `dump-ice-to-disk` and `panic-abort-eh_frame` `run-make` tests to rmake Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). Please try: try-job: x86_64-msvc
☀️ Try build successful - checks-actions |
Very well, I am removing that check for colons ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR. I have some concerns about dump-ice-to-disk
regarding test parity and a question about a FIXME in panic_abort-eh_frame
.
@rustbot author |
💔 Test failed - checks-actions |
Ah I didn't r- fast enough, this failed the rollup too #127991 (comment). Almost all tests except i686-mingw passed, updated the try jobs. |
☔ The latest upstream changes (presumably #127663) made this pull request unmergeable. Please resolve the merge conflicts. |
e7cfc85
to
184649c
Compare
I restored the ignore on this specific platform, let me know if that is adequate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, the other changes look good, but the ignore on the panic-abort
test is a bit interesting, I'll wait for people who are more familiar to see if it's intentional that it fails on i686-pc-windows-gnu
.
//@ ignore-i686-pc-windows-gnu | ||
// Reason: the DW_CFA symbol appears on this specific architecture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: cc @Nilstrieb (since you were the reviewer of #112403) and @saethlin, since I lack context for this test: is it intentional that this property is only preserved for linux, but not necessarily for i686-pc-windows-gnu
, or is this a bug? The original test was only-linux
but I couldn't find reasons for why it had to be only-linux
other than it was impacting RfL so obviously we would want to test it on linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The context is that everything under -Zub-checks creates non-unwinding panics, so it is a problem for RFL and quite goofy in general if enabling -Zub-checks produces new sections which are only for unwinding.
I don't know enough about this target to say whether it has this section with UB checks disabled. If it has this section with UB checks off that seems like a separate bug, we can handle that in another issue. If it gains this section when UB checks are enabled on this target specifically I would really like to know why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this is intended behavior that uwtable is forced on Windows (cf. #128136 (comment)), see
requires_uwtable: true, |
Can you revert this to //@ only-linux
and add a comment to that effect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me!
☔ The latest upstream changes (presumably #128109) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, r=me after a rebase + adding the FIXME linking to #128136.
//@ ignore-i686-pc-windows-gnu | ||
// Reason: the DW_CFA symbol appears on this specific architecture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this is intended behavior that uwtable is forced on Windows (cf. #128136 (comment)), see
requires_uwtable: true, |
Can you revert this to //@ only-linux
and add a comment to that effect?
There's something weird about this. The Given this, why would |
This comment has been minimized.
This comment has been minimized.
Let's keep it only-linux and just document that currently uwtables are forced on windows targers (apparently they're eventually forced even on windows-gnu). AFAICT we only are trying to guarantee this property on linux, but not for Windows. |
Rollup of 7 pull requests Successful merges: - rust-lang#126575 (Make it crystal clear what lint `type_alias_bounds` actually signifies) - rust-lang#127017 (Extend rules of dead code analysis for impls for adts to impls for types refer to adts) - rust-lang#127523 (Migrate `dump-ice-to-disk` and `panic-abort-eh_frame` `run-make` tests to rmake) - rust-lang#127557 (Add a label to point to the lacking macro name definition) - rust-lang#127989 (Migrate `interdependent-c-libraries`, `compiler-rt-works-on-mingw` and `incr-foreign-head-span` `run-make` tests to rmake) - rust-lang#128099 (migrate tests/run-make/extern-flag-disambiguates to rmake) - rust-lang#128170 (Make Clone::clone a lang item) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#127523 - Oneirical:treasure-test, r=jieyouxu Migrate `dump-ice-to-disk` and `panic-abort-eh_frame` `run-make` tests to rmake Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html). Please try: try-job: x86_64-msvc try-job: i686-mingw
🤔 bors you tested it already |
Part of #121876 and the associated Google Summer of Code project.
Please try:
try-job: x86_64-msvc
try-job: i686-mingw