fix(samd): fall back to cores/arduino when build.core dir is missing#320
Merged
Conversation
…pawn lint After f86817a (refactor lib.rs → submodules) the four std::process::Command::new sites in fbuild-python/daemon.rs ended up inside `match` arms two lines below their `allow-direct-spawn:` comment. The detector in ci/find_direct_subprocess.py only looks at "same line or the line immediately above", so the annotation no longer covered the actual call sites and CI started reporting: NEW direct spawns without an `allow-direct-spawn: <reason>` marker: crates/fbuild-python/src/daemon.rs:96 crates/fbuild-python/src/daemon.rs:97 crates/fbuild-python/src/daemon.rs:247 crates/fbuild-python/src/daemon.rs:248 This blocks every PR's "Lint subprocess spawns" check (e.g. #306 for fbuild#304, which is the actual FastLED-vs-PIO size regression we need to land). Fix: add an inline `// allow-direct-spawn:` annotation immediately above each of the four arms. Behavior unchanged.
…ist cmsis_dsp_lib
`Validate Board Definitions` workflow has been failing 39 boards
drifted from PlatformIO. The two largest categories are pure schema
drift that this PR resolves; the remaining ~14 are configuration-
choice differences that need maintainer review.
## Fixed: tinyuf2 partition CSV rename (18 boards)
Upstream Adafruit (and the espressif32 platform's Adafruit board
defs) renamed `tinyuf2-partitions-<size>.csv` ->
`partitions-<size>-tinyuf2.csv`. Our bundled board JSONs still pointed
to the old name. Renamed in:
adafruit_feather_esp32s2{,_reversetft,_tft}
adafruit_feather_esp32s3{,_nopsram,_reversetft,_tft}
adafruit_funhouse_esp32s2
adafruit_magtag29_esp32s2
adafruit_matrixportal_esp32s3
adafruit_metro_esp32s2
adafruit_metro_esp32s3
adafruit_qtpy_esp32s2
adafruit_qtpy_esp32s3_n4r2
adafruit_qtpy_esp32s3_nopsram
adafruit_qualia_s3_rgb666
featheresp32-s2
wt32-sc01-plus
All swaps are mechanical s/tinyuf2-partitions-<N>MB.csv/partitions-<N>MB-tinyuf2.csv/.
## Fixed: 4d_systems_esp32s3_gen4_r8n16 partitions
`esp_sr_16.csv` -> `default_16MB.csv` per upstream.
## Fixed: validator whitelist for cmsis_dsp_lib (8 teensy boards)
`build.cmsis_dsp_lib` was added by fbuild#300 (PR#313) as an
intentional fbuild-only schema extension — Teensyduino's makefile
picks the matching `arm_cortexM*_math` archive at link time, but
fbuild needs it declared statically. The validator was flagging all
8 teensy variants because the diff treats every extra key in the
actual asset as drift.
Added `FBUILD_EXTENSION_BUILD_FIELDS` whitelist (currently just
`cmsis_dsp_lib`) and strip those from the actual side before diffing.
Documented in-place so future fbuild-only extensions have a place to
land with a one-line note.
## Not in scope
Remaining ~14 boards have configuration-choice drift (UM boards
`-DARDUINO_USB_MODE=1` difference, m5stack variant rename, seeed_
xiao_esp32c6 USB defines, arduino_nano_esp32 BOARD_USES_HW_GPIO_NUMBERS
vs BOARD_HAS_PIN_REMAP, um_tinys2 extras). Each requires a real
decision about which value is correct for fbuild's compile path —
not auto-resolvable by a snapshot rebase. Leaving those for a
separate maintainer-driven sweep.
…319) Every `Build SAMD51J` workflow run on main fails at: fatal error: WVariant.h: No such file or directory variants/feather_m4/variant.h:43:10 43 | #include "WVariant.h" Root cause: Adafruit's ArduinoCore-samd v1.7.16 (the SAMD framework fbuild installs) ships only `cores/arduino/` — verified via GitHub API. Every Adafruit SAMD board declares `build.core = "adafruit"` in its board JSON (matching PlatformIO upstream, so the board-validator is happy). fbuild's `SamdCores::get_core_dir(core_name)` literally joined `cores/<core_name>`, so it asked for `cores/adafruit/`, which doesn't exist. The variant compile's include-path then pointed at a phantom directory and `WVariant.h` (which lives in `cores/arduino/`) was unreachable. PlatformIO's atmelsam builder handles this by treating `build.core` as a vendor brand label rather than a literal subdirectory name. Mirror that behavior: if `cores/<core_name>/` doesn't exist, fall back to `cores/arduino/` (the canonical Arduino-compatible cores directory name). Refactored the fallback into a small `resolve_core_dir_with_arduino_ fallback` helper that's straightforwardly unit-testable. Three new tests cover the three branches (literal name wins, fallback when named dir absent, return literal when both absent so the eventual file-open error has a meaningful path). Local test results: 9/9 samd_core tests pass, 412/412 fbuild-packages test suite passes. Fixes #319.
|
Warning Review limit reached
More reviews will be available in 27 minutes and 17 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 (23)
✨ 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 |
…ollow-up) The get_core_dir fallback in the previous commit is necessary but not sufficient. `BoardConfig::get_include_paths(framework_root)` builds the compile-time include list using the LITERAL `build.core` value from the board JSON — for Adafruit SAMD boards that's "adafruit", which produces a `-I.../cores/adafruit` path that doesn't exist. The variant compile's `#include "WVariant.h"` then misses because: - same-dir lookup (variants/feather_m4/) — not there - `-I.../cores/adafruit` (phantom) — directory doesn't exist - other -I entries (system includes, variant dir) — no WVariant.h `install_samd_core` already has a resolved `core_dir` (from `SamdCores::get_core_dir`, which falls back to cores/arduino/ per the previous commit). Inject it into the system_includes vector returned by `install_samd_core` so the orchestrator's eventual compile sees `-I.../cores/arduino` and the headers resolve. Inserted at position 0 to make the active core dir the first search hit, matching what PlatformIO's atmelsam builder does. Refs #319.
Member
Author
|
Pushed a follow-on ( Root cause for the second half:
Fix: Together the two commits should clear the SAMD51J failure on next main push. |
zackees
added a commit
that referenced
this pull request
May 30, 2026
Bundles four user-facing fixes merged since 2.2.9: - #319, #320: SAMD51 cores/arduino fallback + variant include injection (Build SAMD51J was 100% red) - #321, #322: nRF52 nRF52DK -> pca10056 variant alias for Adafruit framework (Build nRF52840 DK was 100% red) - #298 (already shipped 2.2.9, board JSON drift cleanup): #318 catches up the tinyuf2 partition renames + adds cmsis_dsp_lib validator whitelist (Validate Boards was failing on 39 boards, now 14) - #317: daemon.rs Command::new annotations (Lint subprocess spawns was red — blocked every PR) Downstream FastLED bumps fbuild==2.2.9 to fbuild==2.2.10 to pick up these fixes.
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 free
to 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
Build SAMD51Jfails on every main run with:Reproducible: every recent run of the workflow.
Root cause
Adafruit's
ArduinoCore-samdv1.7.16 (the SAMD framework fbuild installs) ships onlycores/arduino/— verified via the GitHub API:But every Adafruit SAMD board declares
build.core = "adafruit"in its board JSON (matching PIO's upstream, so the board-validator agrees).SamdCores::get_core_dir(core_name)literally joinedcores/<core_name>, so it pointed atcores/adafruit/— which doesn't exist. The variant compile's include path entry was a phantom directory andWVariant.h(which lives incores/arduino/) was unreachable.PIO's
atmelsambuilder handles this by treatingbuild.coreas a vendor brand label rather than a literal subdirectory. fbuild needed the same.Fix
Extract the fallback into a tiny pure helper, then use it from
get_core_dir:get_core_sources(core_name)already goes throughget_core_dir, so the fix propagates.Scope: SAMD only
Limited to
SamdCoresfor now. The other Arduino-compatible core managers (ArduinoCorefor AVR,Esp32Framework,Esp8266Framework, etc.) have the same shape of bug but no current failing CI exposes them. Easier to extend the pattern if/when we see it bite again than to speculatively change every Framework impl.Test plan
cargo test --lib -p fbuild-packages samd_core— 9/9 passing (3 new tests cover literal-wins, fallback, and no-fallback-available branches)cargo test --lib -p fbuild-packages— 412/412 passingBuild SAMD51Jworkflow turns green on next main pushBuild SAMD21,Build SAMD21 Zero,Build SAMD51PFixes #319.