[d3d12 wgl] Upgrade to `windows 0.59` crates
Connections Depends on https://github.com/Traverse-Research/gpu-allocator/pull/258 and https://github.com/Xudong-Huang/generator-rs/pull/72.
https://github.com/microsoft/windows-rs/releases/tag/0.61.0
Description
The latest windows 0.59 and windows-core 0.59 crates were just released (strangely tagged 0.61), including some minor code improvements for us. The MSRV has been bumped to 1.74, but wgpu is already on 1.76 anyway.
Testing Not yet, only cross-compiled.
Checklist
- [x] Run
cargo fmt. - [x] Run
taplo format. - [x] Run
cargo clippy. If applicable, add:- [ ]
--target wasm32-unknown-unknown - [ ]
--target wasm32-unknown-emscripten
- [ ]
- [ ] Run
cargo xtask testto run tests. - [ ] Add change to
CHANGELOG.md. See simple instructions inside file. - [ ] Replaced crate patches with actual releases.
Just to set expectations here, we likely can't merge this for a bit, as firefox is currently on 0.58 and migration will be a bit of a thing. @ErichDonGubler will take charge of this.
I don't expect there to be a ton of problems with this PR just hanging out for a while. I'm going to mark this as draft as we can't merge it, but @Wumpf will review it still.
@cwfitzgerald that's all good, I don't expect this to be merged until the upgrade has fully trickled through the ecosystem. Just making sure we have a PR open with all the necessary changes that I couldn't make in the original windows-rs migration due to missing or incorrect upstream annotations.
There are a few things that I'm already aware need to happen in mozilla-central before this can land:
- [ ] Dependencies on
windows- [ ] Bump
gpu-allocatorafter a version releases that consumes https://github.com/Traverse-Research/gpu-allocator/pull/258.- We might need this backported to 0.27.0, since mainline upstream now uses
objc2-metal, which we're blocked on moving to because of #5641 (CC @Jasper-Bekkers, @MarijnS95).
- We might need this backported to 0.27.0, since mainline upstream now uses
- [x]
mtumigrated fromwindows-{bindgen,core}andwindows-targetstowindows>=0.58.0,<0.60.0: https://github.com/mozilla/mtu/pull/65
- [ ] Bump
- [ ] Dependencies on
windows-sys- [x]
mio: https://github.com/tokio-rs/mio/pull/1857 - [ ] Coordinate upgrades with folks working on
neqo: https://github.com/mozilla/neqo/pull/2662 - [x]
audio_thread_priorityneeds migration: https://github.com/mozilla/audio_thread_priority/pull/30 - [x] ~~
tempfilecan have its version range extended: https://github.com/Stebalien/tempfile/pull/319~~ Actually already works, but it has a nitpick issue with its range specification. - [x]
socket2can have its version range extended:- https://github.com/rust-lang/socket2/pull/545
- https://github.com/rust-lang/socket2/pull/579
- [x]
tokioneeds migration: https://github.com/tokio-rs/tokio/pull/7117- [x]
is-terminalcan have its version range extended: https://github.com/sunfishcode/is-terminal/pull/43
- [x]
- [x]
- [ ] Direct dependencies on
windows-targets- [x]
libloadingcan widen its accepted ranges ofwindows-targetsto include 0.53.*: https://github.com/nagisa/rust_libloading/pull/169 - [ ]
backtrace: consume a newlibloadingand update workspace: https://github.com/rust-lang/backtrace-rs/pull/694 - [ ]
parking_lot_core: consume a newbacktraceand update workspace: https://github.com/Amanieu/parking_lot/pull/458
- [x]
Regarding neqo and mtu, we can bump whenever you're ready - just let us know. (CC @mxinden for info).
Regarding
neqoandmtu, we can bump whenever you're ready - just let us know. (CC @mxinden for info).
Under the assumption that windows v0.59 does not introduce a breaking change for neqo or mtu, we can also make neqo and mtu compatible to both windows v0.58 and v0.59 by configuring a range:
windows-sys = { version = ">=0.58, <=0.59"
See https://github.com/quinn-rs/quinn/pull/2021/ as an example from one of our upstream dependencies.
@ErichDonGubler let me know what works best for you.
@mxinden: If a range dependency works for neqo and mtu, then wonderful, let's please do that. 😀 Did you want me to file PRs?
ETA: Filed! See the comment (https://github.com/gfx-rs/wgpu/pull/6876#issuecomment-2581684258) for details.
https://github.com/mozilla/mtu/pull/65 has landed! 🙌🏻
I've updated my dependency index comment to be organized by direct dependency. I've filed the PRs that I'm aware are necessary in the larger Crates.io ecosystem, and am now ready (but not yet trying in earnest) to try compiling mozilla-central with the updated deps.
[ ] Bump
gpu-allocatorafter a version releases that consumes cargo: Bumpwindowscrate range to0.53-0.59Traverse-Research/gpu-allocator#258.
- We might need this backported to 0.27.0, since mainline upstream now uses
objc2-metal, which we're blocked on moving to because of [metal] Useobjc2-metal#5641 (CC @Jasper-Bekkers, @MarijnS95).
@ErichDonGubler I think we can arrange this pretty quickly, let us know when the time is there (and you haven't yet upgraded to objc2-metal by then).
@ErichDonGubler any update on the Mozilla side? looks like a lot of the PRs you referenced are landed by now
Sorry I completely dropped the ball on this and there are quite a few new versions already. I'll get back to the second bump ASAP, as long as Mozilla is okay with the next bumps?
@MarijnS95: The way to keep this tractable is almost certainly to keep uograde steps incremental, so don't feel like you need to throw away efforts to target 0.59 without a strong concrete blocker.
@Wumpf: Some PRs have landed, and some have released, but many of them have not been exposed in a Crates.io release yet. I'm checking those boxes as releases become available on Crates.io, not when the PRs land.
@ErichDonGubler It is expected that the upgrade to 0.61 is incremental on top of 0.59, nothing to be thrown away. But this PR is definitely in need of a rebase and conflict-resolve.
@ErichDonGubler you can cross off mio which released 1.0.4 with the bump just 6 hours ago (tokio also released, but the bump wasn't merged yet).
I'm a little confused by the latest CI error https://github.com/gfx-rs/wgpu/actions/runs/16555966628/job/46817429146?pr=6876:
error: failed to select a version for `libc`.
... required by package `socket2 v0.6.0`
... which satisfies dependency `socket2 = "^0.6.0"` of package `tokio v1.47.0`
... which satisfies dependency `tokio = "^1.47"` of package `cts_runner v26.0.0 (/home/runner/work/wgpu/wgpu/cts_runner)`
versions that meet the requirements `^0.2.172` are: 0.2.174, 0.2.173, 0.2.172
all possible versions conflict with previously selected packages.
previously selected package `libc v0.2.168`
... which satisfies dependency `libc = "^0.2.168"` of package `wgpu-hal v26.0.0 (/home/runner/work/wgpu/wgpu/wgpu-hal)`
... which satisfies path dependency `wgpu-hal` of package `wgpu v26.0.0 (/home/runner/work/wgpu/wgpu/wgpu)`
... which satisfies path dependency `wgpu` of package `wgpu-benchmark v26.0.0 (/home/runner/work/wgpu/wgpu/benches)`
failed to select a version for `libc` which could resolve this conflict
This PR only bumps socket2 from 0.5.10 to 0.6 via tokio 1.47. That 0.5.10 already had a libc 0.2.171 bound, which should similarly be incompatible with that "previously selected libc 0.2.168" for wgpu-hal. However, it seems then the resolver understood it's an open-ended dependency and wgpu-hal can trivially use libc 0.2.172?
The solution is probably simple, just bump our dep to libc = "0.2.172", I just feel like understanding why it didn't need to be libc = "0.2.171" on trunk already.
This is a really weird error that you hit with -Zdirect-minimal-versions. When you have a dependency that has a higher requirement than you do, instead of resolving it to that higher version, it throws an error as it's trying to lock your immediate versions to exactly the versions you need. This may end up being a unnecessary requirement, but would require downgrading a transitive dependency, which -Zdirect-minimal-versions won't do.
Never mind, I was mistaken. While I said that socket2 0.5.10 requires libc 0.2.171 already, tokio 1.64.1 only requires socket2 0.5.5 which requires libc 0.2.149, so the requirement is satisfied when -Zdirect-minimal-versions sets it at libc 0.2.168 for wgpu-hal.
@ErichDonGubler how are we doing here, mozilla side?
@cwfitzgerald: The dependency status index comment is up-to-date, as far as I know. Some upstream PRs have stalled, and now some of the deps. have even started to move on to later windows versions. There's also some complication with some upstream movement to windows-link, instead of windows-sys.
I think we might benefit from retrying with the latest windows crate ecosystem, rather than trying to continue with 0.59.
I think we might benefit from retrying with the latest
windowscrate ecosystem, rather than trying to continue with 0.59.
@ErichDonGubler with that, are you referring to 0.61 from https://github.com/gfx-rs/wgpu/pull/8013 or the latest 0.62 release which barely misses out on range compatibility due to https://github.com/gfx-rs/wgpu/pull/8013#issuecomment-3271792993?
We dropped gpu-allocator 0.28 :)
@MarijnS95:
I think we might benefit from retrying with the latest
windowscrate ecosystem, rather than trying to continue with 0.59.@ErichDonGubler with that, are you referring to
0.61from #8013 or the latest0.62release which barely misses out on range compatibility due to #8013 (comment)?
I'm referring to 0.62, sorry for any confusion!
Much of the ecosystem has caught up with 0.62, with some refusing to use range dependencies. I've also confirmed that we can build it in Firefox, done all the auditing work for new dependencies. The main supply chain task is some upstreaming before review that can be done incrementally per-dependency (see Bug 1991226 - Upgrade/audit most Rust deps. for WGPU revendor and windows 0.62, part 1).
I'll let you judge for yourself, though. I've put a PR for 0.62 up—based on work here—for your perusal: https://github.com/gfx-rs/wgpu/pull/8281
Much of the ecosystem has caught up with 0.62, with some refusing to use range dependencies.
As already highlighted in https://github.com/gfx-rs/wgpu/pull/8013#issuecomment-3271792993 three whole weeks ago wgpu itself cannot take a range dependency because of the Error::from_win32() -> Error::from_thread() rename.
I'll let you judge for yourself, though. I've put a PR for 0.62 up—based on work here—for your perusal: https://github.com/gfx-rs/wgpu/pull/8281
I thought I had also already shared https://github.com/MarijnS95/wgpu/compare/windows-69 with you those three weeks ago, and was merely waiting for confirmation to push it into this PR or #8013.
Note that all these force-pushes to contributor PRs -in particular after such a long time of radio silence- seem a little disingenuous: I cannot even read the CI failure and git rebase --update-refs to amend the fixes into multiple PRs at once, before having git push --force-with-lease tell me that my remote branch changed since checkout? Would be awesome if you could ask next time whether help is needed 🙏
As already highlighted in #8013 (comment) three whole weeks ago
wgpuitself cannot take a range dependency because of theError::from_win32()->Error::from_thread()rename.
Sure, but that only means WGPU specifically suffers from being an all-or-nothing upgrade WRT this windows upgrade. It's still beneficial for WGPU's dependencies to be incrementally upgradeable, because incremental upgrades at Mozilla have a significantly lower risk of getting stuck. If you examine bug 1991226, that's exactly what I'm doing.
When you submitted that comment three weeks ago, this was, unfortunately, not something I could justify looking at (and I'm still not sure that I can drive this to completion, based on Mozilla's current priorities). I'm trying to split a bit of attention to take a look at this now, though, because I think it's important for Mozilla to have concrete reasons for either letting things through or being blocked, rather than "I don't got bandwidth indefinitely, soz."
Note that all these force-pushes to contributor PRs -in particular after such a long time of radio silence- seem a little disingenuous: I cannot even read the CI failure and
git rebase --update-refsto amend the fixes into multiple PRs at once, before havinggit push --force-with-leasetell me that my remote branch changed since checkout? Would be awesome if you could ask next time whether help is needed 🙏
Ah, sorry, I'm not trying to create obstacles for you! I'm happy to switch to appending fixup!s, or giving you diffs via PRs. Force-pushing generally works very well with less dedicated contributions, but you've been quite dedicated. ❤️ Happy to accommodate a better workflow here!
I thought I had also already shared https://github.com/MarijnS95/wgpu/compare/windows-69 with you those three weeks ago, and was merely waiting for confirmation to push it into this PR or https://github.com/gfx-rs/wgpu/pull/8013.
I don't remember this being shared, but I think it's safe to say we've reached consensus for trying to target windows 0.62. I'm fine dismissing #8281, if you intend to reconcile it with #8013.
Sure thing, having downstream dependencies commit to ranges (when possible) would be awesome and I've been trying to sneak them into crates whenever possible. Thus happen to contribute to quite a few too many, and it usually means preempting the maintainer or other contributors who are oblivious to this behavior (and getting the range extended backwards has thus far proven harder).
Hadn't looked at the Bugzilla tracker but it looks like a lot of crates to repeatedly vet; any way that could be reduced by less aggressively bumping patch versions where not necessary for e.g. this Windows upgrade?
Glad to see that you're at least trying to get something going here despite not finding time/priority. Apologies for the hasty response. Makes me fear the ever-increasing stack of PRs, issues and (mostly) complaints on popular repos that I'm supposed to be maintaining, ... 🤷
I'll happily keep #8013 up to date and rebased on this PR, and leave it to you whether to temporarily jump to 0.59 via this PR or go straight to 0.62. Since most context lives here, I can merge it into this PR too.
I'd like to address this tension specifically, since I consider cultural gardening an important exercise:
Note that all these force-pushes to contributor PRs -in particular after such a long time of radio silence- seem a little disingenuous: ...
As I'm sure you know, being radio-silent generally isn't a problem for one-off contributions. This case is already different; IMO, you've definitely earned some negotiating power on how you'd like to communicate, given how much (welcome) work you've been doing. Thoughts? I'm guessing you would have preferred that we told you specifically that we knew we didn't have immediate bandwidth for windows upgrades. Alternatively, we don't mind getting pinged in issues like this if and when we do fail to communicate, though responsiveness can vary between Mozillians and from week to week. Sometimes, we have week-or-longer interruptions (like the recent WebGPU F2F, or Mozilla all-company meetings).
☝🏻 CC @gfx-rs/wgpu.