From 07dcb8f2bb0a28290a2a05d57995cb9ca2a5bf97 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Sat, 1 Nov 2025 17:30:13 -0400 Subject: [PATCH] debugger: Add program and module path fallbacks for debugpy toolchain (#40975) Fixes the Debugpy toolchain detection bug in #40324 When detecting what toolchain (venv) to use in the Debugpy configuration stage, we used to only base it off of the current working directory argument passed to the config. This is wrong behavior for cases like mono repos, where the correct virtual environment to use is nested in another folder. This PR fixes this issue by adding the program and module fields as fallbacks to check for virtual environments. We also added support for program/module relative paths as well when cwd is not None. Release Notes: - debugger: Improve mono repo virtual environment detection with Debugpy --------- Co-authored-by: Remco Smits --- crates/dap_adapters/src/dap_adapters.rs | 86 +++++++++++++------------ crates/dap_adapters/src/python.rs | 75 ++++++++++++++------- 2 files changed, 96 insertions(+), 65 deletions(-) diff --git a/crates/dap_adapters/src/dap_adapters.rs b/crates/dap_adapters/src/dap_adapters.rs index d8a706ba414af2c9e0beb1cffe8357bcece1dc52..2ab9cabc198c4b036301cb92e1f544ae640b898d 100644 --- a/crates/dap_adapters/src/dap_adapters.rs +++ b/crates/dap_adapters/src/dap_adapters.rs @@ -42,61 +42,63 @@ pub fn init(cx: &mut App) { } #[cfg(test)] -struct MockDelegate { - worktree_root: PathBuf, -} +mod test_mocks { + use super::*; -#[cfg(test)] -impl MockDelegate { - fn new() -> Arc { - Arc::new(Self { - worktree_root: PathBuf::from("/tmp/test"), - }) + pub(crate) struct MockDelegate { + worktree_root: PathBuf, } -} -#[cfg(test)] -#[async_trait::async_trait] -impl adapters::DapDelegate for MockDelegate { - fn worktree_id(&self) -> settings::WorktreeId { - settings::WorktreeId::from_usize(0) + impl MockDelegate { + pub(crate) fn new() -> Arc { + Arc::new(Self { + worktree_root: PathBuf::from("/tmp/test"), + }) + } } - fn worktree_root_path(&self) -> &std::path::Path { - &self.worktree_root - } + #[async_trait::async_trait] + impl adapters::DapDelegate for MockDelegate { + fn worktree_id(&self) -> settings::WorktreeId { + settings::WorktreeId::from_usize(0) + } - fn http_client(&self) -> Arc { - unimplemented!("Not needed for tests") - } + fn worktree_root_path(&self) -> &std::path::Path { + &self.worktree_root + } - fn node_runtime(&self) -> node_runtime::NodeRuntime { - unimplemented!("Not needed for tests") - } + fn http_client(&self) -> Arc { + unimplemented!("Not needed for tests") + } - fn toolchain_store(&self) -> Arc { - unimplemented!("Not needed for tests") - } + fn node_runtime(&self) -> node_runtime::NodeRuntime { + unimplemented!("Not needed for tests") + } - fn fs(&self) -> Arc { - unimplemented!("Not needed for tests") - } + fn toolchain_store(&self) -> Arc { + unimplemented!("Not needed for tests") + } - fn output_to_console(&self, _msg: String) {} + fn fs(&self) -> Arc { + unimplemented!("Not needed for tests") + } - async fn which(&self, _command: &std::ffi::OsStr) -> Option { - None - } + fn output_to_console(&self, _msg: String) {} - async fn read_text_file(&self, _path: &util::rel_path::RelPath) -> Result { - Ok(String::new()) - } + async fn which(&self, _command: &std::ffi::OsStr) -> Option { + None + } - async fn shell_env(&self) -> collections::HashMap { - collections::HashMap::default() - } + async fn read_text_file(&self, _path: &util::rel_path::RelPath) -> Result { + Ok(String::new()) + } - fn is_headless(&self) -> bool { - false + async fn shell_env(&self) -> collections::HashMap { + collections::HashMap::default() + } + + fn is_headless(&self) -> bool { + false + } } } diff --git a/crates/dap_adapters/src/python.rs b/crates/dap_adapters/src/python.rs index e718f66c78099044baed837da0ddc7bfa96ffa1c..4d81e5ba851305ae3adc2ee0a6ab6a29f43edd62 100644 --- a/crates/dap_adapters/src/python.rs +++ b/crates/dap_adapters/src/python.rs @@ -824,29 +824,58 @@ impl DebugAdapter for PythonDebugAdapter { .await; } - let base_path = config - .config - .get("cwd") - .and_then(|cwd| { - RelPath::new( - cwd.as_str() - .map(Path::new)? - .strip_prefix(delegate.worktree_root_path()) - .ok()?, - PathStyle::local(), - ) - .ok() + let base_paths = ["cwd", "program", "module"] + .into_iter() + .filter_map(|key| { + config.config.get(key).and_then(|cwd| { + RelPath::new( + cwd.as_str() + .map(Path::new)? + .strip_prefix(delegate.worktree_root_path()) + .ok()?, + PathStyle::local(), + ) + .ok() + }) }) - .unwrap_or_else(|| RelPath::empty().into()); - let toolchain = delegate - .toolchain_store() - .active_toolchain( - delegate.worktree_id(), - base_path.into_arc(), - language::LanguageName::new(Self::LANGUAGE_NAME), - cx, + .chain( + // While Debugpy's wiki saids absolute paths are required, but it actually supports relative paths when cwd is passed in. + // (Which should always be the case because Zed defaults to the cwd worktree root) + // So we want to check that these relative paths find toolchains as well. Otherwise, they won't be checked + // because the strip prefix in the iteration above will return an error + config + .config + .get("cwd") + .map(|_| { + ["program", "module"].into_iter().filter_map(|key| { + config.config.get(key).and_then(|value| { + let path = Path::new(value.as_str()?); + RelPath::new(path, PathStyle::local()).ok() + }) + }) + }) + .into_iter() + .flatten(), ) - .await; + .chain([RelPath::empty().into()]); + + let mut toolchain = None; + + for base_path in base_paths { + if let Some(found_toolchain) = delegate + .toolchain_store() + .active_toolchain( + delegate.worktree_id(), + base_path.into_arc(), + language::LanguageName::new(Self::LANGUAGE_NAME), + cx, + ) + .await + { + toolchain = Some(found_toolchain); + break; + } + } self.fetch_debugpy_whl(toolchain.clone(), delegate) .await @@ -914,7 +943,7 @@ mod tests { let result = adapter .get_installed_binary( - &MockDelegate::new(), + &test_mocks::MockDelegate::new(), &task_def, None, None, @@ -955,7 +984,7 @@ mod tests { let result_host = adapter .get_installed_binary( - &MockDelegate::new(), + &test_mocks::MockDelegate::new(), &task_def_host, None, None,