From 13c9493c317e6f690b75ee62a5518fd00df46a3a Mon Sep 17 00:00:00 2001 From: zackees Date: Sat, 25 Apr 2026 01:12:07 -0700 Subject: [PATCH] feat(library-selection): #205 Phase 6 acceptance gates + Phase 8 CLI/docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continues #205 on top of the foundation merged in PR #207. No-op for runtime behaviour; adds gates that *prove* the foundation behaves as specified, plus a diagnostic CLI and architecture docs. ## Phase 6 — acceptance gates (#[ignore]'d, CI-only) - `crates/fbuild-build/tests/teensylc_acceptance.rs` — builds teensyLC Blink end-to-end and asserts AC#1: * `.bss` <= 3 KB * No `fnet_` / `snooze_` / `RadioHead` / `mbedtls` symbol substrings (#204 regression guard) * `setup` / `loop` symbols present * `compile_commands.json` TU count <= 250 * No FNET / Snooze / RadioHead / mbedtls entries in compile DB - `crates/fbuild-build/tests/stm32_acceptance.rs` — builds an stm32f103c8 sketch that `#include`s `` and asserts AC#4: * `SPIClass` symbol present in ELF * compile_commands.json contains an SPI library entry * No manual allowlist needed (closes #202) Both gates use `fbuild_test_support::{ElfProbe, CompileDb}` from PR #207 and are gated behind `#[ignore]` because they download real toolchains and Teensyduino / STM32duino frameworks. CI runs them via `cargo test -- --ignored`. ## Phase 8 — diagnostic CLI - `fbuild lib-select -e ` — drives the resolver in-process and prints the selected library set. - `--explain` shows per-library trigger header + unresolved includes. - `--json` emits machine-readable output (`{ selected, unresolved, included_files, seeds, framework }`). - Mutually-exclusive flags (`conflicts_with`). - New `crates/fbuild-cli/src/lib_select.rs` (~420 LoC) owns the diagnostic flow without depending on `fbuild-build` — uses `fbuild-packages::library` directly to keep the CLI a leaf binary. - 3 new tests in `crates/fbuild-cli/tests/lib_select.rs` (help-output, missing-project exit code, flag-conflict). ## Phase 8 — architecture docs - `docs/architecture/library-selection.md` — new subsystem doc covering scanner / walker / resolver, path-prefix attribution, two-pass convergence, future Phase 4 cache. - `docs/CLAUDE.md` table updated. - `docs/INDEX.md` FAQ entries added. - `docs/architecture/README.md` lists the new doc. - `tasks/lessons.md` appended with the LDF-semantics lesson. ## Verification - `uv run soldr cargo check --workspace --all-targets` — green. - `uv run soldr cargo clippy --workspace --all-targets -- -D warnings` — green (only pre-existing `clippy.toml` MSRV info note). - `uv run soldr cargo fmt --all --check` — clean. - `RUSTDOCFLAGS="-D warnings" uv run soldr cargo doc --workspace --no-deps` — green. - `uv run soldr cargo test -p fbuild-cli` — 13 passed, 0 failed (3 new lib_select + existing). - Phase 6 acceptance tests `#[ignore]`'d; CI runs them with `--ignored`. ## Out of scope (still tracked) - Phase 4 — zccache K/V memoization (gated on zackees/zccache#130). - Phase 7 — perf gates wired into `bench/fastled-examples`. - Baseline numeric capture (`tasks/baseline-205.md` placeholder). Refs: FastLED/fbuild#202, FastLED/fbuild#204, FastLED/fbuild#205, zackees/zccache#130 Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 3 + crates/fbuild-build/Cargo.toml | 1 + crates/fbuild-build/tests/stm32_acceptance.rs | 122 +++++ .../fbuild-build/tests/teensylc_acceptance.rs | 131 ++++++ crates/fbuild-cli/Cargo.toml | 2 + crates/fbuild-cli/src/lib_select.rs | 419 ++++++++++++++++++ crates/fbuild-cli/src/main.rs | 33 ++ crates/fbuild-cli/tests/lib_select.rs | 93 ++++ docs/CLAUDE.md | 1 + docs/INDEX.md | 2 + docs/architecture/README.md | 1 + docs/architecture/library-selection.md | 150 +++++++ tasks/lessons.md | 8 + 13 files changed, 966 insertions(+) create mode 100644 crates/fbuild-build/tests/stm32_acceptance.rs create mode 100644 crates/fbuild-build/tests/teensylc_acceptance.rs create mode 100644 crates/fbuild-cli/src/lib_select.rs create mode 100644 crates/fbuild-cli/tests/lib_select.rs create mode 100644 docs/architecture/library-selection.md diff --git a/Cargo.lock b/Cargo.lock index 168edd9e..8cf019a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -764,6 +764,7 @@ dependencies = [ "fbuild-library-select", "fbuild-packages", "fbuild-paths", + "fbuild-test-support", "filetime", "regex", "serde", @@ -786,6 +787,7 @@ dependencies = [ "fbuild-config", "fbuild-core", "fbuild-deploy", + "fbuild-library-select", "fbuild-packages", "fbuild-paths", "futures", @@ -797,6 +799,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "walkdir", ] [[package]] diff --git a/crates/fbuild-build/Cargo.toml b/crates/fbuild-build/Cargo.toml index 7d6b12ab..43ad93cc 100644 --- a/crates/fbuild-build/Cargo.toml +++ b/crates/fbuild-build/Cargo.toml @@ -26,3 +26,4 @@ tempfile = { workspace = true } [dev-dependencies] filetime = { workspace = true } +fbuild-test-support = { path = "../fbuild-test-support" } diff --git a/crates/fbuild-build/tests/stm32_acceptance.rs b/crates/fbuild-build/tests/stm32_acceptance.rs new file mode 100644 index 00000000..1e3a6d7b --- /dev/null +++ b/crates/fbuild-build/tests/stm32_acceptance.rs @@ -0,0 +1,122 @@ +//! Acceptance gate for #205 AC#4 / closes #202: stm32f103c8 SPI auto-discovery. +//! +//! This integration test verifies that an stm32f103c8 Blink sketch which +//! `#include`s `` builds with no manual library allowlist, and that +//! the bundled `Arduino_Core_STM32` SPI library is automatically discovered +//! by fbuild's library-selection layer. +//! +//! Run with: +//! `uv run soldr cargo test -p fbuild-build --test stm32_acceptance -- --ignored` +//! +//! Marked `#[ignore]` because it downloads the ARM GCC toolchain plus the +//! STM32duino cores (cached after first run) and performs a full firmware +//! build — too heavy for default `cargo test`. +//! +//! Acceptance criteria (#205 AC#4): +//! 1. The build succeeds. +//! 2. `compile_commands.json` references at least one source file under +//! the SPI library (substring `SPI`). +//! 3. The ELF contains a symbol whose mangled name contains `SPIClass`. + +use std::path::{Path, PathBuf}; + +use fbuild_build::{BuildOrchestrator, BuildParams}; +use fbuild_core::BuildProfile; +use fbuild_test_support::{CompileDb, ElfProbe}; + +#[test] +#[ignore = "downloads STM32duino + builds firmware; CI-only"] +fn stm32f103c8_blink_with_spi_auto_discovers_library_205_ac4() { + // Use a temporary project dir so we can write our own SPI-using sketch + // independent of whatever ships in the fixture. + let tmp = tempfile::TempDir::new().unwrap(); + let project_dir = tmp.path(); + + std::fs::write( + project_dir.join("platformio.ini"), + "[env:stm32f103c8]\n\ + platform = ststm32\n\ + board = bluepill_f103c8\n\ + framework = arduino\n", + ) + .unwrap(); + + let src = project_dir.join("src"); + std::fs::create_dir_all(&src).unwrap(); + std::fs::write( + src.join("main.cpp"), + "#include \n\ + #include \n\ + void setup() { SPI.begin(); }\n\ + void loop() {}\n", + ) + .unwrap(); + + let build_dir = project_dir.join(".fbuild/build"); + let params = BuildParams { + project_dir: project_dir.to_path_buf(), + env_name: "stm32f103c8".to_string(), + clean: true, + profile: BuildProfile::Release, + build_dir: build_dir.clone(), + verbose: true, + jobs: None, + generate_compiledb: true, + compiledb_only: false, + log_sender: None, + symbol_analysis: false, + symbol_analysis_path: None, + no_timestamp: false, + src_dir: None, + pio_env: Default::default(), + extra_build_flags: Vec::new(), + watch_set_cache: None, + }; + + let orchestrator = fbuild_build::stm32::orchestrator::Stm32Orchestrator; + let result = orchestrator + .build(¶ms) + .expect("stm32f103c8 build with SPI must succeed"); + assert!(result.success, "build did not report success"); + + let elf = result + .elf_path + .as_ref() + .expect("stm32 build must produce ELF"); + let probe = ElfProbe::open(elf).expect("ELF parses"); + assert!( + probe + .has_symbol_containing("SPIClass") + .expect("symbol query"), + "AC#4: SPIClass symbol must be present in ELF — closes #202" + ); + + let compdb = locate_compile_commands(&build_dir, "stm32f103c8") + .expect("compile_commands.json should land in build dir"); + let db = CompileDb::from_path(&compdb).expect("parse compile_commands.json"); + let spi_entries: Vec<_> = db.entries_matching("SPI").collect(); + assert!( + !spi_entries.is_empty(), + "AC#4: compile_commands.json must reference an SPI library entry — \ + closes #202; found {} entries with no SPI hit", + db.tu_count() + ); +} + +fn locate_compile_commands(build_dir: &Path, env: &str) -> Option { + let candidates = [ + build_dir.join(env).join("compile_commands.json"), + build_dir.join("compile_commands.json"), + ]; + for c in candidates { + if c.exists() { + return Some(c); + } + } + for entry in walkdir::WalkDir::new(build_dir).into_iter().flatten() { + if entry.file_name() == "compile_commands.json" { + return Some(entry.into_path()); + } + } + None +} diff --git a/crates/fbuild-build/tests/teensylc_acceptance.rs b/crates/fbuild-build/tests/teensylc_acceptance.rs new file mode 100644 index 00000000..1b10f37d --- /dev/null +++ b/crates/fbuild-build/tests/teensylc_acceptance.rs @@ -0,0 +1,131 @@ +//! Phase 6.a acceptance gate for issue #205 on the teensyLC Blink target. +//! +//! Runs the full TeensyOrchestrator build against the in-repo +//! `tests/platform/teensylc` fixture and asserts: +//! +//! * `.bss` size <= 3 KB (#205 AC#1). +//! * No `fnet_*`, `snooze_*`, `RadioHead`, or `mbedtls` symbols leaked into +//! the linked ELF (#205 AC#1, #204 regression guard). +//! * The Arduino/Teensy `setup` and `loop` symbols are present (#205 A-11). +//! * `compile_commands.json` has <= 250 translation units (was 451 pre-fix +//! per the #205 issue body). +//! * `compile_commands.json` references no `FNET`, `Snooze`, `RadioHead`, or +//! `mbedtls` files (#204 root-cause guard). +//! +//! This test downloads Teensyduino + arm-gcc on the first run and is +//! therefore CI-only — it is gated behind `#[ignore]` and runs via +//! `uv run soldr cargo test -p fbuild-build --test teensylc_acceptance -- --ignored`. + +use std::path::PathBuf; + +use fbuild_build::{BuildOrchestrator, BuildParams}; +use fbuild_core::BuildProfile; +use fbuild_test_support::{CompileDb, ElfProbe}; + +#[test] +#[ignore = "downloads Teensyduino + builds firmware; CI-only"] +fn teensylc_blink_meets_205_acceptance_criteria() { + let project_dir = repo_fixture("teensylc"); + let build_dir = tempfile::TempDir::new().unwrap(); + + let params = BuildParams { + project_dir: project_dir.clone(), + env_name: "teensyLC".to_string(), + clean: true, + profile: BuildProfile::Release, + build_dir: build_dir.path().to_path_buf(), + verbose: true, + jobs: None, + generate_compiledb: true, + compiledb_only: false, + log_sender: None, + symbol_analysis: false, + symbol_analysis_path: None, + no_timestamp: false, + src_dir: None, + pio_env: Default::default(), + extra_build_flags: Vec::new(), + watch_set_cache: None, + }; + + let result = fbuild_build::teensy::orchestrator::TeensyOrchestrator + .build(¶ms) + .expect("teensyLC build must succeed for acceptance gate"); + assert!(result.success, "build did not report success"); + + // ── ELF probes (AC#1) ─────────────────────────────────────────────── + let elf = result + .elf_path + .as_ref() + .expect("teensy build must produce ELF"); + let probe = ElfProbe::open(elf).expect("ELF parses"); + let bss = probe.section_size(".bss").expect("bss query"); + assert!(bss <= 3 * 1024, "AC#1: .bss must be <= 3KB; got {bss}"); + + for forbidden in ["fnet_", "snooze_", "RadioHead", "mbedtls"] { + assert!( + !probe + .has_symbol_containing(forbidden) + .expect("symbol query"), + "AC#1: forbidden symbol substring '{forbidden}' present in ELF — \ + #204 regression" + ); + } + for required in ["setup", "loop"] { + assert!( + probe.has_symbol(required).expect("symbol query") + || probe.has_symbol_containing(required).expect("symbol query"), + "A-11: required symbol '{required}' missing from ELF" + ); + } + + // ── compile_commands.json probes (AC#1, A-20..A-22) ───────────────── + let compdb_path = locate_compile_commands(build_dir.path(), "teensyLC") + .expect("compile_commands.json should land in build dir"); + let db = CompileDb::from_path(&compdb_path).expect("parse compile_commands.json"); + assert!( + db.tu_count() <= 250, + "AC#1: TU count must be <= 250; got {} entries", + db.tu_count() + ); + let forbidden_hits = db.forbidden_present(&["FNET", "Snooze", "RadioHead", "mbedtls"]); + assert!( + forbidden_hits.is_empty(), + "AC#1 / #204: compile_commands.json must not include any of \ + FNET/Snooze/RadioHead/mbedtls; found: {:?}", + forbidden_hits + ); +} + +fn repo_fixture(name: &str) -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .unwrap() + .parent() + .unwrap() + .join("tests/platform") + .join(name) +} + +fn locate_compile_commands(build_dir: &std::path::Path, env: &str) -> Option { + // Per fbuild's layout the file lives at one of: + // //compile_commands.json + // /compile_commands.json + // Search both, prefer the per-env path. + let candidates = [ + build_dir.join(env).join("compile_commands.json"), + build_dir.join("compile_commands.json"), + ]; + for c in candidates { + if c.exists() { + return Some(c); + } + } + // Fallback: walk the build_dir for any compile_commands.json. + for entry in walkdir::WalkDir::new(build_dir).into_iter().flatten() { + if entry.file_name() == "compile_commands.json" { + return Some(entry.into_path()); + } + } + None +} diff --git a/crates/fbuild-cli/Cargo.toml b/crates/fbuild-cli/Cargo.toml index 4ee3874f..c28f2ef6 100644 --- a/crates/fbuild-cli/Cargo.toml +++ b/crates/fbuild-cli/Cargo.toml @@ -14,6 +14,7 @@ path = "src/main.rs" fbuild-core = { path = "../fbuild-core" } fbuild-config = { path = "../fbuild-config" } fbuild-deploy = { path = "../fbuild-deploy" } +fbuild-library-select = { path = "../fbuild-library-select" } fbuild-packages = { path = "../fbuild-packages" } fbuild-paths = { path = "../fbuild-paths" } tokio = { workspace = true } @@ -28,3 +29,4 @@ ctrlc = "3.5.2" blake3 = { workspace = true } sha2 = { workspace = true } tempfile = { workspace = true } +walkdir = { workspace = true } diff --git a/crates/fbuild-cli/src/lib_select.rs b/crates/fbuild-cli/src/lib_select.rs new file mode 100644 index 00000000..9330df9f --- /dev/null +++ b/crates/fbuild-cli/src/lib_select.rs @@ -0,0 +1,419 @@ +//! `fbuild lib-select` diagnostic subcommand. +//! +//! Drives the same library-selection resolver the daemon uses (the +//! `fbuild-library-select` crate) directly against an already-installed +//! framework cache and the project's own `src/`/`include/`/`lib/` trees, then +//! prints the result in three flavors: +//! +//! - default: one library name per line, sorted, suitable for piping to grep; +//! - `--explain`: human-readable summary with per-library attribution and the +//! list of unresolved `#include`s — handy for debugging +//! `FastLED/fbuild#202` / `#204`-style "library not found" issues; +//! - `--json`: machine-readable; mutually exclusive with `--explain`. +//! +//! The CLI does **not** download anything: if the framework cache root is +//! missing, it prints a clear "framework not installed; run `fbuild build` +//! once first" message and exits 2. Downloading is the daemon's job. + +use std::path::{Path, PathBuf}; + +use fbuild_config::PlatformIOConfig; +use fbuild_core::Platform; +use fbuild_library_select::{resolve, Selection}; +use fbuild_packages::library::framework_library::discover_framework_libraries; +use fbuild_packages::library::FrameworkLibrary; +use fbuild_packages::Framework; +use serde_json::json; +use walkdir::{DirEntry, WalkDir}; + +/// Top-level entry point. Returns a process exit code. +pub fn run(project_dir: &Path, env: Option<&str>, explain: bool, json: bool) -> i32 { + let project_dir = match std::fs::canonicalize(project_dir) { + Ok(p) => p, + Err(err) => { + eprintln!( + "lib-select: project '{}' not accessible: {}", + project_dir.display(), + err + ); + return 2; + } + }; + + let ini_path = project_dir.join("platformio.ini"); + let config = match PlatformIOConfig::from_path(&ini_path) { + Ok(c) => c, + Err(err) => { + eprintln!("lib-select: failed to read {}: {}", ini_path.display(), err); + return 2; + } + }; + + let env_name: String = match env { + Some(name) => name.to_string(), + None => match config.get_default_environment() { + Some(name) => name.to_string(), + None => { + eprintln!("lib-select: no environment given and no default in platformio.ini"); + return 2; + } + }, + }; + + let env_cfg = match config.get_env_config(&env_name) { + Ok(cfg) => cfg, + Err(err) => { + eprintln!("lib-select: {}", err); + return 2; + } + }; + + let platform_str = env_cfg.get("platform").cloned().unwrap_or_default(); + let board = env_cfg + .get("board") + .cloned() + .unwrap_or_else(|| "unknown".to_string()); + // We deliberately don't read `framework`: every supported `Platform` + // already implies its (single) framework, and PlatformIO's INI uses + // `framework = arduino` near-uniformly for the boards we handle. + + let platform = match Platform::from_platform_str(&platform_str) { + Some(p) => p, + None => { + eprintln!( + "lib-select: unrecognized platform '{}' for env '{}'", + platform_str, env_name + ); + return 2; + } + }; + + let framework_info = match resolve_framework(&project_dir, platform, &board) { + Ok(info) => info, + Err(msg) => { + eprintln!("lib-select: {}", msg); + return 2; + } + }; + + let libraries = discover_framework_libraries(&framework_info.libraries_dir); + + let scan_roots = project_scan_roots(&project_dir); + let seeds = collect_project_seeds(&scan_roots); + + let selection = resolve(&seeds, &scan_roots, &libraries); + + if json { + emit_json( + &project_dir, + &env_name, + &framework_info, + &libraries, + &seeds, + &selection, + ); + } else if explain { + emit_explain( + &project_dir, + &env_name, + &framework_info, + &libraries, + &seeds, + &selection, + ); + } else { + for name in &selection.required_libraries { + println!("{}", name); + } + } + + 0 +} + +/// Resolved framework cache info needed to discover libraries. +struct FrameworkInfo { + name: String, + root: PathBuf, + libraries_dir: PathBuf, +} + +/// Map (platform, board) → an installed framework on disk. +/// +/// Each platform's framework lives at a different cache path. We instantiate +/// the relevant `fbuild_packages::library::*` type and ask it for +/// `get_libraries_dir()`. We do **not** download anything: if the resolved +/// path doesn't exist we return an Err and the caller prints a clear +/// "framework not installed" message. +fn resolve_framework( + project_dir: &Path, + platform: Platform, + board: &str, +) -> Result { + use fbuild_packages::library::{ + Apollo3Cores, AvrFramework, Ch32vCores, Esp32Framework, Esp8266Framework, Nrf52Cores, + RenesasCores, Rp2040Cores, SamCores, SilabsCores, Stm32Cores, TeensyCores, + }; + + /// Build a `FrameworkInfo` from a libraries dir + display name. Pulled out + /// so each match arm can stay a one-liner. + fn info(name: &str, libs: PathBuf) -> (String, PathBuf, PathBuf) { + let root = libs + .parent() + .map(Path::to_path_buf) + .unwrap_or_else(|| libs.clone()); + (name.to_string(), libs, root) + } + + let (name, libraries_dir, root): (String, PathBuf, PathBuf) = match platform { + Platform::Teensy => info( + "framework-arduinoteensy", + TeensyCores::new(project_dir).get_libraries_dir(), + ), + Platform::Ststm32 => info( + "Arduino_Core_STM32", + Stm32Cores::new(project_dir).get_libraries_dir(), + ), + Platform::AtmelAvr | Platform::AtmelMegaAvr => { + // AVR is data-driven by board core; "arduino" covers the + // overwhelming majority of boards (uno, mega, nano, leonardo, ...). + // Boards needing alt cores (MiniCore, ATTinyCore, ...) won't + // resolve — caller can re-run after `fbuild build` populates them. + let fw = AvrFramework::for_core("arduino", project_dir) + .map_err(|e| format!("AVR framework lookup failed for core 'arduino': {}", e))?; + info("framework-arduino-avr", fw.get_libraries_dir()) + } + Platform::Espressif32 => { + // `Esp32Framework::new` accepts an MCU but only consults it for + // post-discovery operations; the libraries dir is the same for + // every ESP32 variant. + info( + "framework-arduinoespressif32", + Esp32Framework::new(project_dir, board).get_libraries_dir(), + ) + } + Platform::Espressif8266 => info( + "framework-arduinoespressif8266", + Esp8266Framework::new(project_dir).get_libraries_dir(), + ), + Platform::RaspberryPi => info( + "framework-arduinopico", + Rp2040Cores::new(project_dir).get_libraries_dir(), + ), + Platform::NordicNrf52 => info( + "framework-arduinoadafruitnrf52", + Nrf52Cores::new(project_dir).get_libraries_dir(), + ), + Platform::AtmelSam => info( + "framework-arduinosam", + SamCores::new(project_dir).get_libraries_dir(), + ), + Platform::RenesasRa => info( + "framework-arduinorenesas", + RenesasCores::new(project_dir).get_libraries_dir(), + ), + Platform::SiliconLabs => info( + "framework-arduinosilabs", + SilabsCores::new(project_dir).get_libraries_dir(), + ), + Platform::Ch32v => info( + "framework-arduinoch32v", + Ch32vCores::new(project_dir).get_libraries_dir(), + ), + Platform::Apollo3 => info( + "framework-arduinoapollo3", + Apollo3Cores::new(project_dir).get_libraries_dir(), + ), + Platform::Wasm => { + return Err( + "WASM (emscripten) does not use a libraries/ tree; lib-select is not applicable" + .to_string(), + ); + } + }; + + if !libraries_dir.is_dir() { + return Err(format!( + "framework not installed at {} (run `fbuild build` once first to populate the cache)", + libraries_dir.display() + )); + } + + Ok(FrameworkInfo { + name, + root, + libraries_dir, + }) +} + +/// Mirror of `fbuild_build::framework_libs::framework_include_scan_roots`. +/// +/// Kept in-crate (rather than depending on `fbuild-build`) so this diagnostic +/// is a leaf binary and the CLI doesn't drag in the orchestrator graph. +fn project_scan_roots(project_dir: &Path) -> Vec { + let mut roots = Vec::new(); + for sub in ["src", "include", "lib"] { + let p = project_dir.join(sub); + if p.exists() && !roots.iter().any(|r: &PathBuf| r == &p) { + roots.push(p); + } + } + roots +} + +fn collect_project_seeds(roots: &[PathBuf]) -> Vec { + let mut seeds = Vec::new(); + for root in roots { + for entry in WalkDir::new(root) + .into_iter() + .filter_entry(should_scan_entry) + .flatten() + { + if !entry.file_type().is_file() { + continue; + } + if is_source_or_header(entry.path()) { + seeds.push(entry.path().to_path_buf()); + } + } + } + seeds +} + +fn should_scan_entry(entry: &DirEntry) -> bool { + let name = entry.file_name().to_string_lossy().to_lowercase(); + !matches!( + name.as_str(), + ".git" + | ".pio" + | ".fbuild" + | ".zap" + | ".build" + | "build" + | "target" + | ".venv" + | "venv" + | "node_modules" + | "__pycache__" + ) +} + +fn is_source_or_header(path: &Path) -> bool { + let ext = path + .extension() + .and_then(|e| e.to_str()) + .unwrap_or_default() + .to_lowercase(); + matches!( + ext.as_str(), + "c" | "cpp" | "cc" | "cxx" | "s" | "ino" | "h" | "hh" | "hpp" | "hxx" + ) +} + +/// For each selected library, find one reached file under its include dirs to +/// show as "triggered by". We canonicalize both sides so prefix matching works +/// across the same path-canonicalization quirks (`/private/var`, `\\?\`, ...) +/// that the resolver itself handles. +fn first_reached_under(included: &[PathBuf], lib: &FrameworkLibrary) -> Option { + let canon_dirs: Vec = lib + .include_dirs + .iter() + .map(|d| std::fs::canonicalize(d).unwrap_or_else(|_| d.clone())) + .collect(); + included + .iter() + .find(|p| canon_dirs.iter().any(|d| p.starts_with(d))) + .cloned() +} + +fn emit_explain( + project_dir: &Path, + env_name: &str, + framework: &FrameworkInfo, + libraries: &[FrameworkLibrary], + seeds: &[PathBuf], + sel: &Selection, +) { + println!("project: {}", project_dir.display()); + println!("env: {}", env_name); + println!( + "framework: {} @ {}", + framework.name, + framework.root.display() + ); + println!(); + println!("selected libraries ({}):", sel.required_libraries.len()); + for name in &sel.required_libraries { + if let Some(lib) = libraries.iter().find(|l| &l.name == name) { + let trigger = first_reached_under(&sel.included_files, lib) + .map(|p| p.display().to_string()) + .unwrap_or_else(|| "".to_string()); + println!( + " {:<14} (include_dirs: {}, sources: {}, triggered by: {})", + lib.name, + lib.include_dirs.len(), + lib.source_files.len(), + trigger + ); + } + } + println!(); + println!("unresolved includes ({}):", sel.unresolved.len()); + for u in &sel.unresolved { + println!(" {}", u); + } + println!(); + println!( + "scanned: {} source files, {} headers reached.", + seeds.len(), + sel.included_files.len() + ); +} + +fn emit_json( + project_dir: &Path, + env_name: &str, + framework: &FrameworkInfo, + libraries: &[FrameworkLibrary], + seeds: &[PathBuf], + sel: &Selection, +) { + let selected: Vec = sel + .required_libraries + .iter() + .map(|name| { + let lib = libraries.iter().find(|l| &l.name == name); + let triggered = lib + .and_then(|l| first_reached_under(&sel.included_files, l)) + .map(|p| p.display().to_string()); + json!({ + "name": name, + "include_dirs": lib.map(|l| l.include_dirs.len()).unwrap_or(0), + "source_count": lib.map(|l| l.source_files.len()).unwrap_or(0), + "triggered_by": triggered, + }) + }) + .collect(); + + let payload = json!({ + "project": project_dir.display().to_string(), + "env": env_name, + "framework": { + "name": framework.name, + "root": framework.root.display().to_string(), + "libraries_dir": framework.libraries_dir.display().to_string(), + }, + "seeds": seeds + .iter() + .map(|p| p.display().to_string()) + .collect::>(), + "included_files": sel + .included_files + .iter() + .map(|p| p.display().to_string()) + .collect::>(), + "selected": selected, + "unresolved": sel.unresolved, + }); + + println!("{}", serde_json::to_string_pretty(&payload).unwrap()); +} diff --git a/crates/fbuild-cli/src/main.rs b/crates/fbuild-cli/src/main.rs index 32bed660..d6dfa3de 100644 --- a/crates/fbuild-cli/src/main.rs +++ b/crates/fbuild-cli/src/main.rs @@ -1,4 +1,5 @@ mod daemon_client; +mod lib_select; mod mcp; use clap::{Parser, Subcommand}; @@ -284,6 +285,22 @@ enum Commands { #[command(subcommand)] action: LnkAction, }, + /// Diagnostic: drive the LDF-style library-selection resolver and print + /// the selected library set. Useful for debugging FastLED/fbuild#202 / + /// `#204`-style "library not found" issues without running a full build. + LibSelect { + /// Project directory (defaults to "."). + project_dir: Option, + /// Target environment. + #[arg(short = 'e', long)] + environment: Option, + /// Show selection origin per library, unresolved headers, etc. + #[arg(long, conflicts_with = "json")] + explain: bool, + /// Emit machine-readable JSON instead of plain text. + #[arg(long, conflicts_with = "explain")] + json: bool, + }, } /// Subcommands for `fbuild lnk`. @@ -423,6 +440,7 @@ const KNOWN_SUBCOMMANDS: &[&str] = &[ "iwyu", "clang-query", "test-emu", + "lib-select", ]; /// Rewrite `fbuild ...` → `fbuild ...` @@ -723,6 +741,21 @@ async fn main() { .await } Some(Commands::Lnk { action }) => run_lnk(action, &top_level_project_dir).await, + Some(Commands::LibSelect { + project_dir, + environment, + explain, + json, + }) => { + let project_dir = resolve_project_dir(project_dir, &top_level_project_dir); + let exit = lib_select::run( + std::path::Path::new(&project_dir), + environment.as_deref(), + explain, + json, + ); + std::process::exit(exit); + } None => { // Default action: deploy with monitor (like Python fbuild) let project_dir = cli.project_dir.unwrap_or_else(|| ".".to_string()); diff --git a/crates/fbuild-cli/tests/lib_select.rs b/crates/fbuild-cli/tests/lib_select.rs new file mode 100644 index 00000000..8919521f --- /dev/null +++ b/crates/fbuild-cli/tests/lib_select.rs @@ -0,0 +1,93 @@ +//! Unit-level tests for the `fbuild lib-select` diagnostic subcommand. +//! +//! These exercise just the CLI surface: argument parsing, conflict +//! resolution between `--explain`/`--json`, and the early-exit paths that +//! don't require a populated framework cache. Heavier end-to-end tests +//! that need a real framework install (Teensy, STM32, ...) are gated +//! behind `#[ignore]` and run manually. + +use std::process::Command; + +/// `fbuild lib-select --help` must exit 0 and document both modes. +#[test] +fn lib_select_help_lists_command() { + let bin = env!("CARGO_BIN_EXE_fbuild"); + // allow-direct-spawn: integration test driver invoking the compiled fbuild binary. + let output = Command::new(bin) + .args(["lib-select", "--help"]) + .output() + .expect("spawn fbuild"); + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "lib-select --help should exit 0\nstdout: {}\nstderr: {}", + stdout, + stderr + ); + assert!( + stdout.contains("--explain"), + "help should document --explain. got:\n{}", + stdout + ); + assert!( + stdout.contains("--json"), + "help should document --json. got:\n{}", + stdout + ); +} + +/// `fbuild lib-select ` must exit non-zero. We don't care about +/// the precise code, only that callers (CI, scripts) can detect the error. +#[test] +fn lib_select_missing_project_exits_nonzero() { + let bin = env!("CARGO_BIN_EXE_fbuild"); + // allow-direct-spawn: integration test driver invoking the compiled fbuild binary. + let output = Command::new(bin) + .args([ + "lib-select", + "/this/path/should/not/exist/and/never/will", + "-e", + "uno", + ]) + .output() + .expect("spawn fbuild"); + + let code = output.status.code().unwrap_or(-1); + assert_ne!( + code, + 0, + "lib-select on a missing project must exit non-zero.\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); +} + +/// `--explain` and `--json` are mutually exclusive (clap `conflicts_with`). +/// Passing both must fail at argument-parse time, not silently pick one. +#[test] +fn lib_select_explain_and_json_conflict() { + let bin = env!("CARGO_BIN_EXE_fbuild"); + // allow-direct-spawn: integration test driver invoking the compiled fbuild binary. + let output = Command::new(bin) + .args(["lib-select", ".", "--explain", "--json"]) + .output() + .expect("spawn fbuild"); + + assert!( + !output.status.success(), + "--explain + --json must be a usage error.\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + let stderr = String::from_utf8_lossy(&output.stderr); + // clap's conflict message contains "cannot be used with" — assert on + // that string so we catch a regression where the conflict gets dropped + // and one flag silently wins. + assert!( + stderr.contains("cannot be used with") || stderr.contains("conflict"), + "expected clap conflict message in stderr.\nstderr: {}", + stderr + ); +} diff --git a/docs/CLAUDE.md b/docs/CLAUDE.md index bbbbdff9..69641c22 100644 --- a/docs/CLAUDE.md +++ b/docs/CLAUDE.md @@ -18,6 +18,7 @@ For a full FAQ-style index of every doc in this repo (human + LLM entry point), | `fbuild-packages` | [overview.md](architecture/overview.md) | | `fbuild-paths` | [overview.md](architecture/overview.md) | | `fbuild-core` | [overview.md](architecture/overview.md) | +| `fbuild-header-scan` / `fbuild-library-select` | [library-selection.md](architecture/library-selection.md) | | Platform-specific issues | [portability.md](architecture/portability.md) | ## Other docs diff --git a/docs/INDEX.md b/docs/INDEX.md index 4d18677c..b62c9aa3 100644 --- a/docs/INDEX.md +++ b/docs/INDEX.md @@ -18,6 +18,8 @@ A grep-friendly FAQ that maps common questions to the file that answers them. Bo | How do the PyO3 Python bindings work? | [architecture/pyo3-bindings.md](architecture/pyo3-bindings.md) | | How does deploy preemption work? | [architecture/deploy-preemption.md](architecture/deploy-preemption.md) | | What are the cross-platform portability constraints? | [architecture/portability.md](architecture/portability.md) | +| How does library selection (LDF) work in fbuild? | [architecture/library-selection.md](architecture/library-selection.md) | +| Why was my library wrongly compiled (#204) / not found (#202)? | [architecture/library-selection.md](architecture/library-selection.md#why) | | Why did we choose X over Y? | [DESIGN_DECISIONS.md](DESIGN_DECISIONS.md) | | What's on the implementation roadmap? | [ROADMAP.md](ROADMAP.md) | | How do I run tests / lint / fmt? | [DEVELOPMENT.md](DEVELOPMENT.md#testing) | diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 2594609a..496e876c 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -11,3 +11,4 @@ Subsystem-specific architecture documentation. See [../CLAUDE.md](../CLAUDE.md) - **`serial.md`** -- Serial port communication and USB-CDC handling - **`pyo3-bindings.md`** -- PyO3 Python bindings for the fbuild-python crate - **`portability.md`** -- Platform-specific considerations (Windows, macOS, Linux) +- **`library-selection.md`** -- LDF-style scanner / walker / resolver subsystem (`fbuild-header-scan`, `fbuild-library-select`) diff --git a/docs/architecture/library-selection.md b/docs/architecture/library-selection.md new file mode 100644 index 00000000..a6a16464 --- /dev/null +++ b/docs/architecture/library-selection.md @@ -0,0 +1,150 @@ +# Library selection (LDF-style) + +> Status: foundation phases (0–3 + Phase 5 framework_libs delegation) landed +> in PR #207. Phase 4 (zccache memoization) tracked at zackees/zccache#130. +> Phase 6 acceptance gates and Phase 7 perf gates are follow-ups in #205. + +## Why + +PlatformIO's LDF picks the right libraries for a sketch but is slow (Python ++ single-threaded + SCons graph overhead). fbuild's earlier basename-matching +helper produced wrong selections under #202 (STM32 SPI not auto-discovered) +and #204 (teensyLC RAM overflow from FNET / Snooze / RadioHead / mbedtls +being wrongly compiled). This subsystem replaces that helper with a +PlatformIO-LDF-faithful, Rust-native, deterministic resolver that orchestrators +call transparently through `fbuild-build::framework_libs`. + +## What + +Three crates form the subsystem: + +- `fbuild-header-scan::scan` — line-oriented C/C++ tokenizer that emits + `IncludeRef` per `#include`. Pure function, no I/O. Tracks comment, + string-literal, raw-string, and char-literal state. Both branches of + `#if` / `#ifdef` are scanned (false positives are acceptable, false + negatives are not). +- `fbuild-header-scan::walk` — BFS over the include graph. Quoted-first + resolution for `"..."`, ordered search-path lookup for `<...>`. Visited + set guards cycles. Output is canonicalized and sorted for deterministic + cache keys. +- `fbuild-library-select::resolve` — PlatformIO-LDF-style two-pass walk: + 1. From project seeds, BFS marks libs whose `include_dirs` contain a + reached path (path-prefix attribution). + 2. Reconciliation: for each selected lib, re-walk seeded with its full + source set; libs newly reached in pass 2 are also marked. + Output `Selection` is sorted-by-name and deduplicated. + +`fbuild-build::framework_libs` is the integration glue — orchestrators +(`teensy/orchestrator.rs`, `stm32/orchestrator.rs`, ...) call +`resolve_framework_library_sources` transparently with no orchestrator-side +code changes. + +## Sequence + +```text +project sources framework libraries +(src/, lib/, include/) (e.g. Arduino_Core_STM32/libraries/*) + │ │ + │ collect_project_seeds │ FrameworkLibrary { name, + ▼ │ include_dirs, source_files } + seeds: Vec │ + │ │ + └────────────┬─────────────────┘ + ▼ + fbuild-library-select::resolve + ├─ pass 1: walk(seeds, project + lib include dirs) + │ └─ for each reached path: + │ attribute by include_dirs prefix → mark lib + ├─ pass 2: for each marked lib, walk(lib.source_files, ...) + │ └─ newly reached paths attribute new libs + └─ Selection { included_files, required_libraries, + source_files, include_dirs, unresolved } + │ + ▼ + fbuild-build::framework_libs + flatten Selection.source_files → Vec + │ + ▼ + orchestrator compile list +``` + +## Why path-prefix attribution + +PlatformIO LDF's `search_deps_recursive` (piolib.py:428) attributes +resolved paths to libs by *include_dirs prefix*, not basename. fbuild does +the same. Concrete consequences: + +- A project including `"foo/config.h"` will not pull in a `Bar` library + whose `bar/config.h` shares a basename. (Closed: misattribution risk.) +- A library is selected only when the walker actually resolves an include + *into* its `include_dirs`. (#204: FNET / Snooze / RadioHead / mbedtls + no longer pulled in for a Blink sketch on teensyLC.) +- STM32 `SPI.h` resolves through `Arduino_Core_STM32/libraries/SPI/src/` + and prefix-attributes to the SPI library — no manual allowlist needed + (#202). + +## Why two-pass (not fixed-point) + +PlatformIO `chain` mode runs BFS from project sources, then ONE +reconciliation pass that re-seeds with each dependent lib's full source set +(piolib.py:1156). Unconverged deps drop silently (L1164–L1167). The +original issue framing ("fixed-point over include closure — typically 2–3 +iterations") was wrong; we match PIO's 2-pass semantics exactly so users +who flip between PlatformIO and fbuild see the same library set. + +## Cache key (Phase 4, not yet shipped) + +The resolver output is a pure function of: + +- sorted blake3s of project source content, +- sorted blake3s of each lib's canonical headers + `library.json` / + `library.properties`, +- ordered search-path list, +- toolchain triple, +- framework install path + version identifier, +- `SCANNER_VERSION` (bumped on tokenizer changes), +- `LDF_MODE_VERSION` (bumped on resolver semantic changes). + +Memoization is gated on the K/V proposal at zackees/zccache#130 +(`tasks/zccache-kv-design.md`). The resolver is already deterministic and +sort-stable, so cache wiring is a pure addition with no behavior change. + +## Determinism + +Walker output is sorted (`BTreeSet` → `Vec`). Resolver output is sorted by +lib name and deduplicated, and `included_files`, `source_files`, and +`include_dirs` are all sorted-and-deduped before return. Same inputs +produce byte-equal `Selection` output, which is what makes Phase 4 cache +keys safe. + +## Tests + +- 34 scanner tests (`crates/fbuild-header-scan/src/scanner.rs`) covering + S-01..S-32 plus panic-safety guards for unterminated comments and strings. +- Walker tests in `walker.rs` (W-01..W-20: resolution order, cycle and + diamond termination, deterministic output ordering, unresolved-include + reporting). +- Resolver tests in `crates/fbuild-library-select/src/lib.rs` including + the #204 regression guard, sort-stability, missing-include-dir + tolerance, and same-basename-different-library disambiguation. +- Acceptance tests for AC#1 (teensyLC), AC#4 (stm32 SPI auto-discovery) + land in Phase 6 via `fbuild-test-support`'s `MiniFramework`, + `ElfProbe`, and `CompileDb` utilities. + +## Future work + +- **Phase 4** — zccache K/V memoization. Gated on zackees/zccache#130 + shipping a versioned `KvStore` API and a 1.4.0 release; see + `tasks/zccache-kv-design.md`. +- **Phase 6** — wire ELF + compile-DB probes through `fbuild-test-support` + into per-board acceptance tests, gating CI on AC#1..#4 from #205. +- **Phase 7** — perf gates wired into `bench/fastled-examples`. +- **Phase 8** — `fbuild lib-select --explain` CLI subcommand and final + deletion of `framework_libs.rs` helpers. + +## References + +- PlatformIO LDF source: `platformio/builder/tools/piolib.py`. +- Issue: FastLED/fbuild#205. +- Closes: FastLED/fbuild#202, FastLED/fbuild#204. +- Cache prerequisite: zackees/zccache#130. diff --git a/tasks/lessons.md b/tasks/lessons.md index d4cecdd9..139d1040 100644 --- a/tasks/lessons.md +++ b/tasks/lessons.md @@ -17,3 +17,11 @@ **Pattern**: PyO3 0.22+ requires explicit `#[pyo3(signature = (...))]` on `__exit__` methods with `Option` parameters. Without it, clippy reports deprecated implicit defaults. **Fix**: Add `#[pyo3(signature = (_exc_type=None, _exc_val=None, _exc_tb=None))]`. + +## 2026-04-25: Match Upstream Semantics, Don't Re-derive Them (#205) + +**Problem**: The original #205 plan called for "fixed-point over include closure (typically 2–3 iterations)" for library selection. PlatformIO LDF is not a fixed-point — it's BFS + ONE reconciliation pass (piolib.py:1156), with unconverged deps dropping silently. + +**Lesson**: When replicating an upstream tool's behavior (PlatformIO, Arduino-CLI, etc.), read the source first and match its semantics exactly. Users who flip between fbuild and the upstream tool should see byte-identical output, not a "more correct" reinterpretation. + +**Bonus**: Path-prefix attribution beats basename matching for library resolution. A library is selected only if the walker resolves an include *into* its `include_dirs`, not because some header shares a basename. Closes #202 and #204.