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

bindgen::ParseCallbacks: support tracking env variable usage for cargo #2400

Merged
merged 4 commits into from
Apr 10, 2023

Conversation

codyps
Copy link
Contributor

@codyps codyps commented Jan 27, 2023

bindgen currently has a bindgen/build.rs which attempts to have cargo rebuild bindgen when TARGET specific env variables change, specifically BINDGEN_EXTRA_CLANG_ARGS_<target>. Unfortunately, this doesn't have the desired effect in most cases.

Specifically, when a crate A has bindgen in build-dependencies, and we're cross compiling A for target T on a host H, bindgen's build.rs will observe TARGET set to H (the host target name) instead of T (because bindgen itself is being built for H and not T). Then, within the build script of crate A, one would use bindgen to generate bindings, and now TARGET is set to T, so different env variables are used.

Allow crates using bindgen in build scripts to correctly handle env variable changes by adding ParseCallbacks::read_env_var() to track env var reads and adding a basic implimentation to CargoCallbacks to emit cargo:rerun-if-env-changed lines.

@bors-servo
Copy link

☔ The latest upstream changes (presumably 2be14a3) made this pull request unmergeable. Please resolve the merge conflicts.

@codyps
Copy link
Contributor Author

codyps commented Mar 29, 2023

@pvdrz ?

@pvdrz pvdrz self-requested a review March 29, 2023 21:32
@pvdrz pvdrz self-assigned this Mar 29, 2023
@pvdrz
Copy link
Contributor

pvdrz commented Mar 29, 2023

Hey! Thanks for the ping, I'll be reviewing this soon. For the time being could you fix the formatting. It seems you're running cargo fmt instead of cargo +nightly fmt.

@bors-servo
Copy link

☔ The latest upstream changes (presumably 040149b) made this pull request unmergeable. Please resolve the merge conflicts.

@pvdrz pvdrz force-pushed the bindgen-parse-cb-read-env branch from ecd9b4f to 461faa3 Compare April 10, 2023 16:09
codyps and others added 4 commits April 10, 2023 11:10
bindgen currently has a `bindgen/build.rs` which attempts to have cargo
rebuild bindgen when TARGET specific env variables change, specifically
`BINDGEN_EXTRA_CLANG_ARGS_<target>`. Unfortunately, this doesn't have
the desired effect in most cases.

Specifically, when a crate `A` has `bindgen` in `build-dependencies`,
and we're cross compiling `A` for target `T` on a host `H`, `bindgen`'s
`build.rs` will observe `TARGET` set to `H` (the host target name)
instead of `T` (because `bindgen` itself is being built for `H` and not
`T`). Then, within the build script of crate `A`, one would use
`bindgen` to generate bindings, and now `TARGET` is set to `T`, so
different env variables are used.

Allow crates using `bindgen` in build scripts to correctly handle env
variable changes by adding `ParseCallbacks::read_env_var()` to track
env var reads and adding a basic implementation to `CargoCallbacks` to
emit `cargo:rerun-if-env-changed` lines.
@pvdrz pvdrz merged commit 5d1c79a into rust-lang:main Apr 10, 2023
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.

3 participants