windows: drop dependency on windows-sys #125
Conversation
src/os/windows/mod.rs
Outdated
| pub const LOAD_LIBRARY_SAFE_CURRENT_DIRS: LOAD_LIBRARY_FLAGS = consts::LOAD_LIBRARY_SAFE_CURRENT_DIRS; | ||
| pub const LOAD_LIBRARY_SAFE_CURRENT_DIRS: LOAD_LIBRARY_FLAGS = 0x00002000; | ||
|
|
||
| #[link(name="kernel32")] |
There was a problem hiding this comment.
If you want to drop windows-sys I'd still recommend using windows-targets. See Amanieu/parking_lot#378
There was a problem hiding this comment.
Thanks for bringing this up and taking time to review this PR!
I don’t entirely follow what problem this crate solves from reading the linked issue. Is it that the community doesn't always expect the linker to have access to ambient import libs, some form of which windows-targets (or rather – its dependencies) provides?
I guess that's reasonable enough, though it still leaves a bad taste in my mouth that using windows-targets is going to be slightly under a megabyte of dependencies which I’ll have to keep up with if I don't want this particular crate to eventually become the only crate that is responsible for pulling that megabyte in. Avoiding this ongoing maintenance effort was pretty much my rationale for migrating away from windows-sys in the first place.
So, I guess I have two overarching goals:
- I want to effectively never think about, or be pressured to, release new versions of this crate solely for dependency updates;
- I want this crate to be minimal in resources it requires to build (including network bandwidth.)
From what I can tell there are only a couple of plausible solutions compatible with these goals:
windows-targetsstops releasing semver incompatible changes as often (or claims that a bound similar to>=0.48.0 && <1.0.0is really likely to continue building and satisfying the above goals;)- I somehow generate import libraries for all the relevant windows targets containing just the required APIs and ship them with libloading (I imagine those could be quite small.)
I wonder if this generating the import libraries thing could be a windows-rs tooling feature request (if it doesn’t yet exist?)
There was a problem hiding this comment.
Long term the recently stabilized raw-dylib should make this a thing of the past. Short term the windows-targets crate should very rarely release a semver-incompatible update. You can of course leave the code as is but that actually has a far bigger implicit dependency on the Windows SDK but may be "good enough" for most dependents. Here's an explainer on the windows-targets crate:
https://kennykerr.ca/rust-getting-started/understanding-windows-targets.html
|
Could also go the windows-bindgen route, I suppose? Would be nice to get this or #136 released to avoid duplicate dependencies downstream. |
|
Thanks for the reminder. Just updating the dependency we have right now seems reasonable enough until we figure out a good way forward.
I'll try to not forget and deal with it this evening
----------------------------------------
1 Mar 2024 14:09:53 Dirkjan Ochtman ***@***.***>:
…
Could also go the windows-bindgen[https://crates.io/crates/windows-bindgen] route, I suppose?
Would be nice to get this or #136[#136] released to avoid duplicate dependencies downstream.
—
Reply to this email directly, view it on GitHub[#125 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAFFZUQCDNINQY2EX45MVE3YWBVY7AVCNFSM6AAAAAAZG6FH2SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZTGA3TOMJTHA].
You are receiving this because you authored the thread.
[Tracking image][https://github.com/notifications/beacon/AAFFZUUONPNHZSMVD747CZDYWBVY7A5CNFSM6AAAAAAZG6FH2SWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTTVTLCJE.gif]
|
|
So, bumping windows-sys is not as trivial as may seem. At least one complication is that it has increased the MSRV to 1.56.0 due to use of edition 2021. I've been careful to not release versions of libloading that bump MSRV without also making a breaking release. This has been alright so far, as these bumps always coincided with other breaking changes, but this time it is not the case. That said, 1.56.0 is old enough that I can justify to my brain a patch bump with a MSRV break. That said, its been a great opportunity to switch to windows-targets anyway... |
nagisa
force-pushed
the
nagisa/drop-windows-sys
branch
2 times, most recently
from
06ab51e
to
41d790e
Compare
|
Looks nice! |
|
Is there a |
|
Its there; looks like there is something else going on. Its fine, I'll figure it out. |
This results in a smaller dependency tree for the users and a slightly less maintenance burden for me (I won’t be losing any sleep over outdated dependency now!) I have chose to make this a patch-point release. I believe this is an entirely compatible release – `LOAD_LIBRARY_FLAGS` is the same typedef to u32 before and after. `HMOODULE` is the same `isize`, etc. The negative is that documentation now displays method types with resolved type names (i.e. `isize` instead of `HMODULE`) which is not great. I would love to make tyese newtypes, but that's gonna need to wait for a breaking change.
nagisa
force-pushed
the
nagisa/drop-windows-sys
branch
from
41d790e
to
f575857
Compare
This results in a smaller dependency tree for the users and a slightly less maintenance burden for me (I won’t be losing any sleep over outdated dependency now!)
I have chose to make this a patch-point release. I believe this is an entirely compatible release –
LOAD_LIBRARY_FLAGSis the same typedef to u32 before and after.HMOODULEis the sameisize, etc. The negative is that documentation now displays method types with resolved type names (i.e.isizeinstead ofHMODULE) which is not great. I would love to make tyese newtypes, but that's gonna need to wait for a breaking change.