add a function to populate flags from an arbitrary environment variable #818
Conversation
My specific use-case is to add ability for the end users to specify crate-specific environment variables, but with how general this method is I could imagine numerous other applications too. Why does this need to be in `cc-rs`, you ask? The code to split up the `CFLAGS` or `CXXFLAGS` environment variables into flags seems trivial today, but I worry that it might not remain the case indefinitely into the future. If everybody is expected to do this in their own build scripts, we’re certain to run into a very inconsistent experience as soon as parsing needs an adjustment.
nagisa
force-pushed
the
nagisa/flag-population
branch
from
5d4573d to
a8c9a61
Compare
| @@ -3133,6 +3139,34 @@ impl Build { | |||
| } | |||
| } | |||
|
|
|||
| fn getenv_with_target_prefixes(&self, var_base: &str) -> Result<String, Error> { | |||
There was a problem hiding this comment.
This function has been moved and renamed to be grouped with other getenv family of functions, with no other changes intended.
| } | ||
| } | ||
|
|
||
| fn envflags(&self, name: &str) -> Result<Vec<String>, Error> { |
There was a problem hiding this comment.
This function has been changed to return an Error when getenv_with_target_prefixes fails in order to allow to propagate this error in the newly added method.
|
This seems fine to me, but probably needs some documentation. |
|
What sort of documentation do you envision (i.e. what sort of questions should it answer) in addition to what is present in the doc-comment and where should it go? |
|
Do you think it's worth mentioning with https://github.com/rust-lang/cc-rs#external-configuration-via-environment-variables? I could see an argument that it's not. |
This is specifically a end-user facing information, so that they are aware of the possibility that there may be additional environment variables as well.
|
Done! I tailored that part of the documentation specifically towards the end-users (people compiling code/crates,) as that whom this documentation appeared to be aimed at. |
|
This all looks good to me, I'll take another look some time it's not 2AM before I merge, though. |
|
Do you have a time in mind for then there might be a new version? I can submit a PR with version bumps if that all it needs. |
My specific use-case is to add ability for the end users to specify crate-specific environment variables, but with how general this method is I could imagine numerous other applications too.
Why does this need to be in
cc-rs, you ask? The code to split up theCFLAGSorCXXFLAGSenvironment variables into flags seems trivial today, but I worry that it might not remain the case indefinitely into the future. If everybody is expected to do this in their own build scripts, we’re certain to run into a very inconsistent experience as soon as parsing needs an adjustment.