-
Notifications
You must be signed in to change notification settings - Fork 294
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
Maintain a more stable ordering of types #12
Comments
This is actually quite annoying for when my WR update cron job pushes patches, resulting in frequent merge conflicts. Is there something we can do to make the ordering stable? The SideOffsets2D_f32 keeps flip-flopping win the u32 variant. |
There are definitely improvements we can make for the ordering. Unfortunately I haven't had time to improve cbindgen recently. The issue with SideOffsets2D_f32 and SideOffsets2D_u32 seems like it could be from some uses of HashMap, which is non deterministic across changes. I pushed a new release that uses BTreeMap instead, which might help your case. Let me know if it's still bad. |
Initial results look good. Thanks! I'm ok with closing this issue for now and reopening it later if it's still a problem. |
Okay cool. Let's keep this issue open for now, there are some other changes I'd like to make in the future. |
I'm going to close this now as I think we do okay here. If there's something specific that comes up, lets open a new issue. |
commit the current generated proxmox-backup-qemu.h as 'current-api.h' and run `diff -up` between the two in build steps, so any changes to the C api are caught at build time. cbindgen seems to be using a BTreeMap for the data, so it should always come out sorted, so this should be a stable-enough check. Link: mozilla/cbindgen#12 Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Anecdotally, it seems like if you change when I a type gets used it can end up changing order in the generated header which causes needless code churn. It might be nice to avoid this somehow.
The text was updated successfully, but these errors were encountered: