-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Rewrite core-no-oom-handling
, issue-24445
and issue-38237
run-make
tests to new rmake.rs
format
#125421
Conversation
This comment has been minimized.
This comment has been minimized.
701cb37
to
670e401
Compare
// This test checks that the core library can still compile correctly | ||
// when the no_global_oom_handling feature is turned on. |
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.
Nit: it only tests that compilation succeeds, it does not check for correctness.
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.
I wrote that exact "correctly" terminology in a couple of similar tests to this one, so I have changed them as well. Thanks!
// A very specific set of circumstances (mainly, implementing Deref, and | ||
// having a procedural macro and a Debug derivation in external crates) caused | ||
// an internal compiler error (ICE) when trying to use rustdoc. This test | ||
// reproduces the exact circumstances which caused the bug and checks | ||
// that it does not happen again. |
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.
Not-a-review-comment: this is very specific... it must've been a pain to figure out lol
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.
I know, right? I find it especially hilarious that the fix for all this, in the linked PR, is literally just adding a single line of code.
One minor nit for a test comment, then r=me after CI is green |
✌️ @Oneirical, you can now approve this pull request! If @jieyouxu told you to " |
670e401
to
aa9ef5c
Compare
This comment has been minimized.
This comment has been minimized.
aa9ef5c
to
1676d84
Compare
This comment has been minimized.
This comment has been minimized.
.arg("--gc-sections") | ||
.arg("-lpthread") | ||
.arg("-ldl") | ||
.out_exe(tmp_dir().join("foo")) |
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.
IIRC out_exe
tries to construct the executable path under tmp_dir()
already, so you just need to pass the exe name, i.e. "foo"
here.
844a2bc
to
33f08fa
Compare
Thanks, r=me after CI is green |
✌️ @Oneirical, you can now approve this pull request! If @jieyouxu told you to " |
This comment has been minimized.
This comment has been minimized.
Well, that's an unexpected error. Here is what I think I understand: there are multiple C compilers, and we want to use GCC, as it is the one that has the I also notice how this test has Is it possible that, in the past where this test was written, the GNU/Linux CI was using GCC by default, and that changed? But then, how could the Makefile version of the test keep passing? I may need your help on this one. |
Not quite: clang and gcc has similar CLI interfaces on this one (e.g. https://clang.llvm.org/docs/CommandGuide/clang.html#cmdoption-Wl-args). The catch here is that the cli flag takes the form
So they're actually taking That is,
is different from
lol. You needed to write |
33f08fa
to
1f17e27
Compare
I think the Thank you so much for figuring this out, I was digging a serious rabbit hole in obscure documentation... Well, no victory just yet, let's see what the CI has to say about it. |
…=jieyouxu Rewrite `core-no-oom-handling`, `issue-24445` and `issue-38237` `run-make` tests to new `rmake.rs` format 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). The test which is now called `non-pie-thread-local` has an unexplained "only-linux" flag. Could it be worth trying to remove it and changing the CI to test non-Linux platforms on it?
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#125165 (Migrate `run-make/pgo-branch-weights` to `rmake`) - rust-lang#125210 (Cleanup: Fix up some diagnostics) - rust-lang#125224 (Migrate `run-make/issue-53964` to `rmake`) - rust-lang#125227 (Migrate `run-make/issue-30063` to `rmake`) - rust-lang#125383 (Rewrite `emit`, `mixing-formats` and `bare-outfile` `run-make` tests in `rmake.rs` format) - rust-lang#125401 (Migrate `run-make/rustdoc-scrape-examples-macros` to `rmake.rs`) - rust-lang#125409 (Rename `FrameworkOnlyWindows` to `RawDylibOnlyWindows`) - rust-lang#125416 (Use correct param-env in `MissingCopyImplementations`) - rust-lang#125421 (Rewrite `core-no-oom-handling`, `issue-24445` and `issue-38237` `run-make` tests to new `rmake.rs` format) r? `@ghost` `@rustbot` modify labels: rollup
…=jieyouxu Rewrite `core-no-oom-handling`, `issue-24445` and `issue-38237` `run-make` tests to new `rmake.rs` format 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). The test which is now called `non-pie-thread-local` has an unexplained "only-linux" flag. Could it be worth trying to remove it and changing the CI to test non-Linux platforms on it?
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#125210 (Cleanup: Fix up some diagnostics) - rust-lang#125224 (Migrate `run-make/issue-53964` to `rmake`) - rust-lang#125227 (Migrate `run-make/issue-30063` to `rmake`) - rust-lang#125383 (Rewrite `emit`, `mixing-formats` and `bare-outfile` `run-make` tests in `rmake.rs` format) - rust-lang#125401 (Migrate `run-make/rustdoc-scrape-examples-macros` to `rmake.rs`) - rust-lang#125409 (Rename `FrameworkOnlyWindows` to `RawDylibOnlyWindows`) - rust-lang#125416 (Use correct param-env in `MissingCopyImplementations`) - rust-lang#125421 (Rewrite `core-no-oom-handling`, `issue-24445` and `issue-38237` `run-make` tests to new `rmake.rs` format) - rust-lang#125438 (Remove unneeded string conversion) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#124297 (Allow coercing functions whose signature differs in opaque types in their defining scope into a shared function pointer type) - rust-lang#124516 (Allow monomorphization time const eval failures if the cause is a type layout issue) - rust-lang#124976 (rustc: Use `tcx.used_crates(())` more) - rust-lang#125210 (Cleanup: Fix up some diagnostics) - rust-lang#125409 (Rename `FrameworkOnlyWindows` to `RawDylibOnlyWindows`) - rust-lang#125416 (Use correct param-env in `MissingCopyImplementations`) - rust-lang#125421 (Rewrite `core-no-oom-handling`, `issue-24445` and `issue-38237` `run-make` tests to new `rmake.rs` format) - rust-lang#125438 (Remove unneeded string conversion) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#125421 - Oneirical:bundle-them-yet-again, r=jieyouxu Rewrite `core-no-oom-handling`, `issue-24445` and `issue-38237` `run-make` tests to new `rmake.rs` format 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). The test which is now called `non-pie-thread-local` has an unexplained "only-linux" flag. Could it be worth trying to remove it and changing the CI to test non-Linux platforms on it?
Part of #121876 and the associated Google Summer of Code project.
The test which is now called
non-pie-thread-local
has an unexplained "only-linux" flag. Could it be worth trying to remove it and changing the CI to test non-Linux platforms on it?