terminal: Resolve env based on the project dir on the target (#41867)

Jakub Konka created

Prior to this change we would always resolve envs when spawning a new
terminal window based on the inherited CLI environment. This works fine
as long as we open a new Zed instance in the terminal when using it
locally only. When using Zed connected to a remote server, it would not
be meaningful however. WIth this change, we correctly ping the remote
for the project-local envs and use that instead. This change should also
fix a pesky issue when updating Zed - after Zed restarts, opening a new
terminal window will not run `direnv` for example.

Release Notes:

- N/A

Change summary

crates/project/src/terminals.rs | 149 ++++++++++++++++++++--------------
crates/vim/src/command.rs       |  24 +++--
2 files changed, 100 insertions(+), 73 deletions(-)

Detailed changes

crates/project/src/terminals.rs 🔗

@@ -2,6 +2,7 @@ use anyhow::Result;
 use collections::HashMap;
 use gpui::{App, AppContext as _, Context, Entity, Task, WeakEntity};
 
+use futures::{FutureExt, future::Shared};
 use itertools::Itertools as _;
 use language::LanguageName;
 use remote::RemoteClient;
@@ -75,16 +76,6 @@ impl Project {
 
         let (completion_tx, completion_rx) = bounded(1);
 
-        // Start with the environment that we might have inherited from the Zed CLI.
-        let mut env = self
-            .environment
-            .read(cx)
-            .get_cli_environment()
-            .unwrap_or_default();
-        // Then extend it with the explicit env variables from the settings, so they take
-        // precedence.
-        env.extend(settings.env);
-
         let local_path = if is_via_remote { None } else { path.clone() };
         let task_state = Some(TaskState {
             spawned_task: spawn_task.clone(),
@@ -99,8 +90,12 @@ impl Project {
                 .unwrap_or_else(get_default_system_shell),
             None => settings.shell.program(),
         };
-
         let is_windows = self.path_style(cx).is_windows();
+        let shell_kind = ShellKind::new(&shell, is_windows);
+
+        // Prepare a task for resolving the environment
+        let env_task =
+            self.resolve_directory_environment(&shell, path.clone(), remote_client.clone(), cx);
 
         let project_path_contexts = self
             .active_entry()
@@ -120,7 +115,8 @@ impl Project {
             .collect::<Vec<_>>();
         let lang_registry = self.languages.clone();
         cx.spawn(async move |project, cx| {
-            let shell_kind = ShellKind::new(&shell, is_windows);
+            let mut env = env_task.await.unwrap_or_default();
+            env.extend(settings.env);
 
             let activation_script = maybe!(async {
                 for toolchain in toolchains {
@@ -295,17 +291,6 @@ impl Project {
         }
         let settings = TerminalSettings::get(settings_location, cx).clone();
         let detect_venv = settings.detect_venv.as_option().is_some();
-
-        // Start with the environment that we might have inherited from the Zed CLI.
-        let mut env = self
-            .environment
-            .read(cx)
-            .get_cli_environment()
-            .unwrap_or_default();
-        // Then extend it with the explicit env variables from the settings, so they take
-        // precedence.
-        env.extend(settings.env);
-
         let local_path = if is_via_remote { None } else { path.clone() };
 
         let project_path_contexts = self
@@ -332,11 +317,17 @@ impl Project {
                 .unwrap_or_else(get_default_system_shell),
             None => settings.shell.program(),
         };
-
         let shell_kind = ShellKind::new(&shell, self.path_style(cx).is_windows());
 
+        // Prepare a task for resolving the environment
+        let env_task =
+            self.resolve_directory_environment(&shell, path.clone(), remote_client.clone(), cx);
+
         let lang_registry = self.languages.clone();
         cx.spawn(async move |project, cx| {
+            let mut env = env_task.await.unwrap_or_default();
+            env.extend(settings.env);
+
             let activation_script = maybe!(async {
                 for toolchain in toolchains {
                     let Some(toolchain) = toolchain.await else {
@@ -469,9 +460,13 @@ impl Project {
         TerminalSettings::get(settings_location, cx)
     }
 
-    pub fn exec_in_shell(&self, command: String, cx: &App) -> Result<smol::process::Command> {
+    pub fn exec_in_shell(
+        &self,
+        command: String,
+        cx: &mut Context<Self>,
+    ) -> Task<Result<smol::process::Command>> {
         let path = self.first_project_directory(cx);
-        let remote_client = self.remote_client.as_ref();
+        let remote_client = self.remote_client.clone();
         let settings = self.terminal_settings(&path, cx).clone();
         let shell = remote_client
             .as_ref()
@@ -482,43 +477,77 @@ impl Project {
         let builder = ShellBuilder::new(&shell, is_windows).non_interactive();
         let (command, args) = builder.build(Some(command), &Vec::new());
 
-        let mut env = self
-            .environment
-            .read(cx)
-            .get_cli_environment()
-            .unwrap_or_default();
-        env.extend(settings.env);
-
-        match remote_client {
-            Some(remote_client) => {
-                let command_template =
-                    remote_client
-                        .read(cx)
-                        .build_command(Some(command), &args, &env, None, None)?;
-                let mut command = std::process::Command::new(command_template.program);
-                command.args(command_template.args);
-                command.envs(command_template.env);
-                Ok(command)
-            }
-            None => {
-                let mut command = std::process::Command::new(command);
-                command.args(args);
-                command.envs(env);
-                if let Some(path) = path {
-                    command.current_dir(path);
+        let env_task = self.resolve_directory_environment(
+            &shell.program(),
+            path.as_ref().map(|p| Arc::from(&**p)),
+            remote_client.clone(),
+            cx,
+        );
+
+        cx.spawn(async move |project, cx| {
+            let mut env = env_task.await.unwrap_or_default();
+            env.extend(settings.env);
+
+            project.update(cx, move |_, cx| {
+                match remote_client {
+                    Some(remote_client) => {
+                        let command_template = remote_client.read(cx).build_command(
+                            Some(command),
+                            &args,
+                            &env,
+                            None,
+                            None,
+                        )?;
+                        let mut command = std::process::Command::new(command_template.program);
+                        command.args(command_template.args);
+                        command.envs(command_template.env);
+                        Ok(command)
+                    }
+                    None => {
+                        let mut command = std::process::Command::new(command);
+                        command.args(args);
+                        command.envs(env);
+                        if let Some(path) = path {
+                            command.current_dir(path);
+                        }
+                        Ok(command)
+                    }
                 }
-                Ok(command)
-            }
-        }
-        .map(|mut process| {
-            util::set_pre_exec_to_start_new_session(&mut process);
-            smol::process::Command::from(process)
+                .map(|mut process| {
+                    util::set_pre_exec_to_start_new_session(&mut process);
+                    smol::process::Command::from(process)
+                })
+            })?
         })
     }
 
     pub fn local_terminal_handles(&self) -> &Vec<WeakEntity<terminal::Terminal>> {
         &self.terminals.local_handles
     }
+
+    fn resolve_directory_environment(
+        &self,
+        shell: &str,
+        path: Option<Arc<Path>>,
+        remote_client: Option<Entity<RemoteClient>>,
+        cx: &mut App,
+    ) -> Shared<Task<Option<HashMap<String, String>>>> {
+        if let Some(path) = &path {
+            let shell = Shell::Program(shell.to_string());
+            self.environment
+                .update(cx, |project_env, cx| match &remote_client {
+                    Some(remote_client) => project_env.remote_directory_environment(
+                        &shell,
+                        path.clone(),
+                        remote_client.clone(),
+                        cx,
+                    ),
+                    None => project_env.local_directory_environment(&shell, path.clone(), cx),
+                })
+        } else {
+            Task::ready(None).shared()
+        }
+    }
 }
 
 fn create_remote_shell(
@@ -528,12 +557,8 @@ fn create_remote_shell(
     remote_client: Entity<RemoteClient>,
     cx: &mut App,
 ) -> Result<(Shell, HashMap<String, String>)> {
-    // Alacritty sets its terminfo to `alacritty`, this requiring hosts to have it installed
-    // to properly display colors.
-    // We do not have the luxury of assuming the host has it installed,
-    // so we set it to a default that does not break the highlighting via ssh.
-    env.entry("TERM".to_string())
-        .or_insert_with(|| "xterm-256color".to_string());
+    // Set default terminfo that does not break the highlighting via ssh.
+    env.insert("TERM".to_string(), "xterm-256color".to_string());
 
     let (program, args) = match spawn_command {
         Some((program, args)) => (Some(program.clone()), args),

crates/vim/src/command.rs 🔗

@@ -2194,21 +2194,23 @@ impl ShellExec {
 
         let Some(range) = input_range else { return };
 
-        let Some(mut process) = project.read(cx).exec_in_shell(command, cx).log_err() else {
-            return;
-        };
-        process.stdout(Stdio::piped());
-        process.stderr(Stdio::piped());
-
-        if input_snapshot.is_some() {
-            process.stdin(Stdio::piped());
-        } else {
-            process.stdin(Stdio::null());
-        };
+        let process_task = project.update(cx, |project, cx| project.exec_in_shell(command, cx));
 
         let is_read = self.is_read;
 
         let task = cx.spawn_in(window, async move |vim, cx| {
+            let Some(mut process) = process_task.await.log_err() else {
+                return;
+            };
+            process.stdout(Stdio::piped());
+            process.stderr(Stdio::piped());
+
+            if input_snapshot.is_some() {
+                process.stdin(Stdio::piped());
+            } else {
+                process.stdin(Stdio::null());
+            };
+
             let Some(mut running) = process.spawn().log_err() else {
                 vim.update_in(cx, |vim, window, cx| {
                     vim.cancel_running_command(window, cx);