Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello @akiselev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the debugging capabilities by integrating native Debug Adapter Protocol (DAP) support for GDB and CUDA-GDB. This allows users to leverage these powerful debuggers with a standardized interface, improving the debugging experience for C/C++ and CUDA projects. The implementation prioritizes native DAP for efficiency and minimal dependencies, while also enhancing project type detection to intelligently recommend the correct debugger. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
@claude review this PR |
|
Claude finished @akiselev's task in 1m 54s —— View job Code Review Complete ✓
SummaryThis PR successfully implements native DAP support for GDB (14+) and CUDA-GDB debuggers. The implementation follows the existing adapter patterns well, includes comprehensive documentation, and has good test coverage. I've identified several issues ranging from critical bugs to minor improvements. Critical Issues1. Version Check Logic Error (gdb_common.rs:35)Bug: The version comparison logic is incorrect and will reject GDB 15.0+ major > 14 || (major == 14 && minor >= 1)This condition is false for GDB 15.0, 16.0, etc. because when Fix: Change to: major >= 15 || (major == 14 && minor >= 1)Or more clearly: (major == 14 && minor >= 1) || major > 14Impact: Users with GDB 15+ will be incorrectly told their version is too old. 2. Missing Public Export (gdb_common.rs)Issue: Integration tests at tests/integration.rs:288 import In src/setup/adapters/mod.rs:9, pub mod gdb_common;But this only makes it public within the Verification needed: Check if 3. Crate Name Mismatch in Tests (integration.rs:288)Issue: Test uses use debugger::setup::adapters::gdb_common::{parse_gdb_version, is_gdb_version_sufficient};Action needed: Verify the crate is named High Priority Issues4. Incomplete Version Parsing (gdb_common.rs:6-14)Issue: Version parsing only extracts the first token starting with a digit, which may fail on some GDB output formats. The current implementation: output.lines().next()
.and_then(|line| {
line.split_whitespace()
.find(|token| token.chars().next().map_or(false, |c| c.is_ascii_digit()))
})Problems:
Example failure case: This would extract "14.1-0ubuntu1" which fails parsing in Recommendation: Add more robust parsing: pub fn parse_gdb_version(output: &str) -> Option<String> {
output
.lines()
.next()
.and_then(|line| {
line.split_whitespace()
.find(|token| token.chars().next().map_or(false, |c| c.is_ascii_digit()))
})
.and_then(|token| {
// Extract just the version number part (e.g., "14.1" from "14.1-ubuntu")
token.split(|c: char| c == '-' || c.is_whitespace())
.next()
.map(|s| s.to_string())
})
}5. Platform Check Inconsistency (cuda_gdb.rs:32 vs detector.rs:32)Issue: CUDA-GDB adapter performs platform check in In detector.rs: // CUDA detection must precede C/C++ (.cu files are valid C++ but require CUDA-GDB)
if has_extension_in_dir(dir, "cu") {
types.push(ProjectType::Cuda);
}This will detect CUDA projects on Windows/macOS, but CUDA-GDB only works on Linux (cuda_gdb.rs:32-34). Impact: Users on macOS/Windows with Recommendation: Either:
6. Registry Ordering Issue (detector.rs:101)Concern: For C/C++ projects, debuggers_for_project returns This inconsistency might confuse users. The README.md states GDB is for "C/C++ debugging with GDB ≥14.1" but C/C++ projects don't get GDB recommended. Questions:
Medium Priority Issues7. Missing Error Context (gdb.rs:86, cuda_gdb.rs:98)Issue: Error messages don't include the attempted path: Err(Error::Internal(format!("Cannot install GDB: {}", reason)))Improvement: Include more context: Err(Error::Internal(format!("Cannot install GDB from {}: {}", path.display(), reason)))This helps users debug installation issues. 8. Duplicate Code (gdb.rs vs cuda_gdb.rs)Observation: The two adapter implementations share nearly identical code:
Recommendation: Consider extracting common functionality into pub fn create_install_result(path: PathBuf) -> impl Future<Output = Result<InstallResult>> {
async move {
let version = get_gdb_version(&path).await;
Ok(InstallResult {
path,
version,
args: vec!["-i=dap".to_string()],
})
}
}This reduces duplication and keeps the adapters DRY. 9. Test Organization (integration.rs:461-531)Observation: The GDB test Suggestion: Consider parameterizing the test or extracting shared test logic: fn test_debugging_workflow_with_adapter(adapter_name: &str, adapter_path: PathBuf) {
// shared test logic
}
#[test]
fn test_basic_debugging_workflow_c_gdb() {
if let Some(path) = gdb_available() {
test_debugging_workflow_with_adapter("gdb", path);
}
}10. Documentation Clarity (README.md:9)Minor: The version requirement states "GDB ≥14.1" but the code comment at gdb_common.rs:35 says "GDB ≥14.1 required for Python-based DAP implementation". Suggestion: Be consistent and add a note about the Python requirement in user-facing docs, as GDB built with Low Priority / Nitpicks11. Code Style: Early Returns (gdb.rs:30-57, cuda_gdb.rs:31-62)The 12. Unused PathBuf Import (gdb.rs:10)Line 10 imports PathBuf but it's only used in function signatures, not in the implementation. Not a problem, just noting for completeness. 13. Test Skip Messages (integration.rs:465, 525)Consider making skip messages more informative: eprintln!("Skipping test: GDB ≥14.1 not available. Install GDB 14.1+ to run this test.");14. Magic Number in Tests (integration.rs:501)The timeout value const BREAKPOINT_TIMEOUT_SECS: u64 = 30;Positive Observations ✅
RecommendationApprove with changes required. The critical issues (version check bug, module visibility) should be fixed before merging, but the overall implementation is solid and well-designed. The medium/low priority issues can be addressed in follow-up PRs if needed. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds native DAP support for GDB and CUDA-GDB, which is a significant and well-structured feature addition. The implementation is thorough, including new installers, project type detection, and integration tests. I've identified a few areas for improvement, primarily concerning code efficiency in the new installer modules and a correctness bug in version parsing. There are also opportunities to reduce code duplication and improve readability. Overall, this is a great contribution, and addressing these points will make the new functionality more robust and maintainable.
| pub fn is_gdb_version_sufficient(version: &str) -> bool { | ||
| let parts: Vec<&str> = version.split('.').collect(); | ||
| let Some(major_str) = parts.get(0) else { | ||
| return false; | ||
| }; | ||
| let Some(minor_str) = parts.get(1) else { | ||
| return false; | ||
| }; | ||
| let Ok(major) = major_str.parse::<u32>() else { | ||
| return false; | ||
| }; | ||
| let Ok(minor) = minor_str.parse::<u32>() else { | ||
| return false; | ||
| }; | ||
|
|
||
| major > 14 || (major == 14 && minor >= 1) | ||
| } |
There was a problem hiding this comment.
The is_gdb_version_sufficient function doesn't correctly handle GDB versions with suffixes, such as 14.2-arch, which your parse_gdb_version function is designed to extract. The call to minor_str.parse::<u32>() will fail for a string like "2-arch", causing the function to incorrectly return false for a valid GDB version.
To fix this, you should parse only the numeric part of the minor version string. This change will also fix the behavior of the gdb_available() test helper in tests/integration.rs.
| pub fn is_gdb_version_sufficient(version: &str) -> bool { | |
| let parts: Vec<&str> = version.split('.').collect(); | |
| let Some(major_str) = parts.get(0) else { | |
| return false; | |
| }; | |
| let Some(minor_str) = parts.get(1) else { | |
| return false; | |
| }; | |
| let Ok(major) = major_str.parse::<u32>() else { | |
| return false; | |
| }; | |
| let Ok(minor) = minor_str.parse::<u32>() else { | |
| return false; | |
| }; | |
| major > 14 || (major == 14 && minor >= 1) | |
| } | |
| pub fn is_gdb_version_sufficient(version: &str) -> bool { | |
| let parts: Vec<&str> = version.split('.').collect(); | |
| let Some(major_str) = parts.get(0) else { | |
| return false; | |
| }; | |
| let Some(minor_str) = parts.get(1) else { | |
| return false; | |
| }; | |
| let Ok(major) = major_str.parse::<u32>() else { | |
| return false; | |
| }; | |
| // Take only the numeric part of the minor version string. | |
| let minor_numeric_str: String = minor_str.chars().take_while(|c| c.is_ascii_digit()).collect(); | |
| let Ok(minor) = minor_numeric_str.parse::<u32>() else { | |
| return false; | |
| }; | |
| major > 14 || (major == 14 && minor >= 1) | |
| } |
| async fn install(&self, _opts: InstallOptions) -> Result<InstallResult> { | ||
| let method = self.best_method().await?; | ||
|
|
||
| match method { | ||
| InstallMethod::AlreadyInstalled { path } => { | ||
| let version = get_gdb_version(&path).await; | ||
| Ok(InstallResult { | ||
| path, | ||
| version, | ||
| args: vec!["-i=dap".to_string()], | ||
| }) | ||
| } | ||
| InstallMethod::NotSupported { reason } => { | ||
| Err(Error::Internal(format!("Cannot install GDB: {}", reason))) | ||
| } | ||
| _ => Err(Error::Internal("Unexpected installation method".to_string())), | ||
| } | ||
| } |
There was a problem hiding this comment.
The install method is inefficient as it repeatedly calls get_gdb_version. The call chain is install -> best_method -> get_gdb_version, and then install -> get_gdb_version again. This results in executing the gdb --version command multiple times, which can be slow.
You can make this more efficient by calling self.status().await? and reusing its result, which avoids re-running which::which and gdb --version.
async fn install(&self, _opts: InstallOptions) -> Result<InstallResult> {
match self.status().await? {
InstallStatus::Installed { path, version } => Ok(InstallResult {
path,
version,
args: vec!["-i=dap".to_string()],
}),
InstallStatus::Broken { reason, .. } => {
Err(Error::Internal(format!("Cannot install GDB: {}", reason)))
}
InstallStatus::NotInstalled => Err(Error::Internal(
"GDB ≥14.1 not found. Install via your system package manager.".to_string(),
)),
}
}| async fn install(&self, _opts: InstallOptions) -> Result<InstallResult> { | ||
| let method = self.best_method().await?; | ||
|
|
||
| match method { | ||
| InstallMethod::AlreadyInstalled { path } => { | ||
| let version = get_gdb_version(&path).await; | ||
| Ok(InstallResult { | ||
| path, | ||
| version, | ||
| args: vec!["-i=dap".to_string()], | ||
| }) | ||
| } | ||
| InstallMethod::NotSupported { reason } => { | ||
| Err(Error::Internal(format!("Cannot install CUDA-GDB: {}", reason))) | ||
| } | ||
| _ => Err(Error::Internal("Unexpected installation method".to_string())), | ||
| } | ||
| } |
There was a problem hiding this comment.
The install method is inefficient. It calls best_method(), which in turn calls get_gdb_version, and then install calls get_gdb_version again. This leads to multiple executions of the cuda-gdb --version command.
To improve performance, you should reuse the result from self.status().await? to avoid redundant command executions.
async fn install(&self, _opts: InstallOptions) -> Result<InstallResult> {
match self.status().await? {
InstallStatus::Installed { path, version } => Ok(InstallResult {
path,
version,
args: vec!["-i=dap".to_string()],
}),
InstallStatus::Broken { reason, .. } => {
Err(Error::Internal(format!("Cannot install CUDA-GDB: {}", reason)))
}
InstallStatus::NotInstalled => Err(Error::Internal(
"CUDA-GDB not found. Install NVIDIA CUDA Toolkit from https://developer.nvidia.com/cuda-downloads".to_string(),
)),
}
}
docs/plan/cuda-gdb.md
Outdated
| | Native DAP over cdt-gdb-adapter | cdt-gdb-adapter requires Node.js runtime → adds ~50MB dependency + complexity → native DAP (`-i=dap`) achieves same goal with zero dependencies → start with simpler approach, add fallback only if needed | | ||
| | Phased implementation (GDB first, then CUDA-GDB) | CUDA-GDB native DAP is undocumented by NVIDIA → testing on standard GDB validates approach with better documentation → if GDB works, CUDA-GDB (same codebase) likely works → reduces risk of discovering issues late | | ||
| | Stdio transport mode | GDB native DAP uses stdin/stdout like lldb-dap → existing DapClient::spawn() handles this pattern → no new transport code needed → matches established adapter integration | | ||
| | Version check ≥14.2 | GDB native DAP requires Python support, added in GDB 14.1 → CUDA-GDB 13.x is based on GDB 14.2 → version check ensures DAP availability → prevents cryptic failures on older GDB | |
There was a problem hiding this comment.
There's a slight inconsistency in the required GDB version mentioned in this plan. This line states ≥14.2, but other parts of the document (e.g., lines 39, 99, 126) and the implementation in gdb_common.rs use ≥14.1. To avoid confusion, it would be best to make this consistent throughout the document, probably by changing this line to ≥14.1.
| async fn status(&self) -> Result<InstallStatus> { | ||
| if let Ok(path) = which::which("gdb") { | ||
| match get_gdb_version(&path).await { | ||
| Some(version) if is_gdb_version_sufficient(&version) => { | ||
| return Ok(InstallStatus::Installed { | ||
| path, | ||
| version: Some(version), | ||
| }); | ||
| } | ||
| Some(version) => { | ||
| return Ok(InstallStatus::Broken { | ||
| path, | ||
| reason: format!( | ||
| "GDB version {} found, but ≥14.1 required for native DAP support", | ||
| version | ||
| ), | ||
| }); | ||
| } | ||
| None => { | ||
| return Ok(InstallStatus::Broken { | ||
| path, | ||
| reason: "Could not determine GDB version".to_string(), | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(InstallStatus::NotInstalled) | ||
| } |
There was a problem hiding this comment.
The logic in the status method is a bit nested and can be simplified to improve readability and maintainability. Using match on which::which and then handling the version check can flatten the structure.
async fn status(&self) -> Result<InstallStatus> {
let path = match which::which("gdb") {
Ok(p) => p,
Err(_) => return Ok(InstallStatus::NotInstalled),
};
match get_gdb_version(&path).await {
Some(version) => {
if is_gdb_version_sufficient(&version) {
Ok(InstallStatus::Installed {
path,
version: Some(version),
})
} else {
Ok(InstallStatus::Broken {
path,
reason: format!(
"GDB version {} found, but ≥14.1 required for native DAP support",
version
),
})
}
}
None => Ok(InstallStatus::Broken {
path,
reason: "Could not determine GDB version".to_string(),
}),
}
}| async fn status(&self) -> Result<InstallStatus> { | ||
| if Platform::current() != Platform::Linux { | ||
| return Ok(InstallStatus::NotInstalled); | ||
| } | ||
|
|
||
| if let Some(path) = find_cuda_gdb() { | ||
| match get_gdb_version(&path).await { | ||
| Some(version) if is_gdb_version_sufficient(&version) => { | ||
| return Ok(InstallStatus::Installed { | ||
| path, | ||
| version: Some(version), | ||
| }); | ||
| } | ||
| Some(version) => { | ||
| return Ok(InstallStatus::Broken { | ||
| path, | ||
| reason: format!( | ||
| "CUDA-GDB version {} found, but ≥14.1 required for native DAP support", | ||
| version | ||
| ), | ||
| }); | ||
| } | ||
| None => { | ||
| return Ok(InstallStatus::Broken { | ||
| path, | ||
| reason: "Could not determine CUDA-GDB version".to_string(), | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(InstallStatus::NotInstalled) | ||
| } |
There was a problem hiding this comment.
Similar to gdb.rs, the logic in the status method is a bit nested. Refactoring it can improve readability and maintainability.
async fn status(&self) -> Result<InstallStatus> {
if Platform::current() != Platform::Linux {
return Ok(InstallStatus::NotInstalled);
}
let path = match find_cuda_gdb() {
Some(p) => p,
None => return Ok(InstallStatus::NotInstalled),
};
match get_gdb_version(&path).await {
Some(version) => {
if is_gdb_version_sufficient(&version) {
Ok(InstallStatus::Installed {
path,
version: Some(version),
})
} else {
Ok(InstallStatus::Broken {
path,
reason: format!(
"CUDA-GDB version {} found, but ≥14.1 required for native DAP support",
version
),
})
}
}
None => Ok(InstallStatus::Broken {
path,
reason: "Could not determine CUDA-GDB version".to_string(),
}),
}
}| fn cuda_gdb_available() -> Option<PathBuf> { | ||
| let default_path = PathBuf::from("/usr/local/cuda/bin/cuda-gdb"); | ||
| if default_path.exists() { | ||
| return Some(default_path); | ||
| } | ||
|
|
||
| if let Ok(cuda_home) = std::env::var("CUDA_HOME") { | ||
| let cuda_home_path = PathBuf::from(cuda_home).join("bin/cuda-gdb"); | ||
| if cuda_home_path.exists() { | ||
| return Some(cuda_home_path); | ||
| } | ||
| } | ||
|
|
||
| which::which("cuda-gdb").ok() | ||
| } |
There was a problem hiding this comment.
The cuda_gdb_available helper function duplicates the path-finding logic from find_cuda_gdb in src/setup/adapters/cuda_gdb.rs. This creates a maintenance burden, as any changes to find_cuda_gdb would need to be manually replicated here.
To avoid this duplication and make this suggestion work, you first need to make find_cuda_gdb public in src/setup/adapters/cuda_gdb.rs:
// in src/setup/adapters/cuda_gdb.rs
pub fn find_cuda_gdb() -> Option<PathBuf> { ... }After that, you can apply the suggested change below to call it directly from your test. This will ensure your test always uses the same detection logic as the application.
fn cuda_gdb_available() -> Option<PathBuf> {
use debugger::setup::adapters::cuda_gdb;
cuda_gdb::find_cuda_gdb()
}There was a problem hiding this comment.
Pull request overview
Adds new native DAP-based debugger adapters for GDB (14.1+) and CUDA-GDB (via -i=dap), plus CUDA project detection and integration tests/docs to support the new adapters.
Changes:
- Introduces new
gdbandcuda-gdbinstallers with shared version-detection utilities. - Registers the new debuggers in the setup registry and adds CUDA project-type detection.
- Extends integration tests and adds adapter documentation/implementation plan.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration.rs | Adds GDB/CUDA-GDB availability helpers and a basic GDB debugging workflow test. |
| src/setup/registry.rs | Registers gdb and cuda-gdb and wires their installers. |
| src/setup/detector.rs | Adds ProjectType::Cuda and maps CUDA projects to cuda-gdb. |
| src/setup/adapters/mod.rs | Exposes new adapter modules. |
| src/setup/adapters/gdb_common.rs | Shared GDB/CUDA-GDB version parsing and version retrieval. |
| src/setup/adapters/gdb.rs | Implements GdbInstaller using native DAP (-i=dap). |
| src/setup/adapters/cuda_gdb.rs | Implements CudaGdbInstaller with CUDA Toolkit path detection. |
| src/setup/adapters/README.md | Documents rationale/requirements for native GDB/CUDA-GDB DAP. |
| src/setup/adapters/CLAUDE.md | Index/update notes for new adapter files. |
| docs/plan/cuda-gdb.md | Adds a detailed implementation plan for GDB/CUDA-GDB support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/setup/adapters/gdb_common.rs
Outdated
| /// Returns first token starting with digit (handles varying GDB output formats) | ||
| pub fn parse_gdb_version(output: &str) -> Option<String> { | ||
| output | ||
| .lines() | ||
| .next() | ||
| .and_then(|line| { | ||
| line.split_whitespace() | ||
| .find(|token| token.chars().next().map_or(false, |c| c.is_ascii_digit())) | ||
| }) | ||
| .map(|s| s.to_string()) |
There was a problem hiding this comment.
parse_gdb_version currently returns the first whitespace token starting with a digit. On common outputs like GNU gdb (Ubuntu 12.1-0ubuntu1~22.04) 12.1, this will pick 12.1-0ubuntu1~22.04 (package version) rather than the actual GDB version token, causing version detection to fail. Consider selecting the last version-like token on the first line, or extracting the first \d+\.\d+ (or \d+(?:\.\d+)+) match and returning only the numeric portion.
| /// Returns first token starting with digit (handles varying GDB output formats) | |
| pub fn parse_gdb_version(output: &str) -> Option<String> { | |
| output | |
| .lines() | |
| .next() | |
| .and_then(|line| { | |
| line.split_whitespace() | |
| .find(|token| token.chars().next().map_or(false, |c| c.is_ascii_digit())) | |
| }) | |
| .map(|s| s.to_string()) | |
| /// Scans the first line for the first dotted numeric version like `\d+(?:\.\d+)+` | |
| /// and returns only the numeric portion (e.g. "12.1" from | |
| /// "GNU gdb (Ubuntu 12.1-0ubuntu1~22.04) 12.1"). | |
| pub fn parse_gdb_version(output: &str) -> Option<String> { | |
| let line = output.lines().next()?; | |
| let bytes = line.as_bytes(); | |
| let mut i = 0; | |
| while i < bytes.len() { | |
| let c = bytes[i] as char; | |
| if c.is_ascii_digit() { | |
| let start = i; | |
| let mut has_dot = false; | |
| i += 1; | |
| while i < bytes.len() { | |
| let c_inner = bytes[i] as char; | |
| if c_inner.is_ascii_digit() { | |
| i += 1; | |
| } else if c_inner == '.' { | |
| has_dot = true; | |
| i += 1; | |
| } else { | |
| break; | |
| } | |
| } | |
| if has_dot { | |
| return Some(line[start..i].to_string()); | |
| } | |
| } else { | |
| i += 1; | |
| } | |
| } | |
| None |
| pub fn is_gdb_version_sufficient(version: &str) -> bool { | ||
| let parts: Vec<&str> = version.split('.').collect(); | ||
| let Some(major_str) = parts.get(0) else { | ||
| return false; | ||
| }; | ||
| let Some(minor_str) = parts.get(1) else { | ||
| return false; | ||
| }; | ||
| let Ok(major) = major_str.parse::<u32>() else { | ||
| return false; | ||
| }; | ||
| let Ok(minor) = minor_str.parse::<u32>() else { | ||
| return false; | ||
| }; |
There was a problem hiding this comment.
is_gdb_version_sufficient assumes the version string is strictly MAJOR.MINOR with both parts purely numeric. If parse_gdb_version returns tokens like 12.1-0ubuntu1~22.04 or 14.2-arch, the minor parse will fail and the installer will incorrectly mark a working GDB as Broken. Consider normalizing the version string first (strip any suffix after the numeric portion) or parsing with a regex to extract major/minor digits.
tests/integration.rs
Outdated
| ctx.create_config("gdb", gdb_path.to_str().unwrap()); | ||
|
|
There was a problem hiding this comment.
This GDB integration test writes a config with args = [], but GDB native DAP requires starting GDB with -i=dap. Without that, debugger start will spawn GDB in CLI mode and the session will fail/hang. Update the test config to include -i=dap for the gdb adapter (e.g., extend create_config to accept args, or special-case gdb/cuda-gdb).
| ctx.create_config("gdb", gdb_path.to_str().unwrap()); | |
| // Manually create a GDB adapter config that starts GDB in native DAP mode. | |
| // This ensures GDB is invoked with `-i=dap` instead of defaulting to CLI mode. | |
| let gdb_command = gdb_path.to_str().expect("Non-UTF-8 GDB path"); | |
| let config_path = ctx.config_dir.join("config.json"); | |
| let config_dir = config_path | |
| .parent() | |
| .expect("config.json should have a parent directory"); | |
| fs::create_dir_all(config_dir).expect("failed to create config directory"); | |
| let config_contents = format!( | |
| r#"{{ | |
| "adapters": {{ | |
| "gdb": {{ | |
| "command": "{}", | |
| "args": ["-i=dap"] | |
| }} | |
| }} | |
| }}"#, | |
| gdb_command | |
| ); | |
| fs::write(&config_path, config_contents).expect("failed to write GDB adapter config"); |
|
|
||
| /// Checks if cuda-gdb is available for testing | ||
| /// | ||
| /// Uses same path search as CudaGdbInstaller::find_cuda_gdb() |
There was a problem hiding this comment.
Doc comment references CudaGdbInstaller::find_cuda_gdb(), but the implementation is a free function find_cuda_gdb() (not an associated method). Update the comment to match the actual code to avoid confusion during maintenance.
| /// Uses same path search as CudaGdbInstaller::find_cuda_gdb() | |
| /// Uses the same path search logic as the `find_cuda_gdb()` helper |
| languages: &["c", "cpp"], | ||
| platforms: &[Platform::Linux, Platform::MacOS, Platform::Windows], | ||
| description: "GDB native DAP adapter", | ||
| primary: true, |
There was a problem hiding this comment.
Marking gdb as primary: true for c/cpp changes primary_debugger_for_language("c") / ("cpp") to return GDB (because it appears first in DEBUGGERS) instead of the previously-primary lldb. If this isn’t intended, set primary to false here or adjust the selection logic to handle multiple primaries deterministically.
| primary: true, | |
| primary: false, |
| // CUDA detection must precede C/C++ (.cu files are valid C++ but require CUDA-GDB) | ||
| if has_extension_in_dir(dir, "cu") { | ||
| types.push(ProjectType::Cuda); | ||
| } |
There was a problem hiding this comment.
CUDA project detection and debugger mapping were added, but there are no unit tests covering .cu detection or debuggers_for_project(ProjectType::Cuda). Since this file already has a test module, add tests for: (1) a directory containing a .cu file is detected as ProjectType::Cuda, and (2) debuggers_for_project(&ProjectType::Cuda) returns cuda-gdb.
| GDB native DAP requires Python support, added in GDB 14.1. Installers verify version at setup time and return `Broken` status for older versions with upgrade instructions. | ||
|
|
||
| CUDA-GDB 13.x is based on GDB 14.2 and inherits DAP support. The installer validates DAP availability during verification via `verify_dap_adapter()`. |
There was a problem hiding this comment.
The README says “GDB native DAP requires Python support, added in GDB 14.1”. GDB’s Python support predates 14.1; the requirement here is that DAP support is available in GDB ≥14.1 and that the specific GDB build has Python enabled. Rewording this will avoid misleading users.
- Add CHANGELOG entry for v0.1.1 (Delve, GDB, CUDA-GDB support) - Bump version to 0.1.1 - Fix unused variable warnings - Add cuda_test to .gitignore Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
-i=dapinterpreter flaggdb_common.rsTest plan
.cufiles when cuda-gdb is installedcargo test🤖 Generated with Claude Code