Github Add support for [env] section in .cargo/config.toml by wickerwaka · Pull...
source link: https://github.com/rust-lang/cargo/pull/9175
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
This adds support for an [env] section in the config.toml files. Environment variables set in this section will be applied to the environment of any processes executed by cargo.
This is implemented to follow the recommendations in #8839 (comment)
Variables have optional force and relative flags. force means the variable can override an existing environment variable. relative means the variable represents a path relative to the location of the directory that contains the .cargo/ directory that contains the config.toml file. A relative variable will have an absolute path prepended to it before setting it in the environment.
[env]
FOOBAR = "Apple"
PATH_TO_SOME_TOOL = { value = "bin/tool", relative = true }
USERNAME = { value = "test_user", force = true }
Fixes #4121
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.
If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.
Please see the contribution instructions for more information.
Thanks for the PR! Since it's been a few months since I've at least thought about this could you detail your expected use case with this as well? I'm not sure if this should close #4121 since that seems like a different feature request for Cargo
Outdated
@@ -1244,6 +1248,39 @@ impl Config {
&self.progress_config
}
/// Create an EnvConfigValue hashmap from the "env" table
fn get_env_config(&self) -> CargoResult<EnvConfig> {
alexcrichton 25 days ago
Member
Would it be possible to do custom serde deserialization (e.g. via #[serde(untagged)]) instead of having custom deserialization logic here?
wickerwaka 24 days ago
Author
Contributor
So I tried that originally and could not get it to work. I had something like:
#[derive(Debug, Deserialize)]
#[serde(transparent)]
pub struct EnvConfig {
vars: HashMap<String, EnvConfigValue>
}
#[derive(Debug, Deserialize)]
#[serde(untagged)]
pub enum EnvConfigValue {
Simple(String),
WithOptions{
value: Value<String>,
force: bool,
relative: bool,
}
}
The problem was serde would never parse the Value<T> type in the WithOptions variant, it worked if it was a regular String. I'm not sure why, maybe the entire "path" from the root of the deserialization needs to be Value<T> types in order for them to be handle correctly in some inner struct?
alexcrichton 23 days ago
Member
I think this structure may work instead?
#[derive(Debug, Deserialize)]
#[serde(untagged)]
pub enum EnvConfigValue {
Simple(String),
WithOptions(WithOptions),
}
#[derive(Debug, Deserialize)]
pub struct WithOptions {
value: Value<String>,
force: bool,
relative: bool,
}wickerwaka 22 days ago
Author
Contributor
No, that doesn't work either. It seems like untagged and the Value<T> types do not play nicely together, it will never select a variant with that type. Latest commit does contain a struct/enum layout that does work though. So no more custom logic.
Thanks for the PR! Since it's been a few months since I've at least thought about this could you detail your expected use case with this as well? I'm not sure if this should close #4121 since that seems like a different feature request for Cargo
My specific use case is that the rust-cpython and pyo3 crates both look for a PYTHON_SYS_EXECUTABLE environment variable to find the python installation it should link against. I work in a large monorepo that contains rust code that is using rust-cpython, a full win64 python installation and a bunch of other stuff. Whenever possible we avoid using environment variables and prefer to use relative paths to find various components. This means the monorepo is easily relocatable to any location on a developers machine and multiple different versions can exist without having to modify/set any environment variables or other system-wide settings. I want to be able to set PYTHON_SYS_EXECUTABLE for a config.toml at or near the root of the monorepo:
[env]
PYTHON_SYS_EXECUTABLE = { value = "tools/lang/python3.7/python.exe", relative = true }
It seems to be a common thread that comes up during any search for "environment variables in cargo", crates people are using need to find libraries or tools (such as openssl, ffmpeg, python) and the crates use environment variables for that. If you have those dependencies at a known relative location you prefer not to have to rely on global environment variables for that. You can set them locally before you run cargo build but then you have all these issues with tools like IDEs that might not be running within that context. @rib calls out several issues related to this from other projects here: #4121 (comment)
You are right, I don't think it solves every aspect of #4121 because there are two different use cases in that issue and the comments. What some would like is a "prebuild" script that runs before dependencies are built that would allow you to set environment variables (and maybe some other settings) that the dependencies would consume. I would love to have a prebuild script myself, but I know there is some reticence about going down that road.
I confirm we have exactly the same use case.
Thanks for the explanation! That all sounds quite reasonable to me and I'd be happy to r+ this especially given that it's unstable. I think we'll want more team sign-off once a motion is made to stabilize, but for now seems good!
@bors: r+
Looks great to me, thanks!
Commit 05acf73 has been approved by alexcrichton
Testing commit 
Test successful - checks-actions
Approved by: alexcrichton
Pushing 4742e82 to master...
Copy link
Flaise commented 19 days ago
This is going to be very helpful for a number of my projects. Thanks guys.
Copy link
rib commented 19 days ago
Big thanks to @wickerwaka for implementing this! I guess #8839 can also be closed based on this?
Copy link
mandeep commented 7 days ago
Perhaps there should be a test that shows what happens if a dependency requires an environment variable. I tried testing this change in that manner with an environment variable that points to the path of a shared library. cargo build worked just fine however cargo run ended with an error with the environment variable not being found. I'm not sure if this is the point of the change, but curious to hear some thoughts.
Perhaps there should be a test that shows what happens if a dependency requires an environment variable. I tried testing this change in that manner with an environment variable that points to the path of a shared library.
cargo buildworked just fine howevercargo runended with an error with the environment variable not being found. I'm not sure if this is the point of the change, but curious to hear some thoughts.
Could you provide some more details on this? Environment variables are getting set for both build and run and my own personal use case is handling a situation where a dependency needs an environment variable set. However, I am not using that environment variable during the run step. Do you have a minimal example you can share?
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK