-
Notifications
You must be signed in to change notification settings - Fork 275
Update to windows-sys 0.61 #621
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
base: master
Are you sure you want to change the base?
Conversation
Tokio and mio already updated to windows-sys 0.61, so socket2 requiring 0.60 makes it hard to avoid multiple copies of windows-sys. Since no code changes are needed, I relaxed the version requirement (still allowing 0.60) to avoid increasing socket2's MSRV.
|
FWIW, the MSRV bump would be 1.70 -> 1.71. It seems that CI doesn't exercise the windows + Rust 1.70 combination, but it would give this error: which unfortunately makes the wrong suggestion, the fix is to run |
|
We're not going to increase the MSRV in v0.6. I also don't see a new (v0.7/v1) release in the near future as we just released v0.6 and I don't want to create churn for windows-sys. We though we finally fixed all the windows-sys related churn, but not yet :( |
|
This PR doesn't increase MSRV, though. It would make it more annoying to use socket2 on Rust 1.70, since cargo's MSRV-aware resolver doesn't always generate a lockfile that works, but using 1.70 would remain entirely possible. |
|
FWIW, many other popular crates with MSRV < 1.71 also depend on windows-sys with multi-version ranges that allow but don't require windows-sys 0.61. Among the top 20 reverse dependencies:
(disclaimer: the anstyle-query version range was merged and released just yesterday, on my request) So what this PR is proposing is a common practice, and anyone who needs MSRV <= 1.70 for Windows builds is probably already used to dealing with the |
|
I think this would be nice to have -- I've definitely used ranges for windows-sys in other crates. |
|
Just ran into this, it seems kind of silly for socket2 to continue using a version that it seems the rest of the ecosystem has moved on from if not abandoned. Hoping that this gets merged and pushed out soon so I can get rid of a pointless duplicate dependency. |
|
To me it seems silly to bump our MSVR as a breaking change in v0.6.x or release v0.7 creating a bunch of churn in the ecosystem 🤷 |
But it wouldn't bump the MSRV, at least assuming this is correct? |
Apart from the fact that this PR should not bump the MSRV for most people (by way of using a range), (1) bumping the MSRV in an otherwise non-breaking version bump is pretty common in the crates.io ecosystem, (2) even if you did an MSRV bump, given that tokio is already at 1.71 MSRV I can't imagine it will actually cause substantial churn. |
|
This PR doesn't increase socket2's MSRV for anyone. It remains possible to use socket2 with Rust 1.70 by selecting a windows-sys version (0.62.2) that is compatible both with Rust 1.70 and "socket2 if this PR was merged". I'm confident asserting this because I tried it myself when I created the PR (see first comment). If maintainers would like more assurance about this, I can try to add a CI job that compiles socket2 on windows with Rust 1.70 (probably in a separate PR). Admittedly, it's possible that some people who want to build socket2 with Rust 1.70 on windows may see build failures they didn't see before. This can happen if they haven't committed a lockfile or if they regenerate the lockfile, and Cargo eagerly chooses the newest version of windows-sys allowed by the version bound in socket2. In those situations, the fix is to manually downgrade the windows-sys version. This is mildly annoying, but it's fixable, not an MSRV bump. If there's anything I can do to help mitigate this, or to otherwise make this PR more acceptable, please let me know. NB: needing manual intervention to downgrade transitive dependencies for MSRV is nothing unusual when maintaining a relatively old MSRV. As @djc mentioned, most libraries out there do not consider MSRV bumps semver-breaking (precisely to avoid churn!) and e.g. anyone using socket2 via tokio on Rust 1.70 must already be pinning tokio to an older version. In the case of windows-sys it just so happened that the change that required a MSRV bump was also semver-breaking (probably not in practice, but in theory yes, and windows-sys does not try very hard to avoid semver-breaking version bumps). In contrast, the other dependency that socket2 has (libc) has previously bumped MSRV in patch releases and may do that again in the future. So whatever MSRV policy socket2 follows, I don't think it can (or should try to) completely shield its users from the "need to pin a transitive dependency broke for the MSRV build" phenomenon. |
|
At the very least, if we do this, we need CI jobs to check windows with each major version of windows-sys in the range. |
Tokio and mio already updated to windows-sys 0.61, so socket2 requiring 0.60 makes it hard to avoid multiple copies of windows-sys. In addition, 0.61 finally drops the dependency on large pre-built import libraries (from
windows-targets) in favor of Rust's raw-dylib feature.Since no code changes are needed, I relaxed the version requirement (still allowing 0.60) to avoid increasing socket2's MSRV.