-
Notifications
You must be signed in to change notification settings - Fork 159
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
Use custom bindings instead of winapi crate. #89
Conversation
What are the I note that it seems like you can just tell winapi not to bundle the libraries (which only happens on the gnu targets anyway): https://github.com/retep998/winapi-rs/blob/0.3/x86_64/build.rs#L10 |
I'm not sure why do we need them, honestly. Looks like some kind of workaround for mingw. And I'm not sure what this flag does, but it doesn't impact |
cc @retep998 - There is some desire to avoid pulling in the While I don't maintain |
@BurntSushi see this comment by @retep998 as well as the motivation section of the raw-dylib RFC for use cases of those crates. |
I don't think any of that really answers my question. Or at least, I am not knowledgeable enough to use those details to extrapolate to ultimate trade offs. For example, it sounds to me like if we just declare the bindings ourselves and drop |
Thanks for the PR and good discussion. I agree with @BurntSushi - it would be great to understand the issue fully before taking on this maintenance burden. Additionally, my gut is that it would be relatively rare for |
@danburkert I'm not sure if you can list all crates on crates.io that depend on a particular crate, so it's hard to say how often But I agree that we should understand why |
@RazrFalcon if you don't need windows anyway then maybe this could help: rust-lang/cargo#7058 |
@est31 No, all platforms are required anyway. |
Therefore In this specific case because you are limiting yourselves strictly to functions included in the bundled subset of MinGW it will work without |
@retep998 Thanks for clarification. Maybe this should be added to the |
This one doesn't depend on winapi. danburkert/memmap-rs#89
My suggestion here is to keep the dependency on |
@BurntSushi I've already made a fork, so it's no longer a problem, at least for me. And no, "a few extra MB of disk space" is very important to me. |
There are two much better alternatives to this:
|
I'm going to be helping out with maintenance for this crate. On balance, I think this PR probably isn't worth it. Thanks though! |
The
winapi
crate requireswinapi-*-pc-windows-gnu
crates, which are pretty heavy (100MB). This blow ups the vendored archives size. Which is important for such a widely used crate.There are only two ways to fix this:
This patch doesn't make this library less safe since
winapi
crate doesn't contain any logic. We simply copy-pastingwinapi
crate definitions.This patch doesn't break comparability/api since it affects only the internal implementation.
The only remaining question is portability. The
winapi
crate does a lot of linking magic, but I'm not sure if we need it at all. UPD since all tests are passed, looks like we don't need it anyway.