Anthony Eid created
There were two bugs that caused the gdb adapter not to work properly,
one on our end and one their end.
The bug on our end was sending `stopOnEntry: null` in our launch request
when stop on entry had no value. I fixed that bug across all dap
adapters
The other bug had to do with python's "great" type system and how we
serialized our unit structs to json; mainly,
`ConfigurationDoneArguments` and `ThreadsArguments`. Gdb seems to follow
a pattern for handling requests where they pass `**args` to a function,
this errors out when the equivalent json is `"arguments": null`.
```py
@capability("supportsConfigurationDoneRequest")
@request("configurationDone", on_dap_thread=True)
def config_done(**args): ### BUG!!
...
```
Release Notes:
- N/ACargo.lock | 2
Cargo.toml | 2
crates/dap_adapters/src/gdb.rs | 34 ++++++++++-
crates/dap_adapters/src/go.rs | 12 +++-
crates/dap_adapters/src/javascript.rs | 21 ++++---
crates/dap_adapters/src/lldb.rs | 21 ++++---
crates/dap_adapters/src/php.rs | 2
crates/debugger_ui/src/debugger_panel.rs | 6 +-
crates/debugger_ui/src/session.rs | 4
crates/debugger_ui/src/tests/debugger_panel.rs | 4
crates/project/src/debugger/dap_command.rs | 6 +-
crates/project/src/debugger/dap_store.rs | 17 ++++--
crates/project/src/debugger/session.rs | 53 +++++++++++++------
13 files changed, 120 insertions(+), 64 deletions(-)
@@ -4050,7 +4050,7 @@ dependencies = [
[[package]]
name = "dap-types"
version = "0.0.1"
-source = "git+https://github.com/zed-industries/dap-types?rev=bfd4af0#bfd4af084bbaa5f344e6925370d7642e41d0b5b8"
+source = "git+https://github.com/zed-industries/dap-types?rev=be69a016ba710191b9fdded28c8b042af4b617f7#be69a016ba710191b9fdded28c8b042af4b617f7"
dependencies = [
"schemars",
"serde",
@@ -425,7 +425,7 @@ core-foundation = "0.10.0"
core-foundation-sys = "0.8.6"
ctor = "0.4.0"
dashmap = "6.0"
-dap-types = { git = "https://github.com/zed-industries/dap-types", rev = "bfd4af0" }
+dap-types = { git = "https://github.com/zed-industries/dap-types", rev = "be69a016ba710191b9fdded28c8b042af4b617f7" }
derive_more = "0.99.17"
dirs = "4.0"
ec4rs = "1.1"
@@ -3,7 +3,7 @@ use std::ffi::OsStr;
use anyhow::{Result, bail};
use async_trait::async_trait;
use gpui::AsyncApp;
-use task::{DebugAdapterConfig, DebugTaskDefinition};
+use task::{DebugAdapterConfig, DebugRequestType, DebugTaskDefinition};
use crate::*;
@@ -74,13 +74,37 @@ impl DebugAdapter for GdbDebugAdapter {
}
fn request_args(&self, config: &DebugTaskDefinition) -> Value {
+ let mut args = json!({
+ "request": match config.request {
+ DebugRequestType::Launch(_) => "launch",
+ DebugRequestType::Attach(_) => "attach",
+ },
+ });
+
+ let map = args.as_object_mut().unwrap();
match &config.request {
- dap::DebugRequestType::Attach(attach_config) => {
- json!({"pid": attach_config.process_id})
+ DebugRequestType::Attach(attach) => {
+ map.insert("pid".into(), attach.process_id.into());
}
- dap::DebugRequestType::Launch(launch_config) => {
- json!({"program": launch_config.program, "cwd": launch_config.cwd, "stopOnEntry": config.stop_on_entry, "args": launch_config.args.clone()})
+
+ DebugRequestType::Launch(launch) => {
+ map.insert("program".into(), launch.program.clone().into());
+
+ if !launch.args.is_empty() {
+ map.insert("args".into(), launch.args.clone().into());
+ }
+
+ if let Some(stop_on_entry) = config.stop_on_entry {
+ map.insert(
+ "stopAtBeginningOfMainSubprogram".into(),
+ stop_on_entry.into(),
+ );
+ }
+ if let Some(cwd) = launch.cwd.as_ref() {
+ map.insert("cwd".into(), cwd.to_string_lossy().into_owned().into());
+ }
}
}
+ args
}
}
@@ -83,19 +83,25 @@ impl DebugAdapter for GoDebugAdapter {
}
fn request_args(&self, config: &DebugTaskDefinition) -> Value {
- match &config.request {
+ let mut args = match &config.request {
dap::DebugRequestType::Attach(attach_config) => {
json!({
"processId": attach_config.process_id,
- "stopOnEntry": config.stop_on_entry,
})
}
dap::DebugRequestType::Launch(launch_config) => json!({
"program": launch_config.program,
"cwd": launch_config.cwd,
- "stopOnEntry": config.stop_on_entry,
"args": launch_config.args
}),
+ };
+
+ let map = args.as_object_mut().unwrap();
+
+ if let Some(stop_on_entry) = config.stop_on_entry {
+ map.insert("stopOnEntry".into(), stop_on_entry.into());
}
+
+ args
}
}
@@ -131,19 +131,20 @@ impl DebugAdapter for JsDebugAdapter {
match &config.request {
DebugRequestType::Attach(attach) => {
map.insert("processId".into(), attach.process_id.into());
- map.insert("stopOnEntry".into(), config.stop_on_entry.into());
}
DebugRequestType::Launch(launch) => {
map.insert("program".into(), launch.program.clone().into());
- map.insert("args".into(), launch.args.clone().into());
- map.insert(
- "cwd".into(),
- launch
- .cwd
- .as_ref()
- .map(|s| s.to_string_lossy().into_owned())
- .into(),
- );
+
+ if !launch.args.is_empty() {
+ map.insert("args".into(), launch.args.clone().into());
+ }
+
+ if let Some(stop_on_entry) = config.stop_on_entry {
+ map.insert("stopOnEntry".into(), stop_on_entry.into());
+ }
+ if let Some(cwd) = launch.cwd.as_ref() {
+ map.insert("cwd".into(), cwd.to_string_lossy().into_owned().into());
+ }
}
}
args
@@ -81,16 +81,17 @@ impl DebugAdapter for LldbDebugAdapter {
}
DebugRequestType::Launch(launch) => {
map.insert("program".into(), launch.program.clone().into());
- map.insert("stopOnEntry".into(), config.stop_on_entry.into());
- map.insert("args".into(), launch.args.clone().into());
- map.insert(
- "cwd".into(),
- launch
- .cwd
- .as_ref()
- .map(|s| s.to_string_lossy().into_owned())
- .into(),
- );
+
+ if !launch.args.is_empty() {
+ map.insert("args".into(), launch.args.clone().into());
+ }
+
+ if let Some(stop_on_entry) = config.stop_on_entry {
+ map.insert("stopOnEntry".into(), stop_on_entry.into());
+ }
+ if let Some(cwd) = launch.cwd.as_ref() {
+ map.insert("cwd".into(), cwd.to_string_lossy().into_owned().into());
+ }
}
}
args
@@ -119,7 +119,7 @@ impl DebugAdapter for PhpDebugAdapter {
"program": launch_config.program,
"cwd": launch_config.cwd,
"args": launch_config.args,
- "stopOnEntry": config.stop_on_entry,
+ "stopOnEntry": config.stop_on_entry.unwrap_or_default(),
})
}
}
@@ -185,7 +185,7 @@ impl DebugPanel {
) -> Vec<Entity<DebugSession>> {
self.sessions
.iter()
- .filter(|item| item.read(cx).session_id(cx) == Some(*client_id))
+ .filter(|item| item.read(cx).session_id(cx) == *client_id)
.map(|item| item.clone())
.collect()
}
@@ -200,7 +200,7 @@ impl DebugPanel {
.find(|item| {
let item = item.read(cx);
- item.session_id(cx) == Some(client_id)
+ item.session_id(cx) == client_id
})
.cloned()
}
@@ -227,7 +227,7 @@ impl DebugPanel {
if self
.sessions
.iter()
- .any(|item| item.read(cx).session_id(cx) == Some(*session_id))
+ .any(|item| item.read(cx).session_id(cx) == *session_id)
{
// We already have an item for this session.
return;
@@ -75,9 +75,9 @@ impl DebugSession {
})
}
- pub(crate) fn session_id(&self, cx: &App) -> Option<SessionId> {
+ pub(crate) fn session_id(&self, cx: &App) -> SessionId {
match &self.mode {
- DebugSessionState::Running(entity) => Some(entity.read(cx).session_id()),
+ DebugSessionState::Running(entity) => entity.read(cx).session_id(),
}
}
@@ -270,7 +270,7 @@ async fn test_we_can_only_have_one_panel_per_debug_session(
.clone()
});
- assert_eq!(client.id(), active_session.read(cx).session_id(cx).unwrap());
+ assert_eq!(client.id(), active_session.read(cx).session_id(cx));
assert_eq!(
ThreadId(1),
running_state.read(cx).selected_thread_id().unwrap()
@@ -307,7 +307,7 @@ async fn test_we_can_only_have_one_panel_per_debug_session(
.clone()
});
- assert_eq!(client.id(), active_session.read(cx).session_id(cx).unwrap());
+ assert_eq!(client.id(), active_session.read(cx).session_id(cx));
assert_eq!(
ThreadId(1),
running_state.read(cx).selected_thread_id().unwrap()
@@ -1479,7 +1479,7 @@ impl LocalDapCommand for ThreadsCommand {
type DapRequest = dap::requests::Threads;
fn to_dap(&self) -> <Self::DapRequest as dap::requests::Request>::Arguments {
- ()
+ dap::ThreadsArgument {}
}
fn response_from_dap(
@@ -1568,7 +1568,7 @@ impl LocalDapCommand for Initialize {
}
#[derive(Clone, Debug, Hash, PartialEq)]
-pub(super) struct ConfigurationDone;
+pub(super) struct ConfigurationDone {}
impl LocalDapCommand for ConfigurationDone {
type Response = ();
@@ -1581,7 +1581,7 @@ impl LocalDapCommand for ConfigurationDone {
}
fn to_dap(&self) -> <Self::DapRequest as dap::requests::Request>::Arguments {
- dap::ConfigurationDoneArguments
+ dap::ConfigurationDoneArguments {}
}
fn response_from_dap(
@@ -843,12 +843,17 @@ fn create_new_session(
cx.notify();
})?;
- match session
- .update(cx, |session, cx| {
- session.initialize_sequence(initialized_rx, cx)
- })?
- .await
- {
+ match {
+ session
+ .update(cx, |session, cx| session.request_initialize(cx))?
+ .await?;
+
+ session
+ .update(cx, |session, cx| {
+ session.initialize_sequence(initialized_rx, cx)
+ })?
+ .await
+ } {
Ok(_) => {}
Err(error) => {
this.update(cx, |this, cx| {
@@ -10,7 +10,7 @@ use super::dap_command::{
VariablesCommand,
};
use super::dap_store::DapAdapterDelegate;
-use anyhow::{Result, anyhow};
+use anyhow::{Context as _, Result, anyhow};
use collections::{HashMap, HashSet, IndexMap, IndexSet};
use dap::adapters::{DebugAdapter, DebugAdapterBinary};
use dap::messages::Response;
@@ -190,7 +190,7 @@ impl LocalMode {
delegate: DapAdapterDelegate,
messages_tx: futures::channel::mpsc::UnboundedSender<Message>,
cx: AsyncApp,
- ) -> Task<Result<(Self, Capabilities)>> {
+ ) -> Task<Result<Self>> {
Self::new_inner(
debug_adapters,
session_id,
@@ -214,7 +214,7 @@ impl LocalMode {
caps: Capabilities,
fail: bool,
cx: AsyncApp,
- ) -> Task<Result<(Self, Capabilities)>> {
+ ) -> Task<Result<Self>> {
use task::DebugRequestDisposition;
let request = match config.request.clone() {
@@ -349,7 +349,7 @@ impl LocalMode {
messages_tx: futures::channel::mpsc::UnboundedSender<Message>,
on_initialized: impl AsyncFnOnce(&mut LocalMode, AsyncApp) + 'static,
cx: AsyncApp,
- ) -> Task<Result<(Self, Capabilities)>> {
+ ) -> Task<Result<Self>> {
cx.spawn(async move |cx| {
let (adapter, binary) =
Self::get_adapter_binary(®istry, &config, &delegate, cx).await?;
@@ -374,11 +374,11 @@ impl LocalMode {
message_handler,
cx.clone(),
)
- .await?
+ .await
+ .with_context(|| "Failed to start communication with debug adapter")?
},
);
- let adapter_id = adapter.name().to_string().to_owned();
let mut session = Self {
client,
adapter,
@@ -387,11 +387,8 @@ impl LocalMode {
};
on_initialized(&mut session, cx.clone()).await;
- let capabilities = session
- .request(Initialize { adapter_id }, cx.background_executor().clone())
- .await?;
- Ok((session, capabilities))
+ Ok(session)
})
}
@@ -541,6 +538,12 @@ impl LocalMode {
self.config.label.clone()
}
+ fn request_initialization(&self, cx: &App) -> Task<Result<Capabilities>> {
+ let adapter_id = self.adapter.name().to_string();
+
+ self.request(Initialize { adapter_id }, cx.background_executor().clone())
+ }
+
fn initialize_sequence(
&self,
capabilities: &Capabilities,
@@ -590,7 +593,7 @@ impl LocalMode {
cx.update(|cx| this.send_all_breakpoints(false, cx))?.await;
if configuration_done_supported {
- this.request(ConfigurationDone, cx.background_executor().clone())
+ this.request(ConfigurationDone {}, cx.background_executor().clone())
} else {
Task::ready(Ok(()))
}
@@ -849,7 +852,7 @@ impl Session {
let (message_tx, message_rx) = futures::channel::mpsc::unbounded();
cx.spawn(async move |cx| {
- let (mode, capabilities) = LocalMode::new(
+ let mode = LocalMode::new(
debug_adapters,
session_id,
parent_session.clone(),
@@ -870,7 +873,6 @@ impl Session {
initialized_tx,
message_rx,
mode,
- capabilities,
cx,
)
})
@@ -893,7 +895,7 @@ impl Session {
let (message_tx, message_rx) = futures::channel::mpsc::unbounded();
cx.spawn(async move |cx| {
- let (mode, capabilities) = LocalMode::new_fake(
+ let mode = LocalMode::new_fake(
session_id,
parent_session.clone(),
breakpoint_store.clone(),
@@ -915,7 +917,6 @@ impl Session {
initialized_tx,
message_rx,
mode,
- capabilities,
cx,
)
})
@@ -1007,6 +1008,25 @@ impl Session {
}
}
+ pub(super) fn request_initialize(&mut self, cx: &mut Context<Self>) -> Task<Result<()>> {
+ match &self.mode {
+ Mode::Local(local_mode) => {
+ let capabilities = local_mode.clone().request_initialization(cx);
+
+ cx.spawn(async move |this, cx| {
+ let capabilities = capabilities.await?;
+ this.update(cx, |session, _| {
+ session.capabilities = capabilities;
+ })?;
+ Ok(())
+ })
+ }
+ Mode::Remote(_) => Task::ready(Err(anyhow!(
+ "Cannot send initialize request from remote session"
+ ))),
+ }
+ }
+
pub(super) fn initialize_sequence(
&mut self,
initialize_rx: oneshot::Receiver<()>,
@@ -1947,7 +1967,6 @@ fn create_local_session(
initialized_tx: oneshot::Sender<()>,
mut message_rx: futures::channel::mpsc::UnboundedReceiver<Message>,
mode: LocalMode,
- capabilities: Capabilities,
cx: &mut Context<Session>,
) -> Session {
let _background_tasks = vec![cx.spawn(async move |this: WeakEntity<Session>, cx| {
@@ -2003,7 +2022,7 @@ fn create_local_session(
child_session_ids: HashSet::default(),
parent_id: parent_session.map(|session| session.read(cx).id),
variables: Default::default(),
- capabilities,
+ capabilities: Capabilities::default(),
thread_states: ThreadStates::default(),
output_token: OutputToken(0),
ignore_breakpoints: false,