Bumping `windows-sys` is a Semver breaking change

[abonander]

• socket2 0.5.5 depends on windows-sys 0.48
• socket2 0.5.6, 0.5.7 depend on windows-sys 0.52

This is a breaking change because windows_sys::Win32::Networking::WinSock::SOCKADDR_STORAGE is reexported as the sockaddr_storage type, making it a part of the public API: https://github.com/rust-lang/socket2/blob/master/src/sys/windows.rs#L63

This can lead to build failures from a seemingly innocuous cargo update:

error[E0308]: mismatched types
    --> ffi\src\external_net.rs:44:24
     |
44   |     sockaddr_out.write(sockaddr.as_storage());
     |                  ----- ^^^^^^^^^^^^^^^^^^^^^ expected `SOCKADDR_STORAGE`, found a different `SOCKADDR_STORAGE`
     |                  |
     |                  arguments to this method are incorrect
     |
     = note: `SOCKADDR_STORAGE` and `SOCKADDR_STORAGE` have similar names, but are actually distinct types
note: `SOCKADDR_STORAGE` is defined in crate `windows_sys`
    --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\windows-sys-0.52.0\src\Windows\Win32\Networking\WinSock\mod.rs:5109:1
     |
5109 | pub struct SOCKADDR_STORAGE {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: `SOCKADDR_STORAGE` is defined in crate `windows_sys`
    --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\windows-sys-0.48.0\src\Windows\Win32\Networking\WinSock\mod.rs:6988:1
     |
6988 | pub struct SOCKADDR_STORAGE {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: method defined here
    --> /rustc/051478957371ee0084a7c0913941d2a8c4757bb9\library\core\src\ptr\mut_ptr.rs:1586:25

0.5.6 should have been released as 0.6.0.

Resolution

• If this reexport is semver-exempt, it needs to be clarified in the documentation. Users need to be advised to pin the socket2 version.
• If this reexport is not semver-exempt, the windows-sys dependency should be marked with a warning in Cargo.toml not to bump it in a patch release.

We've already worked around the breakage, so I don't really care what happens to the released version.

[Thomasdezeeuw]

Yes, this is totally correct.

We have a problem where we want to be able to extract the underlying raw/C types, but that means we depend on external crates in our API. For libc this is mostly not a problem because it's stable-ish -- the update to v1 will have this problem though. Windows-sys is worse because they release way too many version for such a low-level crate.

We can drop all types from external types from our API, but we want to keep the ability to get the underlying raw/C types. This problem will have to be solved. I don't have a solution for now.

[faern]

Any update on this? Has more discussion been had and is there any proposed fix? This issue currently makes it hard for any user of socket2::SockAddr to upgrade windows-sys to 0.59.

[Thomasdezeeuw]

No solution yet, not any further discussion either. So let's start one.

Socket2 aims to expose some of the underlying types so the raw types can be used if socket2 doesn't have a proper const/function/etc. for it yet. That said, we do support a lot of functionality now so maybe this goals isn't worth it anymore.

If we decide that the goal isn't worth it we can close the API a bit more and don't depend on libc/windows-sys types in the public API. Although for SockAddr specifically I'm not sure what the best option is. Maybe an untyped pointer? *const sockaddr, it's effectively dynamically typed already...

Also, if windows-sys could stop release breaking changes every other week that would be great /rant (admittedly this annoys me more than it should)

[Darksonn]

How many types is this a problem for?

[Thomasdezeeuw]

How many types is this a problem for?

Quite a few. For example Type, Domain, etc. implement From<sys::c_int>. I don't expect these to change, but still.

The main problem is SockAddr, which uses a lot of OS specific types.

[faern]

Even if it's ugly I think that the opaque pointer solution might be the least bad solution. Then socket2 becomes usable with any type of Rust binding to the SOCKADDR_STORAGE type (any windows-sys version or even winapi).

The same problem should not be there for sys::c_int since it does not depend on windows-sys:

socket2/src/sys/windows.rs

Line 38 in 56e625e

pub(crate) type c_int = std::os::raw::c_int;

[ChrisDenton]

Also, if windows-sys could stop release breaking changes every other week that would be great /rant (admittedly this annoys me more than it should)

windows-sys currently releases once every ~6 months if there's demand for it. It's generated from win32metadata so it's hard to avoid it be a breaking change until that stabilizes. An alternative may be to use windows-bindgen to generate the types instead of using windows-sys.

[Thomasdezeeuw]

windows-sys currently releases once every ~6 months if there's demand for it. It's generated from win32metadata so it's hard to avoid it be a breaking change until that stabilizes.

Aren't the Windows types at ABI level stable though? I don't develop for Windows myself, but one of the good things I hear about it that Windows takes backwards compatibility very serious. So it's quite annoying for such a low level crate to have breaking change that often. And yes, I think that every 6 months is often if you compare it to e.g. the libc crate which has been on the same 0.2.x version since 9 years, i.e. pretty much all of Rust v1.

Don't get me wrong though, I do appreciate the work being put into it and I fully understand what a v0.x version means :)

An alternative may be to use windows-bindgen to generate the types instead of using windows-sys.

I don't really want to maintain these types myself, personally I think window-sys/libc are the correct places for these types.

[ChrisDenton]

Aren't the Windows types at ABI level stable though?

Yes, but that doesn't mean the metadata is fully accurate in all cases. And a fix anywhere can be semver breaking for the whole crate even if most things stay the same. Note that the Windows API is large. It's somewhat comparable to if you said every library and API that ships with Ubuntu is "the Ubuntu API" and put that in a crate. There are some smaller crates now that focus on a specific area, like windows-registry.

I don't really want to maintain these types myself, personally I think window-sys/libc are the correct places for these types.

Sure. Though I don't think the maintenance cost of windows-bindgen is particular high. E.g. chrono uses it (see here and here).

[Darksonn]

Can we just use void pointers? We do that in Tokio for the few functions we have, e.g. open_with_security_attributes_raw.

[Thomasdezeeuw]

I agree that void pointers seems the only way to solved this at the moment.

[pronebird]

It would have been sensible to release 0.6 with windows-sys 0.59 to unblock the folks from being able to upgrade.