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

Generate rust bindings from mlibc headers #989

Merged
merged 7 commits into from
Feb 26, 2024

Conversation

no92
Copy link
Member

@no92 no92 commented Feb 13, 2024

This PR adds a script to generate valid, working rust-libc bindings. It parses (installed) mlibc headers using libclang, and then outputs rust bindings. A configuration file is supplied to list stuff that needs to be ignored, either due to being broken, unnecessary or already provided in unix/mod.rs or unix/linux_like/mod.rs.

@no92 no92 added bug This issue reports a bug enhancement affects ABI labels Feb 13, 2024
@no92 no92 force-pushed the rust-bindings branch 2 times, most recently from 731f1a2 to f3faec9 Compare February 13, 2024 21:29
Copy link
Contributor

@mintsuki mintsuki left a comment

Choose a reason for hiding this comment

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

Please split up the PR into at least 2: one for the scripts and one for the rest of the mlibc bug fixes.

Copy link
Member

Choose a reason for hiding this comment

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

Probably too awful to do now. But ideally there would be some comments explaining why certain things are ignored.

options/posix/include/dlfcn.h Show resolved Hide resolved
scripts/rust-libc.py Outdated Show resolved Hide resolved
scripts/rust-libc.py Outdated Show resolved Hide resolved
scripts/rust-libc.py Outdated Show resolved Hide resolved
scripts/rust-libc.py Outdated Show resolved Hide resolved
@no92 no92 mentioned this pull request Feb 15, 2024
@no92
Copy link
Member Author

no92 commented Feb 17, 2024

Rebased onto current master.

@no92 no92 changed the base branch from master to abi-break February 17, 2024 22:37
@mintsuki mintsuki force-pushed the abi-break branch 2 times, most recently from 40fe204 to 80b0943 Compare February 22, 2024 10:00
@no92 no92 force-pushed the rust-bindings branch 2 times, most recently from 53bafe4 to fb73d6f Compare February 24, 2024 11:18
@no92 no92 force-pushed the rust-bindings branch 2 times, most recently from 39f95b1 to 7a4562f Compare February 24, 2024 18:02
Copy link
Member

@Geertiebear Geertiebear left a comment

Choose a reason for hiding this comment

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

Aside from the messy python code I don't see a reason why this can't be merged right now. We could always clean up the script if there's ever motivation to.

Copy link
Member

@ArsenArsen ArsenArsen left a comment

Choose a reason for hiding this comment

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

this does not appear to be black-formatted, which, IIRC, we agreed to use for all .py code?

(overall, generating more code like this is quite a good thing, +1)

scripts/rust-libc.py Outdated Show resolved Hide resolved
scripts/rust-libc.py Show resolved Hide resolved
scripts/rust-libc.py Show resolved Hide resolved
scripts/rust-libc.py Outdated Show resolved Hide resolved
@no92
Copy link
Member Author

no92 commented Feb 26, 2024

Updated the script to be formatted with black and addressed some comments.

This script and the configuration file supplied are tested to generate
valid rust bindings (for rust's libc crate) for unix/linux_like/*
targets. This has been tested by building rust's stdlib, alacritty, exa
and ripgrep.

In the future, it might be a worthwhile idea to run this as a GHA check
to catch accidental breakage.
Copy link
Member

@Geertiebear Geertiebear left a comment

Choose a reason for hiding this comment

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

Aesthetically the script looks much better now 🙃 Do still have issues with the code quality but as I mentioned before I'm not of the opinion that should be a blocker for now.

- "B2500000"
- "B3000000"
- "B3500000"
- "B4000000"
Copy link
Member

Choose a reason for hiding this comment

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

Missing a newline at the end of file. We should fix that at the next opportunity but it's not a blocker for me.

Copy link
Member

@Dennisbonke Dennisbonke left a comment

Choose a reason for hiding this comment

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

LGTM

@Dennisbonke Dennisbonke merged commit d2e207f into managarm:abi-break Feb 26, 2024
30 checks passed
@Dennisbonke Dennisbonke deleted the rust-bindings branch February 26, 2024 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects ABI bug This issue reports a bug enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants