Uh oh!
There was an error while loading. Please reload this page.
feat(symbols): ingest toolchain paths from build_info.json - #436
Merged
Conversation
Extends `BuildInfo` with `nm_path` / `cppfilt_path` / `readelf_path` / `objdump_path` and mirrors them onto a PIO-shape `aliases` block so existing PlatformIO consumers (FastLED `ci/util/symbol_analysis.py`, `ci/inspect_binary.py`) read fbuild-built artifacts drop-in. Paths are derived centrally in `BuildInfo::new` from `size_path` using the GCC cross-tool naming convention — no orchestrator surgery needed. Teaches `fbuild symbols` to auto-locate `build_info.json` by walking up from the ELF directory; adds a `--build-info <path>` override. The common case becomes flag-less: fbuild symbols .fbuild/build/uno/firmware.elf fbuild symbols .pio/build/esp32s3/firmware.elf Adds `docs/symbols.md` walking through the resolution order, the `aliases` schema, and PIO/fbuild interop. Closes#428. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Warning Review limit reached
More reviews will be available in 48 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
`cargo fmt --all` flagged a stray blank line in `mod.rs` and a short-line wrap in `tests.rs`. Both were left over from when the file was split out — they pass locally only because rustfmt default formatting matches them when they already exist. CI runs `cargo fmt -- --check`, which doesn't. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 6, 2026
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jun 6, 2026
zackees added a commit
that referenced
this pull request
Jun 29, 2026
…rdening (#833) * dylint(826): port ban_std_pathbuf + ban_unrooted_tempdir from zccache Adopt zccache's two PathBuf/tempdir lints into fbuild with allowlists matching the fbuild source tree: - ban_std_pathbuf: 193-entry allowlist of legacy PathBuf call sites; steers new code at fbuild_core::path::NormalizedPath. - ban_unrooted_tempdir: 92-entry allowlist of legacy unrooted tempdir call sites; steers new code at fbuild_paths::get_cache_root() and the tempfile::*_in(...) variants. Both lints are ON and deny-by-default for new code. Target state is zero allowlist entries; the lists shrink as migrations land. Add the dylint crate dirs to the crate_guard allowlist so future edits to those Cargo.tomls aren't blocked by the monocrate hook. Refs #826, #436, #437, #282. * dylint(826): add 3 net-new fbuild-specific lints Add three lints that enforce fbuild's own architectural rules: - ban_direct_serialport: bans `serialport::*` references outside crates/fbuild-serial/ so the Windows USB-CDC contract stays in one place. Legacy allowlist covers fbuild-cli diagnostics, the daemon device manager, and the fbuild-deploy bootloader paths. - ban_file_based_locks: bans `OpenOptions::create_new(true)`, the `fs2::FileExt` API, and `libc::flock` / `nix::flock`. Allowlist is empty — fbuild has no file-based locks per CLAUDE.md "Key Constraints"; this locks the invariant in. - ban_deploy_tool_direct_invocation: bans `Command::new("esptool" | "esptool.py" | "avrdude" | "picotool" | "dfu-util" | "pyocd")` outside crates/fbuild-deploy/. Pattern-matches the binary-name string literal; complements ban_raw_subprocess (which scopes by spawn shape, not binary). All three follow the ban_raw_subprocess skeleton: pinned nightly, file-path scope (crates/*/src/), file-level allowlist via include_str!, forward-slash normalized. Each carries unit tests for the scope/dir helpers. Refs #826, #694. * fix(daemon/ws): replace 5 unwraps in error-reply paths with fallback The WebSocket error-reply paths in `handle_serial_ws` previously panicked via `serde_json::to_string(&err_msg).unwrap()` if the JSON serializer ever failed. While `SerialServerMessage` always serializes cleanly today, a panic in the cold error path tears down the entire WebSocket — exactly the wrong failure mode for a code path whose only job is to surface an error to the client. Add a small `serialize_or_fallback` helper that returns a hardcoded JSON error frame on serializer failure, and route the 5 affected `Message::Text(...)` constructions through it (open_port failure, unexpected-message-shape, parse-error, attach_reader=None, attached-confirmation send). The 3 remaining `to_string(...).unwrap()` calls at lines ~441/450/467 are on normal data-forwarding paths (not error-reply paths) and are out of scope for this fix; #826 only flagged the error-reply ones as a daemon-stability hazard. Refs #826. * build: add new dylint crates to workspace exclude cargo metadata refuses to operate on dylint sub-crates that are neither members nor excluded; add the 5 new lint crates from #826 to the existing exclude list (alongside ban_raw_subprocess). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Aug 7, 2026
zackees added a commit
that referenced
this pull request
Aug 7, 2026
…1274) (#1278) The per-project build-serialization lock was keyed on the raw, client-supplied project dir (PathBuf::from(&req.project_dir), never normalized). Two requests naming one project with different casing or slash direction (C:\proj vs c:/proj) keyed distinct DashMap entries and received different locks — so two builds could run on the same project concurrently. This is the #436/#437 path-identity bug class as a correctness defect, not a cache annoyance. Key project_locks on fbuild_core::path::NormalizedPath, whose Eq/Hash use the case-folded, slash-normalized, UNC-stripped key. Display stays readable (NormalizedPath derefs to Path; the stored path preserves original casing). Scope is deliberately narrow (see #1274): other path-keyed maps (image_hash_memo, compiler identity cache, LDF walker caches) are safe by construction — daemon-internal, single-source, or canonicalized before insert — and are left as PathBuf. Regression test asserts two spellings of one project share a lock on case-insensitive platforms (Windows/macOS CI) and stay independent on case-sensitive ones. Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes#428 (Phase 1 of #434).
BuildInfowithnm_path/cppfilt_path/readelf_path/objdump_pathplus a PIO-shapealiasesblock that mirrors them onto short keys (nm,c++filt, etc.). PlatformIO consumers (FastLEDci/util/symbol_analysis.py,ci/inspect_binary.py) keep working drop-in against fbuild-built artifacts.BuildInfo::newfromsize_pathusing the GCC cross-tool naming convention (<prefix>-size→<prefix>-{nm,c++filt,readelf,objdump}). All of fbuild's existing orchestrators (AVR, ESP32, RP2040, STM32, Teensy, ESP8266, NRF52, SAM, Renesas, Apollo3, LPC, Silabs, CH32V) ship GCC cross-toolchains, so no orchestrator surgery is needed.fbuild symbolsto auto-locatebuild_info.jsonby walking up from the ELF directory; adds a--build-info <path>explicit override. New resolution order:--nm/--cppfiltCLI flags (user wins).--build-info <path>if provided.build_info.jsonorbuild_info_<env>.json.nm, withc++filtderived by stem.docs/symbols.mdwalking through the resolution order, thealiasesschema, and PIO/fbuild interop.Test plan
soldr cargo check --workspace --all-targets✅soldr cargo clippy -p fbuild-build -p fbuild-cli --all-targets -- -D warnings✅soldr cargo test -p fbuild-build --lib— 627 passed, 0 failedsoldr cargo test -p fbuild-cli --bin fbuild— 25 passed, 0 failedderive_gcc_tool_path_avr_prefix/xtensa_with_exe_suffix/bare_size_falls_back— derivation rulesbuild_info_populates_new_toolchain_fields— the four new fields landbuild_info_aliases_block_mirrors_paths— PIO-shape mirrorbuild_info_aliases_omit_empty_optional_tools— ESP8266 (no ar/objcopy) keeps consumers safe withkey in aliasesemit_writes_aliases_into_json— round-trip via JSONfind_build_info_walks_up_from_elf_dir/prefers_env_specific/returns_none_when_absent— discovery walkerload_build_info_unwraps_single_env— single-env outer object contractresolve_reads_nm_from_build_info_auto_discovery— CLI resolver: build_info auto-pickupresolve_explicit_nm_wins_over_build_info— CLI resolver: explicit override precedenceresolve_explicit_build_info_path_is_honoured— CLI resolver:--build-infoflag🤖 Generated with Claude Code