debugger: Mark DebugAdapterBinary::program as optional (#32534)

Piotr Osiewicz created

This allows us to support debugging with a debug adapter not managed by
Zed. Note that this is not a user facing change, as DebugAdapterBinary
is used to determine how to spawn a debugger. Thus, this should not
break any configs or anything like that.

Closes #ISSUE

Release Notes:

- N/A

Change summary

crates/collab/src/tests/remote_editing_collaboration_tests.rs |  2 
crates/dap/src/adapters.rs                                    |  4 
crates/dap/src/client.rs                                      |  6 
crates/dap/src/transport.rs                                   | 72 +++-
crates/dap_adapters/src/codelldb.rs                           |  2 
crates/dap_adapters/src/gdb.rs                                |  2 
crates/dap_adapters/src/go.rs                                 |  2 
crates/dap_adapters/src/javascript.rs                         | 14 
crates/dap_adapters/src/php.rs                                | 14 
crates/dap_adapters/src/python.rs                             |  2 
crates/dap_adapters/src/ruby.rs                               |  2 
crates/extension_api/wit/since_v0.6.0/dap.wit                 |  2 
crates/project/src/debugger/dap_store.rs                      |  7 
crates/proto/proto/debugger.proto                             |  2 
14 files changed, 80 insertions(+), 53 deletions(-)

Detailed changes

crates/collab/src/tests/remote_editing_collaboration_tests.rs 🔗

@@ -671,7 +671,7 @@ async fn test_remote_server_debugger(
     });
 
     session.update(cx_a, |session, _| {
-        assert_eq!(session.binary().command, "ssh");
+        assert_eq!(session.binary().command.as_deref(), Some("ssh"));
     });
 
     let shutdown_session = workspace.update(cx_a, |workspace, cx| {

crates/dap/src/adapters.rs 🔗

@@ -181,7 +181,7 @@ impl DebugTaskDefinition {
 /// Created from a [DebugTaskDefinition], this struct describes how to spawn the debugger to create a previously-configured debug session.
 #[derive(Debug, Clone, PartialEq)]
 pub struct DebugAdapterBinary {
-    pub command: String,
+    pub command: Option<String>,
     pub arguments: Vec<String>,
     pub envs: HashMap<String, String>,
     pub cwd: Option<PathBuf>,
@@ -437,7 +437,7 @@ impl DebugAdapter for FakeAdapter {
         _: &mut AsyncApp,
     ) -> Result<DebugAdapterBinary> {
         Ok(DebugAdapterBinary {
-            command: "command".into(),
+            command: Some("command".into()),
             arguments: vec![],
             connection: None,
             envs: HashMap::default(),

crates/dap/src/client.rs 🔗

@@ -297,7 +297,7 @@ mod tests {
         let client = DebugAdapterClient::start(
             crate::client::SessionId(1),
             DebugAdapterBinary {
-                command: "command".into(),
+                command: Some("command".into()),
                 arguments: Default::default(),
                 envs: Default::default(),
                 connection: None,
@@ -367,7 +367,7 @@ mod tests {
         let client = DebugAdapterClient::start(
             crate::client::SessionId(1),
             DebugAdapterBinary {
-                command: "command".into(),
+                command: Some("command".into()),
                 arguments: Default::default(),
                 envs: Default::default(),
                 connection: None,
@@ -420,7 +420,7 @@ mod tests {
         let client = DebugAdapterClient::start(
             crate::client::SessionId(1),
             DebugAdapterBinary {
-                command: "command".into(),
+                command: Some("command".into()),
                 arguments: Default::default(),
                 envs: Default::default(),
                 connection: None,

crates/dap/src/transport.rs 🔗

@@ -85,10 +85,12 @@ impl Transport {
             TcpTransport::start(binary, cx)
                 .await
                 .map(|(transports, tcp)| (transports, Self::Tcp(tcp)))
+                .context("Tried to connect to a debug adapter via TCP transport layer")
         } else {
             StdioTransport::start(binary, cx)
                 .await
                 .map(|(transports, stdio)| (transports, Self::Stdio(stdio)))
+                .context("Tried to connect to a debug adapter via stdin/stdout transport layer")
         }
     }
 
@@ -567,7 +569,7 @@ pub struct TcpTransport {
     pub port: u16,
     pub host: Ipv4Addr,
     pub timeout: u64,
-    process: Mutex<Child>,
+    process: Option<Mutex<Child>>,
 }
 
 impl TcpTransport {
@@ -596,17 +598,23 @@ impl TcpTransport {
         let host = connection_args.host;
         let port = connection_args.port;
 
-        let mut command = util::command::new_std_command(&binary.command);
+        let mut process = if let Some(command) = &binary.command {
+            let mut command = util::command::new_std_command(&command);
 
-        if let Some(cwd) = &binary.cwd {
-            command.current_dir(cwd);
-        }
+            if let Some(cwd) = &binary.cwd {
+                command.current_dir(cwd);
+            }
 
-        command.args(&binary.arguments);
-        command.envs(&binary.envs);
+            command.args(&binary.arguments);
+            command.envs(&binary.envs);
 
-        let mut process = Child::spawn(command, Stdio::null())
-            .with_context(|| "failed to start debug adapter.")?;
+            Some(
+                Child::spawn(command, Stdio::null())
+                    .with_context(|| "failed to start debug adapter.")?,
+            )
+        } else {
+            None
+        };
 
         let address = SocketAddrV4::new(host, port);
 
@@ -624,15 +632,18 @@ impl TcpTransport {
                     match TcpStream::connect(address).await {
                         Ok(stream) => return Ok((process, stream.split())),
                         Err(_) => {
-                            if let Ok(Some(_)) = process.try_status() {
-                                let output = process.into_inner().output().await?;
-                                let output = if output.stderr.is_empty() {
-                                    String::from_utf8_lossy(&output.stdout).to_string()
-                                } else {
-                                    String::from_utf8_lossy(&output.stderr).to_string()
-                                };
-                                anyhow::bail!("{output}\nerror: process exited before debugger attached.");
+                            if let Some(p) = &mut process {
+                                if let Ok(Some(_)) = p.try_status() {
+                                    let output = process.take().unwrap().into_inner().output().await?;
+                                    let output = if output.stderr.is_empty() {
+                                        String::from_utf8_lossy(&output.stdout).to_string()
+                                    } else {
+                                        String::from_utf8_lossy(&output.stderr).to_string()
+                                    };
+                                    anyhow::bail!("{output}\nerror: process exited before debugger attached.");
+                                }
                             }
+
                             cx.background_executor().timer(Duration::from_millis(100)).await;
                         }
                     }
@@ -645,13 +656,13 @@ impl TcpTransport {
             host,
             port
         );
-        let stdout = process.stdout.take();
-        let stderr = process.stderr.take();
+        let stdout = process.as_mut().and_then(|p| p.stdout.take());
+        let stderr = process.as_mut().and_then(|p| p.stderr.take());
 
         let this = Self {
             port,
             host,
-            process: Mutex::new(process),
+            process: process.map(Mutex::new),
             timeout,
         };
 
@@ -670,14 +681,18 @@ impl TcpTransport {
     }
 
     async fn kill(&self) {
-        let mut process = self.process.lock().await;
-        Child::kill(&mut process);
+        if let Some(process) = &self.process {
+            let mut process = process.lock().await;
+            Child::kill(&mut process);
+        }
     }
 }
 
 impl Drop for TcpTransport {
     fn drop(&mut self) {
-        self.process.get_mut().kill();
+        if let Some(mut p) = self.process.take() {
+            p.get_mut().kill();
+        }
     }
 }
 
@@ -688,7 +703,12 @@ pub struct StdioTransport {
 impl StdioTransport {
     #[allow(dead_code, reason = "This is used in non test builds of Zed")]
     async fn start(binary: &DebugAdapterBinary, _: AsyncApp) -> Result<(TransportPipe, Self)> {
-        let mut command = util::command::new_std_command(&binary.command);
+        let Some(binary_command) = &binary.command else {
+            bail!(
+                "When using the `stdio` transport, the path to a debug adapter binary must be set by Zed."
+            );
+        };
+        let mut command = util::command::new_std_command(&binary_command);
 
         if let Some(cwd) = &binary.cwd {
             command.current_dir(cwd);
@@ -700,7 +720,7 @@ impl StdioTransport {
         let mut process = Child::spawn(command, Stdio::piped()).with_context(|| {
             format!(
                 "failed to spawn command `{} {}`.",
-                binary.command,
+                binary_command,
                 binary.arguments.join(" ")
             )
         })?;
@@ -715,7 +735,7 @@ impl StdioTransport {
         if stderr.is_none() {
             bail!(
                 "Failed to connect to stderr for debug adapter command {}",
-                &binary.command
+                &binary_command
             );
         }
 

crates/dap_adapters/src/codelldb.rs 🔗

@@ -359,7 +359,7 @@ impl DebugAdapter for CodeLldbDebugAdapter {
         };
 
         Ok(DebugAdapterBinary {
-            command: command.unwrap(),
+            command: Some(command.unwrap()),
             cwd: Some(delegate.worktree_root_path().to_path_buf()),
             arguments: vec![
                 "--settings".into(),

crates/dap_adapters/src/gdb.rs 🔗

@@ -183,7 +183,7 @@ impl DebugAdapter for GdbDebugAdapter {
         };
 
         Ok(DebugAdapterBinary {
-            command: gdb_path,
+            command: Some(gdb_path),
             arguments: vec!["-i=dap".into()],
             envs: HashMap::default(),
             cwd: Some(delegate.worktree_root_path().to_path_buf()),

crates/dap_adapters/src/go.rs 🔗

@@ -463,7 +463,7 @@ impl DebugAdapter for GoDebugAdapter {
         };
 
         Ok(DebugAdapterBinary {
-            command: minidelve_path.to_string_lossy().into_owned(),
+            command: Some(minidelve_path.to_string_lossy().into_owned()),
             arguments,
             cwd: Some(cwd),
             envs: HashMap::default(),

crates/dap_adapters/src/javascript.rs 🔗

@@ -69,12 +69,14 @@ impl JsDebugAdapter {
         let (host, port, timeout) = crate::configure_tcp_connection(tcp_connection).await?;
 
         Ok(DebugAdapterBinary {
-            command: delegate
-                .node_runtime()
-                .binary_path()
-                .await?
-                .to_string_lossy()
-                .into_owned(),
+            command: Some(
+                delegate
+                    .node_runtime()
+                    .binary_path()
+                    .await?
+                    .to_string_lossy()
+                    .into_owned(),
+            ),
             arguments: vec![
                 adapter_path
                     .join(Self::ADAPTER_PATH)

crates/dap_adapters/src/php.rs 🔗

@@ -72,12 +72,14 @@ impl PhpDebugAdapter {
         let (host, port, timeout) = crate::configure_tcp_connection(tcp_connection).await?;
 
         Ok(DebugAdapterBinary {
-            command: delegate
-                .node_runtime()
-                .binary_path()
-                .await?
-                .to_string_lossy()
-                .into_owned(),
+            command: Some(
+                delegate
+                    .node_runtime()
+                    .binary_path()
+                    .await?
+                    .to_string_lossy()
+                    .into_owned(),
+            ),
             arguments: vec![
                 adapter_path
                     .join(Self::ADAPTER_PATH)

crates/dap_adapters/src/python.rs 🔗

@@ -187,7 +187,7 @@ impl PythonDebugAdapter {
         );
 
         Ok(DebugAdapterBinary {
-            command: python_command,
+            command: Some(python_command),
             arguments,
             connection: Some(adapters::TcpArguments {
                 host,

crates/dap_adapters/src/ruby.rs 🔗

@@ -175,7 +175,7 @@ impl DebugAdapter for RubyDebugAdapter {
         arguments.extend(ruby_config.args);
 
         Ok(DebugAdapterBinary {
-            command: rdbg_path.to_string_lossy().to_string(),
+            command: Some(rdbg_path.to_string_lossy().to_string()),
             arguments,
             connection: Some(dap::adapters::TcpArguments {
                 host,

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

@@ -258,14 +258,17 @@ impl DapStore {
 
                     let (program, args) = wrap_for_ssh(
                         &ssh_command,
-                        Some((&binary.command, &binary.arguments)),
+                        binary
+                            .command
+                            .as_ref()
+                            .map(|command| (command, &binary.arguments)),
                         binary.cwd.as_deref(),
                         binary.envs,
                         None,
                     );
 
                     Ok(DebugAdapterBinary {
-                        command: program,
+                        command: Some(program),
                         arguments: args,
                         envs: HashMap::default(),
                         cwd: None,

crates/proto/proto/debugger.proto 🔗

@@ -496,7 +496,7 @@ message GetDebugAdapterBinary {
 }
 
 message DebugAdapterBinary {
-    string command = 1;
+    optional string command = 1;
     repeated string arguments = 2;
     map<string, string> envs = 3;
     optional string cwd = 4;