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

Add env var EXTRA_CLANG_ARGS_<TARGET> #2031

Merged
merged 1 commit into from Apr 15, 2021

Conversation

Orycterope
Copy link
Contributor

Closes #2009

As suggested in the issue, I've done it the same way the cc crate does it in its get_var function.

build.rs Outdated
@@ -79,4 +79,6 @@ fn main() {
println!("cargo:rerun-if-env-changed=LIBCLANG_PATH");
println!("cargo:rerun-if-env-changed=LIBCLANG_STATIC_PATH");
println!("cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS");
println!("cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS_{}", std::env::var("TARGET").unwrap());
println!("cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS_{}", std::env::var("TARGET").unwrap().replace("-", "_"));
Copy link
Contributor

Choose a reason for hiding this comment

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

This unwrap is fine.

src/lib.rs Outdated
env::var("BINDGEN_EXTRA_CLANG_ARGS").ok()
if let Ok(extra_clang_args) = env::var(&format!("BINDGEN_EXTRA_CLANG_ARGS_{}", env::var("TARGET").unwrap()))
.or_else(|_| env::var(&format!("BINDGEN_EXTRA_CLANG_ARGS_{}", env::var("TARGET").unwrap().replace("-", "_"))))
.or_else(|_| env::var("BINDGEN_EXTRA_CLANG_ARGS"))
Copy link
Contributor

Choose a reason for hiding this comment

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

This can't unwrap the env::var() result, because when used from the CLI it might not be set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes ! The unit tests caught it. I'm changing it to .map()

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this to a reusable function? Something like:

fn get_target_dependent_env_var(var: &str) -> Option<String> {
    if let Some(target) = env::var("TARGET") {
        if let Some(v) = env::var(&format!("{}_{}", var, target)) {
            return Some(v);
        }
        if let Some(v) = env::var(&format!("{}_{}", var, target.replace("...")) {
            return Some(v)
        }
    }
    return env::var(var);
}

or such?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done !

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

@emilio emilio merged commit c39c47c into rust-lang:master Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow configuring BINDGEN_EXTRA_CLANG_ARGS per target
3 participants