Skip to content

libstd needs update for pending libc change - #39871

Merged
bors merged 3 commits into
rust-lang:masterfrom
binarycrusader:issue-39868
Feb 19, 2017
Merged

libstd needs update for pending libc change#39871
bors merged 3 commits into
rust-lang:masterfrom
binarycrusader:issue-39868

Conversation

@binarycrusader

@binarycrusaderbinarycrusader commented Feb 16, 2017

Copy link
Copy Markdown
Contributor

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

Fixes#39868

@rust-highfive

rust-highfive commented Feb 16, 2017

Copy link
Copy Markdown
Contributor

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

binarycrusader commented Feb 16, 2017

Copy link
Copy Markdown
ContributorAuthor

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

alexcrichton
alexcrichton reviewed Feb 16, 2017
View reviewed changes
Comment threadsrc/libstd/sys/unix/os.rs Outdated
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,

@alexcrichtonalexcrichtonFeb 16, 2017

Copy link
Copy Markdown
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?

@binarycrusaderbinarycrusaderFeb 16, 2017

Copy link
Copy Markdown
ContributorAuthor

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

binarycrusader commented Feb 16, 2017

Copy link
Copy Markdown
ContributorAuthor

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

@alexcrichton

alexcrichton commented Feb 16, 2017

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented Feb 16, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 5789539 has been approved by alexcrichton

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2017
@GuillaumeGomez
Rollup merge ofrust-lang#39871- binarycrusader:issue-39868, r=alexc…
892d896
…richton
libstd needs update for pending libc change
This updates libstd to accommodate the fixes made in rust-lang/libc#523Fixesrust-lang#39868
@GuillaumeGomezGuillaumeGomez mentioned this pull request Feb 16, 2017
Closed
@est31

est31 commented Feb 16, 2017

Copy link
Copy Markdown
Member

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

@binarycrusader

binarycrusader commented Feb 16, 2017•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@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

alexcrichton commented Feb 16, 2017

Copy link
Copy Markdown
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

alexcrichton commented Feb 16, 2017

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented Feb 16, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 68a9d8b has been approved by alexcrichton

@est31

est31 commented Feb 16, 2017

Copy link
Copy Markdown
Member

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
@frewsxcv
Rollup merge ofrust-lang#39871- binarycrusader:issue-39868, r=alexc…
b556222
…richton
libstd needs update for pending libc change
This updates libstd to accommodate the fixes made in rust-lang/libc#523Fixesrust-lang#39868
@frewsxcvfrewsxcv mentioned this pull request Feb 17, 2017
Closed
@binarycrusader

binarycrusader commented Feb 17, 2017

Copy link
Copy Markdown
ContributorAuthor

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

@binarycrusaderbinarycrusader mentioned this pull request Feb 17, 2017
Merged
@GuillaumeGomezGuillaumeGomez mentioned this pull request Feb 17, 2017
Closed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 17, 2017
@GuillaumeGomez
Rollup merge ofrust-lang#39871- binarycrusader:issue-39868, r=alexc…
295d15d
…richton
libstd needs update for pending libc change
This updates libstd to accommodate the fixes made in rust-lang/libc#523Fixesrust-lang#39868
@bors

bors commented Feb 18, 2017

Copy link
Copy Markdown
Collaborator

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

@bors

bors commented Feb 18, 2017

Copy link
Copy Markdown
Collaborator

💔 Test failed - status-travis

@binarycrusader

binarycrusader commented Feb 19, 2017

Copy link
Copy Markdown
ContributorAuthor

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

@petrochenkov

petrochenkov commented Feb 19, 2017

Copy link
Copy Markdown
Contributor

@bors retry

@bors

bors commented Feb 19, 2017

Copy link
Copy Markdown
Collaborator

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

bors added a commit that referenced this pull request Feb 19, 2017
@bors
Auto merge of#39871- binarycrusader:issue-39868, r=alexcrichton
0128be9
libstd needs update for pending libc change
This updates libstd to accommodate the fixes made in rust-lang/libc#523Fixes#39868
@bors

bors commented Feb 19, 2017

Copy link
Copy Markdown
Collaborator

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

@bors
bors merged commit 68a9d8b into rust-lang:masterFeb 19, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexcrichtonalexcrichtonalexcrichton left review comments

Assignees

@brsonbrson

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

libstd needs update for pending libc change

7 participants

@binarycrusader@rust-highfive@alexcrichton@bors@est31@petrochenkov@brson