Skip to content

feat(debug): add fbuild debug verb with ESP32 native-gdbstub orchestration (#1144) - #1206

Merged
zackees merged 1 commit into
mainfrom
feat/1144-fbuild-debug
Jul 28, 2026
Merged

feat(debug): add fbuild debug verb with ESP32 native-gdbstub orchestration (#1144)#1206
zackees merged 1 commit into
mainfrom
feat/1144-fbuild-debug

Conversation

@zackees

@zackees zackees commented Jul 28, 2026

Copy link
Copy Markdown
Member

Implements the "high value, low effort" arm of #1144 — the fbuild debug verb with the per-target capability matrix and ESP32 native-gdbstub orchestration. The CH32V injected stub and fbuild crashdump remain on the issue (both define their contract jointly with FastLED/soundwave#38).

  • Capability matrix as code: ESP32 family → orchestrate the existing IDF/ROM gdbstub (no injection needed); CH32V → first-class "planned" message linking the reference-implementation issue; AVR → honestly unsupported ("no trap architecture"), per the issue's requirement that this be a first-class message, not a cryptic failure.
  • Orchestration flow: build + flash via the existing deploy path (--no-flash to attach to what's running) → ELF resolution reusing fbuild symbols' discovery (build_info.json → build dirs → loose *.elf) → gdb resolution (derive from the build's own cc_path by gcc→gdb swap, else PATH candidates from the MCU's toolchain prefix) → CDC port from the daemon device list (--port override; auto-pick only when unambiguous; Windows \.\COMx rewriting for gdb) → interactive gdb <elf> -ex "target remote <port>".
  • Honest sdkconfig story: fbuild cannot yet inject CONFIG_ESP_SYSTEM_PANIC_GDBSTUB (docs/sdkconfig.md is a design proposal, not implemented) — the command reads the project's actual sdkconfig, reports the current panic mode, and names the existing build_flags escape hatch instead of faking injection.

Validation (local)

  • soldr cargo test -p fbuild-cli → 262 passed (26 new: matrix, readiness note, gdb derivation/candidates over fake toolchain layouts, argv construction, COM rewriting, ELF fixtures, env/mcu resolution); clippy -D warnings clean; fmt clean; full bash test before merge. The interactive gdb spawn path is deliberately untested (documented — nothing to assert without hardware).

Part of #1144. Hardware validation (real ESP32 attach) noted in the docs as the remaining proof step.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added the fbuild debug command for interactive GDB debugging on supported ESP32 devices.
    • Automatically builds, optionally flashes, detects the device port, locates the correct ELF and GDB executable, and launches the debugging session.
    • Added --no-flash, --port, project directory, and environment options.
    • Added clear capability guidance and clean handling for unsupported targets.
  • Documentation

    • Documented command usage, supported platforms, debugging behavior, and platform limitations.

…ation (#1144)

- Capability matrix as code: ESP32 family => native IDF/ROM gdbstub
  orchestration; CH32V => first-class "planned, pending
  FastLED/soundwave#38" message; AVR => honestly unsupported (no trap
  architecture). Unsupported/planned targets exit cleanly with
  explanatory output, never a stack trace.
- fbuild debug [dir] [-e env] [--no-flash] [--port]: build+deploy via
  the existing deploy path, resolve the exact build's ELF (fbuild
  symbols discovery), resolve the toolchain gdb (build_info cc_path
  gcc->gdb swap, fallback PATH search via the mcu's toolchain prefix),
  resolve the CDC port (daemon device list; --port override; Windows
  \.\COMx rewriting), launch gdb interactively with target remote.
- Honest sdkconfig story: gdbstub-on-panic (CONFIG_ESP_SYSTEM_PANIC_
  GDBSTUB) cannot yet be injected by fbuild (docs/sdkconfig.md is a
  design proposal); the command reports the current panic mode from
  the project's sdkconfig and names the build_flags escape hatch. No
  faked injection.

Part of #1144 (ESP32 arm + capability matrix; CH32V stub and
fbuild crashdump remain, gated on FastLED/soundwave#38).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds fbuild debug with CLI wiring, ESP32 capability handling, build/flash control, ELF and CDC-port discovery, GDB resolution, Windows serial support, interactive attachment, unit tests, and documentation.

Changes

Debug command

Layer / File(s) Summary
CLI contract and dispatch
crates/fbuild-cli/src/cli/args.rs, crates/fbuild-cli/src/cli/mod.rs, crates/fbuild-cli/src/cli/dispatch.rs
Adds debug arguments, argument-order rewriting, module registration, and dispatcher routing.
Target resolution
crates/fbuild-cli/src/cli/debug.rs
Adds platform capability messaging, SDK readiness notes, ELF/environment/MCU resolution, GDB lookup, command construction, and CDC-port discovery.
Build, attach, and documentation
crates/fbuild-cli/src/cli/debug.rs, agents/docs/commands-reference.md, docs/reference/cli.md
Runs build/flash and interactive GDB attachment, validates helpers with unit tests, and documents supported targets and mechanics.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant fbuild_debug as fbuild debug
  participant deploy_run_deploy as deploy::run_deploy
  participant Daemon
  participant GDB
  User->>fbuild_debug: invoke command
  fbuild_debug->>deploy_run_deploy: build and flash unless --no-flash
  fbuild_debug->>Daemon: discover CDC device port
  fbuild_debug->>GDB: launch with ELF, baud, and target remote
  GDB-->>User: interactive debug session
Loading

Possibly related issues

Possibly related PRs

  • FastLED/fbuild#1202 — Adds another top-level CLI subcommand through the shared argument and dispatch mechanisms.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding the fbuild debug command with ESP32 native gdbstub orchestration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/1144-fbuild-debug

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zackees
zackees merged commit c7351bf into main Jul 28, 2026
86 of 93 checks passed
@zackees
zackees deleted the feat/1144-fbuild-debug branch July 28, 2026 05:33

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/fbuild-cli/src/cli/debug.rs`:
- Around line 536-539: Update the gdb launch in the surrounding debug command
flow to use the sanctioned subprocess wrapper; if the interactive session
requires direct Command spawning, add the required allow-direct-spawn marker
with a concise reason that inherited stdio is necessary. Preserve the existing
arguments, status handling, and launch error mapping.
- Around line 352-360: Update resolve_debug_elf and its run_debug caller to
resolve the ELF from the selected env_name/platform/mcu build output rather than
using unfiltered discover_elf_in_project(project_dir). Prefer the selected
environment’s build_info_<env>.json prog_path when available, and ensure the ELF
passed to gdb matches the environment used for toolchain and gdb resolution.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e191269f-b494-4522-83f2-3c226768f6e2

📥 Commits

Reviewing files that changed from the base of the PR and between e4885d3 and 0aa0527.

📒 Files selected for processing (6)
  • agents/docs/commands-reference.md
  • crates/fbuild-cli/src/cli/args.rs
  • crates/fbuild-cli/src/cli/debug.rs
  • crates/fbuild-cli/src/cli/dispatch.rs
  • crates/fbuild-cli/src/cli/mod.rs
  • docs/reference/cli.md

Comment on lines +352 to +360
fn resolve_debug_elf(project_dir: &Path) -> Result<PathBuf> {
discover_elf_in_project(project_dir).ok_or_else(|| {
FbuildError::Other(format!(
"no ELF found under {} — run `fbuild debug` without --no-flash to build one, \
or run `fbuild build` first",
project_dir.display()
))
})
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check whether discover_elf_in_project can disambiguate by environment.
ast-grep outline crates/fbuild-build/src --match discover_elf_in_project --items all
rg -n -A15 'fn discover_elf_in_project' --type=rust

Repository: FastLED/fbuild

Length of output: 166


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Repo files matching debug/discover/elf:"
git ls-files | rg '(^|/)(debug|.*Elf|.*elf|.*build).*\.rs$|crates/fbuild-cli/src/cli/debug\.rs|crates/fbuild-build/src' | head -200

echo
echo "Find discover_elf_in_project text:"
rg -n "discover_elf_in_project|resolve_debug_elf|resolve_debug_target|no-flash|--no-flash|environment|mcu|platform|env_name" -S --glob '*.rs' crates | head -300

echo
echo "Outline crates/fbuild-cli/src/cli/debug.rs symbols:"
ast-grep outline crates/fbuild-cli/src/cli/debug.rs --view expanded | head -200 || true

Repository: FastLED/fbuild

Length of output: 43039


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "debug.rs relevant sections:"
cat -n crates/fbuild-cli/src/cli/debug.rs | sed -n '280,430p'
echo
cat -n crates/fbuild-cli/src/cli/debug.rs | sed -n '430,535p'
echo
cat -n crates/fbuild-cli/src/cli/debug.rs | sed -n '720,805p'

echo
echo "Search for Elf discovery symbols/paths:"
rg -n "firmware\.elf|build_dir|env_name|ProjectDir|ProjectDirResolver|Environment|GetBuildDir|get_build_dir|build.*elf|discover.*elf|glob" -S --glob '*.rs' crates | head -400

echo
echo "Candidate symbols that may implement ELF discovery/build dir selection:"
git ls-files '*.rs' | xargs rg -n "struct ProjectDirResolver|impl ProjectDirResolver|fn get_build_dir|get_build_dir|pub .*BuildDir|BuildDir|BuildDir" --type=rust | head -200

Repository: FastLED/fbuild

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "imports and discover symbol in debug.rs:"
cat -n crates/fbuild-cli/src/cli/debug.rs | sed -n '1,120p'
echo
rg -n "discover_elf_in_project|use .*discover|from .*discover|build.*info|find_build_info_near|load_build_info|glob|WalkDir|walk|firmware\.elf" crates/fbuild-cli/src/cli/debug.rs

echo
echo "symbols with exact target function names:"
rg -n "fn find_build_info_near|fn load_build_info|fn discover_elf_in_project|struct BuildInfo|fn load_build" crates --type=rust -S

echo
echo "candidate crates with relevant names:"
git ls-files '*discover*;*elf*;*.rs' | rg 'fbuild-(core|paths|cli|packages|build|symbols|config)' | head -200

Repository: FastLED/fbuild

Length of output: 9359


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "fbuild-build-engine symbol_analyzer discover_elf_in_project:"
cat -n crates/fbuild-build-engine/src/symbol_analyzer/mod.rs | sed -n '500,600p'
echo
echo "imports/exports around symbol_analyzer:"
cat -n crates/fbuild-build-engine/src/symbol_analyzer/mod.rs | sed -n '1,80p'
echo "imports/exports around symbol_analyzer in engine lib:"
rg -n "mod symbol_analyzer|pub use|symbol_analyzer" crates/fbuild-build-engine/src/lib.rs crates/fbuild-build/src/lib.rs -S

echo
echo "cargo deps for fbuild-core/paths if manifest exports them:"
rg -n "name = \"fbuild-(core|paths|packages-fetch|packages)\"|path = \"fbuild-(core|paths|packages-fetch|packages)\"" crates/Cargo.toml Cargo.toml --type=toml -S

Repository: FastLED/fbuild

Length of output: 144


🏁 Script executed:

#!/bin/bash
set -euo pipefail
time cat -n crates/fbuild-build-engine/src/symbol_analyzer/mod.rs | sed -n '500,580p'

Repository: FastLED/fbuild

Length of output: 3904


Resolve the ELF relative to the selected debug environment.

run_debug selects a specific env_name/platform/mcu, but resolve_debug_elf(project_dir) uses the unfiltered discovery fallback, so with multiple built environments it can hand gdb a stale ELF from another env while later resolving the requested toolchain/gdb. Scope ELF lookup to the resolved environment build output, or load prog_path from that environment’s build_info_<env>.json.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/fbuild-cli/src/cli/debug.rs` around lines 352 - 360, Update
resolve_debug_elf and its run_debug caller to resolve the ELF from the selected
env_name/platform/mcu build output rather than using unfiltered
discover_elf_in_project(project_dir). Prefer the selected environment’s
build_info_<env>.json prog_path when available, and ensure the ELF passed to gdb
matches the environment used for toolchain and gdb resolution.

Comment on lines +536 to +539
let status = Command::new(&gdb_path)
.args(&argv)
.status()
.map_err(|e| FbuildError::Other(format!("failed to launch {}: {e}", gdb_path.display())))?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Locate the sanctioned subprocess wrapper API and its async-safety.
rg -n -C5 'pub fn run_command_passthrough|pub async fn run_command_passthrough|pub fn run_command\b' --type=rust
rg -n 'allow-direct-spawn' --type=rust -C2

Repository: FastLED/fbuild

Length of output: 152


🏁 Script executed:

#!/bin/bash
set -u

echo "== files matching debug.rs =="
fd -a 'debug\.rs$' . | sed 's#^\./##'

echo "== repository rust subprocess-related text broad =="
rg -n 'subprocess|run_command|direct.spawn|allow-direct|Command::new|status\(\)' --type=rust -C2 | head -n 300

echo "== git diff stat/name-only if available =="
git diff --stat || true
git diff -- crates/fbuild-cli/src/cli/debug.rs 2>/dev/null | sed -n '1,220p' || true

echo "== target outline =="
ast-grep outline crates/fbuild-cli/src/cli/debug.rs --match run_debug --view expanded || true

Repository: FastLED/fbuild

Length of output: 346


Route gdb launching through the sanctioned subprocess wrapper or mark it explicitly.

crates/fbuild-cli/src/cli/debug.rs currently uses a direct Command::new(&gdb_path).status() spawn with no allow-direct-spawn: <reason> marker, which conflicts with the Rust build-pipeline directive for non-diagnostic commands. Since this is an interactive gdb session, either add an in-line marker explaining inherited stdio is required, or use the sanctioned subprocess interface so CI/lint can see the bypass is intentional.

🧰 Tools
🪛 GitHub Actions: Lint subprocess spawns / 0_Lint subprocess spawns.txt

[error] 536-536: Direct subprocess spawn detected without an allow-direct-spawn: <reason> marker. Please route via fbuild_core::subprocess::{run_command,run_command_passthrough} or annotate with a one-line reason (see #141).

🪛 GitHub Actions: Lint subprocess spawns / Lint subprocess spawns

[error] 536-536: CI check failed (find_direct_subprocess.py). NEW direct spawn without an allow-direct-spawn: <reason> marker: uses Command::new(&gdb_path).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/fbuild-cli/src/cli/debug.rs` around lines 536 - 539, Update the gdb
launch in the surrounding debug command flow to use the sanctioned subprocess
wrapper; if the interactive session requires direct Command spawning, add the
required allow-direct-spawn marker with a concise reason that inherited stdio is
necessary. Preserve the existing arguments, status handling, and launch error
mapping.

Source: Pipeline failures

@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Jul 29, 2026
zackees added a commit that referenced this pull request Jul 31, 2026
… unused_mut, rustdoc links

main is red on six checks. They are unrelated to each other and accumulated
because the workflows that run them are `pull_request`-triggered, so nothing
re-ran them on main after the merges that broke them (#1197, #1206, #1207).
Found while verifying an unrelated dependency bump (#1215); every failure
there reproduced identically on main.

Dylint: drop `crates/fbuild-cli/src/cli/clangd_config.rs` from
ban_unrooted_tempdir's allowlist -- the file was deleted in #1197.

Lint subprocess spawns: annotate three deliberate direct spawns with
`allow-direct-spawn:` markers (clangd test driver, interactive gdb that must
inherit terminal stdio, detached editor launch that must outlive the CLI).
The scanner only inspects the hit line and the one line directly above it, so
the marker must be a single line -- a wrapped comment silently fails to
register. This also fixes Check (ubuntu-latest), which runs the same script.

Check (macos-latest): `ports.rs` bound `let mut ports` unconditionally while
only `cfg(target_os = "linux")` mutates it, so every other unix target tripped
`-D unused-mut`. The `mut` now exists only inside the Linux cfg via a
shadowing block, rather than an `#[allow]`.

Documentation: six rustdoc errors across three crates. URL templates such as
`?query=<optional filter>` parsed as unclosed HTML tags; several intra-doc
links pointed at `cfg(target_os = "linux")` items that do not exist to resolve
against when docs build on a non-Linux host; three more pointed at private
items. Unresolvable links become code spans, each with a short note on why it
is deliberately not a link.

CH32V006 (`build / build`) is also red on main but is a vendor-C compile error
in the openwch Arduino core, not CI hygiene, and is left for a targeted fix.

Verified locally on Windows, all exit 0: check_dylint_allowlists.py,
find_direct_subprocess.py --fail, cargo doc with RUSTDOCFLAGS=-D warnings,
clippy -D warnings, cargo fmt --check, and `bash test` (77 suites, 0 failed).

The macOS fix could not be verified locally: cross-compiling fbuild-serial to
x86_64-apple-darwin fails on ring's build script needing a macOS `cc`. The
cfg-shadowing pattern was verified in an isolated cargo project (clean under
-D warnings with the cfg both on and off); Check (macos-latest) on this PR is
the real proof.

Co-Authored-By: Claude <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jul 31, 2026
Pinning dylint back to =6.0.1 made the driver build again, so the lints
actually ran for the first time in a while and reported 34 violations across
three crates. None are new: they accumulated in #1197, #1206 and #1207 while
Dylint was failing at earlier steps (a stale allowlist path, then the 6.0.2
driver) and never reached the lint pass.

Handled by category rather than uniformly.

ban_std_pathbuf (30) -- migrated to `fbuild_core::path::NormalizedPath` in
`fbuild-cli` (`ide.rs`, `debug.rs`, `ide_debug.rs`, `clangd_config/{mod,
vscode,zed}.rs`), `fbuild-daemon` (`handlers/libraries.rs`) and the
`sysfs_usb.rs` test fixtures. Not allowlisted: that allowlist's header states
"New files MUST NOT be added here. The target state is zero entries", and
exempting brand-new files would hollow out the gate that was just repaired.
`NormalizedPath` derefs to `Path`, so call sites taking `&Path` are unchanged;
`to_path_buf()` appears only where an external API (`BuildLayout::new`)
demands an owned `PathBuf`.

ban_std_fs_in_async (2) -- `handlers/libraries.rs::installed_dir_names` now
uses async `fbuild_core::fs::read_dir` (the sanctioned re-export of
`tokio::fs`), which makes it and `build_library_entries` async along with
their three unit tests. It also switches the directory test from
`Path::is_dir()` to `entry.file_type().await`: `is_dir()` is a blocking
`stat` on the async worker, so keeping it would have satisfied the lint's
letter while preserving exactly the stall the lint exists to prevent
(#844).

ban_raw_subprocess (2) -- allowlisted with justifications, which is what the
lint's own diagnostic offers for genuinely-justified sites. Both are correct
as raw spawns: `fbuild debug` hands the terminal to gdb, so the child must
inherit this process's stdio rather than be captured; and `fbuild ide`
launches the editor detached so it OUTLIVES the CLI, which is the opposite of
what a containment group guarantees.

Verified: `cargo clippy --workspace --all-targets -- -D warnings`,
`cargo fmt --all --check`, and `bash test` (77 suites, 0 failed) all green.

Dylint itself could not be run locally to pre-verify -- the pinned nightly and
`cargo-dylint` are present, but `dylint-link` fails to link the lint cdylibs
on this Windows host. CI is the proof for that gate.

Co-Authored-By: Claude <noreply@anthropic.com>
zackees added a commit that referenced this pull request Jul 31, 2026
Pinning dylint back to =6.0.1 made the driver build again, so the lints
actually ran for the first time in a while and reported 34 violations across
three crates. None are new: they accumulated in #1197, #1206 and #1207 while
Dylint was failing at earlier steps (a stale allowlist path, then the 6.0.2
driver) and never reached the lint pass.

Handled by category rather than uniformly.

ban_std_pathbuf (30) -- migrated to `fbuild_core::path::NormalizedPath` in
`fbuild-cli` (`ide.rs`, `debug.rs`, `ide_debug.rs`, `clangd_config/{mod,
vscode,zed}.rs`), `fbuild-daemon` (`handlers/libraries.rs`) and the
`sysfs_usb.rs` test fixtures. Not allowlisted: that allowlist's header states
"New files MUST NOT be added here. The target state is zero entries", and
exempting brand-new files would hollow out the gate that was just repaired.
`NormalizedPath` derefs to `Path`, so call sites taking `&Path` are unchanged;
`to_path_buf()` appears only where an external API (`BuildLayout::new`)
demands an owned `PathBuf`.

ban_std_fs_in_async (2) -- `handlers/libraries.rs::installed_dir_names` now
uses async `fbuild_core::fs::read_dir` (the sanctioned re-export of
`tokio::fs`), which makes it and `build_library_entries` async along with
their three unit tests. It also switches the directory test from
`Path::is_dir()` to `entry.file_type().await`: `is_dir()` is a blocking
`stat` on the async worker, so keeping it would have satisfied the lint's
letter while preserving exactly the stall the lint exists to prevent
(#844).

ban_raw_subprocess (2) -- allowlisted with justifications, which is what the
lint's own diagnostic offers for genuinely-justified sites. Both are correct
as raw spawns: `fbuild debug` hands the terminal to gdb, so the child must
inherit this process's stdio rather than be captured; and `fbuild ide`
launches the editor detached so it OUTLIVES the CLI, which is the opposite of
what a containment group guarantees.

Verified: `cargo clippy --workspace --all-targets -- -D warnings`,
`cargo fmt --all --check`, and `bash test` (77 suites, 0 failed) all green.

Dylint itself could not be run locally to pre-verify -- the pinned nightly and
`cargo-dylint` are present, but `dylint-link` fails to link the lint cdylibs
on this Windows host. CI is the proof for that gate.

Co-Authored-By: Claude <noreply@anthropic.com>

Also widens the cargo-cache .gitignore entries to `**/`-prefixed forms. The
existing patterns contain a slash, so git anchors them to the repo root and
they never covered nested crates; running any cargo command inside
`dylints/<lint>/` (each is its own workspace) materializes a full registry
cache there that a `git add -A` will stage.
zackees added a commit that referenced this pull request Jul 31, 2026
Pinning dylint back to =6.0.1 made the driver build again, so the lints
actually ran for the first time in a while and reported 34 violations across
three crates. None are new: they accumulated in #1197, #1206 and #1207 while
Dylint was failing at earlier steps (a stale allowlist path, then the 6.0.2
driver) and never reached the lint pass.

Handled by category rather than uniformly.

ban_std_pathbuf (30) -- migrated to `fbuild_core::path::NormalizedPath` in
`fbuild-cli` (`ide.rs`, `debug.rs`, `ide_debug.rs`, `clangd_config/{mod,
vscode,zed}.rs`), `fbuild-daemon` (`handlers/libraries.rs`) and the
`sysfs_usb.rs` test fixtures. Not allowlisted: that allowlist's header states
"New files MUST NOT be added here. The target state is zero entries", and
exempting brand-new files would hollow out the gate that was just repaired.
`NormalizedPath` derefs to `Path`, so call sites taking `&Path` are unchanged;
`to_path_buf()` appears only where an external API (`BuildLayout::new`)
demands an owned `PathBuf`.

ban_std_fs_in_async (2) -- `handlers/libraries.rs::installed_dir_names` now
uses async `fbuild_core::fs::read_dir` (the sanctioned re-export of
`tokio::fs`), which makes it and `build_library_entries` async along with
their three unit tests. It also switches the directory test from
`Path::is_dir()` to `entry.file_type().await`: `is_dir()` is a blocking
`stat` on the async worker, so keeping it would have satisfied the lint's
letter while preserving exactly the stall the lint exists to prevent
(#844).

ban_raw_subprocess (2) -- allowlisted with justifications, which is what the
lint's own diagnostic offers for genuinely-justified sites. Both are correct
as raw spawns: `fbuild debug` hands the terminal to gdb, so the child must
inherit this process's stdio rather than be captured; and `fbuild ide`
launches the editor detached so it OUTLIVES the CLI, which is the opposite of
what a containment group guarantees.

Verified: `cargo clippy --workspace --all-targets -- -D warnings`,
`cargo fmt --all --check`, and `bash test` (77 suites, 0 failed) all green.

Dylint itself could not be run locally to pre-verify -- the pinned nightly and
`cargo-dylint` are present, but `dylint-link` fails to link the lint cdylibs
on this Windows host. CI is the proof for that gate.

Co-Authored-By: Claude <noreply@anthropic.com>

Also widens the cargo-cache .gitignore entries to `**/`-prefixed forms. The
existing patterns contain a slash, so git anchors them to the repo root and
they never covered nested crates; running any cargo command inside
`dylints/<lint>/` (each is its own workspace) materializes a full registry
cache there that a `git add -A` will stage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant