Skip to content
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

Merged
merged 3 commits into from
Feb 19, 2017

Conversation

binarycrusader
Copy link
Contributor

This updates libstd to accommodate the fixes made in rust-lang/libc#523

Fixes #39868

@rust-highfive
Copy link
Collaborator

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.

@binarycrusader
Copy link
Contributor Author

Please note that this pull request is dependent upon the changes made to libc and cannot be integrated before that one.

@@ -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,
Copy link
Member

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?

Copy link
Contributor Author

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.

@binarycrusader
Copy link
Contributor Author

Ok, I've simplified this as requested and verified that it works as expected (on Solaris).

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 16, 2017

📌 Commit 5789539 has been approved by alexcrichton

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2017
…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
@est31
Copy link
Member

est31 commented Feb 16, 2017

Hmm, shouldn't the libc submodule be updated? Or does this PR update it and github doesn't show it?

@binarycrusader
Copy link
Contributor Author

binarycrusader commented Feb 16, 2017

@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.

@alexcrichton
Copy link
Member

@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:

$ cd src/liblibc
$ git fetch origin
$ git reset --hard origin/master
$ cd ../..
$ git add src/liblibc
$ git commit -m 'Update src/liblibc'

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 16, 2017

📌 Commit 68a9d8b has been approved by alexcrichton

@est31
Copy link
Member

est31 commented Feb 16, 2017

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 :)

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 17, 2017
…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
@binarycrusader
Copy link
Contributor Author

Looks like one of the checks failed due to a network error but everything else passed?

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 17, 2017
…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
@bors
Copy link
Contributor

bors commented Feb 18, 2017

⌛ Testing commit 68a9d8b with merge 3bac2c1...

@bors
Copy link
Contributor

bors commented Feb 18, 2017

💔 Test failed - status-travis

@binarycrusader
Copy link
Contributor Author

Can someone get bors to retry the Travis build? Looks like failure was network or Travis related.

@petrochenkov
Copy link
Contributor

@bors retry

@bors
Copy link
Contributor

bors commented Feb 19, 2017

⌛ Testing commit 68a9d8b with merge 0128be9...

bors added a commit that referenced this pull request Feb 19, 2017
libstd needs update for pending libc change

This  updates libstd to accommodate the fixes made in rust-lang/libc#523

Fixes #39868
@bors
Copy link
Contributor

bors commented Feb 19, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 0128be9 to master...

@bors bors merged commit 68a9d8b into rust-lang:master Feb 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libstd needs update for pending libc change
7 participants