allow using null to unset an environment variable

this makes three changes:
- all callsites of `toolchain::command` are changed to use
  `command(path, extra_env)`, instead of manually adding the env after
  the fact.
- all `map<str, str>` are changed to `map<str, option<str>>`.
- `command` checks for None and calls `env_remove` if so.

this caught several places where environment variables weren't being
propagated:
- when running `rustc --print=target-libdir`
- when running `cargo rustc -- --print=target-spec-json`
- when running the custom DiscoverLinkedProjects config. I *think* this
  is for use with non-cargo build systems, so I didn't change it.

Author: jyn514

crates/load-cargo/src/lib.rs

@@ -66,7 +66,7 @@ pub fn load_workspace_at(
 
 pub fn load_workspace(
     ws: ProjectWorkspace,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
     load_config: &LoadCargoConfig,
 ) -> anyhow::Result<(RootDatabase, vfs::Vfs, Option<ProcMacroClient>)> {
     let (sender, receiver) = unbounded();

crates/proc-macro-api/src/lib.rs

@@ -105,10 +105,11 @@ impl fmt::Display for ServerError {
 
 impl ProcMacroClient {
     /// Spawns an external process as the proc macro server and returns a client connected to it.
-    pub fn spawn(
+    pub fn spawn<'a>(
         process_path: &AbsPath,
-        env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>
-        + Clone,
+        env: impl IntoIterator<
+            Item = (impl AsRef<std::ffi::OsStr>, &'a Option<impl 'a + AsRef<std::ffi::OsStr>>),
+        > + Clone,
     ) -> io::Result<ProcMacroClient> {
         let process = ProcMacroServerProcess::run(process_path, env)?;
         Ok(ProcMacroClient { process: Arc::new(process), path: process_path.to_owned() })

crates/proc-macro-api/src/process.rs

@@ -43,10 +43,11 @@ struct ProcessSrvState {
 
 impl ProcMacroServerProcess {
     /// Starts the proc-macro server and performs a version check
-    pub(crate) fn run(
+    pub(crate) fn run<'a>(
         process_path: &AbsPath,
-        env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>
-        + Clone,
+        env: impl IntoIterator<
+            Item = (impl AsRef<std::ffi::OsStr>, &'a Option<impl 'a + AsRef<std::ffi::OsStr>>),
+        > + Clone,
     ) -> io::Result<ProcMacroServerProcess> {
         let create_srv = || {
             let mut process = Process::run(process_path, env.clone())?;
@@ -193,9 +194,11 @@ struct Process {
 
 impl Process {
     /// Runs a new proc-macro server process with the specified environment variables.
-    fn run(
+    fn run<'a>(
         path: &AbsPath,
-        env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>,
+        env: impl IntoIterator<
+            Item = (impl AsRef<std::ffi::OsStr>, &'a Option<impl 'a + AsRef<std::ffi::OsStr>>),
+        >,
     ) -> io::Result<Process> {
         let child = JodChild(mk_child(path, env)?);
         Ok(Process { child })
@@ -212,14 +215,21 @@ impl Process {
 }
 
 /// Creates and configures a new child process for the proc-macro server.
-fn mk_child(
+fn mk_child<'a>(
     path: &AbsPath,
-    env: impl IntoIterator<Item = (impl AsRef<std::ffi::OsStr>, impl AsRef<std::ffi::OsStr>)>,
+    extra_env: impl IntoIterator<
+        Item = (impl AsRef<std::ffi::OsStr>, &'a Option<impl 'a + AsRef<std::ffi::OsStr>>),
+    >,
 ) -> io::Result<Child> {
     #[allow(clippy::disallowed_methods)]
     let mut cmd = Command::new(path);
-    cmd.envs(env)
-        .env("RUST_ANALYZER_INTERNALS_DO_NOT_USE", "this is unstable")
+    for env in extra_env {
+        match env {
+            (key, Some(val)) => cmd.env(key, val),
+            (key, None) => cmd.env_remove(key),
+        };
+    }
+    cmd.env("RUST_ANALYZER_INTERNALS_DO_NOT_USE", "this is unstable")
         .stdin(Stdio::piped())
         .stdout(Stdio::piped())
         .stderr(Stdio::inherit());

crates/project-model/src/build_dependencies.rs

@@ -163,7 +163,7 @@ impl WorkspaceBuildScripts {
     pub(crate) fn rustc_crates(
         rustc: &CargoWorkspace,
         current_dir: &AbsPath,
-        extra_env: &FxHashMap<String, String>,
+        extra_env: &FxHashMap<String, Option<String>>,
         sysroot: &Sysroot,
     ) -> Self {
         let mut bs = WorkspaceBuildScripts::default();
@@ -172,16 +172,14 @@ impl WorkspaceBuildScripts {
         }
         let res = (|| {
             let target_libdir = (|| {
-                let mut cargo_config = sysroot.tool(Tool::Cargo, current_dir);
-                cargo_config.envs(extra_env);
+                let mut cargo_config = sysroot.tool(Tool::Cargo, current_dir, extra_env);
                 cargo_config
                     .args(["rustc", "-Z", "unstable-options", "--print", "target-libdir"])
                     .env("RUSTC_BOOTSTRAP", "1");
                 if let Ok(it) = utf8_stdout(&mut cargo_config) {
                     return Ok(it);
                 }
-                let mut cmd = sysroot.tool(Tool::Rustc, current_dir);
-                cmd.envs(extra_env);
+                let mut cmd = sysroot.tool(Tool::Rustc, current_dir, extra_env);
                 cmd.args(["--print", "target-libdir"]);
                 utf8_stdout(&mut cmd)
             })()?;
@@ -390,12 +388,12 @@ impl WorkspaceBuildScripts {
     ) -> io::Result<Command> {
         let mut cmd = match config.run_build_script_command.as_deref() {
             Some([program, args @ ..]) => {
-                let mut cmd = toolchain::command(program, current_dir);
+                let mut cmd = toolchain::command(program, current_dir, &config.extra_env);
                 cmd.args(args);
                 cmd
             }
             _ => {
-                let mut cmd = sysroot.tool(Tool::Cargo, current_dir);
+                let mut cmd = sysroot.tool(Tool::Cargo, current_dir, &config.extra_env);
 
                 cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
                 cmd.args(&config.extra_args);
@@ -448,7 +446,6 @@ impl WorkspaceBuildScripts {
             }
         };
 
-        cmd.envs(&config.extra_env);
         if config.wrap_rustc_in_build_scripts {
             // Setup RUSTC_WRAPPER to point to `rust-analyzer` binary itself. We use
             // that to compile only proc macros and build scripts during the initial

crates/project-model/src/cargo_workspace.rs

@@ -104,7 +104,7 @@ pub struct CargoConfig {
     /// Extra args to pass to the cargo command.
     pub extra_args: Vec<String>,
     /// Extra env vars to set when invoking the cargo command
-    pub extra_env: FxHashMap<String, String>,
+    pub extra_env: FxHashMap<String, Option<String>>,
     pub invocation_strategy: InvocationStrategy,
     /// Optional path to use instead of `target` when building
     pub target_dir: Option<Utf8PathBuf>,
@@ -289,7 +289,7 @@ pub struct CargoMetadataConfig {
     /// Extra args to pass to the cargo command.
     pub extra_args: Vec<String>,
     /// Extra env vars to set when invoking the cargo command
-    pub extra_env: FxHashMap<String, String>,
+    pub extra_env: FxHashMap<String, Option<String>>,
 }
 
 // Deserialize helper for the cargo metadata
@@ -343,11 +343,10 @@ impl CargoWorkspace {
         locked: bool,
         progress: &dyn Fn(String),
     ) -> anyhow::Result<(cargo_metadata::Metadata, Option<anyhow::Error>)> {
-        let cargo = sysroot.tool(Tool::Cargo, current_dir);
+        let cargo = sysroot.tool(Tool::Cargo, current_dir, &config.extra_env);
         let mut meta = MetadataCommand::new();
         meta.cargo_path(cargo.get_program());
         cargo.get_envs().for_each(|(var, val)| _ = meta.env(var, val.unwrap_or_default()));
-        config.extra_env.iter().for_each(|(var, val)| _ = meta.env(var, val));
         meta.manifest_path(cargo_toml.to_path_buf());
         match &config.features {
             CargoFeatures::All => {

crates/project-model/src/env.rs

@@ -62,11 +62,10 @@ pub(crate) fn inject_rustc_tool_env(env: &mut Env, cargo_name: &str, kind: Targe
 
 pub(crate) fn cargo_config_env(
     manifest: &ManifestPath,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
     sysroot: &Sysroot,
 ) -> Env {
-    let mut cargo_config = sysroot.tool(Tool::Cargo, manifest.parent());
-    cargo_config.envs(extra_env);
+    let mut cargo_config = sysroot.tool(Tool::Cargo, manifest.parent(), extra_env);
     cargo_config
         .args(["-Z", "unstable-options", "config", "get", "env"])
         .env("RUSTC_BOOTSTRAP", "1");

crates/project-model/src/sysroot.rs

@@ -86,7 +86,7 @@ impl Sysroot {
 
 impl Sysroot {
     /// Attempts to discover the toolchain's sysroot from the given `dir`.
-    pub fn discover(dir: &AbsPath, extra_env: &FxHashMap<String, String>) -> Sysroot {
+    pub fn discover(dir: &AbsPath, extra_env: &FxHashMap<String, Option<String>>) -> Sysroot {
         let sysroot_dir = discover_sysroot_dir(dir, extra_env);
         let rust_lib_src_dir = sysroot_dir.as_ref().ok().map(|sysroot_dir| {
             discover_rust_lib_src_dir_or_add_component(sysroot_dir, dir, extra_env)
@@ -96,7 +96,7 @@ impl Sysroot {
 
     pub fn discover_with_src_override(
         current_dir: &AbsPath,
-        extra_env: &FxHashMap<String, String>,
+        extra_env: &FxHashMap<String, Option<String>>,
         rust_lib_src_dir: AbsPathBuf,
     ) -> Sysroot {
         let sysroot_dir = discover_sysroot_dir(current_dir, extra_env);
@@ -118,7 +118,12 @@ impl Sysroot {
     }
 
     /// Returns a command to run a tool preferring the cargo proxies if the sysroot exists.
-    pub fn tool(&self, tool: Tool, current_dir: impl AsRef<Path>) -> Command {
+    pub fn tool(
+        &self,
+        tool: Tool,
+        current_dir: impl AsRef<Path>,
+        envs: &FxHashMap<String, Option<String>>,
+    ) -> Command {
         match self.root() {
             Some(root) => {
                 // special case rustc, we can look that up directly in the sysroot's bin folder
@@ -127,15 +132,15 @@ impl Sysroot {
                     if let Some(path) =
                         probe_for_binary(root.join("bin").join(Tool::Rustc.name()).into())
                     {
-                        return toolchain::command(path, current_dir);
+                        return toolchain::command(path, current_dir, envs);
                     }
                 }
 
-                let mut cmd = toolchain::command(tool.prefer_proxy(), current_dir);
+                let mut cmd = toolchain::command(tool.prefer_proxy(), current_dir, envs);
                 cmd.env("RUSTUP_TOOLCHAIN", AsRef::<std::path::Path>::as_ref(root));
                 cmd
             }
-            _ => toolchain::command(tool.path(), current_dir),
+            _ => toolchain::command(tool.path(), current_dir, envs),
         }
     }
 
@@ -292,7 +297,7 @@ impl Sysroot {
         // the sysroot uses `public-dependency`, so we make cargo think it's a nightly
         cargo_config.extra_env.insert(
             "__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS".to_owned(),
-            "nightly".to_owned(),
+            Some("nightly".to_owned()),
         );
 
         let (mut res, _) = match CargoWorkspace::fetch_metadata(
@@ -368,10 +373,9 @@ impl Sysroot {
 
 fn discover_sysroot_dir(
     current_dir: &AbsPath,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
 ) -> Result<AbsPathBuf> {
-    let mut rustc = toolchain::command(Tool::Rustc.path(), current_dir);
-    rustc.envs(extra_env);
+    let mut rustc = toolchain::command(Tool::Rustc.path(), current_dir, extra_env);
     rustc.current_dir(current_dir).args(["--print", "sysroot"]);
     tracing::debug!("Discovering sysroot by {:?}", rustc);
     let stdout = utf8_stdout(&mut rustc)?;
@@ -398,12 +402,11 @@ fn discover_rust_lib_src_dir(sysroot_path: &AbsPathBuf) -> Option<AbsPathBuf> {
 fn discover_rust_lib_src_dir_or_add_component(
     sysroot_path: &AbsPathBuf,
     current_dir: &AbsPath,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
 ) -> Result<AbsPathBuf> {
     discover_rust_lib_src_dir(sysroot_path)
         .or_else(|| {
-            let mut rustup = toolchain::command(Tool::Rustup.prefer_proxy(), current_dir);
-            rustup.envs(extra_env);
+            let mut rustup = toolchain::command(Tool::Rustup.prefer_proxy(), current_dir, extra_env);
             rustup.args(["component", "add", "rust-src"]);
             tracing::info!("adding rust-src component by {:?}", rustup);
             utf8_stdout(&mut rustup).ok()?;

crates/project-model/src/toolchain_info/rustc_cfg.rs

@@ -11,7 +11,7 @@ use crate::{toolchain_info::QueryConfig, utf8_stdout};
 pub fn get(
     config: QueryConfig<'_>,
     target: Option<&str>,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
 ) -> Vec<CfgAtom> {
     let _p = tracing::info_span!("rustc_cfg::get").entered();
 
@@ -58,14 +58,13 @@ pub fn get(
 
 fn rustc_print_cfg(
     target: Option<&str>,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
     config: QueryConfig<'_>,
 ) -> anyhow::Result<String> {
     const RUSTC_ARGS: [&str; 2] = ["--print", "cfg"];
     let (sysroot, current_dir) = match config {
         QueryConfig::Cargo(sysroot, cargo_toml) => {
-            let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent());
-            cmd.envs(extra_env);
+            let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent(), extra_env);
             cmd.args(["rustc", "-Z", "unstable-options"]).args(RUSTC_ARGS);
             if let Some(target) = target {
                 cmd.args(["--target", target]);
@@ -86,8 +85,7 @@ fn rustc_print_cfg(
         QueryConfig::Rustc(sysroot, current_dir) => (sysroot, current_dir),
     };
 
-    let mut cmd = sysroot.tool(Tool::Rustc, current_dir);
-    cmd.envs(extra_env);
+    let mut cmd = sysroot.tool(Tool::Rustc, current_dir, extra_env);
     cmd.args(RUSTC_ARGS);
     cmd.arg("-O");
     if let Some(target) = target {

crates/project-model/src/toolchain_info/target_data_layout.rs

@@ -10,7 +10,7 @@ use crate::{Sysroot, toolchain_info::QueryConfig, utf8_stdout};
 pub fn get(
     config: QueryConfig<'_>,
     target: Option<&str>,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
 ) -> anyhow::Result<String> {
     const RUSTC_ARGS: [&str; 2] = ["--print", "target-spec-json"];
     let process = |output: String| {
@@ -21,8 +21,7 @@ pub fn get(
     };
     let (sysroot, current_dir) = match config {
         QueryConfig::Cargo(sysroot, cargo_toml) => {
-            let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent());
-            cmd.envs(extra_env);
+            let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent(), extra_env);
             cmd.env("RUSTC_BOOTSTRAP", "1");
             cmd.args(["rustc", "-Z", "unstable-options"]).args(RUSTC_ARGS).args([
                 "--",
@@ -43,11 +42,8 @@ pub fn get(
         QueryConfig::Rustc(sysroot, current_dir) => (sysroot, current_dir),
     };
 
-    let mut cmd = Sysroot::tool(sysroot, Tool::Rustc, current_dir);
-    cmd.envs(extra_env)
-        .env("RUSTC_BOOTSTRAP", "1")
-        .args(["-Z", "unstable-options"])
-        .args(RUSTC_ARGS);
+    let mut cmd = Sysroot::tool(sysroot, Tool::Rustc, current_dir, extra_env);
+    cmd.env("RUSTC_BOOTSTRAP", "1").args(["-Z", "unstable-options"]).args(RUSTC_ARGS);
     if let Some(target) = target {
         cmd.args(["--target", target]);
     }

crates/project-model/src/toolchain_info/target_tuple.rs

@@ -12,7 +12,7 @@ use crate::{ManifestPath, Sysroot, toolchain_info::QueryConfig, utf8_stdout};
 pub fn get(
     config: QueryConfig<'_>,
     target: Option<&str>,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
 ) -> anyhow::Result<Vec<String>> {
     let _p = tracing::info_span!("target_tuple::get").entered();
     if let Some(target) = target {
@@ -32,12 +32,11 @@ pub fn get(
 }
 
 fn rustc_discover_host_tuple(
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
     sysroot: &Sysroot,
     current_dir: &Path,
 ) -> anyhow::Result<String> {
-    let mut cmd = sysroot.tool(Tool::Rustc, current_dir);
-    cmd.envs(extra_env);
+    let mut cmd = sysroot.tool(Tool::Rustc, current_dir, extra_env);
     cmd.arg("-vV");
     let stdout = utf8_stdout(&mut cmd)
         .with_context(|| format!("unable to discover host platform via `{cmd:?}`"))?;
@@ -53,11 +52,10 @@ fn rustc_discover_host_tuple(
 
 fn cargo_config_build_target(
     cargo_toml: &ManifestPath,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
     sysroot: &Sysroot,
 ) -> Option<Vec<String>> {
-    let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent());
-    cmd.envs(extra_env);
+    let mut cmd = sysroot.tool(Tool::Cargo, cargo_toml.parent(), extra_env);
     cmd.current_dir(cargo_toml.parent()).env("RUSTC_BOOTSTRAP", "1");
     cmd.args(["-Z", "unstable-options", "config", "get", "build.target"]);
     // if successful we receive `build.target = "target-tuple"`

crates/project-model/src/toolchain_info/version.rs

@@ -9,17 +9,16 @@ use crate::{toolchain_info::QueryConfig, utf8_stdout};
 
 pub(crate) fn get(
     config: QueryConfig<'_>,
-    extra_env: &FxHashMap<String, String>,
+    extra_env: &FxHashMap<String, Option<String>>,
 ) -> Result<Option<Version>, anyhow::Error> {
     let (mut cmd, prefix) = match config {
         QueryConfig::Cargo(sysroot, cargo_toml) => {
-            (sysroot.tool(Tool::Cargo, cargo_toml.parent()), "cargo ")
+            (sysroot.tool(Tool::Cargo, cargo_toml.parent(), extra_env), "cargo ")
         }
         QueryConfig::Rustc(sysroot, current_dir) => {
-            (sysroot.tool(Tool::Rustc, current_dir), "rustc ")
+            (sysroot.tool(Tool::Rustc, current_dir, extra_env), "rustc ")
         }
     };
-    cmd.envs(extra_env);
     cmd.arg("--version");
     let out = utf8_stdout(&mut cmd).with_context(|| format!("Failed to query rust toolchain version via `{cmd:?}`, is your toolchain setup correctly?"))?;