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

unix: Add openpty(3) and forkpty(3) for non-Apple platforms #246

Merged
merged 1 commit into from
Apr 3, 2016

Conversation

kamalmarhubi
Copy link
Contributor

The functions were added for Apple in #202. Adding them to other
platforms was pending an amendment to RFC 1291 to expand the scope of
libc to include libutil. The amendment was merged as
rust-lang/rfcs#1529

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

Thanks! Can you add the link-to-libutil in the same place that we link to libc? That way we won't double-link it as the standard library will already be linking to it.

@kamalmarhubi
Copy link
Contributor Author

Is f0d652f what you meant?

I'll squash before merge.

@kamalmarhubi
Copy link
Contributor Author

Is f0d652f what you meant?

Evidently not quite; the linkers are not happy.

@kamalmarhubi
Copy link
Contributor Author

Or maybe the issue is that CI should somehow not build with stdbuild?

@alexcrichton
Copy link
Member

Hm right, we're adding a new library so the libstd libc doesn't link these just yet. I don't believe there's any harm for linking these libraries twice, so yeah I think that we'll need to add link directives in the libc crate itself for them for now.

It's ok to not extract out the functions into a separate extern block, let's just attach the #[link] directives to a one-line extern{} statement for now (or on top of the already-large list of functions)

@kamalmarhubi kamalmarhubi force-pushed the openpty branch 2 times, most recently from b5234a4 to 8996e12 Compare April 1, 2016 19:58
@kamalmarhubi
Copy link
Contributor Author

Fixed (I think), and squash / rebased. :-)

@alexcrichton
Copy link
Member

@bors: r+ 8996e12

@bors
Copy link
Contributor

bors commented Apr 1, 2016

⌛ Testing commit 8996e12 with merge 2fdd92e...

bors added a commit that referenced this pull request Apr 1, 2016
unix: Add openpty(3) and forkpty(3) for non-Apple platforms

The functions were added for Apple in #202. Adding them to other
platforms was pending an amendment to RFC 1291 to expand the scope of
libc to include libutil. The amendment was merged as
  rust-lang/rfcs#1529
@bors
Copy link
Contributor

bors commented Apr 1, 2016

💔 Test failed - travis

The functions were added for Apple in rust-lang#202. Adding them to other
platforms was pending an amendment to RFC 1291 to expand the scope of
libc to include libutil. The amendment was merged as
  rust-lang/rfcs#1529
@kamalmarhubi
Copy link
Contributor Author

Missed the link attribute on freebsd.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 2, 2016

📌 Commit 9c4af10 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Apr 2, 2016

⌛ Testing commit 9c4af10 with merge f288e18...

bors added a commit that referenced this pull request Apr 2, 2016
unix: Add openpty(3) and forkpty(3) for non-Apple platforms

The functions were added for Apple in #202. Adding them to other
platforms was pending an amendment to RFC 1291 to expand the scope of
libc to include libutil. The amendment was merged as
  rust-lang/rfcs#1529
@bors
Copy link
Contributor

bors commented Apr 3, 2016

☀️ Test successful - status-appveyor, travis

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.

None yet

4 participants