Environment loading fixes (#19144)

Stanislav Alekseev created 1 year ago

Closes #19040
Addresses the problem with annoying error messages on windows (see
comment from SomeoneToIgnore on #18567)

Release Notes:

- Fixed the bug where language servers from PATH would sometimes be
prioritised over the ones from `direnv`
- Stopped running environment loading on windows as it didn't work
anyways due to `SHELL` not being set

Change summary

crates/project/src/direnv.rs      | 10 +++++--
crates/project/src/environment.rs | 42 ++++++++++++++++++++++-----------
2 files changed, 35 insertions(+), 17 deletions(-)

Detailed changes

crates/project/src/direnv.rs

@@ -1,7 +1,7 @@
 use crate::environment::EnvironmentErrorMessage;
 use std::process::ExitStatus;
 
-#[cfg(not(any(test, feature = "test-support")))]
+#[cfg(not(any(target_os = "windows", test, feature = "test-support")))]
 use {collections::HashMap, std::path::Path, util::ResultExt};
 
 #[derive(Clone)]
@@ -30,14 +30,18 @@ impl From<DirenvError> for Option<EnvironmentErrorMessage> {
     }
 }
 
-#[cfg(not(any(test, feature = "test-support")))]
-pub async fn load_direnv_environment(dir: &Path) -> Result<HashMap<String, String>, DirenvError> {
+#[cfg(not(any(target_os = "windows", test, feature = "test-support")))]
+pub async fn load_direnv_environment(
+    env: &HashMap<String, String>,
+    dir: &Path,
+) -> Result<HashMap<String, String>, DirenvError> {
     let Ok(direnv_path) = which::which("direnv") else {
         return Err(DirenvError::NotFound);
     };
 
     let Some(direnv_output) = smol::process::Command::new(direnv_path)
         .args(["export", "json"])
+        .envs(env)
         .env("TERM", "dumb")
         .current_dir(dir)
         .output()

crates/project/src/environment.rs

@@ -208,7 +208,19 @@ async fn load_shell_environment(
     (Some(fake_env), None)
 }
 
-#[cfg(not(any(test, feature = "test-support")))]
+#[cfg(all(target_os = "windows", not(any(test, feature = "test-support"))))]
+async fn load_shell_environment(
+    _dir: &Path,
+    _load_direnv: &DirenvSettings,
+) -> (
+    Option<HashMap<String, String>>,
+    Option<EnvironmentErrorMessage>,
+) {
+    // TODO the current code works with Unix $SHELL only, implement environment loading on windows
+    (None, None)
+}
+
+#[cfg(not(any(target_os = "windows", test, feature = "test-support")))]
 async fn load_shell_environment(
     dir: &Path,
     load_direnv: &DirenvSettings,
@@ -225,18 +237,6 @@ async fn load_shell_environment(
         (None, Some(message))
     }
 
-    let (direnv_environment, direnv_error) = match load_direnv {
-        DirenvSettings::ShellHook => (None, None),
-        DirenvSettings::Direct => match load_direnv_environment(dir).await {
-            Ok(env) => (Some(env), None),
-            Err(err) => (
-                None,
-                <Option<EnvironmentErrorMessage> as From<DirenvError>>::from(err),
-            ),
-        },
-    };
-    let direnv_environment = direnv_environment.unwrap_or(HashMap::default());
-
     let marker = "ZED_SHELL_START";
     let Some(shell) = std::env::var("SHELL").log_err() else {
         return message("Failed to get login environment. SHELL environment variable is not set");
@@ -279,7 +279,6 @@ async fn load_shell_environment(
 
     let Some(output) = smol::process::Command::new(&shell)
         .args(["-l", "-i", "-c", &command])
-        .envs(direnv_environment)
         .output()
         .await
         .log_err()
@@ -305,5 +304,20 @@ async fn load_shell_environment(
         parsed_env.insert(key, value);
     });
 
+    let (direnv_environment, direnv_error) = match load_direnv {
+        DirenvSettings::ShellHook => (None, None),
+        DirenvSettings::Direct => match load_direnv_environment(&parsed_env, dir).await {
+            Ok(env) => (Some(env), None),
+            Err(err) => (
+                None,
+                <Option<EnvironmentErrorMessage> as From<DirenvError>>::from(err),
+            ),
+        },
+    };
+
+    for (key, value) in direnv_environment.unwrap_or(HashMap::default()) {
+        parsed_env.insert(key, value);
+    }
+
     (Some(parsed_env), direnv_error)
 }