-
Notifications
You must be signed in to change notification settings - Fork 13k
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
cleanup: remove the *san Cargo features from std #39860
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
@bors: r+ Thanks! |
📌 Commit d22891e has been approved by |
cleanup: remove the *san Cargo features from std these belong to a previous iteration of the sanitizer implementation r? @alexcrichton cc @whitequark
cleanup: remove the *san Cargo features from std these belong to a previous iteration of the sanitizer implementation r? @alexcrichton cc @whitequark
This PR might have caused this issue: https://travis-ci.org/rust-lang/rust/jobs/202521389 |
⌛ Testing commit d22891e with merge 1d20fc1... |
💔 Test failed - status-travis |
This looks wrong because rustc_asan and friends are only dependencies of std on x86_64 Linux but this is testing for arm-linux. It seems that bootstrap has build the dependency graph of std assuming that the target is x86_64-linux. bootstrap is using |
If I'm reding this correctly, bootstrap uses
A much simpler but hacky solution would be to blacklist rustc_asan and company in check.rs; that way we could filter away those crates so they are never passed to the @alexcrichton thoughts? ^ |
@japaric I think the easiest solution would be to add this to all sanitizer [lib]
test = false The lib isn't intended to be unit tested, right? |
Oh, is that enough? OK. r? @alexcrichton |
@bors: r+ |
📌 Commit 2496f8a has been approved by |
⌛ Testing commit 2496f8a with merge 98b8826... |
💔 Test failed - status-travis |
@bors: retry
* network error
…On Thu, Feb 23, 2017 at 6:46 AM, bors ***@***.***> wrote:
💔 Test failed - status-travis
<https://travis-ci.org/rust-lang/rust/builds/204569095>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39860 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95JO0W0-MOpIsjhMc8EVDNlnLEnVpks5rfX-RgaJpZM4MCToI>
.
|
☔ The latest upstream changes (presumably #39851) made this pull request unmergeable. Please resolve the merge conflicts. |
🔒 Merge conflict |
ping @japaric (rebase) |
these belong to a previous iteration of the sanitizer implementation
@alexcrichton rebased |
@bors: r+ |
📌 Commit 28baa27 has been approved by |
cleanup: remove the *san Cargo features from std these belong to a previous iteration of the sanitizer implementation r? @alexcrichton cc @whitequark
☀️ Test successful - status-appveyor, status-travis |
these belong to a previous iteration of the sanitizer implementation
r? @alexcrichton
cc @whitequark