From b284b1a0b86715d9ac945034f6923f2551ce630b Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 21 Aug 2025 19:08:26 +0200 Subject: [PATCH] remote: Fetch shell on ssh remote to use for preparing commands (#36690) Prerequisite for https://github.com/zed-industries/zed/pull/36576 to allow us to differentiate the shell in a remote. Release Notes: - N/A --- 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(-) diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 0574091851..9991395f35 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/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); diff --git a/crates/project/src/debugger/dap_store.rs b/crates/project/src/debugger/dap_store.rs index 834bf2c2d2..2906c32ff4 100644 --- a/crates/project/src/debugger/dap_store.rs +++ b/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 diff --git a/crates/project/src/debugger/locators/cargo.rs b/crates/project/src/debugger/locators/cargo.rs index 3e28fac8af..b2f9580f9c 100644 --- a/crates/project/src/debugger/locators/cargo.rs +++ b/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::>(), ); let mut child = util::command::new_smol_command(program) .args(args) diff --git a/crates/project/src/terminals.rs b/crates/project/src/terminals.rs index e9582e73fd..b009b357fe 100644 --- a/crates/project/src/terminals.rs +++ b/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>, pub path_style: PathStyle, + pub shell: String, } impl Project { @@ -87,12 +89,18 @@ impl Project { pub fn ssh_details(&self, cx: &App) -> Option { 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)>, path: Option<&Path>, @@ -645,16 +662,11 @@ pub fn wrap_for_ssh( path_style: PathStyle, ) -> (String, Vec) { let to_run = if let Some((command, args)) = command { - // DEFAULT_REMOTE_SHELL is '"${SHELL:-sh}"' so must not be escaped - let command: Option> = if command == DEFAULT_REMOTE_SHELL { - Some(command.into()) - } else { - shlex::try_quote(command).ok() - }; + let command: Option> = 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(); diff --git a/crates/remote/src/remote.rs b/crates/remote/src/remote.rs index 43eb59c0ae..71895f1678 100644 --- a/crates/remote/src/remote.rs +++ b/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, }; diff --git a/crates/remote/src/ssh_session.rs b/crates/remote/src/ssh_session.rs index a26f4be661..c02d0ad7e7 100644 --- a/crates/remote/src/ssh_session.rs +++ b/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, pub envs: Option>, } +#[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 { 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, 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; diff --git a/crates/task/src/shell_builder.rs b/crates/task/src/shell_builder.rs index 770312bafc..de4ddc00f4 100644 --- a/crates/task/src/shell_builder.rs +++ b/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, - task_args: &Vec, + task_args: &[String], ) -> (String, Vec) { 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(), diff --git a/crates/task/src/task.rs b/crates/task/src/task.rs index 85e654eff4..eb9e59f087 100644 --- a/crates/task/src/task.rs +++ b/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, diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index f40c4870f1..6b17911487 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -481,14 +481,17 @@ impl TerminalPanel { window: &mut Window, cx: &mut Context, ) -> Task>> { - 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); diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index b430120314..1192b14812 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -166,7 +166,7 @@ impl> From for SanitizedPath { } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum PathStyle { Posix, Windows,