inotify: Add AsFd to allow using with epoll (issue #1998) #1999
Conversation
|
Huh I don't understand the CI build failures. It works for me on stable rust (1.67.0). Yet it seems to be running on a much older version since it complains about AsFd not being a thing (supported since 1.63.0, which is the version that adds OwnedFd too, which was already used in that file). |
|
I did a change (where AsFd/BorrowedFd are imported from), but I do not understand why it was needed, as it builds without that change on my system. I would appreciate some explanation on this! It also seems I get other errors building on CI than locally as well. I'm really confused by this. How do I build locally the same was as on CI? |
|
I don't remember the exact version numbers, but one version of Rust stabilized the This PR looks good, but you don't need to bother with a test that short. If a one-line function can't be tested without essentially just restating what that function does, then the test adds no value. |
VorpalBlade
force-pushed
the
feature/epoll-asfd
branch
from
8556276 to
7830042
Compare
Ah, that explains it. A bit of a footgun though in my opinion!
I have removed the test now and squashed the changes into a single commit. Should be ready to merge! |
This resolves issue #1998 and allows
Inotifyto be used byEpollby adding AsFd.I'm not entirely sure about the unit test. Maybe it would be possible to do a more comperhensive check by contructing inotify using
from_raw_fdand checking that I get the same value back. However, that would basically mean duplicatingInotify::newand that feels a bit pointless.Another option would be to create an integration test to combine
InotifyandEpoll.Fixes #1998