remote: Fetch shell on ssh remote to use for preparing commands (#36690)

Lukas Wirth created

Prerequisite for https://github.com/zed-industries/zed/pull/36576 to
allow us to differentiate the shell in a remote.

Release Notes:

- N/A

Change summary

crates/debugger_ui/src/session/running.rs     |  7 +
crates/project/src/debugger/dap_store.rs      | 13 +++-
crates/project/src/debugger/locators/cargo.rs |  4 
crates/project/src/terminals.rs               | 36 +++++++++----
crates/remote/src/remote.rs                   |  4 
crates/remote/src/ssh_session.rs              | 38 +++++++++++++
crates/task/src/shell_builder.rs              | 54 ++++++++++++--------
crates/task/src/task.rs                       |  2 
crates/terminal_view/src/terminal_panel.rs    | 13 +++-
crates/util/src/paths.rs                      |  2 
10 files changed, 121 insertions(+), 52 deletions(-)

Detailed changes

crates/debugger_ui/src/session/running.rs 🔗

@@ -916,7 +916,10 @@ impl RunningState {
         let task_store = project.read(cx).task_store().downgrade();
         let weak_project = project.downgrade();
         let weak_workspace = workspace.downgrade();
-        let is_local = project.read(cx).is_local();
+        let ssh_info = project
+            .read(cx)
+            .ssh_client()
+            .and_then(|it| it.read(cx).ssh_info());
 
         cx.spawn_in(window, async move |this, cx| {
             let DebugScenario {
@@ -1000,7 +1003,7 @@ impl RunningState {
                     None
                 };
 
-                let builder = ShellBuilder::new(is_local, &task.resolved.shell);
+                let builder = ShellBuilder::new(ssh_info.as_ref().map(|info| &*info.shell), &task.resolved.shell);
                 let command_label = builder.command_label(&task.resolved.command_label);
                 let (command, args) =
                     builder.build(task.resolved.command.clone(), &task.resolved.args);

crates/project/src/debugger/dap_store.rs 🔗

@@ -34,7 +34,7 @@ use http_client::HttpClient;
 use language::{Buffer, LanguageToolchainStore, language_settings::InlayHintKind};
 use node_runtime::NodeRuntime;
 
-use remote::{SshRemoteClient, ssh_session::SshArgs};
+use remote::{SshInfo, SshRemoteClient, ssh_session::SshArgs};
 use rpc::{
     AnyProtoClient, TypedEnvelope,
     proto::{self},
@@ -254,14 +254,18 @@ impl DapStore {
                 cx.spawn(async move |_, cx| {
                     let response = request.await?;
                     let binary = DebugAdapterBinary::from_proto(response)?;
-                    let (mut ssh_command, envs, path_style) =
+                    let (mut ssh_command, envs, path_style, ssh_shell) =
                         ssh_client.read_with(cx, |ssh, _| {
-                            let (SshArgs { arguments, envs }, path_style) =
-                                ssh.ssh_info().context("SSH arguments not found")?;
+                            let SshInfo {
+                                args: SshArgs { arguments, envs },
+                                path_style,
+                                shell,
+                            } = ssh.ssh_info().context("SSH arguments not found")?;
                             anyhow::Ok((
                                 SshCommand { arguments },
                                 envs.unwrap_or_default(),
                                 path_style,
+                                shell,
                             ))
                         })??;
 
@@ -280,6 +284,7 @@ impl DapStore {
                     }
 
                     let (program, args) = wrap_for_ssh(
+                        &ssh_shell,
                         &ssh_command,
                         binary
                             .command

crates/project/src/debugger/locators/cargo.rs 🔗

@@ -117,7 +117,7 @@ impl DapLocator for CargoLocator {
             .cwd
             .clone()
             .context("Couldn't get cwd from debug config which is needed for locators")?;
-        let builder = ShellBuilder::new(true, &build_config.shell).non_interactive();
+        let builder = ShellBuilder::new(None, &build_config.shell).non_interactive();
         let (program, args) = builder.build(
             Some("cargo".into()),
             &build_config
@@ -126,7 +126,7 @@ impl DapLocator for CargoLocator {
                 .cloned()
                 .take_while(|arg| arg != "--")
                 .chain(Some("--message-format=json".to_owned()))
-                .collect(),
+                .collect::<Vec<_>>(),
         );
         let mut child = util::command::new_smol_command(program)
             .args(args)

crates/project/src/terminals.rs 🔗

@@ -4,7 +4,7 @@ use collections::HashMap;
 use gpui::{App, AppContext as _, Context, Entity, Task, WeakEntity};
 use itertools::Itertools;
 use language::LanguageName;
-use remote::ssh_session::SshArgs;
+use remote::{SshInfo, ssh_session::SshArgs};
 use settings::{Settings, SettingsLocation};
 use smol::channel::bounded;
 use std::{
@@ -13,7 +13,7 @@ use std::{
     path::{Path, PathBuf},
     sync::Arc,
 };
-use task::{DEFAULT_REMOTE_SHELL, Shell, ShellBuilder, SpawnInTerminal};
+use task::{Shell, ShellBuilder, SpawnInTerminal};
 use terminal::{
     TaskState, TaskStatus, Terminal, TerminalBuilder,
     terminal_settings::{self, ActivateScript, TerminalSettings, VenvSettings},
@@ -58,11 +58,13 @@ impl SshCommand {
     }
 }
 
+#[derive(Debug)]
 pub struct SshDetails {
     pub host: String,
     pub ssh_command: SshCommand,
     pub envs: Option<HashMap<String, String>>,
     pub path_style: PathStyle,
+    pub shell: String,
 }
 
 impl Project {
@@ -87,12 +89,18 @@ impl Project {
     pub fn ssh_details(&self, cx: &App) -> Option<SshDetails> {
         if let Some(ssh_client) = &self.ssh_client {
             let ssh_client = ssh_client.read(cx);
-            if let Some((SshArgs { arguments, envs }, path_style)) = ssh_client.ssh_info() {
+            if let Some(SshInfo {
+                args: SshArgs { arguments, envs },
+                path_style,
+                shell,
+            }) = ssh_client.ssh_info()
+            {
                 return Some(SshDetails {
                     host: ssh_client.connection_options().host,
                     ssh_command: SshCommand { arguments },
                     envs,
                     path_style,
+                    shell,
                 });
             }
         }
@@ -165,7 +173,9 @@ impl Project {
         let ssh_details = self.ssh_details(cx);
         let settings = self.terminal_settings(&path, cx).clone();
 
-        let builder = ShellBuilder::new(ssh_details.is_none(), &settings.shell).non_interactive();
+        let builder =
+            ShellBuilder::new(ssh_details.as_ref().map(|ssh| &*ssh.shell), &settings.shell)
+                .non_interactive();
         let (command, args) = builder.build(Some(command), &Vec::new());
 
         let mut env = self
@@ -180,9 +190,11 @@ impl Project {
                 ssh_command,
                 envs,
                 path_style,
+                shell,
                 ..
             }) => {
                 let (command, args) = wrap_for_ssh(
+                    &shell,
                     &ssh_command,
                     Some((&command, &args)),
                     path.as_deref(),
@@ -280,6 +292,7 @@ impl Project {
                         ssh_command,
                         envs,
                         path_style,
+                        shell,
                     }) => {
                         log::debug!("Connecting to a remote server: {ssh_command:?}");
 
@@ -291,6 +304,7 @@ impl Project {
                             .or_insert_with(|| "xterm-256color".to_string());
 
                         let (program, args) = wrap_for_ssh(
+                            &shell,
                             &ssh_command,
                             None,
                             path.as_deref(),
@@ -343,11 +357,13 @@ impl Project {
                         ssh_command,
                         envs,
                         path_style,
+                        shell,
                     }) => {
                         log::debug!("Connecting to a remote server: {ssh_command:?}");
                         env.entry("TERM".to_string())
                             .or_insert_with(|| "xterm-256color".to_string());
                         let (program, args) = wrap_for_ssh(
+                            &shell,
                             &ssh_command,
                             spawn_task
                                 .command
@@ -637,6 +653,7 @@ impl Project {
 }
 
 pub fn wrap_for_ssh(
+    shell: &str,
     ssh_command: &SshCommand,
     command: Option<(&String, &Vec<String>)>,
     path: Option<&Path>,
@@ -645,16 +662,11 @@ pub fn wrap_for_ssh(
     path_style: PathStyle,
 ) -> (String, Vec<String>) {
     let to_run = if let Some((command, args)) = command {
-        // DEFAULT_REMOTE_SHELL is '"${SHELL:-sh}"' so must not be escaped
-        let command: Option<Cow<str>> = if command == DEFAULT_REMOTE_SHELL {
-            Some(command.into())
-        } else {
-            shlex::try_quote(command).ok()
-        };
+        let command: Option<Cow<str>> = shlex::try_quote(command).ok();
         let args = args.iter().filter_map(|arg| shlex::try_quote(arg).ok());
         command.into_iter().chain(args).join(" ")
     } else {
-        "exec ${SHELL:-sh} -l".to_string()
+        format!("exec {shell} -l")
     };
 
     let mut env_changes = String::new();
@@ -688,7 +700,7 @@ pub fn wrap_for_ssh(
     } else {
         format!("cd; {env_changes} {to_run}")
     };
-    let shell_invocation = format!("sh -c {}", shlex::try_quote(&commands).unwrap());
+    let shell_invocation = format!("{shell} -c {}", shlex::try_quote(&commands).unwrap());
 
     let program = "ssh".to_string();
     let mut args = ssh_command.arguments.clone();

crates/remote/src/remote.rs 🔗

@@ -4,6 +4,6 @@ pub mod proxy;
 pub mod ssh_session;
 
 pub use ssh_session::{
-    ConnectionState, SshClientDelegate, SshConnectionOptions, SshPlatform, SshRemoteClient,
-    SshRemoteEvent,
+    ConnectionState, SshClientDelegate, SshConnectionOptions, SshInfo, SshPlatform,
+    SshRemoteClient, SshRemoteEvent,
 };

crates/remote/src/ssh_session.rs 🔗

@@ -89,11 +89,19 @@ pub struct SshConnectionOptions {
     pub upload_binary_over_ssh: bool,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct SshArgs {
     pub arguments: Vec<String>,
     pub envs: Option<HashMap<String, String>>,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct SshInfo {
+    pub args: SshArgs,
+    pub path_style: PathStyle,
+    pub shell: String,
+}
+
 #[macro_export]
 macro_rules! shell_script {
     ($fmt:expr, $($name:ident = $arg:expr),+ $(,)?) => {{
@@ -471,6 +479,16 @@ impl SshSocket {
 
         Ok(SshPlatform { os, arch })
     }
+
+    async fn shell(&self) -> String {
+        match self.run_command("sh", &["-c", "echo $SHELL"]).await {
+            Ok(shell) => shell.trim().to_owned(),
+            Err(e) => {
+                log::error!("Failed to get shell: {e}");
+                "sh".to_owned()
+            }
+        }
+    }
 }
 
 const MAX_MISSED_HEARTBEATS: usize = 5;
@@ -1152,12 +1170,16 @@ impl SshRemoteClient {
         cx.notify();
     }
 
-    pub fn ssh_info(&self) -> Option<(SshArgs, PathStyle)> {
+    pub fn ssh_info(&self) -> Option<SshInfo> {
         self.state
             .lock()
             .as_ref()
             .and_then(|state| state.ssh_connection())
-            .map(|ssh_connection| (ssh_connection.ssh_args(), ssh_connection.path_style()))
+            .map(|ssh_connection| SshInfo {
+                args: ssh_connection.ssh_args(),
+                path_style: ssh_connection.path_style(),
+                shell: ssh_connection.shell(),
+            })
     }
 
     pub fn upload_directory(
@@ -1392,6 +1414,7 @@ trait RemoteConnection: Send + Sync {
     fn ssh_args(&self) -> SshArgs;
     fn connection_options(&self) -> SshConnectionOptions;
     fn path_style(&self) -> PathStyle;
+    fn shell(&self) -> String;
 
     #[cfg(any(test, feature = "test-support"))]
     fn simulate_disconnect(&self, _: &AsyncApp) {}
@@ -1403,6 +1426,7 @@ struct SshRemoteConnection {
     remote_binary_path: Option<RemotePathBuf>,
     ssh_platform: SshPlatform,
     ssh_path_style: PathStyle,
+    ssh_shell: String,
     _temp_dir: TempDir,
 }
 
@@ -1429,6 +1453,10 @@ impl RemoteConnection for SshRemoteConnection {
         self.socket.connection_options.clone()
     }
 
+    fn shell(&self) -> String {
+        self.ssh_shell.clone()
+    }
+
     fn upload_directory(
         &self,
         src_path: PathBuf,
@@ -1642,6 +1670,7 @@ impl SshRemoteConnection {
             "windows" => PathStyle::Windows,
             _ => PathStyle::Posix,
         };
+        let ssh_shell = socket.shell().await;
 
         let mut this = Self {
             socket,
@@ -1650,6 +1679,7 @@ impl SshRemoteConnection {
             remote_binary_path: None,
             ssh_path_style,
             ssh_platform,
+            ssh_shell,
         };
 
         let (release_channel, version, commit) = cx.update(|cx| {
@@ -2686,6 +2716,10 @@ mod fake {
         fn path_style(&self) -> PathStyle {
             PathStyle::current()
         }
+
+        fn shell(&self) -> String {
+            "sh".to_owned()
+        }
     }
 
     pub(super) struct Delegate;

crates/task/src/shell_builder.rs 🔗

@@ -1,26 +1,40 @@
 use crate::Shell;
 
-#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
-enum ShellKind {
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum ShellKind {
     #[default]
     Posix,
+    Csh,
+    Fish,
     Powershell,
     Nushell,
     Cmd,
 }
 
 impl ShellKind {
-    fn new(program: &str) -> Self {
+    pub fn system() -> Self {
+        Self::new(&system_shell())
+    }
+
+    pub fn new(program: &str) -> Self {
+        #[cfg(windows)]
+        let (_, program) = program.rsplit_once('\\').unwrap_or(("", program));
+        #[cfg(not(windows))]
+        let (_, program) = program.rsplit_once('/').unwrap_or(("", program));
         if program == "powershell"
-            || program.ends_with("powershell.exe")
+            || program == "powershell.exe"
             || program == "pwsh"
-            || program.ends_with("pwsh.exe")
+            || program == "pwsh.exe"
         {
             ShellKind::Powershell
-        } else if program == "cmd" || program.ends_with("cmd.exe") {
+        } else if program == "cmd" || program == "cmd.exe" {
             ShellKind::Cmd
         } else if program == "nu" {
             ShellKind::Nushell
+        } else if program == "fish" {
+            ShellKind::Fish
+        } else if program == "csh" {
+            ShellKind::Csh
         } else {
             // Someother shell detected, the user might install and use a
             // unix-like shell.
@@ -33,6 +47,8 @@ impl ShellKind {
             Self::Powershell => Self::to_powershell_variable(input),
             Self::Cmd => Self::to_cmd_variable(input),
             Self::Posix => input.to_owned(),
+            Self::Fish => input.to_owned(),
+            Self::Csh => input.to_owned(),
             Self::Nushell => Self::to_nushell_variable(input),
         }
     }
@@ -153,7 +169,7 @@ impl ShellKind {
         match self {
             ShellKind::Powershell => vec!["-C".to_owned(), combined_command],
             ShellKind::Cmd => vec!["/C".to_owned(), combined_command],
-            ShellKind::Posix | ShellKind::Nushell => interactive
+            ShellKind::Posix | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh => interactive
                 .then(|| "-i".to_owned())
                 .into_iter()
                 .chain(["-c".to_owned(), combined_command])
@@ -184,19 +200,14 @@ pub struct ShellBuilder {
     kind: ShellKind,
 }
 
-pub static DEFAULT_REMOTE_SHELL: &str = "\"${SHELL:-sh}\"";
-
 impl ShellBuilder {
     /// Create a new ShellBuilder as configured.
-    pub fn new(is_local: bool, shell: &Shell) -> Self {
+    pub fn new(remote_system_shell: Option<&str>, shell: &Shell) -> Self {
         let (program, args) = match shell {
-            Shell::System => {
-                if is_local {
-                    (system_shell(), Vec::new())
-                } else {
-                    (DEFAULT_REMOTE_SHELL.to_string(), Vec::new())
-                }
-            }
+            Shell::System => match remote_system_shell {
+                Some(remote_shell) => (remote_shell.to_string(), Vec::new()),
+                None => (system_shell(), Vec::new()),
+            },
             Shell::Program(shell) => (shell.clone(), Vec::new()),
             Shell::WithArguments { program, args, .. } => (program.clone(), args.clone()),
         };
@@ -212,6 +223,7 @@ impl ShellBuilder {
         self.interactive = false;
         self
     }
+
     /// Returns the label to show in the terminal tab
     pub fn command_label(&self, command_label: &str) -> String {
         match self.kind {
@@ -221,7 +233,7 @@ impl ShellBuilder {
             ShellKind::Cmd => {
                 format!("{} /C '{}'", self.program, command_label)
             }
-            ShellKind::Posix | ShellKind::Nushell => {
+            ShellKind::Posix | ShellKind::Nushell | ShellKind::Fish | ShellKind::Csh => {
                 let interactivity = self.interactive.then_some("-i ").unwrap_or_default();
                 format!(
                     "{} {interactivity}-c '$\"{}\"'",
@@ -234,7 +246,7 @@ impl ShellBuilder {
     pub fn build(
         mut self,
         task_command: Option<String>,
-        task_args: &Vec<String>,
+        task_args: &[String],
     ) -> (String, Vec<String>) {
         if let Some(task_command) = task_command {
             let combined_command = task_args.iter().fold(task_command, |mut command, arg| {
@@ -258,11 +270,11 @@ mod test {
     #[test]
     fn test_nu_shell_variable_substitution() {
         let shell = Shell::Program("nu".to_owned());
-        let shell_builder = ShellBuilder::new(true, &shell);
+        let shell_builder = ShellBuilder::new(None, &shell);
 
         let (program, args) = shell_builder.build(
             Some("echo".into()),
-            &vec![
+            &[
                 "${hello}".to_string(),
                 "$world".to_string(),
                 "nothing".to_string(),

crates/task/src/task.rs 🔗

@@ -22,7 +22,7 @@ pub use debug_format::{
     AttachRequest, BuildTaskDefinition, DebugRequest, DebugScenario, DebugTaskFile, LaunchRequest,
     Request, TcpArgumentsTemplate, ZedDebugConfig,
 };
-pub use shell_builder::{DEFAULT_REMOTE_SHELL, ShellBuilder};
+pub use shell_builder::{ShellBuilder, ShellKind};
 pub use task_template::{
     DebugArgsRequest, HideStrategy, RevealStrategy, TaskTemplate, TaskTemplates,
     substitute_variables_in_map, substitute_variables_in_str,

crates/terminal_view/src/terminal_panel.rs 🔗

@@ -481,14 +481,17 @@ impl TerminalPanel {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Task<Result<WeakEntity<Terminal>>> {
-        let Ok(is_local) = self
-            .workspace
-            .update(cx, |workspace, cx| workspace.project().read(cx).is_local())
-        else {
+        let Ok((ssh_client, false)) = self.workspace.update(cx, |workspace, cx| {
+            let project = workspace.project().read(cx);
+            (
+                project.ssh_client().and_then(|it| it.read(cx).ssh_info()),
+                project.is_via_collab(),
+            )
+        }) else {
             return Task::ready(Err(anyhow!("Project is not local")));
         };
 
-        let builder = ShellBuilder::new(is_local, &task.shell);
+        let builder = ShellBuilder::new(ssh_client.as_ref().map(|info| &*info.shell), &task.shell);
         let command_label = builder.command_label(&task.command_label);
         let (command, args) = builder.build(task.command.clone(), &task.args);
 

crates/util/src/paths.rs 🔗

@@ -166,7 +166,7 @@ impl<T: AsRef<Path>> From<T> for SanitizedPath {
     }
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum PathStyle {
     Posix,
     Windows,