Use `Build::getenv` instead of `env::var*` in anywhere that makes sense

rust-lang / cc-rs

Rust library for build scripts to compile C/C++ code into a Rust library

https://docs.rs/cc

Apache License 2.0

1.76k stars 425 forks source link

Use `Build::getenv` instead of `env::var*` in anywhere that makes sense #1103

Closed NobodyXu closed 1 week ago

NobodyXu commented 1 week ago

So that we would get:

recompilation when the env is changed
cached env

Also some optimisation

NobodyXu commented 1 week ago

Might make sense to ban std::env::var and std::env::var_os via. Clippy's disallowed_methods functionality? And then explicitly allow it inside the implementation of getenv

Thanks!

Applied the suggestion, and turns out that the entire windows_registry is using env::var*

Took me a while to change them to use a generic function.

cc @thomcc @madsmtm

NobodyXu commented 1 week ago

cc @madsmtm This PR should cover most env variables in #906

If there's any missing, feel free to open a comment.

NobodyXu commented 1 week ago

Updated the code to remove any generics in windows_regsitry.

Having generics would make cc takes longer to compile and generate larger binary, and I don't think it would give us any improvements to performance.