Uh oh!
There was an error while loading. Please reload this page.
Improved error reporting when target sysroot is missing. - #37424
Conversation
rust-highfive
commented
Oct 27, 2016
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (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. |
nrc
commented
Oct 27, 2016
bors
commented
Oct 28, 2016
☔ The latest upstream changes (presumably #37350) made this pull request unmergeable. Please resolve the merge conflicts. |
Thanks @shiver! This looks good to me, with the exception that it needs a test. I'd say the best way to do it is to add a test to You can crib off other tests in that directory for details. |
shiver
commented
Nov 9, 2016
Excellent! Thanks for the feedback. I'll hopefully push the test in the next day or two. |
There was a problem hiding this comment.
This should be --target={}, right?
There was a problem hiding this comment.
Sorry about that.
Definitely a bug. I was having some issues with the tests, sometimes all passing (possibly because I wasn't recompiling rustc correctly), so I didn't pick that up.
Hopefully the latest commit addresses this.
There was a problem hiding this comment.
Perhaps this could be "may not be installed" as it may actually be and we could be error'ing for some other reason?
There was a problem hiding this comment.
Yeah, that seems like a better idea.
I have changed the message to read "the <target> target may not be installed".
alexcrichton
commented
Nov 17, 2016
Thanks! Want to squash the commits? |
shiver
commented
Nov 17, 2016
Hopefully squashed in the correct manner. Let me know if you see any issues though! |
alexcrichton
commented
Nov 17, 2016
@bors: r+ Thanks! |
bors
commented
Nov 17, 2016
📌 Commit 66de87f has been approved by |
bors
commented
Nov 17, 2016
Improved error reporting when target sysroot is missing. Attempts to resolve#37131. This is my first pull request on rust, so I would greatly appreciate any feedback you have on this. Thanks!
Attempts to resolve#37131.
This is my first pull request on rust, so I would greatly appreciate any feedback you have on this.
Thanks!