Merge pull request #20554 from dpaoliello/extraenv

ChayimFriedman2 · web-flow · commit 40cd34da04a0 · 2025-10-21T18:10:57.000Z
Improvements for resolving the value of the `env!` macro
diff --git 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
@@ -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<CargoConfigFile>) -> Env {
+pub(crate) fn cargo_config_env(
+    manifest: &ManifestPath,
+    config: &Option<CargoConfigFile>,
+    extra_env: &FxHashMap<String, Option<String>>,
+) -> 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<CargoCon
     let base = <_ as AsRef<Utf8Path>>::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,17 +132,42 @@ 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"
   }
 }
 "#;
     let config: CargoConfigFile = serde_json::from_str(raw).unwrap();
     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
@@ -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::<Env>();
+                // 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(|| {
