Skip to content

Commit 056474c

Browse files
authored
fix(samd): fall back to cores/arduino when build.core dir is missing (#320)
* fix(lint): annotate Python daemon Command::new sites for subprocess-spawn 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. * fix(boards): catch up tinyuf2 partition rename + esp_sr drift; whitelist 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. * fix(samd): fall back to cores/arduino when build.core dir is missing (#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. * fix(samd): inject resolved core_dir into compile include path (#319 follow-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.
1 parent 872a291 commit 056474c

2 files changed

Lines changed: 85 additions & 1 deletion

File tree

crates/fbuild-build/src/sam/orchestrator.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,20 @@ fn install_samd_core(
365365
];
366366
// Also include the variant dir for variant.h
367367
includes.push(variant_dir.clone());
368+
// And the resolved core dir for Arduino.h / WVariant.h.
369+
//
370+
// `BoardConfig::get_include_paths` already emits
371+
// `framework_root/cores/<board.core>`, which for Adafruit SAMD boards is
372+
// a vendor-brand label (e.g. "adafruit") that the framework doesn't
373+
// actually ship as a directory — only `cores/arduino/` exists. That
374+
// literal path becomes a phantom `-I` and `#include "Arduino.h"` /
375+
// `#include "WVariant.h"` lookups miss. `core_dir` here was produced by
376+
// `SamdCores::get_core_dir` which falls back to `cores/arduino/` when the
377+
// literal subdir is absent (FastLED/fbuild#319), so injecting it into the
378+
// compile include path is what actually resolves the headers. Putting
379+
// it BEFORE the phantom is a no-op but makes the active dir the first
380+
// hit during search, matching what PlatformIO's atmelsam builder does.
381+
includes.insert(0, core_dir.clone());
368382

369383
Ok((
370384
framework_dir,

crates/fbuild-packages/src/library/samd_core.rs

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,17 @@ impl SamdCores {
7373
}
7474

7575
/// Get the core source directory for a specific core name.
76+
///
77+
/// PlatformIO's `build.core` field is a vendor-branding label more than
78+
/// a real subdirectory name — Adafruit's ArduinoCore-samd (the SAMD
79+
/// framework we install) ships only `cores/arduino/`, but every Adafruit
80+
/// SAMD board declares `build.core = "adafruit"` so the literal
81+
/// `cores/adafruit/` lookup misses. Mirror PlatformIO's atmelsam builder
82+
/// fallback: if `cores/<core_name>/` doesn't exist on disk, fall back to
83+
/// `cores/arduino/` (the canonical name for Arduino-compatible cores).
84+
/// See FastLED/fbuild#319.
7685
pub fn get_core_dir(&self, core_name: &str) -> PathBuf {
77-
self.get_cores_dir().join(core_name)
86+
resolve_core_dir_with_arduino_fallback(&self.get_cores_dir(), core_name)
7887
}
7988

8089
/// Get the variant directory for a specific variant name.
@@ -194,6 +203,25 @@ fn collect_sources(dir: &Path) -> Vec<PathBuf> {
194203
sources
195204
}
196205

206+
/// Resolve a core source dir under `cores_dir`, falling back to
207+
/// `cores/arduino/` when the literal `<core_name>` subdirectory is missing.
208+
///
209+
/// Adafruit (and some other vendor) Arduino-compatible cores set
210+
/// `build.core = "<vendor>"` in their board JSON for branding even though
211+
/// the actual sources live in `cores/arduino/`. PlatformIO's atmelsam
212+
/// builder handles this transparently; fbuild needs to do the same.
213+
fn resolve_core_dir_with_arduino_fallback(cores_dir: &Path, core_name: &str) -> PathBuf {
214+
let primary = cores_dir.join(core_name);
215+
if primary.is_dir() {
216+
return primary;
217+
}
218+
let fallback = cores_dir.join("arduino");
219+
if fallback.is_dir() {
220+
return fallback;
221+
}
222+
primary
223+
}
224+
197225
#[cfg(test)]
198226
mod tests {
199227
use super::*;
@@ -249,4 +277,46 @@ mod tests {
249277
let result = SamdCores::validate(tmp.path());
250278
assert!(result.is_err());
251279
}
280+
281+
/// FastLED/fbuild#319: Adafruit SAMD boards declare `build.core = "adafruit"`
282+
/// (a vendor brand label) but the framework only ships `cores/arduino/`.
283+
/// The resolver must fall back to `cores/arduino/` when the literal
284+
/// `cores/<core_name>/` doesn't exist on disk, mirroring PIO's atmelsam
285+
/// builder behavior.
286+
#[test]
287+
fn resolve_falls_back_to_arduino_when_named_dir_missing() {
288+
let tmp = tempfile::TempDir::new().unwrap();
289+
let cores_dir = tmp.path().join("cores");
290+
std::fs::create_dir_all(cores_dir.join("arduino")).unwrap();
291+
// No `cores/adafruit/` exists.
292+
293+
let resolved = resolve_core_dir_with_arduino_fallback(&cores_dir, "adafruit");
294+
assert_eq!(resolved, cores_dir.join("arduino"));
295+
}
296+
297+
/// Sanity: when the named core dir *does* exist, the fallback must not
298+
/// kick in — honor the literal name.
299+
#[test]
300+
fn resolve_uses_literal_name_when_present() {
301+
let tmp = tempfile::TempDir::new().unwrap();
302+
let cores_dir = tmp.path().join("cores");
303+
std::fs::create_dir_all(cores_dir.join("custom_vendor")).unwrap();
304+
std::fs::create_dir_all(cores_dir.join("arduino")).unwrap();
305+
306+
let resolved = resolve_core_dir_with_arduino_fallback(&cores_dir, "custom_vendor");
307+
assert_eq!(resolved, cores_dir.join("custom_vendor"));
308+
}
309+
310+
/// When neither the named dir nor `cores/arduino/` exists, return the
311+
/// literal `cores/<name>/` so the eventual file-open error surfaces with
312+
/// a meaningful path (don't synthesize a nonexistent fallback).
313+
#[test]
314+
fn resolve_returns_literal_when_no_fallback_available() {
315+
let tmp = tempfile::TempDir::new().unwrap();
316+
let cores_dir = tmp.path().join("cores");
317+
// No cores subdirs at all.
318+
319+
let resolved = resolve_core_dir_with_arduino_fallback(&cores_dir, "vendor");
320+
assert_eq!(resolved, cores_dir.join("vendor"));
321+
}
252322
}

0 commit comments

Comments
 (0)