-
Notifications
You must be signed in to change notification settings - Fork 222
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
Store file id in an enum #494
Conversation
I'm honestly not convinced by the tradeoff. It's a lot of additional code to allow for windows XP support (which is not even Tier2 and kind of deprecated). I also had trouble finding much information about what exactly the difference between highres / lowres are. But I may be missing something. Note that the BSD build is currently failing. (I'll fast-track #495 before this, as I'm releasing the new requested debouncer version.) |
Then again, notify is about tackling OS complexity and getting this right once in file-id may be worth it. |
That was my idea. Right now it's 64 + 64 bits (Linux and MacOS), or should it be 64 + 128 bits to accommodate the new Windows types? Just to clarify, the LowRes variant (32 + 64 bits) gets constructed by the old I don't expect there to be any additions to these types any time soon, but an enum would make that possible. The alternative is to stick to the 64 + 64 bits struct and use the 64 lower bits of the 128 bits type for windows. This would be the same behavior as with the winapi crate. |
Then let's do this. The enum variant should be marked |
As far as I'm concerned, the user should mostly just compare and maybe serialize/deserialize ids. |
Ping: this will need a rebase and a fix for BSD |
@0xpr03 do you have an idea, why the BSD build is failing? |
Hm, I'd try pushing it again, the linking error makes no sense to me. If anything this should've been broken already when introducing the file-id crate initially. (And it's breaking inside rusts std.) Sadly github actions won't let me re-run this one manually. |
915c55c
to
bf4b490
Compare
Something something linking error on freeBSD for file-id with Which is apparently some kind of GNU extension that should be existing in another way on BSD - and I can't say why it's failing now.. |
So what we need is a sysroot for freeBSD, otherwise the binary ends up failing to link*. Basically move all the freeBSD stuff into a container and build it inside there, as ghactions has no native bsd support. * you just need to compile a hello world binary, unrelated to the code |
👍 Good job, thanks! |
This turns the
FileId
struct into an enum with three variants:The variants are chosen depending on the operating system and on windows depending on the windows API function used.
My idea behind these changes was to make
file-id
more "stable" when using the serialization feature and also to make is more "correct".To be honest, I'm not sure if it is actually worth it.
Then again the complexity is only marginally higher and if memory footprint is a concern, users are free to copy the values into their own data structure.
What do you guys think?