Use custom bindings instead of winapi crate.

[RazrFalcon]

The winapi crate requires winapi-*-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:

• Wait for Tracking issue for RFC 2627: #[link(kind="raw-dylib")] rust-lang/rust#58713, but looks like it will not be fixed anytime soon.
• Use custom bindings (we need like 5 methods anyway).

This patch doesn't make this library less safe since winapi crate doesn't contain any logic. We simply copy-pasting winapi 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.

[BurntSushi]

What are the winapi-*-pc-windows-gnu crates providing, and why don't we need them here?

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

[RazrFalcon]

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 cargo build and cargo vendor. winapi-*-pc-windows-gnu will still be downloaded and built, but not linked.

[BurntSushi]

cc @retep998 - There is some desire to avoid pulling in the winapi-*-pc-windows-gnu crates, by dropping the winapi dependency and inlining the bindings. What downsides does this have?

While I don't maintain memmap, I do maintain several crates that do use winapi, so I'd like to understand this issue more fully.

[est31]

@BurntSushi see this comment by @retep998 as well as the motivation section of the raw-dylib RFC for use cases of those crates.

[BurntSushi]

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 winapi completely (like this PR does), then it sounds like something on the gnu targets will break. The extent and nature of that breakage isn't clear to me.

[danburkert]

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 memmap to be the only crate depending on winapi in any given applications dependency hierarchy, so fixing it here isn't very high impact.

[RazrFalcon]

@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 memmap is used with winapi. Personally, I need this for my fairly large crate, and this will be the only dependency that uses winapi. And it's pretty important, since it reduces the vendored archive size in half (7MB -> 3.5MB). And in 95% of the cases this archive will be used by linux distros that doesn't care about winapi at all.

But I agree that we should understand why winapi uses those libraries at all and how this will impact memmap.

[est31]

@RazrFalcon if you don't need windows anyway then maybe this could help: rust-lang/cargo#7058

[RazrFalcon]

@est31 No, all platforms are required anyway.

[retep998]

• The pc-windows-gnu targets come with a bundled subset of MinGW which means I cannot assume a full MinGW toolchain exists. Many functions come from system import libraries not included in the bundled subset of MinGW.
• Even when the MinGW import library I need exists, it is usually woefully out of date and does not contain newer functions.

Therefore winapi has target specific dependencies which bundle import libraries that are up to date. I cannot remove them as dependencies of winapi without breaking very real use cases where people try to build something using the pc-windows-gnu toolchain and get linker errors because a library doesn't exist or a symbol is unresolved. Until the raw-dylib feature is implemented and stabilized, there is nothing that winapi can do. I already try to minimize the impact as much as possible by making them target specific.

In this specific case because you are limiting yourselves strictly to functions included in the bundled subset of MinGW it will work without winapi's bundled import libraries. Also because you are only using system libraries already linked to by std you don't need to do any linking magic.

[RazrFalcon]

@retep998 Thanks for clarification. Maybe this should be added to the winapi readme?

[BurntSushi]

My suggestion here is to keep the dependency on winapi. It sounds like the stated downside here is a few extra MB of disk space. IMO, this is not worth it. In part because of @danburkert's point where winapi is a very common dependency. It's hard to build a Rust application with Windows support without it. IMO, a few extra MBs of disk space is not an urgent issue that should be fixed by the crate ecosystem. Instead, the tooling should find a way to fix this.

[RazrFalcon]

@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.

[est31]

There are two much better alternatives to this:

• The best alternative would be to implement the raw-dylib RFC. Tracking issue for RFC 2627: #[link(kind="raw-dylib")] rust-lang/rust#58713
• The second best way would be to find out whether the .lib files could be preprocessed to be smaller. winapi/rustc only needs a small fraction of the info contained in them, so figuring out how to remove the redundant parts can likely achieve great gains in size. It's the second best way because once the raw-dylib RFC is being implemented, it won't be needed any more.

[BurntSushi]

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!