-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Use libtest from crates.io for compiletest #59381
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
📌 Commit 4522d6848b09246072130cc3945e9497c8b95610 has been approved by |
LGTM. @alexcrichton would it be possible to give this higher priority ? |
@bors: p=1 |
⌛ Testing commit 4522d6848b09246072130cc3945e9497c8b95610 with merge a1a6bc9b263d87af33263f9a16e24ba1441f8446... |
💔 Test failed - checks-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors r- Can we remove the |
Done |
@bjorn3 have you checked that removing the |
c060813
to
4522d68
Compare
Oops didn't test. Force pushed that commit away. |
@bors: r+ |
📌 Commit 4522d6848b09246072130cc3945e9497c8b95610 has been approved by |
⌛ Testing commit 4522d6848b09246072130cc3945e9497c8b95610 with merge 19db5af84833b010d99b76bcabcbafd6f215f5f1... |
💔 Test failed - status-appveyor |
📌 Commit 97262760bf9fce244ef73c69390dbcb246c1e28b has been approved by |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
This fixes using latest nightly + local-rebuild=true for testing
9726276
to
8516a88
Compare
Rebased |
@bors r=Manishearth |
📌 Commit 8516a88 has been approved by |
⌛ Testing commit 8516a88 with merge e065a87849d312c9451d68b37195f3ff399bfa25... |
💔 Test failed - status-appveyor |
On windows |
We should probably close this PR, since the change that motivated it was reverted. What we'd probably want to do, is update compiletest-rs to contain the latest upstream changes, remove |
Okay |
@gnzlbg given the weird build system errors we were experiencing it may be worth testing out libtest as a part of compiletest first. But works either way, really. |
@manish compiletest-rs would just continue to just use the “test” crate on
nightly. That should work both in and out of tree (for stable Rust a
different crate is used).
|
I mean, ideally, as a part of this compiletest-rs becomes stable-only, since libtest is stable |
libtest on crates.io does not work on stable Rust.
…On Sun 14. Apr 2019 at 17:38, Manish Goregaokar ***@***.***> wrote:
I mean, ideally, as a part of this compiletest-rs becomes stable-only,
since libtest is stable
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#59381 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA3Npqnbl66TQ-JpNnFLkjsQ7rT0hI7nks5vg0uJgaJpZM4cE1Lg>
.
|
What's the reason behind that? Can we fix that first? |
We could, There is a PR open to libtest that does that, but it’s unclear
whether that libtest can be used in tree.
|
Ah. |
This fixes using latest nightly + local-rebuild=true for testing
Fixes #58890 and #59264