Skip to content
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

Auto generate rust bindings using rust bindgen #4723

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

youyuanwu
Copy link
Contributor

@youyuanwu youyuanwu commented Dec 31, 2024

Description

The FFI definitions for rust should be auto generated instead of manually written.
This PR adds a new rust crate with auto generated bindings aiming to replace or remove all the manual bindings in the existing crate. Existing code is not changed. In future, the safe rust wrappers should be written on top of this generated bindings.

supersede #4696 . This PR uses rust bindgen instead of windows bindgen that has better cross platform support.
Generated code is checked in, and an "overwrite" feature can be used to regenerate the code.
MacOs currently uses Linux bindings, we need to generate for MacOs eventually (I don't have a Mac).

Testing

NA. Generated code compiles.

Documentation

NA

Copy link

codecov bot commented Dec 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.53%. Comparing base (1f248f9) to head (b8e6e9a).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4723      +/-   ##
==========================================
- Coverage   87.07%   85.53%   -1.55%     
==========================================
  Files          56       56              
  Lines       17378    17378              
==========================================
- Hits        15132    14864     -268     
- Misses       2246     2514     +268     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nibanks nibanks added the Language: Rust Related to the Rust interop layer label Dec 31, 2024
@youyuanwu youyuanwu force-pushed the users/youyuanwu/bindgen branch from a279309 to 4124e45 Compare January 7, 2025 04:45
@youyuanwu youyuanwu marked this pull request as ready for review January 7, 2025 05:17
@youyuanwu youyuanwu requested a review from a team as a code owner January 7, 2025 05:17
src/lib.rs Show resolved Hide resolved

#include "msquic.h"

// bindgen or clang has problem with c marcro functions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this requires people to know to update this file if a new QUIC_STATUS_* is added. Any way to automate validation for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know any way to automate this currently. But this is much better than the current manually written code, where you need to add the new constant for every OS in the lib.rs file.
Even if u automate this, I expect the higher level error handling code still needs to be manually updated.


/// Read the c header and generate rust bindings.
#[cfg(feature = "overwrite")]
fn overwrite_bindgen() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update the GitHub action to ensure the bindings are up to date?

@nibanks nibanks merged commit cc31197 into microsoft:main Jan 8, 2025
489 of 490 checks passed
@youyuanwu youyuanwu deleted the users/youyuanwu/bindgen branch January 8, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language: Rust Related to the Rust interop layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants