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

Upgrade to Bitflags 1.0 #801

Merged
merged 1 commit into from
Dec 3, 2017
Merged

Upgrade to Bitflags 1.0 #801

merged 1 commit into from
Dec 3, 2017

Conversation

Susurrus
Copy link
Contributor

The libc_bitflags! macro was replaced with a non-recursive one supporting
only public structs. I could not figure out how to make the old macro work
with the upgrade, so I reworked part of the bitflags! macro directly to suit
our needs, much as the original recursive macro was made. There are no uses
of this macro for non-public structs, so this is not a problem for internal code.

Closes #766.

@Susurrus
Copy link
Contributor Author

This should probably also include a CHANGELOG entry since it does change how constants are scoped.

@Susurrus
Copy link
Contributor Author

This makes Rust 1.20 our base requirement. I've updated Travis, but the buildbots will need to be updated separately for this to pass there.

On the 23rd Rust 1.22 will be released, so once that's done requiring this newer version of Rust will fall in line with our N-2 minimum supported Rust standard.

@Susurrus Susurrus force-pushed the bitflags_1_0 branch 5 times, most recently from e93e17c to 59858fc Compare November 23, 2017 02:11
@Susurrus
Copy link
Contributor Author

@asomers With this change we're now seeing epoll failures during testing as the only error (besides your FreeBSD buildbots not being Rust 1.20). Any idea what could be causing this?

@asomers
Copy link
Member

asomers commented Nov 27, 2017

@Susurrus this is due to the Rust upgrade. I saw this months ago in the Beta channel. At first I suspected a code generation bug in Rust itself, but now I'm not so sure. It could be that something in nix's epoll code is overrunning a buffer or something. I never did get to the bottom of it. But this patch makes the problem a little more obvious.
https://gist.github.com/asomers/198774d364042e85ffcbdb7eaeaa256f

@Susurrus Susurrus mentioned this pull request Dec 1, 2017
@Susurrus
Copy link
Contributor Author

Susurrus commented Dec 1, 2017

Hmmm, maybe we should test on stable in CI instead of beta. I didn't realize we weren't doing that at all anymore.

@Susurrus
Copy link
Contributor Author

Susurrus commented Dec 1, 2017

Blocking on #805.

@Susurrus
Copy link
Contributor Author

Susurrus commented Dec 2, 2017

With #805 merged, this is just waiting on an update of the FreeBSD buildbots. @asomers when would you be able to update those?

The libc_bitflags! macro was replaced with a non-recursive one supporting
only public structs. I could not figure out how to make the old macro work
with the upgrade, so I reworked part of the bitflags! macro directly to suit
our needs, much as the original recursive macro was made. There are no uses
of this macro for non-public structs, so this is not a problem for internal code.
@asomers
Copy link
Member

asomers commented Dec 2, 2017

@Susurrus The amd64 buildbot should be good to go. But I'm out of space on my VM :( so it will take a little while longer to upgrade the i386 buildbot.

@Susurrus
Copy link
Contributor Author

Susurrus commented Dec 2, 2017

@asomers Great, thanks! I've restarted this on the amd64 one and we'll see what happens. As for the i386 I guess we'll need to block on that for this to do things "the right way". What would be a rough ETA on that?

@Susurrus
Copy link
Contributor Author

Susurrus commented Dec 2, 2017

Bam! It worked, so I'm betting i386 will work as well.

The downside to upgrading the buildbot outside of this PR is now other PRs will be testing on 1.20 there. So I'd love to get this merged ASAP to keep things in phase.

@asomers
Copy link
Member

asomers commented Dec 3, 2017

The buildbots should all be working now. I've restarted the failed i386 build.

@asomers
Copy link
Member

asomers commented Dec 3, 2017

bors r+

bors bot added a commit that referenced this pull request Dec 3, 2017
801: Upgrade to Bitflags 1.0 r=asomers a=Susurrus

The libc_bitflags! macro was replaced with a non-recursive one supporting
only public structs. I could not figure out how to make the old macro work
with the upgrade, so I reworked part of the bitflags! macro directly to suit
our needs, much as the original recursive macro was made. There are no uses
of this macro for non-public structs, so this is not a problem for internal code.

Closes #766.
@bors
Copy link
Contributor

bors bot commented Dec 3, 2017

@bors bors bot merged commit e1baab9 into nix-rust:master Dec 3, 2017
@Susurrus Susurrus deleted the bitflags_1_0 branch December 3, 2017 04:29
@lucab lucab mentioned this pull request Dec 3, 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.

2 participants