From 554381f8055bb81b07ae6f7c61281aac214f086c Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Wed, 27 Aug 2025 15:28:32 -0700 Subject: [PATCH] Allow env vars set in cargo.extraEnv to be resolved by the env! macro --- crates/project-model/src/cargo_workspace.rs | 11 +-- crates/project-model/src/env.rs | 78 ++++++++++++++++----- crates/project-model/src/workspace.rs | 41 ++++++----- 3 files changed, 91 insertions(+), 39 deletions(-) diff --git a/crates/project-model/src/cargo_workspace.rs b/crates/project-model/src/cargo_workspace.rs index 04fb2275893c..76ba01f3a263 100644 --- a/crates/project-model/src/cargo_workspace.rs +++ b/crates/project-model/src/cargo_workspace.rs @@ -49,8 +49,9 @@ pub struct CargoWorkspace { is_virtual_workspace: bool, /// Whether this workspace represents the sysroot workspace. is_sysroot: bool, - /// Environment variables set in the `.cargo/config` file. - config_env: Env, + /// Environment variables set in the `.cargo/config` file and the extraEnv + /// configuration option. + env: Env, requires_rustc_private: bool, } @@ -325,7 +326,7 @@ impl CargoWorkspace { pub fn new( mut meta: cargo_metadata::Metadata, ws_manifest_path: ManifestPath, - cargo_config_env: Env, + cargo_env: Env, is_sysroot: bool, ) -> CargoWorkspace { let mut pkg_by_id = FxHashMap::default(); @@ -498,7 +499,7 @@ impl CargoWorkspace { is_virtual_workspace, requires_rustc_private, is_sysroot, - config_env: cargo_config_env, + env: cargo_env, } } @@ -589,7 +590,7 @@ impl CargoWorkspace { } pub fn env(&self) -> &Env { - &self.config_env + &self.env } pub fn is_sysroot(&self) -> bool { diff --git a/crates/project-model/src/env.rs b/crates/project-model/src/env.rs index d281492fc98c..ae0458af7aa7 100644 --- a/crates/project-model/src/env.rs +++ b/crates/project-model/src/env.rs @@ -1,6 +1,7 @@ //! Cargo-like environment variables injection. use base_db::Env; use paths::Utf8Path; +use rustc_hash::FxHashMap; use toolchain::Tool; use crate::{ManifestPath, PackageData, TargetKind, cargo_config_file::CargoConfigFile}; @@ -60,8 +61,14 @@ pub(crate) fn inject_rustc_tool_env(env: &mut Env, cargo_name: &str, kind: Targe env.set("CARGO_CRATE_NAME", cargo_name.replace('-', "_")); } -pub(crate) fn cargo_config_env(manifest: &ManifestPath, config: &Option) -> Env { +pub(crate) fn cargo_config_env( + manifest: &ManifestPath, + config: &Option, + extra_env: &FxHashMap>, +) -> Env { let mut env = Env::default(); + env.extend(extra_env.iter().filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone())))); + let Some(serde_json::Value::Object(env_json)) = config.as_ref().and_then(|c| c.get("env")) else { return env; @@ -72,22 +79,34 @@ pub(crate) fn cargo_config_env(manifest: &ManifestPath, config: &Option>::as_ref(manifest.parent()); for (key, entry) in env_json { - let serde_json::Value::Object(entry) = entry else { - continue; - }; - let Some(value) = entry.get("value").and_then(|v| v.as_str()) else { - continue; - }; + let value = match entry { + serde_json::Value::String(s) => s.clone(), + serde_json::Value::Object(entry) => { + // Each entry MUST have a `value` key. + let Some(value) = entry.get("value").and_then(|v| v.as_str()) else { + continue; + }; + // If the entry already exists in the environment AND the `force` key is not set to + // true, then don't overwrite the value. + if extra_env.get(key).is_some_and(Option::is_some) + && !entry.get("force").and_then(|v| v.as_bool()).unwrap_or(false) + { + continue; + } - let value = if entry - .get("relative") - .and_then(|v| v.as_bool()) - .is_some_and(std::convert::identity) - { - base.join(value).to_string() - } else { - value.to_owned() + if entry + .get("relative") + .and_then(|v| v.as_bool()) + .is_some_and(std::convert::identity) + { + base.join(value).to_string() + } else { + value.to_owned() + } + } + _ => continue, }; + env.insert(key, value); } @@ -113,7 +132,19 @@ fn parse_output_cargo_config_env_works() { }, "TEST": { "value": "test" - } + }, + "FORCED": { + "value": "test", + "force": true + }, + "UNFORCED": { + "value": "test", + "force": false + }, + "OVERWRITTEN": { + "value": "test" + }, + "NOT_AN_OBJECT": "value" } } "#; @@ -121,9 +152,22 @@ fn parse_output_cargo_config_env_works() { let cwd = paths::Utf8PathBuf::try_from(std::env::current_dir().unwrap()).unwrap(); let manifest = paths::AbsPathBuf::assert(cwd.join("Cargo.toml")); let manifest = ManifestPath::try_from(manifest).unwrap(); - let env = cargo_config_env(&manifest, &Some(config)); + let extra_env = [ + ("FORCED", Some("ignored")), + ("UNFORCED", Some("newvalue")), + ("OVERWRITTEN", Some("newvalue")), + ("TEST", None), + ] + .iter() + .map(|(k, v)| (k.to_string(), v.map(ToString::to_string))) + .collect(); + let env = cargo_config_env(&manifest, &Some(config), &extra_env); assert_eq!(env.get("CARGO_WORKSPACE_DIR").as_deref(), Some(cwd.join("").as_str())); assert_eq!(env.get("RELATIVE").as_deref(), Some(cwd.join("../relative").as_str())); assert_eq!(env.get("INVALID").as_deref(), Some("../relative")); assert_eq!(env.get("TEST").as_deref(), Some("test")); + assert_eq!(env.get("FORCED").as_deref(), Some("test")); + assert_eq!(env.get("UNFORCED").as_deref(), Some("newvalue")); + assert_eq!(env.get("OVERWRITTEN").as_deref(), Some("newvalue")); + assert_eq!(env.get("NOT_AN_OBJECT").as_deref(), Some("value")); } diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 22b84791aee9..b88db419574d 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -387,36 +387,36 @@ impl ProjectWorkspace { progress, ) }); - let cargo_config_extra_env = - s.spawn(move || cargo_config_env(cargo_toml, &config_file)); + let cargo_env = + s.spawn(move || cargo_config_env(cargo_toml, &config_file, &config.extra_env)); thread::Result::Ok(( rustc_cfg.join()?, target_data.join()?, rustc_dir.join()?, loaded_sysroot.join()?, cargo_metadata.join()?, - cargo_config_extra_env.join()?, + cargo_env.join()?, )) }); - let ( - rustc_cfg, - data_layout, - mut rustc, - loaded_sysroot, - cargo_metadata, - cargo_config_extra_env, - ) = match join { - Ok(it) => it, - Err(e) => std::panic::resume_unwind(e), - }; + let (rustc_cfg, data_layout, mut rustc, loaded_sysroot, cargo_metadata, mut cargo_env) = + match join { + Ok(it) => it, + Err(e) => std::panic::resume_unwind(e), + }; + + for (key, value) in config.extra_env.iter() { + if let Some(value) = value { + cargo_env.insert(key.clone(), value.clone()); + } + } let (meta, error) = cargo_metadata.with_context(|| { format!( "Failed to read Cargo metadata from Cargo.toml file {cargo_toml}, {toolchain:?}", ) })?; - let cargo = CargoWorkspace::new(meta, cargo_toml.clone(), cargo_config_extra_env, false); + let cargo = CargoWorkspace::new(meta, cargo_toml.clone(), cargo_env, false); if let Some(loaded_sysroot) = loaded_sysroot { tracing::info!(src_root = ?sysroot.rust_lib_src_root(), root = %loaded_sysroot, "Loaded sysroot"); sysroot.set_workspace(loaded_sysroot); @@ -586,7 +586,8 @@ impl ProjectWorkspace { .unwrap_or_else(|| dir.join("target").into()); let cargo_script = fetch_metadata.exec(&target_dir, false, &|_| ()).ok().map(|(ws, error)| { - let cargo_config_extra_env = cargo_config_env(detached_file, &config_file); + let cargo_config_extra_env = + cargo_config_env(detached_file, &config_file, &config.extra_env); ( CargoWorkspace::new(ws, detached_file.clone(), cargo_config_extra_env, false), WorkspaceBuildScripts::default(), @@ -1089,7 +1090,13 @@ fn project_json_to_crate_graph( }, file_id, )| { - let env = env.clone().into_iter().collect(); + let mut env = env.clone().into_iter().collect::(); + // Override existing env vars with those from `extra_env` + env.extend( + extra_env + .iter() + .filter_map(|(k, v)| v.as_ref().map(|v| (k.clone(), v.clone()))), + ); let target_cfgs = match target.as_deref() { Some(target) => cfg_cache.entry(target).or_insert_with(|| {