Propagate .cargo/config.toml [env] settings to the process environment
#16
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nment Leaving the caller to read and merge config environment values with the process environment is not only cumbersome as seen in [rust-mobile/ndk#233], but these values need to be propagated into the process environment anyway to be usable by any subprocess spawned by the caller (ie. `cargo-apk` spawning `cargo rustc`, which can internally spawn a bunch of compilers and linkers too). Instead, do this automatically when the caller invokes `Subcommand::new` while still leaving the door open for crates to read the environment from the public `subcommand.manifest.env` member. [rust-mobile/ndk#233]: rust-mobile/ndk#233
MarijnS95
commented
Mar 22, 2022
Comment on lines
-99
to
-120
| /// It is interpreted as path and canonicalized relative to [`Self::workspace`] if | ||
| /// [`EnvOption::Value::relative`] is set. | ||
| /// | ||
| /// Process environment variables (from [`std::env::var()`]) have [precedence] | ||
| /// unless [`EnvOption::Value::force`] is set. This value is also returned if | ||
| /// the given key was not set under `[env]`. | ||
| /// | ||
| /// [precedence]: https://doc.rust-lang.org/cargo/reference/config.html#env | ||
| pub fn resolve_env(&self, key: &str) -> Result<Cow<'_, str>> { | ||
| let config_var = self.config.env.as_ref().and_then(|env| env.get(key)); | ||
|
|
||
| // Environment variables always have precedence unless | ||
| // the extended format is used to set `force = true`: | ||
| if let Some(env_option @ EnvOption::Value { force: true, .. }) = config_var { | ||
| // Errors iresolving (canonicalizing, really) the config variable take precedence, too: | ||
| return env_option.resolve_value(&self.workspace); | ||
| } | ||
|
|
||
| let process_var = std::env::var(key); | ||
| if process_var != Err(VarError::NotPresent) { | ||
| // Errors from env::var() also have precedence here: | ||
| return Ok(process_var?.into()); |
There was a problem hiding this comment.
This was a public function, not sure if we should keep it (and the tests associated with it)?
If we remove it (saves some maintenance overhead, I don't think the precedence rules still make sense when calling this function after set_env_vars() has been used anyway), this is a breaking change.
|
Thanks @dvc94ch, I'll go ahead and merge+publish this again as I really want these changes now, we can do another breaking followup release once your awesome |
MarijnS95
added a commit
to rust-mobile/ndk
that referenced
this pull request
Mar 22, 2022
Environment variables can be set and override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. The entire handling of these variables is implemented in rust-mobile/cargo-subcommand#12 and rust-mobile/cargo-subcommand#16 which immediately populate the process environment with these variables when `Subcommand::new()` is called by `cargo-apk`.
MarijnS95
added a commit
to rust-mobile/ndk
that referenced
this pull request
Mar 22, 2022
Environment variables can be set and override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. The entire handling of these variables is implemented in rust-mobile/cargo-subcommand#12 and rust-mobile/cargo-subcommand#16 which immediately populate the process environment with these variables when `Subcommand::new()` is called by `cargo-apk`.
MarijnS95
added a commit
to rust-mobile/ndk
that referenced
this pull request
Mar 25, 2022
) Environment variables can be set and override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. The entire handling of these variables is implemented in rust-mobile/cargo-subcommand#12 and rust-mobile/cargo-subcommand#16 which immediately populate the process environment with these variables when `Subcommand::new()` is called by `cargo-apk`.
MarijnS95
added a commit
to rust-mobile/xbuild
that referenced
this pull request
May 3, 2023
…nment Environment variables can be set and optionally override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. Besides exposing variables to all other processes called by `xbuild`, this also allows `xbuild` itself to be driven by variables set in `.cargo/config.toml`, such as `$ANDROID_HOME` needed for #116. rust-mobile/cargo-subcommand#12 rust-mobile/cargo-subcommand#16
MarijnS95
added a commit
to rust-mobile/xbuild
that referenced
this pull request
May 15, 2023
…nment Environment variables can be set and optionally override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. Besides exposing variables to all other processes called by `xbuild`, this also allows `xbuild` itself to be driven by variables set in `.cargo/config.toml`, such as `$ANDROID_HOME` needed for #116. rust-mobile/cargo-subcommand#12 rust-mobile/cargo-subcommand#16
MarijnS95
added a commit
to rust-mobile/xbuild
that referenced
this pull request
Aug 17, 2023
…nment Environment variables can be set and optionally override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. Besides exposing variables to all other processes called by `xbuild`, this also allows `xbuild` itself to be driven by variables set in `.cargo/config.toml`, such as `$ANDROID_HOME` needed for #116. rust-mobile/cargo-subcommand#12 rust-mobile/cargo-subcommand#16
MarijnS95
added a commit
to rust-mobile/xbuild
that referenced
this pull request
Aug 22, 2023
…nment (#117) * Propagate `.cargo/config.toml` `[env]` settings to the process environment Environment variables can be set and optionally override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. Besides exposing variables to all other processes called by `xbuild`, this also allows `xbuild` itself to be driven by variables set in `.cargo/config.toml`, such as `$ANDROID_HOME` needed for #116. rust-mobile/cargo-subcommand#12 rust-mobile/cargo-subcommand#16 * cargo/config: Don't canonicalize joined `[env]` `relative` paths Cargo doesn't do this either, and canonicalization requires the path to exist which it does not have to.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Leaving the caller to read and merge config environment values with the process environment is not only cumbersome as seen in rust-windowing/android-ndk-rs#233, but these values need to be propagated into the process environment anyway to be usable by any subprocess spawned by the caller (ie.
cargo-apkspawningcargo rustc, which can internally spawn a bunch of compilers and linkers too).Instead, do this automatically when the caller invokes
Subcommand::newwhile still leaving the door open for crates to read the environment from the publicsubcommand.manifest.envmember.