util: Fix shell kind in windows based on program path (#39696)

Marco Mihai Condrache created

Closes #39614

The `ShellKind` struct is built on Windows' side, meaning that when
connecting to remotes, we fall back to PowerShell construction, even if
the shell program we are spawning is a unix program.

This broke tasks creation since we are using the shell kind to construct
args:


https://github.com/zed-industries/zed/blob/d04ac864b8cb78ee888ce03323e34c17a6dd16b9/crates/project/src/terminals.rs#L149

In normal terminals this only affected activation scripts (only place
where shell kind is used)

I don't have a Windows machine to test it, so I would appreciate any
help with testing!

Release Notes:

- Fixed an issue where tasks could not be executed in Windows WSL

---------

Signed-off-by: Marco Mihai Condrache <52580954+marcocondrache@users.noreply.github.com>

Change summary

crates/acp_thread/src/acp_thread.rs           |  8 +++--
crates/agent_servers/src/acp.rs               |  5 +++
crates/assistant_tools/src/terminal_tool.rs   |  3 +
crates/debugger_ui/src/session/running.rs     |  3 +
crates/project/src/debugger/locators/cargo.rs |  2 
crates/project/src/terminals.rs               | 13 ++++++---
crates/prompt_store/src/prompts.rs            |  3 +
crates/task/src/shell_builder.rs              |  8 +++---
crates/terminal/src/terminal.rs               |  6 ++--
crates/terminal_view/src/terminal_panel.rs    | 27 ++++++++------------
crates/util/src/shell.rs                      | 18 ++++++-------
crates/util/src/shell_env.rs                  |  4 +-
12 files changed, 53 insertions(+), 47 deletions(-)

Detailed changes

crates/acp_thread/src/acp_thread.rs 🔗

@@ -2112,6 +2112,7 @@ impl AcpThread {
 
         let project = self.project.clone();
         let language_registry = project.read(cx).languages().clone();
+        let is_windows = project.read(cx).path_style(cx).is_windows();
 
         let terminal_id = acp::TerminalId(Uuid::new_v4().to_string().into());
         let terminal_task = cx.spawn({
@@ -2125,9 +2126,10 @@ impl AcpThread {
                             .and_then(|r| r.read(cx).default_system_shell())
                     })?
                     .unwrap_or_else(|| get_default_system_shell_preferring_bash());
-                let (task_command, task_args) = ShellBuilder::new(&Shell::Program(shell))
-                    .redirect_stdin_to_dev_null()
-                    .build(Some(command.clone()), &args);
+                let (task_command, task_args) =
+                    ShellBuilder::new(&Shell::Program(shell), is_windows)
+                        .redirect_stdin_to_dev_null()
+                        .build(Some(command.clone()), &args);
                 let terminal = project
                     .update(cx, |project, cx| {
                         project.create_terminal_task(

crates/agent_servers/src/acp.rs 🔗

@@ -838,7 +838,10 @@ impl acp::Client for ClientDelegate {
                     .map(Shell::Program)
             })?
             .unwrap_or(task::Shell::System);
-        let (task_command, task_args) = task::ShellBuilder::new(&shell)
+        let is_windows = project
+            .read_with(&self.cx, |project, cx| project.path_style(cx).is_windows())
+            .unwrap_or(cfg!(windows));
+        let (task_command, task_args) = task::ShellBuilder::new(&shell, is_windows)
             .redirect_stdin_to_dev_null()
             .build(Some(args.command.clone()), &args.args);
 

crates/assistant_tools/src/terminal_tool.rs 🔗

@@ -136,6 +136,7 @@ impl Tool for TerminalTool {
             }),
             None => Task::ready(None).shared(),
         };
+        let is_windows = project.read(cx).path_style(cx).is_windows();
         let shell = project
             .update(cx, |project, cx| {
                 project
@@ -155,7 +156,7 @@ impl Tool for TerminalTool {
         let build_cmd = {
             let input_command = input.command.clone();
             move || {
-                ShellBuilder::new(&Shell::Program(shell))
+                ShellBuilder::new(&Shell::Program(shell), is_windows)
                     .redirect_stdin_to_dev_null()
                     .build(Some(input_command), &[])
             }

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

@@ -937,6 +937,7 @@ impl RunningState {
         let task_store = project.read(cx).task_store().downgrade();
         let weak_project = project.downgrade();
         let weak_workspace = workspace.downgrade();
+        let is_windows = project.read(cx).path_style(cx).is_windows();
         let remote_shell = project
             .read(cx)
             .remote_client()
@@ -1029,7 +1030,7 @@ impl RunningState {
                     task.resolved.shell = Shell::Program(remote_shell);
                 }
 
-                let builder = ShellBuilder::new(&task.resolved.shell);
+                let builder = ShellBuilder::new(&task.resolved.shell, is_windows);
                 let command_label = builder.command_label(task.resolved.command.as_deref().unwrap_or(""));
                 let (command, args) =
                     builder.build(task.resolved.command.clone(), &task.resolved.args);

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(&build_config.shell).non_interactive();
+        let builder = ShellBuilder::new(&build_config.shell, cfg!(windows)).non_interactive();
         let (program, args) = builder.build(
             Some("cargo".into()),
             &build_config

crates/project/src/terminals.rs 🔗

@@ -101,6 +101,8 @@ impl Project {
             None => settings.shell.program(),
         };
 
+        let is_windows = self.path_style(cx).is_windows();
+
         let project_path_contexts = self
             .active_entry()
             .and_then(|entry_id| self.path_for_entry(entry_id, cx))
@@ -120,7 +122,7 @@ impl Project {
         let lang_registry = self.languages.clone();
         let fs = self.fs.clone();
         cx.spawn(async move |project, cx| {
-            let shell_kind = ShellKind::new(&shell);
+            let shell_kind = ShellKind::new(&shell, is_windows);
             let activation_script = maybe!(async {
                 for toolchain in toolchains {
                     let Some(toolchain) = toolchain.await else {
@@ -329,13 +331,15 @@ impl Project {
             .map(|p| self.active_toolchain(p, LanguageName::new("Python"), cx))
             .collect::<Vec<_>>();
         let remote_client = self.remote_client.clone();
-        let shell_kind = ShellKind::new(&match &remote_client {
+        let shell = match &remote_client {
             Some(remote_client) => remote_client
                 .read(cx)
                 .shell()
                 .unwrap_or_else(get_default_system_shell),
             None => settings.shell.program(),
-        });
+        };
+
+        let shell_kind = ShellKind::new(&shell, self.path_style(cx).is_windows());
 
         let lang_registry = self.languages.clone();
         let fs = self.fs.clone();
@@ -476,7 +480,8 @@ impl Project {
             .and_then(|remote_client| remote_client.read(cx).shell())
             .map(Shell::Program)
             .unwrap_or_else(|| settings.shell.clone());
-        let builder = ShellBuilder::new(&shell).non_interactive();
+        let is_windows = self.path_style(cx).is_windows();
+        let builder = ShellBuilder::new(&shell, is_windows).non_interactive();
         let (command, args) = builder.build(Some(command), &Vec::new());
 
         let mut env = self

crates/prompt_store/src/prompts.rs 🔗

@@ -45,7 +45,8 @@ impl ProjectContext {
             user_rules: default_user_rules,
             os: std::env::consts::OS.to_string(),
             arch: std::env::consts::ARCH.to_string(),
-            shell: ShellKind::new(&get_default_system_shell_preferring_bash()).to_string(),
+            shell: ShellKind::new(&get_default_system_shell_preferring_bash(), cfg!(windows))
+                .to_string(),
         }
     }
 }

crates/task/src/shell_builder.rs 🔗

@@ -18,14 +18,14 @@ pub struct ShellBuilder {
 
 impl ShellBuilder {
     /// Create a new ShellBuilder as configured.
-    pub fn new(shell: &Shell) -> Self {
+    pub fn new(shell: &Shell, is_windows: bool) -> Self {
         let (program, args) = match shell {
             Shell::System => (get_system_shell(), Vec::new()),
             Shell::Program(shell) => (shell.clone(), Vec::new()),
             Shell::WithArguments { program, args, .. } => (program.clone(), args.clone()),
         };
 
-        let kind = ShellKind::new(&program);
+        let kind = ShellKind::new(&program, is_windows);
         Self {
             program,
             args,
@@ -120,7 +120,7 @@ mod test {
     #[test]
     fn test_nu_shell_variable_substitution() {
         let shell = Shell::Program("nu".to_owned());
-        let shell_builder = ShellBuilder::new(&shell);
+        let shell_builder = ShellBuilder::new(&shell, false);
 
         let (program, args) = shell_builder.build(
             Some("echo".into()),
@@ -148,7 +148,7 @@ mod test {
     #[test]
     fn redirect_stdin_to_dev_null_precedence() {
         let shell = Shell::Program("nu".to_owned());
-        let shell_builder = ShellBuilder::new(&shell);
+        let shell_builder = ShellBuilder::new(&shell, false);
 
         let (program, args) = shell_builder
             .redirect_stdin_to_dev_null()

crates/terminal/src/terminal.rs 🔗

@@ -2391,8 +2391,8 @@ mod tests {
         cx.executor().allow_parking();
 
         let (completion_tx, completion_rx) = smol::channel::unbounded();
-        let (program, args) =
-            ShellBuilder::new(&Shell::System).build(Some("echo".to_owned()), &["hello".to_owned()]);
+        let (program, args) = ShellBuilder::new(&Shell::System, false)
+            .build(Some("echo".to_owned()), &["hello".to_owned()]);
         let terminal = cx.new(|cx| {
             TerminalBuilder::new(
                 None,
@@ -2510,7 +2510,7 @@ mod tests {
         cx.executor().allow_parking();
 
         let (completion_tx, completion_rx) = smol::channel::unbounded();
-        let (program, args) = ShellBuilder::new(&Shell::System)
+        let (program, args) = ShellBuilder::new(&Shell::System, false)
             .build(Some("asdasdasdasd".to_owned()), &["@@@@@".to_owned()]);
         let terminal = cx.new(|cx| {
             TerminalBuilder::new(

crates/terminal_view/src/terminal_panel.rs 🔗

@@ -526,23 +526,18 @@ impl TerminalPanel {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Task<Result<WeakEntity<Terminal>>> {
-        let remote_client = self
-            .workspace
-            .update(cx, |workspace, cx| {
-                let project = workspace.project().read(cx);
-                if project.is_via_collab() {
-                    Err(anyhow!("cannot spawn tasks as a guest"))
-                } else {
-                    Ok(project.remote_client())
-                }
-            })
-            .flatten();
-
-        let remote_client = match remote_client {
-            Ok(remote_client) => remote_client,
-            Err(e) => return Task::ready(Err(e)),
+        let Some(workspace) = self.workspace.upgrade() else {
+            return Task::ready(Err(anyhow!("failed to read workspace")));
         };
 
+        let project = workspace.read(cx).project().read(cx);
+
+        if project.is_via_collab() {
+            return Task::ready(Err(anyhow!("cannot spawn tasks as a guest")));
+        }
+
+        let remote_client = project.remote_client();
+        let is_windows = project.path_style(cx).is_windows();
         let remote_shell = remote_client
             .as_ref()
             .and_then(|remote_client| remote_client.read(cx).shell());
@@ -555,7 +550,7 @@ impl TerminalPanel {
             task.shell.clone()
         };
 
-        let builder = ShellBuilder::new(&shell);
+        let builder = ShellBuilder::new(&shell, is_windows);
         let command_label = builder.command_label(task.command.as_deref().unwrap_or(""));
         let (command, args) = builder.build(task.command.clone(), &task.args);
 

crates/util/src/shell.rs 🔗

@@ -171,18 +171,16 @@ impl fmt::Display for ShellKind {
 
 impl ShellKind {
     pub fn system() -> Self {
-        Self::new(&get_system_shell())
+        Self::new(&get_system_shell(), cfg!(windows))
     }
 
-    pub fn new(program: impl AsRef<Path>) -> Self {
+    pub fn new(program: impl AsRef<Path>, is_windows: bool) -> Self {
         let program = program.as_ref();
-        let Some(program) = program.file_stem().and_then(|s| s.to_str()) else {
-            return if cfg!(windows) {
-                ShellKind::PowerShell
-            } else {
-                ShellKind::Posix
-            };
-        };
+        let program = program
+            .file_stem()
+            .unwrap_or_else(|| program.as_os_str())
+            .to_string_lossy();
+
         if program == "powershell" || program == "pwsh" {
             ShellKind::PowerShell
         } else if program == "cmd" {
@@ -200,7 +198,7 @@ impl ShellKind {
         } else if program == "sh" || program == "bash" {
             ShellKind::Posix
         } else {
-            if cfg!(windows) {
+            if is_windows {
                 ShellKind::PowerShell
             } else {
                 // Some other shell detected, the user might install and use a

crates/util/src/shell_env.rs 🔗

@@ -36,7 +36,7 @@ async fn capture_unix(
     use std::process::Stdio;
 
     let zed_path = super::get_shell_safe_zed_path()?;
-    let shell_kind = ShellKind::new(shell_path);
+    let shell_kind = ShellKind::new(shell_path, false);
 
     let mut command_string = String::new();
     let mut command = std::process::Command::new(shell_path);
@@ -131,7 +131,7 @@ async fn capture_windows(
     let zed_path =
         std::env::current_exe().context("Failed to determine current zed executable path.")?;
 
-    let shell_kind = ShellKind::new(shell_path);
+    let shell_kind = ShellKind::new(shell_path, true);
     let env_output = match shell_kind {
         ShellKind::Posix | ShellKind::Csh | ShellKind::Tcsh | ShellKind::Rc | ShellKind::Fish => {
             return Err(anyhow::anyhow!("unsupported shell kind"));