Skip to content

Add cfgs (everywhere) in liblibc for NaCl targets. #21581

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 24, 2015
Merged

Add cfgs (everywhere) in liblibc for NaCl targets. #21581

merged 1 commit into from
Feb 24, 2015

Conversation

DiamondLovesYou
Copy link
Contributor

This does not allow Rust proper to target NaCl; this just adds support for NaCl
crosses to liblibc on crates.io.

@rust-highfive
Copy link
Contributor

r? @pcwalton

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

@DiamondLovesYou
Copy link
Contributor Author

This is preventing some of my projects from building.

r? @alexcrichton

@alexcrichton
Copy link
Member

I'd like to start trying hard to cut down on the explosion of #[cfg] in liblibc as it really does seem to be getting out of hand. To that end, could you make a few modifications?

  • The top-level pub use can all go back to pub use foo::* now that globs are stable.
  • Each sub-block should avoid one-off #[cfg] (this PR adds a lot), if that happens then I think it should be separated to a whole new block.

@DiamondLovesYou DiamondLovesYou force-pushed the nacl-libc branch 2 times, most recently from e77f637 to 2136e63 Compare February 16, 2015 18:27
@DiamondLovesYou
Copy link
Contributor Author

@alexcrichton Done!

alexcrichton
alexcrichton reviewed Feb 16, 2015
View reviewed changes
src/liblibc/lib.rs
@@ -3152,7 +3016,6 @@ pub mod consts {
pub const F_SETFL : c_int = 4;

pub const SIGTRAP : c_int = 5;
pub const SIGPIPE: c_int = 13;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this module not be changing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SIGPIPE is exported in posix88, conflicting with this (and the others).

@DiamondLovesYou
Copy link
Contributor Author

@alexcrichton I've addressed your comments.

@alexcrichton
Copy link
Member

@bors: r+ c70e4ce

Thanks!

@bors
Copy link
Collaborator

bors commented Feb 17, 2015

⌛ Testing commit c70e4ce with merge 439f290...

@bors
Copy link
Collaborator

bors commented Feb 17, 2015

💔 Test failed - auto-win-32-nopt-t

@DiamondLovesYou
Copy link
Contributor Author

@alexcrichton I've issued a possible fix (no local win machine available to test).

@alexcrichton
Copy link
Member

Thanks! Could you squash the commits down together as well?

@DiamondLovesYou
Copy link
Contributor Author

@alexcrichton Done!

@alexcrichton
Copy link
Member

@bors: r+ a8004e3

@bors
Copy link
Collaborator

bors commented Feb 17, 2015

⌛ Testing commit a8004e3 with merge 9abe865...

@bors
Copy link
Collaborator

bors commented Feb 17, 2015

💔 Test failed - auto-win-32-opt

@DiamondLovesYou
Copy link
Contributor Author

@alexcrichton And again.

@bors
Copy link
Collaborator

bors commented Feb 22, 2015

💔 Test failed - auto-mac-64-nopt-t

@Manishearth
Copy link
Member

@bors: retry

(slave lost)

@DiamondLovesYou
Copy link
Contributor Author

I didn't know it was possible to cause so much pain and suffering.

@Manishearth
Copy link
Member

Not your fault, I blame the windows builders.

I'm looking up what the appropriate sacrifice to appease the Windows gods is right now. 😿

@bors
Copy link
Collaborator

bors commented Feb 22, 2015

⌛ Testing commit 949d1ff with merge 1394b43...

@huonw
Copy link
Member

huonw commented Feb 22, 2015

Something weird seems to be going on with #22548, and this PR happened to be caught up. Closing temporarily.

@huonw huonw closed this Feb 22, 2015
@Manishearth
Copy link
Member

(My fault, sorry about this. We'll get your PR through once the rollup passes.)

@huonw huonw reopened this Feb 22, 2015
@huonw
Copy link
Member

huonw commented Feb 22, 2015

@bors retry

@huonw
Copy link
Member

huonw commented Feb 22, 2015

@bors r=alexcrichton 949d

@Manishearth
Copy link
Member

@bors: p=1

Let's get platform specific stuff out of the way first

@bors
Copy link
Collaborator

bors commented Feb 22, 2015

⌛ Testing commit 949d1ff with merge 5ef794a...

@bors
Copy link
Collaborator

bors commented Feb 22, 2015

💔 Test failed - auto-mac-32-opt

@alexcrichton
Copy link
Member

@bors: retry

bors added a commit that referenced this pull request Feb 23, 2015
@bors
Auto merge of #21581 - DiamondLovesYou:nacl-libc, r=alexcrichton
3d01f4a 
This does not allow Rust proper to target NaCl; this just adds support for NaCl
crosses to `liblibc` on crates.io.
@bors
Copy link
Collaborator

bors commented Feb 23, 2015

⌛ Testing commit 949d1ff with merge 3d01f4a...

@bors
Copy link
Collaborator

bors commented Feb 23, 2015

💔 Test failed - auto-win-32-nopt-t

@Manishearth
Copy link
Member

@bors: retry

@bors
Copy link
Collaborator

bors commented Feb 24, 2015

⌛ Testing commit 949d1ff with merge 8cd30db...

@bors
Copy link
Collaborator

bors commented Feb 24, 2015

⌛ Testing commit 949d1ff with merge 0ef56da...

bors added a commit that referenced this pull request Feb 24, 2015
@bors
Auto merge of #21581 - DiamondLovesYou:nacl-libc, r=alexcrichton
0ef56da 
This does not allow Rust proper to target NaCl; this just adds support for NaCl
crosses to `liblibc` on crates.io.
@bors
Copy link
Collaborator

bors commented Feb 24, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

@bors bors merged commit 949d1ff into rust-lang:master Feb 24, 2015
@Manishearth
Copy link
Member

Yes!

@DiamondLovesYou
Copy link
Contributor Author

\o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@DiamondLovesYou @rust-highfive @alexcrichton @bors @Manishearth @huonw @pcwalton