Rework how bindings are generated#66
Merged
Phantomical merged 1 commit intorust-xed:masterfrom Jan 16, 2024
Merged
Conversation
It has been a few years since I have touched the xed-sys crate and advancements to the rust ecosystem have made some of the hacks we used to need obsolete. This commit reworks how and when bindings are generated. The new approach has two main advantages: - With no extra features enabled the build script doesn't even have to run bindgen at all. It can just use the bundled bindings directly. - We no longer have to maintain a separate c2rust translation of a subset of the functions exposed by XED. A new experimental feature in bindgen can now do this (mostly) automatically. I have also thrown in the enc2 feature that is in the process of being introduced by rust-xed#65. We do end up needing bindgen when the enc2 feature is enabled as the generated bindings would be too large to submit to crates.io. (They are >40MB!). With that said, here's how things work now: 1. Bindings for the default featureset are generated in advance by running `./generate.sh`. XED's headers don't contain any platform-dependent code so this should remain portable to any platforms where XED is supported. 2. When the bindgen feature is enabled we ignore the bindings from step 1 and instead generate them in build.rs. With the new experimental --wrap-static-fns bindgen option we can drop the janky c2rust step that translates versions of these functions to rust so that they can be included. Instead bindgen generates an additional C file that declares a set of new functions that forward to the static functions, the generated bindings refer to these forwarding functions instead of the original ones.
Phantomical
force-pushed
the
rework-xed-build
branch
from
1bf3c5d to
3803776
Compare
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It has been a few years since I have touched the xed-sys crate and advancements to the rust ecosystem have made some of the hacks we used to need obsolete. This commit reworks how and when bindings are generated.
The new approach has two main advantages:
I have also thrown in the enc2 feature that is in the process of being introduced by #65. We do end up needing bindgen when the enc2 feature is enabled as the generated bindings would be too large to submit to crates.io. (They are >40MB!).
With that said, here's how things work now:
./generate.sh. XED's headers don't contain any platform-dependent code so this should remain portable to any platforms where XED is supported.With the new experimental --wrap-static-fns bindgen option we can drop the janky c2rust step that translates versions of these functions to rust so that they can be included. Instead bindgen generates an additional C file that declares a set of new functions that forward to the static functions, the generated bindings refer to these forwarding functions instead of the original ones.