Closed kennykerr closed 1 year ago
I would somewhat prefer we keep these bindings the way they are, given that this is included in the stdlib. It seems somewhat risky to switch to windows-sys, which has a fairly idiosyncratic way of linking (not saying it's bad, but I'm not sure it's something we want to do for effectively all rust users...)
Going to close this for now - will be back! 😉
It seems somewhat risky to switch to windows-sys, which has a fairly idiosyncratic way of linking (not saying it's bad, but I'm not sure it's something we want to do for effectively all rust users
I'd love to understand what you mean by this. What's idiosyncratic about the way it links? winapi
also provides libs, just not consistently. The windows-sys
crate just provides uniform libs for a range of compilers and targets:
Yeah, it's definitely possible I'm being a bit paranoid and it would be fine. I would like us to be sure that that's the case before we merge this though, and do it with the understanding that this cause those to be linked into every Rust program.
For example, in the past I had issues in cases where symbols were provided both by windows-sys
and another library. In my case the bindings from libsqlite3.dll
were overwritten by ones from windows-sys, due to them also appearing in winsqlite3.dll
. This has been fixed, but still, providing such comprehensive bindings worries me a bit, due to the risk for things like this.
Btw, incorporating windows-sys into stdlib proper is very much on my radar. I think the implementation of the raw-dylib feature would be a good point to start doing this.
Maintaining separate bindings is error prone and, imho, wasted effort so it'd be great if there's only one source of truth.
Thanks for the feedback!
I was just chatting with @wesleywiser and he suggested I take a stab at porting backtrace-rs
to windows-sys
. This was mostly an experiment - I meant to open a PR on my fork and do some further validation first. So no pressure either way. 😊
I will say the libs have come a long way since those early days. The libs are now quite mature and specifically avoid overlaps with non-Windows APIs like SQLite. I would say they're now also more robust and correct than the libs that you get via winapi
and the Windows SDK. They've had about 12 months to bake and shake out any issues thanks to early adoption by tokio
and parking_lot
but I do appreciate your concern. Something to think about for sure.
I'd love to have all these manual bindings deleted at some point as well, it's of course good to centralize these sorts of definitions in a vetted location like windows-sys
.
That being said the hardest part of this transition is probably going to be how backtrace
-the-crate is integrated into the standard library. This crate can't easily switch to windows-sys
without that being supported. I'd recommend getting that solved first, effectively deleting library/std/src/sys/windows/c.rs
, and then once that's migrated this should be able to trivially follow.
Sounds good - let me know if I can help in any way.
Oops, didn't mean to submit this just yet. 😊
Anyway, this is a simple port of the
backtrace
crate fromwinapi
towindows-sys
. It ensures thatdbghelp.dll
is still delay loaded, in the same way as before, while avoiding a ton of unnecessary redefinitions.