Add a new configuration settings to set env vars when running cargo, rustc, etc. commands: cargo.extraEnv and checkOnSave.extraEnv #13058
Conversation
There was a problem hiding this comment.
I like this change, you lined out good reasons for it as well. One other good one is that this finally gives a way for configuring the rust processes on other clients than VSCode (as server.extraEnv is a VSCode only config).
I am somewhat opposed against reloading the workspace when the env changes though, we don't do this for any other config either that can affect the workspace loading right now either (like the buildscripts configs). We currently delegate this task to the clients. We might want to change that behavior but I'd rather we think about that one separately (a new issue for that might be good).
dpaoliello
changed the title
dpaoliello
force-pushed
the
extraenv
branch
from
910733a
to
c407cc5
Compare
| pub fn check_on_save_extra_env(&self) -> FxHashMap<String, String> { | ||
| let mut extra_env = self.data.cargo_extraEnv.clone(); | ||
| extra_env.extend(self.data.checkOnSave_extraEnv.clone()); | ||
| extra_env | ||
| } |
There was a problem hiding this comment.
I think we should fall back to cargo_extraEnv, not extend them (this is in line with the checkOnSave configs).
There was a problem hiding this comment.
Hm, actually I am not sure what would make more sense here
There was a problem hiding this comment.
I prefer this approach: using extend means that individual variables are overwritten, which matches how env vars work in general (i.e., you get the ambient env vars, then the server.extraEnv overwrites some of those, then cargo.extraEnv overwrites some of those and checkOnSave.extraEnv overwrites some of those).
If checkOnSave.extraEnv replaces cargo.extraEnv, then I suspect that folks will have to duplicate a lot of items to keep the two aligned (e.g., PATH and CARGO_HOME are likely to be the same, although I could see someone wanting to have a slightly different RUSTFLAGS to add more args for checkOnSave.extraEnv).
There was a problem hiding this comment.
Ye, I in this case extending does make more sense I agree, I have left it as was done in this PR.
|
Thanks! |
|
☀️ Test successful - checks-actions |
It can be extremely useful to be able to set environment variables when rust-analyzer is running various cargo or rustc commands (such as
cargo check,cargo --print cfgorcargo metadata): users may want to set customRUSTFLAGS, changePATHto use a custom toolchain or set a differentCARGO_HOME.There is the existing
server.extraEnvsetting that allows env vars to be set when the rust-analyzer server is launched, but using this as the recommended mechanism to also configure cargo/rust has some drawbacks:PATHfor cargo/rustc without affecting the rust-analyzer server).server.extraEnvdoesn't indicate that cargo/rustc will be affected but renaming it tocargo.extraEnvdoesn't indicate that the rust-analyzer server would be affected.This change adds two new configuration settings:
cargo.extraEnvandcheckOnSave.extraEnvthat can be used to change the environment for the rust-analyzer server after launch (thus affecting any process that rust-analyzer invokes) and thecargo checkcommand respectively.cargo.extraEnvsupports dynamic changes by keeping track of the pre-change values of environment variables, thus it can undo changes made previously before applying the new configuration (and then requesting a workspace reload).