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

Include more cc_toolchain flags in RustBindgen actions #2358

Merged
merged 2 commits into from
Dec 27, 2023

Conversation

UebelAndre
Copy link
Collaborator

@UebelAndre UebelAndre commented Dec 25, 2023

This change parses cc_toolchain flags for c++-compile actions for use in bindgen. This change aims to allow the action to gain toolchain specific include paths as well as a more accurate --sysroot.

@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Dec 25, 2023

@illicitonion @scentini @krasimirgg @hlopko do you folks know if there's a way just to grab arguments from cc_toolchain using cc_common or something? As you can see in this PR I'm only interested in include flags. Ideally I would be able to detect those directly in _rust_bindgen_impl by allowing users to optionally pass a list of toolchain features to include in the RustBindgen action.

@UebelAndre UebelAndre force-pushed the bindgen-fmt branch 2 times, most recently from 8d579d4 to a1f3c19 Compare December 26, 2023 16:36
@UebelAndre
Copy link
Collaborator Author

@illicitonion @scentini @krasimirgg @hlopko do you folks know if there's a way just to grab arguments from cc_toolchain using cc_common or something? As you can see in this PR I'm only interested in include flags. Ideally I would be able to detect those directly in _rust_bindgen_impl by allowing users to optionally pass a list of toolchain features to include in the RustBindgen action.

I think I found a solution!

@UebelAndre UebelAndre changed the title Parse include flags from cc_lib target on Bindgen rules. Include more cc_toolchain flags in RustBindgen actions Dec 26, 2023
Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

This generally looks good but I have a couple of questions on it

bindgen/private/bindgen.bzl Show resolved Hide resolved
bindgen/private/bindgen.bzl Show resolved Hide resolved
@UebelAndre UebelAndre merged commit c85ce76 into bazelbuild:main Dec 27, 2023
3 checks passed
@UebelAndre UebelAndre deleted the bindgen-fmt branch December 27, 2023 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants