diff --git a/.clud/settings.json b/.clud/settings.json index fe20ecf61..2bded3ca2 100644 --- a/.clud/settings.json +++ b/.clud/settings.json @@ -2,7 +2,6 @@ "optimize": { "rust": { "install_soldr": true, - "soldr_version": "0.7.11", "use_soldr_shims": true } } diff --git a/.github/workflows/README.md b/.github/workflows/README.md index a214d08d7..6c5af3b69 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -51,6 +51,7 @@ block fails CI with a copy-paste fix. ## Scheduled Benchmarks - **`benchmark-build-comparison.yml`** -- Arduino CLI vs PlatformIO vs fbuild Blink cold/warm benchmark; runs nightly, manually, and for relevant pushes to `main`, then force-publishes the one-commit `benchmark-stats` branch and deploys its site to GitHub Pages +- **`esp32s3-size-parity.yml`** -- builds an ESP32-S3 Blink with PlatformIO and fbuild on the same pinned pioarduino platform and fails if fbuild's `firmware.bin` is larger (#1432); runs once a day and manually, never on push/PR ## Per-Board Builds (push/PR) diff --git a/.github/workflows/esp32s3-size-parity.yml b/.github/workflows/esp32s3-size-parity.yml new file mode 100644 index 000000000..8480299e5 --- /dev/null +++ b/.github/workflows/esp32s3-size-parity.yml @@ -0,0 +1,55 @@ +name: ESP32-S3 size parity (#1432) + +on: + # Two cold ESP32 builds (PlatformIO + fbuild) per run, so this is a smoke + # test that runs at most once a day, plus on demand. No push/PR trigger. + workflow_dispatch: + schedule: + - cron: '41 8 * * *' + +# Non-PR events key on run_id so each gets its own group -- a shared group +# keeps only ONE pending run, which would silently drop a queued dispatch. +concurrency: + group: esp32s3-size-parity.yml-${{ github.run_id }} + cancel-in-progress: false + +permissions: + contents: read + +env: + CARGO_TERM_COLOR: always + RUSTFLAGS: "-D warnings" + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" + PLATFORMIO_CORE_VERSION: "6.1.19" + +jobs: + size-parity: + name: fbuild <= PlatformIO (esp32s3 Blink) + runs-on: ubuntu-24.04 + timeout-minutes: 60 + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + + - uses: astral-sh/setup-uv@v3 + + - name: Setup soldr + uses: zackees/setup-soldr@v0 + with: + version: 0.8.23 + cache: true + build-cache: true + target-cache: true + prebuild-deps: none + linker: platform-default + cache-payload-warn-bytes: 2GiB + + # pioarduino's platform refuses Python 3.14+ ("Python version must be + # between 3.10 and 3.13"), so pin the interpreter PlatformIO runs on. + - name: Install PlatformIO + run: uv tool install --python 3.13 "platformio==${PLATFORMIO_CORE_VERSION}" + + - name: Compare ESP32-S3 Blink firmware size + run: | + soldr cargo test -p fbuild-build --test esp32s3_size_parity -- --ignored --nocapture --test-threads=1 diff --git a/Cargo.lock b/Cargo.lock index d38a183c6..bd0f719bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1229,7 +1229,6 @@ dependencies = [ "fbuild-paths", "fbuild-serial", "futures", - "libc", "owo-colors", "regex", "reqwest", diff --git a/ci/platform_boundary_research.tsv b/ci/platform_boundary_research.tsv index bf666dc4e..f53f19465 100644 --- a/ci/platform_boundary_research.tsv +++ b/ci/platform_boundary_research.tsv @@ -6,6 +6,9 @@ crates/fbuild-core/Cargo.toml 56 native_dependency libc fs host_mechanic crates/fbuild-core/Cargo.toml 58 target_dependency_table [target.'cfg(windows)'.dependencies] fs host_mechanic crates/fbuild-core/Cargo.toml 66 native_dependency windows-sys fs host_mechanic crates/fbuild-core/src/platform/executable.rs 56 native_path std::env::current_exe host_executable host_mechanic +crates/fbuild-core/src/platform/linux/device.rs 33 native_path std::os::unix::fs::OpenOptionsExt fs host_mechanic +crates/fbuild-core/src/platform/linux/device.rs 41 native_path libc:: process host_mechanic +crates/fbuild-core/src/platform/linux/device.rs 41 native_path libc:: process host_mechanic crates/fbuild-core/src/platform/linux/fs.rs 2 native_path std::os::unix::fs::PermissionsExt fs host_mechanic crates/fbuild-core/src/platform/linux/fs.rs 40 native_path std::os::unix::fs::symlink fs host_mechanic crates/fbuild-core/src/platform/linux/fs.rs 84 native_path std::os::unix::ffi::OsStrExt process host_mechanic diff --git a/crates/fbuild-build-engine/src/linker.rs b/crates/fbuild-build-engine/src/linker.rs index 763b1cf15..f26e53bec 100644 --- a/crates/fbuild-build-engine/src/linker.rs +++ b/crates/fbuild-build-engine/src/linker.rs @@ -107,6 +107,21 @@ fn elf_is_up_to_date<'a>(elf_path: &Path, inputs: impl Iterator PathBuf { + let mut name = archive.as_os_str().to_owned(); + name.push(".inputs"); + PathBuf::from(name) +} + +fn archive_manifest(objects: &[PathBuf]) -> String { + objects + .iter() + .map(|o| o.to_string_lossy()) + .collect::>() + .join("\n") +} + /// Trait for platform-specific linkers. /// /// FastLED/fbuild#820 (Phase B of #813): every method that invokes a @@ -413,6 +428,36 @@ impl LinkerBase { Ok(()) } + /// Whether `archive` already holds exactly `objects` and no object is newer. + /// + /// The object list is compared through a sidecar manifest: an mtime check + /// alone would keep members of objects that no longer exist, and a stale + /// member can still satisfy a symbol at link time. + pub fn archive_is_current(archive: &Path, objects: &[PathBuf]) -> bool { + match std::fs::read_to_string(archive_manifest_path(archive)) { + Ok(manifest) => { + manifest == archive_manifest(objects) && elf_is_up_to_date(archive, objects.iter()) + } + Err(_) => false, + } + } + + /// Rebuild `archive` from `objects` unless [`Self::archive_is_current`] + /// holds, so an unchanged archive keeps its mtime and the link can skip. + pub async fn archive_if_stale( + ar_path: &Path, + objects: &[PathBuf], + archive: &Path, + tool_label: &str, + ) -> Result<()> { + if Self::archive_is_current(archive, objects) { + return Ok(()); + } + Self::archive(ar_path, objects, archive, tool_label).await?; + std::fs::write(archive_manifest_path(archive), archive_manifest(objects))?; + Ok(()) + } + /// Report firmware size by running the size tool and parsing its output. pub async fn report_size( size_path: &Path, @@ -590,6 +635,51 @@ impl LinkerBase { mod tests { use super::*; + fn set_mtime(path: &Path, time: std::time::SystemTime) { + std::fs::File::options() + .write(true) + .open(path) + .unwrap() + .set_modified(time) + .unwrap(); + } + + #[test] + fn archive_is_current_requires_the_same_objects_and_no_newer_object() { + let tmp = tempfile::TempDir::new().unwrap(); + let a = tmp.path().join("a.o"); + let b = tmp.path().join("b.o"); + std::fs::write(&a, b"a").unwrap(); + std::fs::write(&b, b"b").unwrap(); + let archive = tmp.path().join("libcore.a"); + let objects = vec![a.clone(), b.clone()]; + assert!( + !LinkerBase::archive_is_current(&archive, &objects), + "no archive yet" + ); + + // What `archive_if_stale` leaves behind, without running `ar`. + std::fs::write(&archive, b"!\n").unwrap(); + std::fs::write(archive_manifest_path(&archive), archive_manifest(&objects)).unwrap(); + let past = std::time::SystemTime::now() - std::time::Duration::from_secs(60); + set_mtime(&a, past); + set_mtime(&b, past); + assert!(LinkerBase::archive_is_current(&archive, &objects)); + + assert!( + !LinkerBase::archive_is_current(&archive, std::slice::from_ref(&a)), + "an object left the build, so its member is stale" + ); + set_mtime( + &b, + std::time::SystemTime::now() + std::time::Duration::from_secs(60), + ); + assert!( + !LinkerBase::archive_is_current(&archive, &objects), + "an object was rebuilt after the archive" + ); + } + /// Absolute path for the running platform (`/x` is *not* absolute on /// Windows — it has a root but no drive prefix). fn abs(tail: &str) -> PathBuf { diff --git a/crates/fbuild-build-engine/src/package_override.rs b/crates/fbuild-build-engine/src/package_override.rs index 9bc0c99a0..8c45d67f9 100644 --- a/crates/fbuild-build-engine/src/package_override.rs +++ b/crates/fbuild-build-engine/src/package_override.rs @@ -33,6 +33,19 @@ pub fn resolve_override( fbuild_config::parse_platform_packages_value(raw, package_name) } +/// Look up the pin for a platform package such as `platform-espressif32`. +/// +/// A `platform_packages` entry wins, as in PlatformIO. Otherwise a +/// `platform = ` pin applies: ignoring it silently built against a +/// different framework release than the one the ini named (FastLED/fbuild#1432). +pub fn resolve_platform_override( + env_config: &HashMap, + package_name: &str, +) -> Option { + resolve_override(env_config, package_name) + .or_else(|| fbuild_config::parse_platform_archive_url(env_config.get("platform")?)) +} + #[cfg(test)] mod tests { use super::*; @@ -82,6 +95,31 @@ mod tests { assert_eq!(ovr.version, "0.0.0+gdeadbee"); } + const PLATFORM_54: &str = "https://github.com/pioarduino/platform-espressif32/releases/download/54.03.20/platform-espressif32.zip"; + const PLATFORM_55: &str = "https://github.com/pioarduino/platform-espressif32/releases/download/55.03.35/platform-espressif32.zip"; + + #[test] + fn platform_archive_url_pins_the_platform_package() { + let env = env(&[("platform", PLATFORM_54)]); + let ovr = resolve_platform_override(&env, "platform-espressif32").expect("pin honored"); + assert_eq!(ovr.url, PLATFORM_54); + assert_eq!(ovr.version, "54.03.20"); + } + + #[test] + fn platform_packages_entry_wins_over_platform_url() { + let packages = format!("platform-espressif32@{PLATFORM_55}"); + let env = env(&[("platform", PLATFORM_54), ("platform_packages", &packages)]); + let ovr = resolve_platform_override(&env, "platform-espressif32").unwrap(); + assert_eq!(ovr.url, PLATFORM_55); + } + + #[test] + fn unpinned_platform_name_resolves_no_override() { + let env = env(&[("platform", "espressif32")]); + assert!(resolve_platform_override(&env, "platform-espressif32").is_none()); + } + #[test] fn version_pin_only_returns_none() { // `name @ 1.2.3` is a registry version pin, not a URL override. diff --git a/crates/fbuild-build-esp/src/esp32/configs/esp32.json b/crates/fbuild-build-esp/src/esp32/configs/esp32.json index 64abfa184..95ed099f4 100644 --- a/crates/fbuild-build-esp/src/esp32/configs/esp32.json +++ b/crates/fbuild-build-esp/src/esp32/configs/esp32.json @@ -125,11 +125,11 @@ "profiles": { "release": { - "compile_flags": ["-Os", "-flto=auto", "-fno-fat-lto-objects", "-fno-omit-frame-pointer"], + "compile_flags": ["-Os", "-flto=auto", "-fno-fat-lto-objects"], "link_flags": ["-flto=auto", "-fuse-linker-plugin"] }, "quick": { - "compile_flags": ["-Os", "-g0", "-fno-omit-frame-pointer"], + "compile_flags": ["-Os", "-g0"], "link_flags": [] } }, diff --git a/crates/fbuild-build-esp/src/esp32/configs/esp32s2.json b/crates/fbuild-build-esp/src/esp32/configs/esp32s2.json index 3c59fd2a8..1a1fb9e84 100644 --- a/crates/fbuild-build-esp/src/esp32/configs/esp32s2.json +++ b/crates/fbuild-build-esp/src/esp32/configs/esp32s2.json @@ -120,11 +120,11 @@ "profiles": { "release": { - "compile_flags": ["-Os", "-flto=auto", "-fno-fat-lto-objects", "-fno-omit-frame-pointer"], + "compile_flags": ["-Os", "-flto=auto", "-fno-fat-lto-objects"], "link_flags": ["-flto=auto", "-fuse-linker-plugin"] }, "quick": { - "compile_flags": ["-Os", "-g0", "-fno-omit-frame-pointer"], + "compile_flags": ["-Os", "-g0"], "link_flags": [] } }, diff --git a/crates/fbuild-build-esp/src/esp32/configs/esp32s3.json b/crates/fbuild-build-esp/src/esp32/configs/esp32s3.json index 8873467d6..61f56127f 100644 --- a/crates/fbuild-build-esp/src/esp32/configs/esp32s3.json +++ b/crates/fbuild-build-esp/src/esp32/configs/esp32s3.json @@ -126,11 +126,11 @@ "profiles": { "release": { - "compile_flags": ["-Os", "-flto=auto", "-fno-fat-lto-objects", "-fno-omit-frame-pointer"], + "compile_flags": ["-Os", "-flto=auto", "-fno-fat-lto-objects"], "link_flags": ["-flto=auto", "-fuse-linker-plugin"] }, "quick": { - "compile_flags": ["-Os", "-g0", "-fno-omit-frame-pointer"], + "compile_flags": ["-Os", "-g0"], "link_flags": [] } }, diff --git a/crates/fbuild-build-esp/src/esp32/esp32_compiler.rs b/crates/fbuild-build-esp/src/esp32/esp32_compiler.rs index 6bce57020..2618bb740 100644 --- a/crates/fbuild-build-esp/src/esp32/esp32_compiler.rs +++ b/crates/fbuild-build-esp/src/esp32/esp32_compiler.rs @@ -266,6 +266,10 @@ mod tests { use crate::esp32::mcu_config::get_mcu_config; fn test_compiler(mcu: &str) -> Esp32Compiler { + test_compiler_with_profile(mcu, BuildProfile::Release) + } + + fn test_compiler_with_profile(mcu: &str, profile: BuildProfile) -> Esp32Compiler { let config = get_mcu_config(mcu).unwrap(); let mut defines = config.defines_map(); defines.insert("PLATFORMIO".to_string(), "1".to_string()); @@ -279,7 +283,7 @@ mod tests { "160000000L", defines, vec![PathBuf::from("/framework/cores/esp32")], - BuildProfile::Release, + profile, false, ) } @@ -320,6 +324,25 @@ mod tests { assert!(!flags.iter().any(|f| f.starts_with("-march="))); } + #[test] + fn xtensa_profiles_omit_the_frame_pointer() { + // Xtensa backtraces unwind through the windowed ABI, so a frame pointer + // gives the crash decoder nothing and costs flash in every compiled + // function. PlatformIO does not pass it (FastLED/fbuild#1432). + for mcu in ["esp32", "esp32s2", "esp32s3"] { + for profile in [BuildProfile::Release, BuildProfile::Quick] { + let compiler = test_compiler_with_profile(mcu, profile); + for flags in [compiler.c_flags(), compiler.cpp_flags()] { + assert!( + !flags.contains(&"-fno-omit-frame-pointer".to_string()), + "{mcu} {} profile keeps -fno-omit-frame-pointer", + profile.as_dir_name() + ); + } + } + } + } + #[test] fn test_defines_in_flags() { let compiler = test_compiler("esp32c6"); diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs index 900c45ea5..dc6d2994a 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs @@ -17,7 +17,7 @@ use super::cdc::warn_if_cdc_on_boot; use super::embed_stage::stage_embed_files; use super::fingerprint::Esp32FingerprintMetadata; use super::framework_libs::compile_framework_builtin_libs; -use super::helpers::{compile_db_is_current, profile_label}; +use super::helpers::{compile_db_is_current, framework_macro_prefix_map, profile_label}; use super::local_libs::compile_local_libraries; use super::packages::resolve_pioarduino_packages; @@ -304,6 +304,8 @@ impl BuildOrchestrator for Esp32Orchestrator { // Read user build_flags early — needed for both library and sketch compilation. // SDK defines (from flags/defines) are prepended so user flags can override them. let mut user_flags = sdk_defines; + // Before the user's build_flags, so their own prefix maps still win. + user_flags.extend(framework_macro_prefix_map(&core_dir)); let mut user_build_flags = ctx.config.get_build_flags(¶ms.env_name)?; user_build_flags.extend(params.extra_build_flags.clone()); user_flags.extend(user_build_flags.clone()); @@ -791,8 +793,31 @@ impl BuildOrchestrator for Esp32Orchestrator { }; // 12-13. Link + convert - // Library archives join core_objects in the archives parameter - let mut all_archives: Vec = core_objects; + // The core reaches the linker as an archive, like PlatformIO's + // libFrameworkArduino.a. Loose objects are always linked whole, which + // kept every core file's constructors and the SDK code they reach: + // about 11 KB on an ESP32-S3 Blink (FastLED/fbuild#1432). + let mut all_archives: Vec = Vec::new(); + if !core_objects.is_empty() { + let _g = perf.phase("archive-core"); + let core_archive = core_build_dir.join("libFrameworkArduino.a"); + let ar_path = toolchain.get_ar_path(); + let gcc_ar_path = toolchain.get_gcc_ar_path(); + let archiver = crate::pipeline::pick_archiver( + &ar_path, + &gcc_ar_path, + &compiler.c_flags(), + &compiler.cpp_flags(), + ); + crate::linker::LinkerBase::archive_if_stale( + archiver, + &core_objects, + &core_archive, + "ar", + ) + .await?; + all_archives.push(core_archive); + } all_archives.extend(library_archives); let linker = Esp32Linker::new( diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/helpers.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/helpers.rs index 1bfb09fb7..ecda4830b 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/helpers.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/helpers.rs @@ -87,3 +87,43 @@ pub(super) fn compile_db_is_current(build_dir: &Path, project_dir: &Path) -> boo } crate::compile_database::CompileDatabase::expected_output_path(build_dir, project_dir).exists() } + +/// `-fmacro-prefix-map` that shortens the framework's install root in `__FILE__`. +/// +/// The core's log macros embed `__FILE__` in flash, so fbuild's long cache path +/// cost bytes PlatformIO's shorter package path did not (FastLED/fbuild#1432). +/// Only macros are remapped: debug info keeps absolute paths for addr2line. +/// `core_dir` is the framework's `cores/` directory. +pub(super) fn framework_macro_prefix_map(core_dir: &Path) -> Option { + let root = core_dir.parent()?.parent()?; + Some(format!( + "-fmacro-prefix-map={}=framework-arduinoespressif32", + root.display() + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn macro_prefix_map_names_the_framework_root() { + let core_dir = Path::new("/cache/framework-arduinoespressif32/abc/3.3.5/esp32-3.3.5") + .join("cores") + .join("esp32"); + let flag = framework_macro_prefix_map(&core_dir).unwrap(); + let root = core_dir.parent().unwrap().parent().unwrap(); + assert_eq!( + flag, + format!( + "-fmacro-prefix-map={}=framework-arduinoespressif32", + root.display() + ) + ); + } + + #[test] + fn macro_prefix_map_needs_a_cores_parent() { + assert_eq!(framework_macro_prefix_map(Path::new("esp32")), None); + } +} diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/packages.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/packages.rs index 0250bbe6e..6188c13d0 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/packages.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/packages.rs @@ -27,15 +27,19 @@ pub(super) async fn resolve_pioarduino_packages( Option, )> { // Ensure pioarduino platform (contains platform.json with metadata URLs). - // Honor `platform_packages = platform-espressif32@#` from the env - // section (FastLED/fbuild#672): if set, the override URL replaces the - // const-pinned default and gets its own cache subdir via - // `PackageBase::with_override`. - let platform_ovr = env_config - .and_then(|env| crate::package_override::resolve_override(env, "platform-espressif32")); + // Honor `platform_packages = platform-espressif32@#` + // (FastLED/fbuild#672), then `platform = ` + // (FastLED/fbuild#1432): the pin replaces the const-pinned default and gets + // its own cache subdir via `PackageBase::with_override`. + let platform_ovr = env_config.and_then(|env| { + crate::package_override::resolve_platform_override(env, "platform-espressif32") + }); let platform = match platform_ovr { Some(o) => fbuild_packages::library::Esp32Platform::with_override(project_dir, o), - None => fbuild_packages::library::Esp32Platform::new(project_dir), + None => { + warn_unhonored_platform_pin(env_config); + fbuild_packages::library::Esp32Platform::new(project_dir) + } }; fbuild_packages::Package::ensure_installed(&platform).await?; @@ -140,6 +144,23 @@ pub(super) async fn resolve_pioarduino_packages( Ok((toolchain, framework, esptool_py)) } +/// Name a `platform` pin fbuild cannot honor instead of dropping it silently +/// (FastLED/fbuild#1407). Registry pins and git URLs fall back to the +/// pioarduino stable platform, which carries a different framework release. +fn warn_unhonored_platform_pin(env_config: Option<&HashMap>) { + let Some(value) = env_config.and_then(|env| env.get("platform")) else { + return; + }; + let value = value.trim(); + if value.contains('@') || value.contains("://") { + tracing::warn!( + "platform pin `{value}` is not a downloadable archive URL; building with the \ + pioarduino stable platform instead. Pin a release with `platform = \ + https://github.com/pioarduino/platform-espressif32/releases/download//platform-espressif32.zip`" + ); + } +} + /// Provision the managed `tool-esptoolpy` package (the tasmota PyInstaller /// standalone binary) from `platform.json` and return the path to the /// `esptool` executable. diff --git a/crates/fbuild-build/tests/README.md b/crates/fbuild-build/tests/README.md index cf85f9f65..b6e3698d8 100644 --- a/crates/fbuild-build/tests/README.md +++ b/crates/fbuild-build/tests/README.md @@ -3,3 +3,5 @@ Tests that download real toolchains and compile real sketches. Marked `#[ignore]` so they don't run during normal `uv run test`. Run with: `soldr cargo test -p fbuild-build -- --ignored` + +`esp32s3_size_parity.rs` also needs PlatformIO (`pio`, or `FBUILD_PARITY_PIO`). It asserts that fbuild's ESP32-S3 Blink `firmware.bin` is no larger than PlatformIO's build of the same pinned platform (FastLED/fbuild#1432), and runs daily via `.github/workflows/esp32s3-size-parity.yml`. Locally, stop any running fbuild daemon first (`fbuild daemon stop`): the test starts its own compile backend, which needs the zccache cache root a live daemon holds. diff --git a/crates/fbuild-build/tests/esp32s3_size_parity.rs b/crates/fbuild-build/tests/esp32s3_size_parity.rs new file mode 100644 index 000000000..50b9a0210 --- /dev/null +++ b/crates/fbuild-build/tests/esp32s3_size_parity.rs @@ -0,0 +1,186 @@ +//! Size-parity smoke test for FastLED/fbuild#1432: an ESP32-S3 Blink built by +//! fbuild must not produce a larger flash image than PlatformIO builds from the +//! same `platformio.ini`. +//! +//! Both builds pin the same pioarduino platform release, so the comparison +//! measures fbuild's build pipeline rather than a framework version change. +//! +//! `#[ignore]`-marked because it needs PlatformIO (`pio` on PATH, or the path +//! in `FBUILD_PARITY_PIO`) and downloads the ESP32 toolchain and framework for +//! both tools. `.github/workflows/esp32s3-size-parity.yml` runs it once a day: +//! +//! ```text +//! soldr cargo test -p fbuild-build --test esp32s3_size_parity -- --ignored --nocapture +//! ``` + +use std::fs; +use std::path::Path; + +use fbuild_build::{BuildOrchestrator, BuildParams, compile_backend}; +use fbuild_core::BuildProfile; +use fbuild_core::path::NormalizedPath; + +const ENV_NAME: &str = "esp32s3"; + +/// The pioarduino release both builds pin (FastLED's `generic-esp` pin). +const PLATFORM_URL: &str = "https://github.com/pioarduino/platform-espressif32/releases/download/55.03.35/platform-espressif32.zip"; + +/// 15-min wall-clock cap for `--ignored` real-toolchain tests (FastLED/fbuild#806). +const REAL_BUILD_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(900); + +/// The orchestrator compiles through the process-wide compile backend +/// (FastLED/fbuild#800), which only the daemon wires at startup. +async fn install_test_compile_backend() { + static INSTALL: tokio::sync::OnceCell<()> = tokio::sync::OnceCell::const_new(); + INSTALL + .get_or_init(|| async { + let backend = compile_backend::CompileBackend::start() + .await + .expect("compile backend starts for size parity test"); + compile_backend::install_global(backend); + }) + .await; +} + +fn write_blink_project(project_dir: &Path) { + fs::write( + project_dir.join("platformio.ini"), + format!( + "[env:{ENV_NAME}]\nplatform = {PLATFORM_URL}\nboard = esp32-s3-devkitc-1\nframework = arduino\n" + ), + ) + .unwrap(); + + let src_dir = project_dir.join("src"); + fs::create_dir_all(&src_dir).unwrap(); + fs::write( + src_dir.join("main.cpp"), + "\ +#include + +void setup() { + pinMode(2, OUTPUT); +} + +void loop() { + digitalWrite(2, HIGH); + delay(1000); + digitalWrite(2, LOW); + delay(1000); +} +", + ) + .unwrap(); +} + +/// Build with PlatformIO and return the directory holding its artifacts. +async fn build_with_platformio(project_dir: &Path) -> NormalizedPath { + let pio = std::env::var("FBUILD_PARITY_PIO").unwrap_or_else(|_| "pio".to_string()); + let project = project_dir.to_str().expect("temp project path is UTF-8"); + // Same budget as the fbuild build: a stalled PlatformIO build must not + // hold the job past it (FastLED/fbuild#806). + let output = fbuild_core::subprocess::run_command( + &[pio.as_str(), "run", "-e", ENV_NAME, "-d", project], + None, + None, + Some(REAL_BUILD_TIMEOUT), + ) + .await + .unwrap_or_else(|e| panic!("failed to run PlatformIO ({pio}): {e}")); + assert!( + output.success(), + "PlatformIO build failed (exit {}):\n{}\n{}", + output.exit_code, + output.stdout, + output.stderr + ); + NormalizedPath::from(project_dir.join(".pio/build").join(ENV_NAME)) +} + +/// Build with fbuild's ESP32 orchestrator and return its artifact directory. +async fn build_with_fbuild(project_dir: &Path) -> NormalizedPath { + let build_dir = project_dir.join(format!( + "{}/{}/{ENV_NAME}/release", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + )); + let params = BuildParams { + project_dir: project_dir.to_path_buf(), + env_name: ENV_NAME.to_string(), + clean_all: false, + clean_only: false, + clean: true, + profile: BuildProfile::Release, + build_dir: build_dir.clone(), + verbose: false, + jobs: None, + generate_compiledb: false, + 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, + bloat_analysis: false, + caller_path: None, + }; + let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; + let result = tokio::time::timeout(REAL_BUILD_TIMEOUT, orchestrator.build(¶ms)) + .await + .expect("fbuild build exceeded the real-toolchain budget (FastLED/fbuild#806)") + .expect("fbuild build should succeed"); + assert!(result.success, "fbuild build should report success"); + NormalizedPath::from(build_dir) +} + +/// Non-debug ELF sections with their sizes, for a readable failure report. +fn section_sizes(elf: &Path) -> Vec<(String, u64)> { + use object::{Object, ObjectSection}; + let bytes = fs::read(elf).unwrap_or_else(|e| panic!("read {}: {e}", elf.display())); + let file = object::File::parse(&*bytes).expect("parse ELF"); + file.sections() + .filter(|s| s.size() > 0) + .filter_map(|s| { + let name = s.name().ok()?; + (!name.starts_with(".debug") && !name.is_empty()).then(|| (name.to_string(), s.size())) + }) + .collect() +} + +fn report(label: &str, artifacts: &Path) -> u64 { + let bin = artifacts.join("firmware.bin"); + let size = fs::metadata(&bin) + .unwrap_or_else(|e| panic!("{label} firmware.bin missing at {}: {e}", bin.display())) + .len(); + println!("{label}: firmware.bin = {size} B"); + for (name, bytes) in section_sizes(&artifacts.join("firmware.elf")) { + println!(" {name:<24} {bytes:>10}"); + } + size +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +#[ignore = "requires PlatformIO and downloads the ESP32 toolchain (~hundreds of MB) twice"] +async fn esp32s3_blink_is_no_larger_than_platformio() { + install_test_compile_backend().await; + // Separate project dirs so neither tool sees the other's build output. + let pio_tmp = tempfile::TempDir::new().unwrap(); + let fbuild_tmp = tempfile::TempDir::new().unwrap(); + write_blink_project(pio_tmp.path()); + write_blink_project(fbuild_tmp.path()); + + let pio_artifacts = build_with_platformio(pio_tmp.path()).await; + let fbuild_artifacts = build_with_fbuild(fbuild_tmp.path()).await; + + let pio_size = report("PlatformIO", &pio_artifacts); + let fbuild_size = report("fbuild", &fbuild_artifacts); + assert!( + fbuild_size <= pio_size, + "fbuild's ESP32-S3 Blink firmware.bin is {} B larger than PlatformIO's \ + (fbuild={fbuild_size} B, PlatformIO={pio_size} B); see the section tables above", + fbuild_size - pio_size + ); +} diff --git a/crates/fbuild-cli/Cargo.toml b/crates/fbuild-cli/Cargo.toml index cb07fe555..1eddbd3ea 100644 --- a/crates/fbuild-cli/Cargo.toml +++ b/crates/fbuild-cli/Cargo.toml @@ -39,9 +39,4 @@ sha2 = { workspace = true } tempfile = { workspace = true } walkdir = { workspace = true } owo-colors = { workspace = true } - -# O_NONBLOCK / O_NOCTTY for the read-only serial probe in `port doctor` -# (FastLED/fbuild#1424). Unix-only: the probe itself is cfg'd to Linux. -[target.'cfg(unix)'.dependencies] -libc = { workspace = true } semver = { workspace = true } diff --git a/crates/fbuild-cli/src/cli/port_doctor.rs b/crates/fbuild-cli/src/cli/port_doctor.rs index 37a20169e..3b59bd696 100644 --- a/crates/fbuild-cli/src/cli/port_doctor.rs +++ b/crates/fbuild-cli/src/cli/port_doctor.rs @@ -80,39 +80,10 @@ pub fn diagnose(port: &DetectedPort, power_rows: &[(String, bool)]) -> PortDiagn } /// Whether the current process can open `port`, or `None` where the question -/// is not meaningful. -/// -/// Linux only. Elsewhere serial access is not group-gated the same way and a -/// speculative open would be a side effect in a command documented as -/// strictly read-only. Opening for read is enough to surface `EACCES` and -/// does not disturb a device: no DTR/RTS assertion, no write. -#[cfg(target_os = "linux")] +/// is not meaningful. The host mechanics live behind the platform facade +/// (see [`fbuild_core::platform::device::probe_serial_openable`]). pub fn probe_openable(port: &str) -> Option { - use std::io::ErrorKind; - use std::os::unix::fs::OpenOptionsExt; - // O_NONBLOCK: without CLOCAL set, a terminal open blocks until carrier - // detect is asserted, which would hang a command documented as a quick - // read-only diagnostic. O_NOCTTY: never let the probe acquire a - // controlling terminal -- signals delivered to that terminal would then - // reach fbuild. Both matter here: an open on a contended port was - // measured at 13.3 s on the bench that motivated FastLED/fbuild#1424. - match std::fs::OpenOptions::new() - .read(true) - .custom_flags(libc::O_NONBLOCK | libc::O_NOCTTY) - .open(port) - { - Ok(_) => Some(true), - Err(e) if e.kind() == ErrorKind::PermissionDenied => Some(false), - // Busy, absent, or anything else is a different question that the - // presence/problem-code verdict already covers. Claiming "not - // openable" here would blame permissions for an unrelated fault. - Err(_) => None, - } -} - -#[cfg(not(target_os = "linux"))] -pub fn probe_openable(_port: &str) -> Option { - None + fbuild_core::platform::device::probe_serial_openable(port) } pub fn verdict(diagnosis: &PortDiagnosis) -> Verdict { diff --git a/crates/fbuild-config/src/lib.rs b/crates/fbuild-config/src/lib.rs index ef72b2fde..da689fda1 100644 --- a/crates/fbuild-config/src/lib.rs +++ b/crates/fbuild-config/src/lib.rs @@ -23,5 +23,6 @@ pub use pio_env::{ scan_warn_only, }; pub use platform_packages::{ - PackageOverride, parse_platform_packages_entry, parse_platform_packages_value, + PackageOverride, parse_platform_archive_url, parse_platform_packages_entry, + parse_platform_packages_value, }; diff --git a/crates/fbuild-config/src/platform_packages.rs b/crates/fbuild-config/src/platform_packages.rs index 2b9971613..1a2a1bc36 100644 --- a/crates/fbuild-config/src/platform_packages.rs +++ b/crates/fbuild-config/src/platform_packages.rs @@ -141,6 +141,26 @@ pub fn parse_platform_packages_value(value: &str, package_name: &str) -> Option< .next() } +/// Archive extensions fbuild can download and unpack. +const ARCHIVE_EXTENSIONS: [&str; 4] = [".zip", ".tar.gz", ".tar.bz2", ".tar.xz"]; + +/// Parse an env's `platform = ` as a pinned, downloadable platform archive. +/// +/// PlatformIO accepts a release archive URL as the `platform` value, and that is +/// how consumers pin a pioarduino release. Only `http(s)` URLs ending in an +/// archive extension qualify, because those are what fbuild can fetch. Registry +/// pins (`espressif32@6.5.0`), bare names and git URLs return `None`. +pub fn parse_platform_archive_url(value: &str) -> Option { + let url = value.trim(); + if !(url.starts_with("http://") || url.starts_with("https://")) { + return None; + } + if !ARCHIVE_EXTENSIONS.iter().any(|ext| url.ends_with(ext)) { + return None; + } + Some(PackageOverride::new(url, version_from_url_tail(url))) +} + fn version_string(sha: &str) -> String { // `0.0.0+g` — keeps the cache-subdir distinct from the default // pin (which uses its own `+g` pattern). The `0.0.0+` base @@ -149,7 +169,25 @@ fn version_string(sha: &str) -> String { format!("0.0.0+g{}", short) } +/// The `` of a GitHub `.../releases/download//` URL, when it +/// is safe to use as a cache path segment. +fn release_tag(url: &str) -> Option<&str> { + let (_, rest) = url.split_once("/releases/download/")?; + let tag = rest.split('/').next()?; + let path_safe = tag + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '_' | '+')); + // `.` and `..` pass the character check but name the cache directory + // itself or its parent. + let names_a_directory = matches!(tag, "" | "." | ".."); + (path_safe && !names_a_directory).then_some(tag) +} + fn version_from_url_tail(url: &str) -> String { + // A release asset URL names its version in the tag segment. + if let Some(tag) = release_tag(url) { + return tag.to_string(); + } // For `https://.../archive/.tar.gz` (no explicit `#sha`), pull the // sha out of the URL tail. Falls back to a generic `override` tag if the // tail isn't a recognizable sha-shaped token. @@ -248,6 +286,56 @@ mod tests { assert!(got.url.contains("ArduinoCore-LPC8xx")); } + #[test] + fn platform_release_zip_url_is_an_archive_pin() { + let url = "https://github.com/pioarduino/platform-espressif32/releases/download/54.03.20/platform-espressif32.zip"; + let got = parse_platform_archive_url(url).unwrap(); + assert_eq!(got.url, url); + assert_eq!(got.version, "54.03.20"); + assert_eq!(got.checksum, None); + } + + #[test] + fn platform_archive_tarball_url_is_an_archive_pin() { + let url = "https://github.com/pioarduino/platform-espressif32/archive/abcdef1234567890abcdef1234567890abcdef12.tar.gz"; + let got = parse_platform_archive_url(url).unwrap(); + assert_eq!(got.url, url); + assert_eq!(got.version, "0.0.0+gabcdef1"); + } + + #[test] + fn platform_values_that_are_not_downloadable_archives_are_not_pins() { + for value in [ + "", + "espressif32", + "espressif32@6.5.0", + "https://github.com/pioarduino/platform-espressif32.git#develop", + "https://github.com/pioarduino/platform-espressif32", + ] { + assert_eq!(parse_platform_archive_url(value), None, "{value:?}"); + } + } + + #[test] + fn release_download_url_in_platform_packages_uses_the_release_tag() { + let line = "platform-espressif32@https://github.com/pioarduino/platform-espressif32/releases/download/55.03.35/platform-espressif32.zip"; + let got = parse_platform_packages_entry(line, "platform-espressif32").unwrap(); + assert_eq!(got.version, "55.03.35"); + } + + #[test] + fn dot_release_tags_are_never_cache_versions() { + // `.` and `..` would name the cache directory itself or its parent, + // which could pass for an installed package and skip the download. + for tag in [".", ".."] { + let url = format!( + "https://github.com/pioarduino/platform-espressif32/releases/download/{tag}/platform-espressif32.zip" + ); + let got = parse_platform_archive_url(&url).unwrap(); + assert_eq!(got.version, "0.0.0+override", "{tag:?}"); + } + } + #[test] fn trailing_comma_or_semicolon_tolerated() { let line = "framework-arduino-lpc8xx@zackees/ArduinoCore-LPC8xx#abc,"; diff --git a/crates/fbuild-core/src/platform/device.rs b/crates/fbuild-core/src/platform/device.rs index ed540f84a..f618b9546 100644 --- a/crates/fbuild-core/src/platform/device.rs +++ b/crates/fbuild-core/src/platform/device.rs @@ -139,6 +139,17 @@ pub fn detect_serial_kernel_driver(port_name: &str) -> Option super::selected::device::detect_serial_kernel_driver(port_name) } +/// Whether the current process may open the serial devnode `port_name`. +/// +/// `Some(false)` means the open was refused with permission denied. `None` +/// means the host does not gate serial access by permissions the same way, or +/// the open failed for a reason other than permissions. Where a probe runs it +/// opens read-only without asserting DTR/RTS, so it never disturbs the device +/// (FastLED/fbuild#1424). +pub fn probe_serial_openable(port_name: &str) -> Option { + super::selected::device::probe_serial_openable(port_name) +} + /// Live sysfs USB topology root (`/sys/bus/usb/devices`-shaped) when the /// host provides one, `None` elsewhere. pub fn live_sysfs_usb_root() -> Option { diff --git a/crates/fbuild-core/src/platform/linux/device.rs b/crates/fbuild-core/src/platform/linux/device.rs index 86a7be0b4..d892021da 100644 --- a/crates/fbuild-core/src/platform/linux/device.rs +++ b/crates/fbuild-core/src/platform/linux/device.rs @@ -25,6 +25,31 @@ pub(crate) fn detect_serial_kernel_driver(port_name: &str) -> Option Option { + use std::os::unix::fs::OpenOptionsExt; + // O_NONBLOCK: without CLOCAL set, a terminal open blocks until carrier + // detect is asserted, which would hang a quick read-only diagnostic. + // O_NOCTTY: never let the probe acquire a controlling terminal -- signals + // delivered to that terminal would then reach fbuild. Both matter: an open + // on a contended port was measured at 13.3 s on the #1424 bench. + match std::fs::OpenOptions::new() + .read(true) + .custom_flags(libc::O_NONBLOCK | libc::O_NOCTTY) + .open(port_name) + { + Ok(_) => Some(true), + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => Some(false), + // Busy, absent, or anything else is a different question that the + // presence/problem-code verdict already covers. Claiming "not + // openable" here would blame permissions for an unrelated fault. + Err(_) => None, + } +} + pub(crate) fn live_sysfs_usb_root() -> Option { let root = Path::new(SYSFS_USB_ROOT); if !root.is_dir() { diff --git a/crates/fbuild-core/src/platform/macos/device.rs b/crates/fbuild-core/src/platform/macos/device.rs index 10ab1d7a8..05235e0b8 100644 --- a/crates/fbuild-core/src/platform/macos/device.rs +++ b/crates/fbuild-core/src/platform/macos/device.rs @@ -24,6 +24,12 @@ pub(crate) fn detect_serial_kernel_driver(port_name: &str) -> Option Option { + // Serial nodes are not group-gated the way Linux dialout nodes are, and a + // speculative open would be a side effect in a read-only diagnostic. + None +} + pub(crate) fn live_sysfs_usb_root() -> Option { None } diff --git a/crates/fbuild-core/src/platform/windows/device.rs b/crates/fbuild-core/src/platform/windows/device.rs index cc453302f..b79158598 100644 --- a/crates/fbuild-core/src/platform/windows/device.rs +++ b/crates/fbuild-core/src/platform/windows/device.rs @@ -120,6 +120,12 @@ pub(crate) fn detect_serial_kernel_driver(_port_name: &str) -> Option Option { + // COM port access is not group-gated the way Linux dialout nodes are, and + // a speculative open would be a side effect in a read-only diagnostic. + None +} + pub(crate) fn live_sysfs_usb_root() -> Option { None } diff --git a/docs/reference/platformio-ini.md b/docs/reference/platformio-ini.md index 29355c9e8..f2fa08caf 100644 --- a/docs/reference/platformio-ini.md +++ b/docs/reference/platformio-ini.md @@ -86,6 +86,21 @@ Supported behavior includes: The library selection design is documented in [`docs/architecture/library-selection.md`](../architecture/library-selection.md). +## ESP32 Platform Pins + +For ESP32 environments fbuild honors a pioarduino release archive as the +`platform` value, as PlatformIO does, and builds against the framework that +release names (FastLED/fbuild#1432): + +```ini +platform = https://github.com/pioarduino/platform-espressif32/releases/download/55.03.35/platform-espressif32.zip +``` + +A `platform_packages = platform-espressif32@` entry takes precedence. A +bare `espressif32` uses the pioarduino `stable` release. Registry pins +(`espressif32@6.5.0`) and git URLs cannot be fetched; fbuild logs a warning and +uses `stable` instead (FastLED/fbuild#1407). + ## ESP QEMU Flash Mode ESP32-family QEMU requires DIO flash mode: