Skip to content

pass some flags to bindgen #2051

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

ibigbug
Copy link
Contributor

@ibigbug ibigbug commented Jul 5, 2023

Issue: while some crates such as boring-sys needs rust-bindgen to run, which is not uncommon. however the rust-bindgen expects the include dir/sysroot to be passed in separately and it doesn't seem to read the CFLAG/CXXFLAGS at all, which makes sense as it only generates the binding without compiling the source.

So I wanted to potentially pass the include dirs as much as possible to the bindgen, so that errors like <sys/types.h> not found while building boringssl can be addressed.

@ibigbug
Copy link
Contributor Author

ibigbug commented Jul 7, 2023

ping @UebelAndre thoughs?

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Hey! Sorry for the delay!

I struggle to make up my mind with this change. I dislike referencing a specific tool (bindgen or otherwise) in these rules and think they should be ignorant of anything other than calls to rustc. But I do see the convenience as I've had to annotate many crates which use bindgen. I feel like there is a larger feature request for being able to more generally customize how cargo_build_script works. I'm unsure if that would be something handled by some Bazel toolchain or implemented in crate_universe, but I think there's value in keeping cargo_build_script generic.

Any thoughts on handling this logic elsewhere?

@ibigbug
Copy link
Contributor Author

ibigbug commented Aug 8, 2023

hey thanks for the feedback and I totally get it that this tool should not be tied to a specific third party tool and I asked myself same question many times before drafting the PR. and the reason i'd like to mention is:

  1. it's sometimes quite hard to markup the bindgen environment, for example, on non hermetic build, and the case for darwin would be injecting something like /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/ to the BINDGEN_EXTRA_CLANG_ARGS_* which looks a bit ugly (and I'm not sure if it's that I didn't use it correctly)

  2. considering the fact that bindgen is tightly closed to the entire rust eco system and this rules_rust comes with a bindgen target too and that's how I see bindgen is unlike a random 3rd party tool but more like part of the build system.

  3. and setting bindgen with the bazel target is sometimes impossilbe/less-intuitive as for example, the boring-sys requires bindgen, and in it's build script and it's not quite easy to hijack the build script and also the dependencies of other pkgs that relies on boring-sys

  4. I really couldn't see another place that would be able to get the reference of both cc_toolchain paths and the bindgen env 🤔

@UebelAndre further thoughts?

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.

2 participants