From bbfcd885ab9289613ff523ecb6c6b9864abf7fcc Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 6 May 2025 18:39:49 +0200 Subject: [PATCH] debugger: Allow locators to generate full debug scenarios (#30014) Closes #ISSUE Release Notes: - N/A --------- Co-authored-by: Anthony Co-authored-by: Remco Smits --- crates/dap/src/registry.rs | 16 ++-- crates/debugger_ui/src/session/running.rs | 86 +++++++++++++++---- crates/editor/src/editor.rs | 5 +- crates/languages/src/rust.rs | 41 +-------- crates/project/src/debugger/dap_store.rs | 81 +++++------------ crates/project/src/debugger/locators/cargo.rs | 72 ++++++++++++---- crates/proto/proto/debugger.proto | 1 + crates/task/src/debug_format.rs | 16 +++- crates/task/src/lib.rs | 25 ++++-- 9 files changed, 197 insertions(+), 146 deletions(-) diff --git a/crates/dap/src/registry.rs b/crates/dap/src/registry.rs index 4a733109ae..c631452b8c 100644 --- a/crates/dap/src/registry.rs +++ b/crates/dap/src/registry.rs @@ -1,9 +1,9 @@ use anyhow::Result; use async_trait::async_trait; use collections::FxHashMap; -use gpui::{App, Global}; +use gpui::{App, Global, SharedString}; use parking_lot::RwLock; -use task::{DebugRequest, SpawnInTerminal}; +use task::{DebugRequest, DebugScenario, SpawnInTerminal, TaskTemplate}; use crate::adapters::{DebugAdapter, DebugAdapterName}; use std::{collections::BTreeMap, sync::Arc}; @@ -11,15 +11,17 @@ use std::{collections::BTreeMap, sync::Arc}; /// Given a user build configuration, locator creates a fill-in debug target ([DebugRequest]) on behalf of the user. #[async_trait] pub trait DapLocator: Send + Sync { + fn name(&self) -> SharedString; /// Determines whether this locator can generate debug target for given task. - fn accepts(&self, build_config: &SpawnInTerminal) -> bool; + fn create_scenario(&self, build_config: &TaskTemplate, adapter: &str) -> Option; + async fn run(&self, build_config: SpawnInTerminal) -> Result; } #[derive(Default)] struct DapRegistryState { adapters: BTreeMap>, - locators: FxHashMap>, + locators: FxHashMap>, } #[derive(Clone, Default)] @@ -48,15 +50,15 @@ impl DapRegistry { ); } - pub fn add_locator(&self, name: String, locator: Arc) { - let _previous_value = self.0.write().locators.insert(name, locator); + pub fn add_locator(&self, locator: Arc) { + let _previous_value = self.0.write().locators.insert(locator.name(), locator); debug_assert!( _previous_value.is_none(), "Attempted to insert a new debug locator when one is already registered" ); } - pub fn locators(&self) -> FxHashMap> { + pub fn locators(&self) -> FxHashMap> { self.0.read().locators.clone() } diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 34baa44caa..5c2a769116 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -38,8 +38,8 @@ use serde_json::Value; use settings::Settings; use stack_frame_list::StackFrameList; use task::{ - DebugScenario, LaunchRequest, TaskContext, substitute_variables_in_map, - substitute_variables_in_str, + BuildTaskDefinition, DebugScenario, LaunchRequest, ShellBuilder, SpawnInTerminal, TaskContext, + substitute_variables_in_map, substitute_variables_in_str, }; use terminal_view::TerminalView; use ui::{ @@ -696,6 +696,7 @@ 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(); cx.spawn_in(window, async move |this, cx| { let DebugScenario { adapter, @@ -706,27 +707,68 @@ impl RunningState { tcp_connection, stop_on_entry, } = scenario; - let request = if let Some(request) = request { - request - } else if let Some(build) = build { - let Some(task) = task_store.update(cx, |this, cx| { - this.task_inventory().and_then(|inventory| { - inventory - .read(cx) - .task_template_by_label(buffer, worktree_id, &build, cx) - }) - })? - else { - anyhow::bail!("Couldn't find task template for {:?}", build) + let build_output = if let Some(build) = build { + let (task, locator_name) = match build { + BuildTaskDefinition::Template { + task_template, + locator_name, + } => (task_template, locator_name), + BuildTaskDefinition::ByName(ref label) => { + let Some(task) = task_store.update(cx, |this, cx| { + this.task_inventory().and_then(|inventory| { + inventory.read(cx).task_template_by_label( + buffer, + worktree_id, + &label, + cx, + ) + }) + })? + else { + anyhow::bail!("Couldn't find task template for {:?}", build) + }; + (task, None) + } }; + let locator_name = if let Some(locator_name) = locator_name { + debug_assert!(request.is_none()); + Some(locator_name) + } else if request.is_none() { + dap_store + .update(cx, |this, cx| { + this.debug_scenario_for_build_task(task.clone(), adapter.clone(), cx) + .and_then(|scenario| match scenario.build { + Some(BuildTaskDefinition::Template { + locator_name, .. + }) => locator_name, + _ => None, + }) + }) + .ok() + .flatten() + } else { + None + }; + let Some(task) = task.resolve_task("debug-build-task", &task_context) else { anyhow::bail!("Could not resolve task variables within a debug scenario"); }; + let builder = ShellBuilder::new(is_local, &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); + + let task_with_shell = SpawnInTerminal { + command_label, + command, + args, + ..task.resolved.clone() + }; let terminal = project .update_in(cx, |project, window, cx| { project.create_terminal( - TerminalKind::Task(task.resolved.clone()), + TerminalKind::Task(task_with_shell.clone()), window.window_handle(), cx, ) @@ -761,9 +803,19 @@ impl RunningState { if !exit_status.success() { anyhow::bail!("Build failed"); } - + Some((task.resolved.clone(), locator_name)) + } else { + None + }; + let request = if let Some(request) = request { + request + } else if let Some((task, locator_name)) = build_output { + let locator_name = locator_name + .ok_or_else(|| anyhow!("Could not find a valid locator for a build task"))?; dap_store - .update(cx, |this, cx| this.run_debug_locator(task.resolved, cx))? + .update(cx, |this, cx| { + this.run_debug_locator(&locator_name, task, cx) + })? .await? } else { return Err(anyhow!("No request or build provided")); diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 79a4873c20..01567e0208 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5216,10 +5216,7 @@ impl Editor { for (_, task) in &resolved_tasks.templates { if let Some(scenario) = this .debug_scenario_for_build_task( - task.resolved.clone(), - SharedString::from( - task.original_task().label.clone(), - ), + task.original_task().clone(), debug_adapter.clone(), cx, ) diff --git a/crates/languages/src/rust.rs b/crates/languages/src/rust.rs index 0e28d69c55..726ce021f8 100644 --- a/crates/languages/src/rust.rs +++ b/crates/languages/src/rust.rs @@ -651,11 +651,6 @@ impl ContextProvider for RustContextProvider { } else { vec!["run".into()] }; - let build_task_args = if let Some(package_to_run) = package_to_run { - vec!["build".into(), "-p".into(), package_to_run] - } else { - vec!["build".into()] - }; let mut task_templates = vec![ TaskTemplate { label: format!( @@ -689,8 +684,8 @@ impl ContextProvider for RustContextProvider { "test".into(), "-p".into(), RUST_PACKAGE_TASK_VARIABLE.template_value(), - RUST_TEST_NAME_TASK_VARIABLE.template_value(), "--".into(), + RUST_TEST_NAME_TASK_VARIABLE.template_value(), "--nocapture".into(), ], tags: vec!["rust-test".to_owned()], @@ -709,9 +704,9 @@ impl ContextProvider for RustContextProvider { "--doc".into(), "-p".into(), RUST_PACKAGE_TASK_VARIABLE.template_value(), - RUST_DOC_TEST_NAME_TASK_VARIABLE.template_value(), "--".into(), "--nocapture".into(), + RUST_DOC_TEST_NAME_TASK_VARIABLE.template_value(), ], tags: vec!["rust-doc-test".to_owned()], cwd: Some("$ZED_DIRNAME".to_owned()), @@ -728,6 +723,7 @@ impl ContextProvider for RustContextProvider { "test".into(), "-p".into(), RUST_PACKAGE_TASK_VARIABLE.template_value(), + "--".into(), RUST_TEST_FRAGMENT_TASK_VARIABLE.template_value(), ], tags: vec!["rust-mod-test".to_owned()], @@ -783,37 +779,6 @@ impl ContextProvider for RustContextProvider { cwd: Some("$ZED_DIRNAME".to_owned()), ..TaskTemplate::default() }, - TaskTemplate { - label: format!( - "Build {} {} (package: {})", - RUST_BIN_KIND_TASK_VARIABLE.template_value(), - RUST_BIN_NAME_TASK_VARIABLE.template_value(), - RUST_PACKAGE_TASK_VARIABLE.template_value(), - ), - cwd: Some("$ZED_DIRNAME".to_owned()), - command: "cargo".into(), - args: build_task_args, - tags: vec!["rust-main".to_owned()], - ..TaskTemplate::default() - }, - TaskTemplate { - label: format!( - "Build Test '{}' (package: {})", - RUST_TEST_NAME_TASK_VARIABLE.template_value(), - RUST_PACKAGE_TASK_VARIABLE.template_value(), - ), - command: "cargo".into(), - args: vec![ - "test".into(), - "-p".into(), - RUST_PACKAGE_TASK_VARIABLE.template_value(), - RUST_TEST_NAME_TASK_VARIABLE.template_value(), - "--no-run".into(), - ], - tags: vec!["rust-test".to_owned()], - cwd: Some("$ZED_DIRNAME".to_owned()), - ..TaskTemplate::default() - }, ]; if let Some(custom_target_dir) = custom_target_dir { diff --git a/crates/project/src/debugger/dap_store.rs b/crates/project/src/debugger/dap_store.rs index be9139318c..965e0c6e7e 100644 --- a/crates/project/src/debugger/dap_store.rs +++ b/crates/project/src/debugger/dap_store.rs @@ -46,7 +46,7 @@ use std::{ path::{Path, PathBuf}, sync::{Arc, Once}, }; -use task::{DebugScenario, SpawnInTerminal}; +use task::{DebugScenario, SpawnInTerminal, TaskTemplate}; use util::{ResultExt as _, merge_json_value_into}; use worktree::Worktree; @@ -99,8 +99,7 @@ impl DapStore { pub fn init(client: &AnyProtoClient, cx: &mut App) { static ADD_LOCATORS: Once = Once::new(); ADD_LOCATORS.call_once(|| { - DapRegistry::global(cx) - .add_locator("cargo".into(), Arc::new(locators::cargo::CargoLocator {})) + DapRegistry::global(cx).add_locator(Arc::new(locators::cargo::CargoLocator {})) }); client.add_entity_request_handler(Self::handle_run_debug_locator); client.add_entity_request_handler(Self::handle_get_debug_adapter_binary); @@ -282,78 +281,38 @@ impl DapStore { pub fn debug_scenario_for_build_task( &self, - mut build: SpawnInTerminal, - unresoved_label: SharedString, + build: TaskTemplate, adapter: SharedString, cx: &mut App, ) -> Option { - build.args = build - .args - .into_iter() - .map(|arg| { - if arg.starts_with("$") { - arg.strip_prefix("$") - .and_then(|arg| build.env.get(arg).map(ToOwned::to_owned)) - .unwrap_or_else(|| arg) - } else { - arg - } - }) - .collect(); - DapRegistry::global(cx) .locators() .values() - .find(|locator| locator.accepts(&build)) - .map(|_| DebugScenario { - adapter, - label: format!("Debug `{}`", build.label).into(), - build: Some(unresoved_label), - request: None, - initialize_args: None, - tcp_connection: None, - stop_on_entry: None, - }) + .find_map(|locator| locator.create_scenario(&build, &adapter)) } pub fn run_debug_locator( &mut self, - mut build_command: SpawnInTerminal, + locator_name: &str, + build_command: SpawnInTerminal, cx: &mut Context, ) -> Task> { match &self.mode { DapStoreMode::Local(_) => { // Pre-resolve args with existing environment. - build_command.args = build_command - .args - .into_iter() - .map(|arg| { - if arg.starts_with("$") { - arg.strip_prefix("$") - .and_then(|arg| build_command.env.get(arg).map(ToOwned::to_owned)) - .unwrap_or_else(|| arg) - } else { - arg - } - }) - .collect(); - let locators = DapRegistry::global(cx) - .locators() - .values() - .filter(|locator| locator.accepts(&build_command)) - .cloned() - .collect::>(); - if !locators.is_empty() { + let locators = DapRegistry::global(cx).locators(); + let locator = locators.get(locator_name); + + if let Some(locator) = locator.cloned() { cx.background_spawn(async move { - for locator in locators { - let result = locator - .run(build_command.clone()) - .await - .log_with_level(log::Level::Error); - if let Some(result) = result { - return Ok(result); - } + let result = locator + .run(build_command.clone()) + .await + .log_with_level(log::Level::Error); + if let Some(result) = result { + return Ok(result); } + Err(anyhow!( "None of the locators for task `{}` completed successfully", build_command.label @@ -370,6 +329,7 @@ impl DapStore { let request = ssh.upstream_client.request(proto::RunDebugLocators { project_id: ssh.upstream_project_id, build_command: Some(build_command.to_proto()), + locator: locator_name.to_owned(), }); cx.background_spawn(async move { let response = request.await?; @@ -789,8 +749,11 @@ impl DapStore { .build_command .ok_or_else(|| anyhow!("missing definition"))?; let build_task = SpawnInTerminal::from_proto(task); + let locator = envelope.payload.locator; let request = this - .update(&mut cx, |this, cx| this.run_debug_locator(build_task, cx))? + .update(&mut cx, |this, cx| { + this.run_debug_locator(&locator, build_task, cx) + })? .await?; Ok(request.to_proto()) diff --git a/crates/project/src/debugger/locators/cargo.rs b/crates/project/src/debugger/locators/cargo.rs index cd665562c5..4531c97e50 100644 --- a/crates/project/src/debugger/locators/cargo.rs +++ b/crates/project/src/debugger/locators/cargo.rs @@ -1,12 +1,13 @@ use anyhow::{Result, anyhow}; use async_trait::async_trait; use dap::{DapLocator, DebugRequest}; +use gpui::SharedString; use serde_json::Value; use smol::{ io::AsyncReadExt, process::{Command, Stdio}, }; -use task::SpawnInTerminal; +use task::{BuildTaskDefinition, DebugScenario, ShellBuilder, SpawnInTerminal, TaskTemplate}; pub(crate) struct CargoLocator; @@ -37,18 +38,51 @@ async fn find_best_executable(executables: &[String], test_name: &str) -> Option } #[async_trait] impl DapLocator for CargoLocator { - fn accepts(&self, build_config: &SpawnInTerminal) -> bool { + fn name(&self) -> SharedString { + SharedString::new_static("rust-cargo-locator") + } + fn create_scenario(&self, build_config: &TaskTemplate, adapter: &str) -> Option { if build_config.command != "cargo" { - return false; + return None; } - let Some(command) = build_config.args.first().map(|s| s.as_str()) else { - return false; - }; - if matches!(command, "check" | "run") { - return false; + let mut task_template = build_config.clone(); + let cargo_action = task_template.args.first_mut()?; + if cargo_action == "check" { + return None; } - !matches!(command, "test" | "bench") - || build_config.args.iter().any(|arg| arg == "--no-run") + + match cargo_action.as_ref() { + "run" => { + *cargo_action = "build".to_owned(); + } + "test" | "bench" => { + let delimiter = task_template + .args + .iter() + .position(|arg| arg == "--") + .unwrap_or(task_template.args.len()); + if !task_template.args[..delimiter] + .iter() + .any(|arg| arg == "--no-run") + { + task_template.args.insert(delimiter, "--no-run".to_owned()); + } + } + _ => {} + } + let label = format!("Debug `{}`", build_config.label); + Some(DebugScenario { + adapter: adapter.to_owned().into(), + label: SharedString::from(label), + build: Some(BuildTaskDefinition::Template { + task_template, + locator_name: Some(self.name()), + }), + request: None, + initialize_args: None, + tcp_connection: None, + stop_on_entry: None, + }) } async fn run(&self, build_config: SpawnInTerminal) -> Result { @@ -57,10 +91,19 @@ impl DapLocator for CargoLocator { "Couldn't get cwd from debug config which is needed for locators" )); }; - - let mut child = Command::new("cargo") - .args(&build_config.args) - .arg("--message-format=json") + let builder = ShellBuilder::new(true, &build_config.shell).non_interactive(); + let (program, args) = builder.build( + "cargo".into(), + &build_config + .args + .iter() + .cloned() + .take_while(|arg| arg != "--") + .chain(Some("--message-format=json".to_owned())) + .collect(), + ); + let mut child = Command::new(program) + .args(args) .envs(build_config.env.iter().map(|(k, v)| (k.clone(), v.clone()))) .current_dir(cwd) .stdout(Stdio::piped()) @@ -89,7 +132,6 @@ impl DapLocator for CargoLocator { if executables.is_empty() { return Err(anyhow!("Couldn't get executable in cargo locator")); }; - let is_test = build_config.args.first().map_or(false, |arg| arg == "test"); let mut test_name = None; diff --git a/crates/proto/proto/debugger.proto b/crates/proto/proto/debugger.proto index 8a8f037413..ad98053c05 100644 --- a/crates/proto/proto/debugger.proto +++ b/crates/proto/proto/debugger.proto @@ -580,6 +580,7 @@ message DebugAdapterBinary { message RunDebugLocators { uint64 project_id = 1; SpawnInTerminal build_command = 2; + string locator = 3; } message DebugRequest { diff --git a/crates/task/src/debug_format.rs b/crates/task/src/debug_format.rs index 068b0885b0..c5f62f36af 100644 --- a/crates/task/src/debug_format.rs +++ b/crates/task/src/debug_format.rs @@ -6,6 +6,8 @@ use serde::{Deserialize, Serialize}; use std::path::PathBuf; use std::{net::Ipv4Addr, path::Path}; +use crate::TaskTemplate; + /// Represents the host information of the debug adapter #[derive(Default, Deserialize, Serialize, PartialEq, Eq, JsonSchema, Clone, Debug)] pub struct TcpArgumentsTemplate { @@ -171,6 +173,18 @@ impl From for DebugRequest { } } +#[derive(Deserialize, Serialize, PartialEq, Eq, JsonSchema, Clone, Debug)] +#[serde(untagged)] +#[allow(clippy::large_enum_variant)] +pub enum BuildTaskDefinition { + ByName(SharedString), + Template { + #[serde(flatten)] + task_template: TaskTemplate, + #[serde(skip)] + locator_name: Option, + }, +} /// This struct represent a user created debug task #[derive(Deserialize, Serialize, PartialEq, Eq, JsonSchema, Clone, Debug)] #[serde(rename_all = "snake_case")] @@ -180,7 +194,7 @@ pub struct DebugScenario { pub label: SharedString, /// A task to run prior to spawning the debuggee. #[serde(default)] - pub build: Option, + pub build: Option, #[serde(flatten)] pub request: Option, /// Additional initialization arguments to be sent on DAP initialization diff --git a/crates/task/src/lib.rs b/crates/task/src/lib.rs index 1e7b93a161..c0c77c0924 100644 --- a/crates/task/src/lib.rs +++ b/crates/task/src/lib.rs @@ -16,7 +16,8 @@ use std::path::PathBuf; use std::str::FromStr; pub use debug_format::{ - AttachRequest, DebugRequest, DebugScenario, DebugTaskFile, LaunchRequest, TcpArgumentsTemplate, + AttachRequest, BuildTaskDefinition, DebugRequest, DebugScenario, DebugTaskFile, LaunchRequest, + TcpArgumentsTemplate, }; pub use task_template::{ DebugArgsRequest, HideStrategy, RevealStrategy, TaskTemplate, TaskTemplates, @@ -341,6 +342,7 @@ enum WindowsShellType { pub struct ShellBuilder { program: String, args: Vec, + interactive: bool, } pub static DEFAULT_REMOTE_SHELL: &str = "\"${SHELL:-sh}\""; @@ -359,7 +361,15 @@ impl ShellBuilder { Shell::Program(shell) => (shell.clone(), Vec::new()), Shell::WithArguments { program, args, .. } => (program.clone(), args.clone()), }; - Self { program, args } + Self { + program, + args, + interactive: true, + } + } + pub fn non_interactive(mut self) -> Self { + self.interactive = false; + self } } @@ -367,7 +377,8 @@ impl ShellBuilder { impl ShellBuilder { /// Returns the label to show in the terminal tab pub fn command_label(&self, command_label: &str) -> String { - format!("{} -i -c '{}'", self.program, command_label) + let interactivity = self.interactive.then_some("-i ").unwrap_or_default(); + format!("{} {interactivity}-c '{}'", self.program, command_label) } /// Returns the program and arguments to run this task in a shell. @@ -379,8 +390,12 @@ impl ShellBuilder { command.push_str(&arg); command }); - self.args - .extend(["-i".to_owned(), "-c".to_owned(), combined_command]); + self.args.extend( + self.interactive + .then(|| "-i".to_owned()) + .into_iter() + .chain(["-c".to_owned(), combined_command]), + ); (self.program, self.args) }