Fix netbsd kevent for breakage introduced by libc #790
Conversation
Susurrus
force-pushed
the
fix_netbsd_kevent
branch
2 times, most recently
from
f167c8e to
f100d4e
Compare
|
So to be clear, the problem is that libc switched kevent.filter from i32 to u32? In addition to fixing Nix for 0.10.0, should we release a 0.9.1 that contains a maximum allowed version of libc in Cargo.toml to prevent people from running into this problem? |
|
Probably. What we should definitely do during a release is to use a locked version of libc instead of specifying a minimum version. Because their backwards compatibility guarantees are different from ours. |
Susurrus
force-pushed
the
fix_netbsd_kevent
branch
from
f100d4e to
dc130d2
Compare
|
Supposedly libc always guarantees backwards compatibility. In the past, they've even refused to let me fix buggy definitions because it would break backwards compatibility. This kevent business is the first time I've seen otherwise. |
|
@asomers They only guarantee backwards compatibility for certain targets (Tier 1 I think), not all targets, since that doesn't match our target list, we have this issue. |
src/sys/event.rs
Outdated
| libc_enum!{ | ||
| #[cfg_attr(target_os = "netbsd", repr(u32))] | ||
| #[cfg_attr(not(target_os = "netbsd"), repr(i16))] | ||
| #[derive(Clone, Copy, Debug, PartialEq)] |
There was a problem hiding this comment.
Looks like the build is breaking because all of these derived things are also derived by libc_enum!.
There was a problem hiding this comment.
Yes, WIP since I don't know how to crosscompile to these platforms from Linux.
There was a problem hiding this comment.
Cross compiling is blissfully easy. It's the best thing about FFI. If you use rustup, you can do something like rustup target add i686-unknown-freebsd then cargo build --target=i686-unknown-freebsd.
src/sys/event.rs
Outdated
| EVFILT_VM = libc::EVFILT_VM, | ||
| EVFILT_VNODE = libc::EVFILT_VNODE, | ||
| EVFILT_WRITE = libc::EVFILT_WRITE, | ||
| cfg_if!{ |
There was a problem hiding this comment.
Why use a cfg_if! instead of just plain #[cfg()] like the old code did?
There was a problem hiding this comment.
Remnant of my first revision. This was supposed to make things clearer, but can you can't put the cfg_attr(..., repr(..)) inside it, so it's less useful.
Susurrus
force-pushed
the
fix_netbsd_kevent
branch
from
dc130d2 to
3fc70cb
Compare
Susurrus
changed the title
The datatype for kevent.filter is u32 on NetBSD and i16 on all other supported platforms. This was recently fixed in upstream libc, breaking this API, so this fixes it. This change also modernizes the code a bit to unify the EventFilter datatype across platforms and switch to the libc_enum! macro.
Susurrus
force-pushed
the
fix_netbsd_kevent
branch
from
3fc70cb to
5a7d5b1
Compare
Susurrus
changed the title
|
@asomers I think this should be GTM, please review. |
|
Everything LGTM bors r+ |
No description provided.