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

Winapi in the public API of mio #658

Open
dtolnay opened this Issue Aug 18, 2017 · 6 comments

Comments

5 participants
@dtolnay
Copy link

dtolnay commented Aug 18, 2017

On windows, the mio::windows::Overlapped::new and as_mut_ptr methods refer to winapi in their signature.

fn new(cb: fn(_: &winapi::minwinbase::OVERLAPPED_ENTRY)) -> Overlapped;
fn as_mut_ptr(&self) -> *mut winapi::minwinbase::OVERLAPPED;

Since winapi is pre-1.0, this makes it risky to stabilize mio. If a future version of winapi were to change these structs in certain ways, mio may be forced into a breaking change to pick them up.

Let's either figure out a way to keep these winapi types out of mio's public API, or lay out a plan for how we would deal with a hypothetical future release of winapi. If the latter, the plan should be clearly documented in comments in the windows module.

@dtolnay

This comment has been minimized.

Copy link
Author

dtolnay commented Aug 23, 2017

From IRC:

<WindowsBunny> winapi 0.3 is coming very soon and you better want to upgrade to it

@carllerche

This comment has been minimized.

Copy link
Owner

carllerche commented Aug 23, 2017

Basically, we should figure out how to remove winapi from all public apis (unless it plans to hit 1.0 soon)

@retep998

This comment has been minimized.

Copy link

retep998 commented Aug 23, 2017

winapi has no plans for 1.0 at the moment due to several critical Rust features still being either unstable or unimplemented.

@carllerche carllerche added this to the v0.7 milestone Nov 7, 2017

@carllerche carllerche removed the help wanted label Nov 7, 2017

@steffengy

This comment has been minimized.

Copy link
Contributor

steffengy commented Mar 23, 2018

@carllerche
This can be closed since the winapi 0.3 update.

Since this still hasn't landed, and we can't really backport the 0.3 migration,
are there news for mio on the 0.7 front?
Any plans to push some open points that won't see progress to another milestone,
insteadof delaying this indefinitely?

@carllerche

This comment has been minimized.

Copy link
Owner

carllerche commented Mar 23, 2018

The main issue right now is time on my part. My current focus is on getting the Tokio stack stable.

I can try to triage the remaining 0.7 issues to see if others can jump on them. The biggest one is going to be to figure out multi thread polling.

Is there urgency on getting a winapi update released? The impact should be minimal and mostly be limited to having multiple versions of the crate compiled.

@steffengy

This comment has been minimized.

Copy link
Contributor

steffengy commented Mar 23, 2018

That's understandable and totally reasonable. In regards to impact there's

  • crate-duplication (which affects most async things, mostly showing in build times)
  • a few mostly internal changes

So nothing that warrants making this a critical priority (so far).

I personally just don't like that it's not remotely foreseeable when this will land
It could be just a few months, but it also could be towards the end of 2018 already.

Currently it doesn't seem realistic to me,
that mio 0.7 will be ready before the next (major stable) tokio release.
So I somehow fear this will lead to stabilizing tokio and then delaying the mio update even further along, since nobody likes to break tokio shortly after it becomes somewhat stable.

So maybe splitting in a 0.7 release (with the points that are implementable short-term) and a 0.8 release might make sense along the way.

Some triaging/documenting of further todos surely is helpful, I'll certainly take a look.

@hawkw hawkw added the windows label Aug 16, 2018

@hawkw hawkw added this to Needs triage in Triage Aug 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.