Replace miow, winapi, and ntapi with windows-sys #1556
Conversation
This updates miow to 0.4, which now uses the windows-sys crate instead of winapi, as the former is maintained and updated frequently as opposed to winapi. The windows-sys crate also covers more of the Windows API surface, which also allowed me to remove the dependency on ntapi (as it still depends on winapi). There was only a single function, `NtCancelIoFileEx` that was present in ntapi but not windows-sys, so I merely added the extern declaration in the one place it was used as it is not worth bringing in a dependency just for that.
|
I think @Noah-Kennedy was working on this as well. |
|
Might be worth switching |
|
Here's a PR to update |
I think it was @Noah-Kennedy idea to switch directly to use windows-sys/winapi as well, but I'm not 100% sure about that. |
|
I think its probably best to just use either winapi or windows-sys directly and not depend on miow. |
|
I can remove the dependency on miow in this PR if that's what's wanted? I love removing dependencies. |
Sounds good. |
Jake-Shadle
changed the title
|
|
||
| /// Wrapper around a Windows HANDLE so that we close it upon drop in all scenarios | ||
| #[derive(Debug)] | ||
| pub struct Handle(HANDLE); |
There was a problem hiding this comment.
Not a review comment, but future work: we can replace this with std::os::windows::io::OwnedHandle (currently unstable).
| const SIO_BSP_HANDLE: u32 = IOC_OUT | IOC_WS2 | 27; // 1_207_959_579u32 | ||
| const SIO_BSP_HANDLE_SELECT: u32 = IOC_OUT | IOC_WS2 | 28; // 1_207_959_580u32 | ||
| const SIO_BSP_HANDLE_POLL: u32 = IOC_OUT | IOC_WS2 | 29; // 1_207_959_581u32 | ||
| const SIO_BASE_HANDLE: u32 = IOC_OUT | IOC_WS2 | 34; // 1_207_959_586u32 |
There was a problem hiding this comment.
Are these not in windows-sys?
There was a problem hiding this comment.
LGTM, @Noah-Kennedy can you review this as well?
Some future work:
- Move the
SIO_BSP_*constants, Various Winsock constants missing microsoft/win32metadata#844. - Replace
Handlewithstd::os::windows::io::OwnedHandle(once stable). - Remove the feature for
AF_INET(6)constants, seeAF_INETandAF_INET6should not be exclusive toWin32::NetworkManagement::IpHelpermicrosoft/win32metadata#845.
|
One open question is should this be a part of v0.8.x or v0.9.x? We don't use winapi's types publicly, but the rest of the ecosystem might still be using it, having both winapi and windows-sys might become a problem. |
| /// ports and waiting to receive the completion notification on the port. | ||
| pub unsafe fn read_overlapped( | ||
| &self, | ||
| buf: &mut [u8], |
There was a problem hiding this comment.
I've never quite been a fan of how miow did this, as its kinda a footgun. Still, I can't think of anything actively harmful about taking this in as an arg.
|
Thanks @Jake-Shadle! We can do some of the future work lined out in #1556 (review) in a future pr |
|
Created #1558 to keep track of that future work. |
This removes the dependency on
miow,winapi, andntapiin favor of just usingwindows-sysdirectly. A few types/functions frommiowwere copied, and added, namely theHandlewrapper (forCloseHandledropping) and theCompletionPortandCompletionStatustypes.There was only a single function,
NtCancelIoFileExthat was present inntapibut notwindows-sys, so I merely added the extern declaration in the one place it was used as it is not worth bringing in a dependency just for that.Note this uses the latest
window-sysversion, but I think the version restriction can be loosened to>=0.32as (probably) the only actual relevant change for mio was the change ofHANDLEfrom*mut c_voidtoisize. This is unfortunately a downside of thewindows-syscrate, its minor version is bumped fairly frequently, but other than changes such as the one previously mentioned, most of those versions won't actually meaningfully affect mio's usage, and having an exact version requirement could mean duplicate versions of thewindows-syscrates being used in downstream crates if they use a slightly newer version of it that is otherwise fully API compatible. Though of course the safest is to have>=0.32, <=0.34to avoid a minor version > 0.34 actually having breaking changes for mio and downstream crates failing to compile unless they manually pin the version.