-
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
libstd needs update for pending libc change #39871
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (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. |
Please note that this pull request is dependent upon the changes made to libc and cannot be integrated before that one. |
src/libstd/sys/unix/os.rs
Outdated
@@ -483,7 +483,6 @@ pub fn home_dir() -> Option<PathBuf> { | |||
target_os = "nacl", | |||
target_os = "emscripten")))] | |||
unsafe fn fallback() -> Option<OsString> { | |||
#[cfg(not(target_os = "solaris"))] | |||
unsafe fn getpwduid_r(me: libc::uid_t, passwd: &mut libc::passwd, |
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.
Now that there's no abstraction, could this wrapper be removed entirely?
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.
Ah, you're right. I was attempting to minimise changes here, but the wrapper could be removed entirely. Let me simplify this further and rebuild.
Ok, I've simplified this as requested and verified that it works as expected (on Solaris). |
@bors: r+ |
📌 Commit 5789539 has been approved by |
…richton libstd needs update for pending libc change This updates libstd to accommodate the fixes made in rust-lang/libc#523 Fixes rust-lang#39868
Hmm, shouldn't the libc submodule be updated? Or does this PR update it and github doesn't show it? |
@est31 This pr does not update the submodule; 1) because I'm relatively new to git and have no idea what the right way is to do that 2) I wasn't certain what rust's policy is for updating submodules. That's why I explicitly noted in the pull request that this change depends upon the changes made in libc. My apologies if there is something more that I should do. If someone would explain to me exactly what I should be doing for pull requests like this, I'm happy to make the requested change. |
@bors: r- Oh good catch @est31! @binarycrusader I don't blame you, I try to stay away from submodules with a 10 foot pole... In any case it's totally fine to update the libc submodule whenever, so you're more than welcome to do so. I believe the easiest way to do so is:
|
@bors: r+ |
📌 Commit 68a9d8b has been approved by |
To be honest, I'd have had no idea how to do this myself either. I'm using git since some time already, but I still don't know how to deal with submodules :) |
…richton libstd needs update for pending libc change This updates libstd to accommodate the fixes made in rust-lang/libc#523 Fixes rust-lang#39868
Looks like one of the checks failed due to a network error but everything else passed? |
…richton libstd needs update for pending libc change This updates libstd to accommodate the fixes made in rust-lang/libc#523 Fixes rust-lang#39868
⌛ Testing commit 68a9d8b with merge 3bac2c1... |
💔 Test failed - status-travis |
Can someone get bors to retry the Travis build? Looks like failure was network or Travis related. |
@bors retry |
libstd needs update for pending libc change This updates libstd to accommodate the fixes made in rust-lang/libc#523 Fixes #39868
☀️ Test successful - status-appveyor, status-travis |
This updates libstd to accommodate the fixes made in rust-lang/libc#523
Fixes #39868