Repository: rust-lang/rust-analyzer
Pull request #18807 (2024-12-31 13:52:24)

fix: Populate cargo config env vars for crates
Noticed we weren't actually filling these in yet

Fixes https://github.com/rust-lang/rust-analyzer/pull/18807


Patch:
diff --git a/crates/project-model/src/cargo_workspace.rs b/crates/project-model/src/cargo_workspace.rs
index 4ffe3a6265..c125c141cd 100644
--- a/crates/project-model/src/cargo_workspace.rs
+++ b/crates/project-model/src/cargo_workspace.rs
@@ -4,6 +4,7 @@ use std::ops;
 use std::str::from_utf8;
 
 use anyhow::Context;
+use base_db::Env;
 use cargo_metadata::{CargoOpt, MetadataCommand};
 use la_arena::{Arena, Idx};
 use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
@@ -34,6 +35,8 @@ pub struct CargoWorkspace {
     target_directory: AbsPathBuf,
     manifest_path: ManifestPath,
     is_virtual_workspace: bool,
+    /// Environment variables set in the `.cargo/config` file.
+    config_env: Env,
 }
 
 impl ops::Index<Package> for CargoWorkspace {
@@ -395,6 +398,7 @@ impl CargoWorkspace {
     pub fn new(
         mut meta: cargo_metadata::Metadata,
         ws_manifest_path: ManifestPath,
+        cargo_config_env: Env,
     ) -> CargoWorkspace {
         let mut pkg_by_id = FxHashMap::default();
         let mut packages = Arena::default();
@@ -516,6 +520,7 @@ impl CargoWorkspace {
             target_directory,
             manifest_path: ws_manifest_path,
             is_virtual_workspace,
+            config_env: cargo_config_env,
         }
     }
 
@@ -602,4 +607,8 @@ impl CargoWorkspace {
     pub fn is_virtual_workspace(&self) -> bool {
         self.is_virtual_workspace
     }
+
+    pub fn env(&self) -> &Env {
+        &self.config_env
+    }
 }
diff --git a/crates/project-model/src/env.rs b/crates/project-model/src/env.rs
index b4714b764a..53c35ef6a7 100644
--- a/crates/project-model/src/env.rs
+++ b/crates/project-model/src/env.rs
@@ -1,5 +1,6 @@
 //! Cargo-like environment variables injection.
 use base_db::Env;
+use paths::Utf8Path;
 use rustc_hash::FxHashMap;
 use toolchain::Tool;
 
@@ -73,7 +74,7 @@ pub(crate) fn cargo_config_env(
     manifest: &ManifestPath,
     extra_env: &FxHashMap<String, String>,
     sysroot: &Sysroot,
-) -> FxHashMap<String, String> {
+) -> Env {
     let mut cargo_config = sysroot.tool(Tool::Cargo, manifest.parent());
     cargo_config.envs(extra_env);
     cargo_config
@@ -85,7 +86,7 @@ pub(crate) fn cargo_config_env(
     // if successful we receive `env.key.value = "value" per entry
     tracing::debug!("Discovering cargo config env by {:?}", cargo_config);
     utf8_stdout(&mut cargo_config)
-        .map(parse_output_cargo_config_env)
+        .map(|stdout| parse_output_cargo_config_env(manifest, stdout))
         .inspect(|env| {
             tracing::debug!("Discovered cargo config env: {:?}", env);
         })
@@ -95,18 +96,38 @@ pub(crate) fn cargo_config_env(
         .unwrap_or_default()
 }
 
-fn parse_output_cargo_config_env(stdout: String) -> FxHashMap<String, String> {
-    stdout
-        .lines()
-        .filter_map(|l| l.strip_prefix("env."))
-        .filter_map(|l| l.split_once(" = "))
-        .filter_map(|(k, v)| {
-            if k.contains('.') {
-                k.strip_suffix(".value").zip(Some(v))
-            } else {
-                Some((k, v))
+fn parse_output_cargo_config_env(manifest: &ManifestPath, stdout: String) -> Env {
+    let mut env = Env::default();
+    let mut relatives = vec![];
+    for (key, val) in
+        stdout.lines().filter_map(|l| l.strip_prefix("env.")).filter_map(|l| l.split_once(" = "))
+    {
+        let val = val.trim_matches('"').to_owned();
+        if let Some((key, modifier)) = key.split_once('.') {
+            match modifier {
+                "relative" => relatives.push((key, val)),
+                "value" => _ = env.insert(key, val),
+                _ => {
+                    tracing::warn!(
+                        "Unknown modifier in cargo config env: {}, expected `relative` or `value`",
+                        modifier
+                    );
+                    continue;
+                }
             }
-        })
-        .map(|(key, value)| (key.to_owned(), value.trim_matches('"').to_owned()))
-        .collect()
+        } else {
+            env.insert(key, val);
+        }
+    }
+    // FIXME: The base here should be the parent of the `.cargo/config` file, not the manifest.
+    // But cargo does not provide this information.
+    let base = <_ as AsRef<Utf8Path>>::as_ref(manifest.parent());
+    for (key, val) in relatives {
+        if let Some(val) = env.get(&val) {
+            env.insert(key, base.join(val).to_string());
+        } else {
+            env.insert(key, base.to_string());
+        }
+    }
+    env
 }
diff --git a/crates/project-model/src/sysroot.rs b/crates/project-model/src/sysroot.rs
index b0fdd3fa41..9d3dc37c58 100644
--- a/crates/project-model/src/sysroot.rs
+++ b/crates/project-model/src/sysroot.rs
@@ -422,7 +422,7 @@ impl Sysroot {
             res.packages.remove(idx);
         });
 
-        let cargo_workspace = CargoWorkspace::new(res, library_manifest);
+        let cargo_workspace = CargoWorkspace::new(res, library_manifest, Default::default());
         Some(Sysroot {
             root: sysroot_dir.clone(),
             src_root: Some(sysroot_src_dir.clone()),
diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs
index 233f94203e..9598316178 100644
--- a/crates/project-model/src/workspace.rs
+++ b/crates/project-model/src/workspace.rs
@@ -78,8 +78,6 @@ pub enum ProjectWorkspaceKind {
         /// The rustc workspace loaded for this workspace. An `Err(None)` means loading has been
         /// disabled or was otherwise not requested.
         rustc: Result<Box<(CargoWorkspace, WorkspaceBuildScripts)>, Option<String>>,
-        /// Environment variables set in the `.cargo/config` file.
-        cargo_config_extra_env: FxHashMap<String, String>,
         set_test: bool,
     },
     /// Project workspace was specified using a `rust-project.json` file.
@@ -99,8 +97,6 @@ pub enum ProjectWorkspaceKind {
         file: ManifestPath,
         /// Is this file a cargo script file?
         cargo: Option<(CargoWorkspace, WorkspaceBuildScripts, Option<Arc<anyhow::Error>>)>,
-        /// Environment variables set in the `.cargo/config` file.
-        cargo_config_extra_env: FxHashMap<String, String>,
         set_test: bool,
     },
 }
@@ -110,14 +106,7 @@ impl fmt::Debug for ProjectWorkspace {
         // Make sure this isn't too verbose.
         let Self { kind, sysroot, rustc_cfg, toolchain, target_layout, cfg_overrides } = self;
         match kind {
-            ProjectWorkspaceKind::Cargo {
-                cargo,
-                error: _,
-                build_scripts,
-                rustc,
-                cargo_config_extra_env,
-                set_test,
-            } => f
+            ProjectWorkspaceKind::Cargo { cargo, error: _, build_scripts, rustc, set_test } => f
                 .debug_struct("Cargo")
                 .field("root", &cargo.workspace_root().file_name())
                 .field("n_packages", &cargo.packages().len())
@@ -130,7 +119,6 @@ impl fmt::Debug for ProjectWorkspace {
                 .field("n_cfg_overrides", &cfg_overrides.len())
                 .field("toolchain", &toolchain)
                 .field("data_layout", &target_layout)
-                .field("cargo_config_extra_env", &cargo_config_extra_env)
                 .field("set_test", set_test)
                 .field("build_scripts", &build_scripts.error().unwrap_or("ok"))
                 .finish(),
@@ -146,12 +134,7 @@ impl fmt::Debug for ProjectWorkspace {
 
                 debug_struct.finish()
             }
-            ProjectWorkspaceKind::DetachedFile {
-                file,
-                cargo: cargo_script,
-                cargo_config_extra_env,
-                set_test,
-            } => f
+            ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, set_test } => f
                 .debug_struct("DetachedFiles")
                 .field("file", &file)
                 .field("cargo_script", &cargo_script.is_some())
@@ -161,7 +144,6 @@ impl fmt::Debug for ProjectWorkspace {
                 .field("toolchain", &toolchain)
                 .field("data_layout", &target_layout)
                 .field("n_cfg_overrides", &cfg_overrides.len())
-                .field("cargo_config_extra_env", &cargo_config_extra_env)
                 .field("set_test", set_test)
                 .finish(),
         }
@@ -289,14 +271,14 @@ impl ProjectWorkspace {
                 progress,
             ) {
                 Ok((meta, _error)) => {
-                    let workspace = CargoWorkspace::new(meta, cargo_toml.clone());
-                    let buildscripts = WorkspaceBuildScripts::rustc_crates(
+                    let workspace = CargoWorkspace::new(meta, cargo_toml.clone(), Env::default());
+                    let build_scripts = WorkspaceBuildScripts::rustc_crates(
                         &workspace,
                         cargo_toml.parent(),
                         &config.extra_env,
                         &sysroot,
                     );
-                    Ok(Box::new((workspace, buildscripts)))
+                    Ok(Box::new((workspace, build_scripts)))
                 }
                 Err(e) => {
                     tracing::error!(
@@ -348,14 +330,13 @@ impl ProjectWorkspace {
                 "Failed to read Cargo metadata from Cargo.toml file {cargo_toml}, {toolchain:?}",
             )
         })?;
-        let cargo = CargoWorkspace::new(meta, cargo_toml.clone());
         let cargo_config_extra_env = cargo_config_env(cargo_toml, &config.extra_env, &sysroot);
+        let cargo = CargoWorkspace::new(meta, cargo_toml.clone(), cargo_config_extra_env);
         Ok(ProjectWorkspace {
             kind: ProjectWorkspaceKind::Cargo {
                 cargo,
                 build_scripts: WorkspaceBuildScripts::default(),
                 rustc,
-                cargo_config_extra_env,
                 error: error.map(Arc::new),
                 set_test: config.set_test,
             },
@@ -450,19 +431,19 @@ impl ProjectWorkspace {
         )
         .ok()
         .map(|(ws, error)| {
+            let cargo_config_extra_env =
+                cargo_config_env(detached_file, &config.extra_env, &sysroot);
             (
-                CargoWorkspace::new(ws, detached_file.clone()),
+                CargoWorkspace::new(ws, detached_file.clone(), cargo_config_extra_env),
                 WorkspaceBuildScripts::default(),
                 error.map(Arc::new),
             )
         });
 
-        let cargo_config_extra_env = cargo_config_env(detached_file, &config.extra_env, &sysroot);
         Ok(ProjectWorkspace {
             kind: ProjectWorkspaceKind::DetachedFile {
                 file: detached_file.to_owned(),
                 cargo: cargo_script,
-                cargo_config_extra_env,
                 set_test: config.set_test,
             },
             sysroot,
@@ -643,14 +624,7 @@ impl ProjectWorkspace {
                 .chain(mk_sysroot())
                 .unique()
                 .collect(),
-            ProjectWorkspaceKind::Cargo {
-                cargo,
-                rustc,
-                build_scripts,
-                cargo_config_extra_env: _,
-                error: _,
-                set_test: _,
-            } => {
+            ProjectWorkspaceKind::Cargo { cargo, rustc, build_scripts, error: _, set_test: _ } => {
                 cargo
                     .packages()
                     .map(|pkg| {
@@ -787,23 +761,18 @@ impl ProjectWorkspace {
                 extra_env,
                 cfg_overrides,
             ),
-            ProjectWorkspaceKind::Cargo {
-                cargo,
-                rustc,
-                build_scripts,
-                cargo_config_extra_env: _,
-                error: _,
-                set_test,
-            } => cargo_to_crate_graph(
-                load,
-                rustc.as_ref().map(|a| a.as_ref()).ok(),
-                cargo,
-                sysroot,
-                rustc_cfg.clone(),
-                cfg_overrides,
-                build_scripts,
-                *set_test,
-            ),
+            ProjectWorkspaceKind::Cargo { cargo, rustc, build_scripts, error: _, set_test } => {
+                cargo_to_crate_graph(
+                    load,
+                    rustc.as_ref().map(|a| a.as_ref()).ok(),
+                    cargo,
+                    sysroot,
+                    rustc_cfg.clone(),
+                    cfg_overrides,
+                    build_scripts,
+                    *set_test,
+                )
+            }
             ProjectWorkspaceKind::DetachedFile { file, cargo: cargo_script, set_test, .. } => {
                 if let Some((cargo, build_scripts, _)) = cargo_script {
                     cargo_to_crate_graph(
@@ -848,7 +817,6 @@ impl ProjectWorkspace {
                 ProjectWorkspaceKind::Cargo {
                     cargo,
                     rustc,
-                    cargo_config_extra_env,
                     build_scripts: _,
                     error: _,
                     set_test: _,
@@ -856,16 +824,11 @@ impl ProjectWorkspace {
                 ProjectWorkspaceKind::Cargo {
                     cargo: o_cargo,
                     rustc: o_rustc,
-                    cargo_config_extra_env: o_cargo_config_extra_env,
                     build_scripts: _,
                     error: _,
                     set_test: _,
                 },
-            ) => {
-                cargo == o_cargo
-                    && rustc == o_rustc
-                    && cargo_config_extra_env == o_cargo_config_extra_env
-            }
+            ) => cargo == o_cargo && rustc == o_rustc,
             (ProjectWorkspaceKind::Json(project), ProjectWorkspaceKind::Json(o_project)) => {
                 project == o_project
             }
@@ -873,20 +836,14 @@ impl ProjectWorkspace {
                 ProjectWorkspaceKind::DetachedFile {
                     file,
                     cargo: Some((cargo_script, _, _)),
-                    cargo_config_extra_env,
                     set_test: _,
                 },
                 ProjectWorkspaceKind::DetachedFile {
                     file: o_file,
                     cargo: Some((o_cargo_script, _, _)),
-                    cargo_config_extra_env: o_cargo_config_extra_env,
                     set_test: _,
                 },
-            ) => {
-                file == o_file
-                    && cargo_script == o_cargo_script
-                    && cargo_config_extra_env == o_cargo_config_extra_env
-            }
+            ) => file == o_file && cargo_script == o_cargo_script,
             _ => return false,
         }) && sysroot == o_sysroot
             && rustc_cfg == o_rustc_cfg
@@ -1402,7 +1359,7 @@ fn add_target_crate_root(
         opts
     };
 
-    let mut env = Env::default();
+    let mut env = cargo.env().clone();
     inject_cargo_package_env(&mut env, pkg);
     inject_cargo_env(&mut env);
     inject_rustc_tool_env(&mut env, cargo, cargo_name, kind);
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs
index 1996c2b642..0add2cdf5a 100644
--- a/crates/rust-analyzer/src/reload.rs
+++ b/crates/rust-analyzer/src/reload.rs
@@ -630,13 +630,10 @@ impl GlobalState {
                 };
 
                 let env: FxHashMap<_, _> = match &ws.kind {
-                    ProjectWorkspaceKind::Cargo { cargo_config_extra_env, .. }
-                    | ProjectWorkspaceKind::DetachedFile {
-                        cargo: Some(_),
-                        cargo_config_extra_env,
-                        ..
-                    } => cargo_config_extra_env
-                        .iter()
+                    ProjectWorkspaceKind::Cargo { cargo, .. }
+                    | ProjectWorkspaceKind::DetachedFile { cargo: Some((cargo, ..)), .. } => cargo
+                        .env()
+                        .into_iter()
                         .chain(self.config.extra_env(None))
                         .map(|(a, b)| (a.clone(), b.clone()))
                         .chain(


Test patch:
diff --git a/crates/project-model/src/tests.rs b/crates/project-model/src/tests.rs
index 9fc2d6116a..cf77b875d8 100644
--- a/crates/project-model/src/tests.rs
+++ b/crates/project-model/src/tests.rs
@@ -28,13 +28,12 @@ fn load_cargo_with_overrides(
     let meta: Metadata = get_test_json_file(file);
     let manifest_path =
         ManifestPath::try_from(AbsPathBuf::try_from(meta.workspace_root.clone()).unwrap()).unwrap();
-    let cargo_workspace = CargoWorkspace::new(meta, manifest_path);
+    let cargo_workspace = CargoWorkspace::new(meta, manifest_path, Default::default());
     let project_workspace = ProjectWorkspace {
         kind: ProjectWorkspaceKind::Cargo {
             cargo: cargo_workspace,
             build_scripts: WorkspaceBuildScripts::default(),
             rustc: Err(None),
-            cargo_config_extra_env: Default::default(),
             error: None,
             set_test: true,
         },
@@ -228,7 +227,7 @@ fn smoke_test_real_sysroot_cargo() {
     let meta: Metadata = get_test_json_file("hello-world-metadata.json");
     let manifest_path =
         ManifestPath::try_from(AbsPathBuf::try_from(meta.workspace_root.clone()).unwrap()).unwrap();
-    let cargo_workspace = CargoWorkspace::new(meta, manifest_path);
+    let cargo_workspace = CargoWorkspace::new(meta, manifest_path, Default::default());
     let sysroot = Sysroot::discover(
         AbsPath::assert(Utf8Path::new(env!("CARGO_MANIFEST_DIR"))),
         &Default::default(),
@@ -240,7 +239,6 @@ fn smoke_test_real_sysroot_cargo() {
             cargo: cargo_workspace,
             build_scripts: WorkspaceBuildScripts::default(),
             rustc: Err(None),
-            cargo_config_extra_env: Default::default(),
             error: None,
             set_test: true,
         },
diff --git a/crates/rust-analyzer/src/cli/rustc_tests.rs b/crates/rust-analyzer/src/cli/rustc_tests.rs
index dabc71b1b9..e45492e17e 100644
--- a/crates/rust-analyzer/src/cli/rustc_tests.rs
+++ b/crates/rust-analyzer/src/cli/rustc_tests.rs
@@ -89,7 +89,6 @@ impl Tester {
             kind: ProjectWorkspaceKind::DetachedFile {
                 file: ManifestPath::try_from(tmp_file).unwrap(),
                 cargo: None,
-                cargo_config_extra_env: Default::default(),
                 set_test: true,
             },
             sysroot,