remote: Fix wsl failing to start on some setups (#39612)

Lukas Wirth created

Closes https://github.com/zed-industries/zed/issues/39433

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Change summary

Cargo.lock                         |  1 
crates/remote/Cargo.toml           |  1 
crates/remote/src/remote_client.rs | 49 +++++++++++++++++++++----------
crates/remote/src/transport.rs     |  1 
crates/remote/src/transport/ssh.rs | 49 +++++++++++++------------------
crates/remote/src/transport/wsl.rs | 47 +++++++++++++++---------------
6 files changed, 77 insertions(+), 71 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -13007,7 +13007,6 @@ dependencies = [
  "fs",
  "futures 0.3.31",
  "gpui",
- "itertools 0.14.0",
  "log",
  "parking_lot",
  "paths",

crates/remote/Cargo.toml 🔗

@@ -25,7 +25,6 @@ collections.workspace = true
 fs.workspace = true
 futures.workspace = true
 gpui.workspace = true
-itertools.workspace = true
 log.workspace = true
 parking_lot.workspace = true
 paths.workspace = true

crates/remote/src/remote_client.rs 🔗

@@ -22,7 +22,7 @@ use futures::{
 };
 use gpui::{
     App, AppContext as _, AsyncApp, BackgroundExecutor, BorrowAppContext, Context, Entity,
-    EventEmitter, Global, SemanticVersion, Task, WeakEntity,
+    EventEmitter, FutureExt, Global, SemanticVersion, Task, WeakEntity,
 };
 use parking_lot::Mutex;
 
@@ -356,26 +356,43 @@ impl RemoteClient {
                     cx,
                 );
 
-                let multiplex_task = Self::monitor(this.downgrade(), io_task, cx);
-
-                let timeout = cx.background_executor().timer(HEARTBEAT_TIMEOUT).fuse();
-                futures::pin_mut!(timeout);
-
-                select_biased! {
-                    ready = client.wait_for_remote_started() => {
-                        if ready.is_none() {
-                            let error = anyhow::anyhow!("remote client exited before becoming ready");
-                            log::error!("failed to establish connection: {}", error);
-                            return Err(error);
+                let ready = client
+                    .wait_for_remote_started()
+                    .with_timeout(HEARTBEAT_TIMEOUT, cx.background_executor())
+                    .await;
+                match ready {
+                    Ok(Some(_)) => {}
+                    Ok(None) => {
+                        let mut error = "remote client exited before becoming ready".to_owned();
+                        if let Some(status) = io_task.now_or_never() {
+                            match status {
+                                Ok(exit_code) => {
+                                    error.push_str(&format!(", exit_code={exit_code:?}"))
+                                }
+                                Err(e) => error.push_str(&format!(", error={e:?}")),
+                            }
+                        }
+                        let error = anyhow::anyhow!("{error}");
+                        log::error!("failed to establish connection: {}", error);
+                        return Err(error);
+                    }
+                    Err(_) => {
+                        let mut error =
+                            "remote client did not become ready within the timeout".to_owned();
+                        if let Some(status) = io_task.now_or_never() {
+                            match status {
+                                Ok(exit_code) => {
+                                    error.push_str(&format!(", exit_code={exit_code:?}"))
+                                }
+                                Err(e) => error.push_str(&format!(", error={e:?}")),
+                            }
                         }
-                    },
-                    _ = timeout => {
-                        let error = anyhow::anyhow!("remote client did not become ready within the timeout");
+                        let error = anyhow::anyhow!("{error}");
                         log::error!("failed to establish connection: {}", error);
                         return Err(error);
                     }
                 }
-
+                let multiplex_task = Self::monitor(this.downgrade(), io_task, cx);
                 if let Err(error) = client.ping(HEARTBEAT_TIMEOUT).await {
                     log::error!("failed to establish connection: {}", error);
                     return Err(error);

crates/remote/src/transport.rs 🔗

@@ -107,7 +107,6 @@ fn handle_rpc_messages_over_child_process_stdio(
                 result.context("stderr")
             }
         };
-
         let status = ssh_proxy_process.status().await?.code().unwrap_or(1);
         match result {
             Ok(_) => Ok(status),

crates/remote/src/transport/ssh.rs 🔗

@@ -11,7 +11,6 @@ use futures::{
     select_biased,
 };
 use gpui::{App, AppContext as _, AsyncApp, SemanticVersion, Task};
-use itertools::Itertools;
 use parking_lot::Mutex;
 use paths::remote_server_dir_relative;
 use release_channel::{AppCommitSha, AppVersion, ReleaseChannel};
@@ -22,7 +21,6 @@ use smol::{
     process::{self, Child, Stdio},
 };
 use std::{
-    iter,
     path::{Path, PathBuf},
     sync::Arc,
     time::Instant,
@@ -206,30 +204,24 @@ impl RemoteConnection for SshRemoteConnection {
             return Task::ready(Err(anyhow!("Remote binary path not set")));
         };
 
-        let mut start_proxy_command = shell_script!(
-            "exec {binary_path} proxy --identifier {identifier}",
-            binary_path = &remote_binary_path.display(self.path_style()),
-            identifier = &unique_identifier,
-        );
-
+        let mut proxy_args = vec![];
         for env_var in ["RUST_LOG", "RUST_BACKTRACE", "ZED_GENERATE_MINIDUMPS"] {
             if let Some(value) = std::env::var(env_var).ok() {
-                start_proxy_command = format!(
-                    "{}={} {} ",
-                    env_var,
-                    shlex::try_quote(&value).unwrap(),
-                    start_proxy_command,
-                );
+                proxy_args.push(format!("{}='{}'", env_var, value));
             }
         }
+        proxy_args.push(remote_binary_path.display(self.path_style()).into_owned());
+        proxy_args.push("proxy".to_owned());
+        proxy_args.push("--identifier".to_owned());
+        proxy_args.push(unique_identifier);
 
         if reconnect {
-            start_proxy_command.push_str(" --reconnect");
+            proxy_args.push("--reconnect".to_owned());
         }
 
         let ssh_proxy_process = match self
             .socket
-            .ssh_command("sh", &["-c", &start_proxy_command])
+            .ssh_command("env", &proxy_args)
             // IMPORTANT: we kill this process when we drop the task that uses it.
             .kill_on_drop(true)
             .spawn()
@@ -718,24 +710,23 @@ impl SshSocket {
     // Furthermore, some setups (e.g. Coder) will change directory when SSH'ing
     // into a machine. You must use `cd` to get back to $HOME.
     // You need to do it like this: $ ssh host "cd; sh -c 'ls -l /tmp'"
-    fn ssh_command(&self, program: &str, args: &[&str]) -> process::Command {
+    fn ssh_command(&self, program: &str, args: &[impl AsRef<str>]) -> process::Command {
         let mut command = util::command::new_smol_command("ssh");
-        let to_run = iter::once(&program)
-            .chain(args.iter())
-            .map(|token| {
-                // We're trying to work with: sh, bash, zsh, fish, tcsh, ...?
-                debug_assert!(
-                    !token.contains('\n'),
-                    "multiline arguments do not work in all shells"
-                );
-                shlex::try_quote(token).unwrap()
-            })
-            .join(" ");
+        let mut to_run = shlex::try_quote(program).unwrap().into_owned();
+        for arg in args {
+            // We're trying to work with: sh, bash, zsh, fish, tcsh, ...?
+            debug_assert!(
+                !arg.as_ref().contains('\n'),
+                "multiline arguments do not work in all shells"
+            );
+            to_run.push(' ');
+            to_run.push_str(&shlex::try_quote(arg.as_ref()).unwrap());
+        }
         let to_run = format!("cd; {to_run}");
-        log::debug!("ssh {} {:?}", self.connection_options.ssh_url(), to_run);
         self.ssh_options(&mut command)
             .arg(self.connection_options.ssh_url())
             .arg(to_run);
+        log::debug!("ssh {:?}", command);
         command
     }
 

crates/remote/src/transport/wsl.rs 🔗

@@ -11,6 +11,7 @@ use release_channel::{AppCommitSha, AppVersion, ReleaseChannel};
 use rpc::proto::Envelope;
 use smol::{fs, process};
 use std::{
+    ffi::OsStr,
     fmt::Write as _,
     path::{Path, PathBuf},
     process::Stdio,
@@ -110,7 +111,7 @@ impl WslRemoteConnection {
         windows_path_to_wsl_path_impl(&self.connection_options, source).await
     }
 
-    fn wsl_command(&self, program: &str, args: &[&str]) -> process::Command {
+    fn wsl_command(&self, program: &str, args: &[impl AsRef<OsStr>]) -> process::Command {
         wsl_command_impl(&self.connection_options, program, args)
     }
 
@@ -297,23 +298,22 @@ impl RemoteConnection for WslRemoteConnection {
             return Task::ready(Err(anyhow!("Remote binary path not set")));
         };
 
-        let mut proxy_command = format!(
-            "exec {} proxy --identifier {}",
-            remote_binary_path.display(PathStyle::Posix),
-            unique_identifier
-        );
-
-        if reconnect {
-            proxy_command.push_str(" --reconnect");
-        }
-
+        let mut proxy_args = vec![];
         for env_var in ["RUST_LOG", "RUST_BACKTRACE", "ZED_GENERATE_MINIDUMPS"] {
             if let Some(value) = std::env::var(env_var).ok() {
-                proxy_command = format!("{}='{}' {}", env_var, value, proxy_command);
+                proxy_args.push(format!("{}='{}'", env_var, value));
             }
         }
+        proxy_args.push(remote_binary_path.display(PathStyle::Posix).into_owned());
+        proxy_args.push("proxy".to_owned());
+        proxy_args.push("--identifier".to_owned());
+        proxy_args.push(unique_identifier);
+
+        if reconnect {
+            proxy_args.push("--reconnect".to_owned());
+        }
         let proxy_process = match self
-            .wsl_command("sh", &["-lc", &proxy_command])
+            .wsl_command("env", &proxy_args)
             .kill_on_drop(true)
             .spawn()
         {
@@ -387,22 +387,22 @@ impl RemoteConnection for WslRemoteConnection {
             .map(|working_dir| RemotePathBuf::new(working_dir, PathStyle::Posix).to_string())
             .unwrap_or("~".to_string());
 
-        let mut script = String::new();
+        let mut exec = String::from("exec env ");
 
         for (k, v) in env.iter() {
-            write!(&mut script, "{}='{}' ", k, v).unwrap();
+            if let Some((k, v)) = shlex::try_quote(k).ok().zip(shlex::try_quote(v).ok()) {
+                write!(exec, "{}={} ", k, v).unwrap();
+            }
         }
 
         if let Some(program) = program {
-            let command = shlex::try_quote(&program)?;
-            script.push_str(&command);
+            write!(exec, "{}", shlex::try_quote(&program)?).unwrap();
             for arg in args {
                 let arg = shlex::try_quote(&arg)?;
-                script.push_str(" ");
-                script.push_str(&arg);
+                write!(exec, " {}", &arg).unwrap();
             }
         } else {
-            write!(&mut script, "exec {} -l", self.shell).unwrap();
+            write!(&mut exec, "{} -l", self.shell).unwrap();
         }
 
         let wsl_args = if let Some(user) = &self.connection_options.user {
@@ -416,7 +416,7 @@ impl RemoteConnection for WslRemoteConnection {
                 "--".to_string(),
                 self.shell.clone(),
                 "-c".to_string(),
-                script,
+                exec,
             ]
         } else {
             vec![
@@ -427,7 +427,7 @@ impl RemoteConnection for WslRemoteConnection {
                 "--".to_string(),
                 self.shell.clone(),
                 "-c".to_string(),
-                script,
+                exec,
             ]
         };
 
@@ -476,7 +476,7 @@ async fn windows_path_to_wsl_path_impl(
 fn wsl_command_impl(
     options: &WslConnectionOptions,
     program: &str,
-    args: &[&str],
+    args: &[impl AsRef<OsStr>],
 ) -> process::Command {
     let mut command = util::command::new_smol_command("wsl.exe");
 
@@ -495,6 +495,7 @@ fn wsl_command_impl(
         .arg(program)
         .args(args);
 
+    log::debug!("wsl {:?}", command);
     command
 }