diff --git a/crates/fbuild-build/src/esp32/esp32_linker.rs b/crates/fbuild-build/src/esp32/esp32_linker.rs index 63763ad5..296c7ff6 100644 --- a/crates/fbuild-build/src/esp32/esp32_linker.rs +++ b/crates/fbuild-build/src/esp32/esp32_linker.rs @@ -49,6 +49,46 @@ pub fn f_flash_to_esptool_freq(f_flash: Option<&str>, default_freq: &str) -> Str } } +/// Build the argv for an esptool `elf2image` invocation. +/// +/// When esptool was provisioned, the standalone binary is invoked directly; +/// otherwise it falls back to an `esptool` on PATH. Shared by the firmware +/// conversion path here and the bootloader conversion path in +/// `orchestrator::boot_artifacts` so both honor the same provisioned tool +/// (FastLED/fbuild#954). The `--chip` flag is a global option and therefore +/// precedes the `elf2image` subcommand, matching the esptool v4/v5 CLI. +#[allow(clippy::too_many_arguments)] +pub(crate) fn esptool_elf2image_argv( + esptool_bin: Option<&Path>, + chip: &str, + flash_mode: &str, + flash_freq: &str, + flash_size: &str, + elf: &str, + out_bin: &str, +) -> Vec { + let mut argv: Vec = Vec::new(); + match esptool_bin { + Some(bin) => argv.push(bin.to_string_lossy().to_string()), + None => argv.push("esptool".to_string()), + } + argv.extend([ + "--chip".to_string(), + chip.to_string(), + "elf2image".to_string(), + "--flash-mode".to_string(), + flash_mode.to_string(), + "--flash-freq".to_string(), + flash_freq.to_string(), + "--flash-size".to_string(), + flash_size.to_string(), + elf.to_string(), + "-o".to_string(), + out_bin.to_string(), + ]); + argv +} + /// ESP32-specific linker using RISC-V or Xtensa GCC as the link driver. pub struct Esp32Linker { gcc_path: PathBuf, @@ -72,6 +112,9 @@ pub struct Esp32Linker { flash_freq: String, max_flash: Option, max_ram: Option, + /// Path to the provisioned standalone esptool binary, if available. `None` + /// falls back to an `esptool` on PATH. See FastLED/fbuild#954. + esptool_bin: Option, verbose: bool, } @@ -91,6 +134,7 @@ impl Esp32Linker { flash_freq: &str, max_flash: Option, max_ram: Option, + esptool_bin: Option, verbose: bool, ) -> Self { let flash_mode = flash_mode.unwrap_or_else(|| mcu_config.default_flash_mode().to_string()); @@ -108,6 +152,7 @@ impl Esp32Linker { flash_freq: flash_freq.to_string(), max_flash, max_ram, + esptool_bin, verbose, } } @@ -367,25 +412,22 @@ impl Linker for Esp32Linker { } // Determine flash size from max_flash config (bytes → human-readable). // elf2image doesn't support "detect" — needs an explicit size. - let args = [ - "esptool", - "--chip", + // Prefer the provisioned standalone esptool binary; fall back to an + // `esptool` on PATH (FastLED/fbuild#954). + let argv = esptool_elf2image_argv( + self.esptool_bin.as_deref(), chip, - "elf2image", - "--flash-mode", &self.flash_mode, - "--flash-freq", &self.flash_freq, - "--flash-size", &flash_size, &elf_str, - "-o", &bin_str, - ]; + ); + let args: Vec<&str> = argv.iter().map(|s| s.as_str()).collect(); - tracing::info!("elf2image: {}", args.join(" ")); + tracing::info!("elf2image: {}", argv.join(" ")); - match run_command(&args, None, None, Some(std::time::Duration::from_secs(30))).await { + match run_command(&args, None, None, Some(std::time::Duration::from_secs(60))).await { Ok(result) if result.success() => { let cache = self.current_bin_cache(&elf_out, &flash_size)?; if let Err(e) = save_json(&self.bin_cache_path(output_dir), &cache) { @@ -400,7 +442,8 @@ impl Linker for Esp32Linker { ))), Err(e) => Err(fbuild_core::FbuildError::BuildFailed(format!( "esptool not found — cannot convert firmware.elf to firmware.bin.\n\ - Install with: pip install esptool\nError: {}", + fbuild normally provisions esptool automatically; if provisioning \ + failed, install with: pip install esptool\nError: {}", e ))), } @@ -475,6 +518,7 @@ mod tests { "80m", Some(3145728), Some(327680), + None, false, ) } @@ -504,6 +548,7 @@ mod tests { "80m", Some(4 * 1024 * 1024), Some(327680), + None, false, ); let tmp = tempfile::TempDir::new().unwrap(); @@ -568,6 +613,7 @@ mod tests { "80m", Some(3145728), Some(327680), + None, false, ); let flags = linker.linker_flags(); @@ -611,6 +657,7 @@ mod tests { "80m", Some(3145728), Some(327680), + None, false, ); let flags = linker.linker_flags(); diff --git a/crates/fbuild-build/src/esp32/orchestrator/boot_artifacts.rs b/crates/fbuild-build/src/esp32/orchestrator/boot_artifacts.rs index 2f900494..b5bc1c38 100644 --- a/crates/fbuild-build/src/esp32/orchestrator/boot_artifacts.rs +++ b/crates/fbuild-build/src/esp32/orchestrator/boot_artifacts.rs @@ -13,6 +13,7 @@ use super::super::mcu_config::Esp32McuConfig; /// exception: an explicitly configured `board_build.partitions` CSV that /// resolves nowhere is a hard error (FastLED/fbuild#955) — silently flashing /// a different partition table is worse than failing. +#[allow(clippy::too_many_arguments)] pub(super) async fn prepare_boot_artifacts( build_dir: &Path, project_dir: &Path, @@ -20,6 +21,7 @@ pub(super) async fn prepare_boot_artifacts( board: &fbuild_config::BoardConfig, mcu_config: &Esp32McuConfig, flash_freq: &str, + esptool_bin: Option<&Path>, perf: &mut crate::perf_log::PerfTimer, ) -> Result<()> { let boot_artifacts_started = Instant::now(); @@ -76,26 +78,23 @@ pub(super) async fn prepare_boot_artifacts( board.max_flash, mcu_config.default_flash_size(), ); - let args = [ - "esptool", - "--chip", + // Prefer the provisioned standalone esptool binary; fall back to an + // `esptool` on PATH (FastLED/fbuild#954). + let argv = crate::esp32::esp32_linker::esptool_elf2image_argv( + esptool_bin, &board.mcu, - "elf2image", - "--flash-mode", boot_flash_mode, - "--flash-freq", flash_freq, - "--flash-size", flash_size, &boot_elf_str, - "-o", &boot_dst_str, - ]; + ); + let args: Vec<&str> = argv.iter().map(|s| s.as_str()).collect(); match fbuild_core::subprocess::run_command( &args, None, None, - Some(std::time::Duration::from_secs(30)), + Some(std::time::Duration::from_secs(60)), ) .await { diff --git a/crates/fbuild-build/src/esp32/orchestrator/build.rs b/crates/fbuild-build/src/esp32/orchestrator/build.rs index 615261b4..6d21cf19 100644 --- a/crates/fbuild-build/src/esp32/orchestrator/build.rs +++ b/crates/fbuild-build/src/esp32/orchestrator/build.rs @@ -75,7 +75,7 @@ impl BuildOrchestrator for Esp32Orchestrator { // packages can honor consumer-pinned URLs (FastLED/fbuild#672). let env_config = ctx.config.get_env_config(¶ms.env_name).ok(); let _resolve_phase = perf.phase("pioarduino-resolve"); - let (toolchain, framework) = resolve_pioarduino_packages( + let (toolchain, framework, esptool_bin) = resolve_pioarduino_packages( ¶ms.project_dir, &ctx.board.mcu, &mcu_config, @@ -793,6 +793,7 @@ impl BuildOrchestrator for Esp32Orchestrator { &flash_freq, ctx.board.max_flash, ctx.board.max_ram, + esptool_bin.clone(), params.verbose, ); @@ -821,6 +822,7 @@ impl BuildOrchestrator for Esp32Orchestrator { &ctx.board, &mcu_config, &flash_freq, + esptool_bin.as_deref(), &mut perf, ) .await?; diff --git a/crates/fbuild-build/src/esp32/orchestrator/packages.rs b/crates/fbuild-build/src/esp32/orchestrator/packages.rs index d5eda70c..a71c0987 100644 --- a/crates/fbuild-build/src/esp32/orchestrator/packages.rs +++ b/crates/fbuild-build/src/esp32/orchestrator/packages.rs @@ -1,7 +1,7 @@ //! Package resolution for pioarduino (platform.json, framework, toolchain). use std::collections::HashMap; -use std::path::Path; +use std::path::{Path, PathBuf}; use fbuild_core::Result; @@ -23,6 +23,7 @@ pub(super) async fn resolve_pioarduino_packages( ) -> Result<( fbuild_packages::toolchain::Esp32Toolchain, fbuild_packages::library::Esp32Framework, + Option, )> { // Ensure pioarduino platform (contains platform.json with metadata URLs). // Honor `platform_packages = platform-espressif32@#` from the env @@ -107,10 +108,20 @@ pub(super) async fn resolve_pioarduino_packages( } Ok::<(), fbuild_core::FbuildError>(()) }; - // Both futures run to completion; surface BOTH errors if both fail so the - // framework/SDK-libs failure (often the more actionable one) isn't dropped - // in favor of the toolchain error (CodeRabbit review on #967). - let (toolchain_res, framework_res) = tokio::join!(toolchain_fut, framework_fut); + // Provision the managed `tool-esptoolpy` package CONCURRENTLY with the + // toolchain + framework. esptool converts firmware.elf → firmware.bin at + // link time (FastLED/fbuild#954); provisioning it removes the pristine- + // machine "esptool not found — pip install esptool" failure. Best-effort: + // a resolution miss logs a warning and returns `None`, and the linker + // falls back to an `esptool` on PATH. + let esptool_fut = resolve_esptool(&platform, project_dir); + + // The three futures run to completion; surface BOTH the toolchain and + // framework errors if both fail so the framework/SDK-libs failure (often + // the more actionable one) isn't dropped in favor of the toolchain error + // (CodeRabbit review on #967). + let (toolchain_res, framework_res, esptool_py) = + tokio::join!(toolchain_fut, framework_fut, esptool_fut); match (toolchain_res, framework_res) { (Err(tc), Err(fw)) => { return Err(fbuild_core::FbuildError::BuildFailed(format!( @@ -121,7 +132,45 @@ pub(super) async fn resolve_pioarduino_packages( (Ok(_), Ok(_)) => {} } - Ok((toolchain, framework)) + Ok((toolchain, framework, esptool_py)) +} + +/// Provision the managed `tool-esptoolpy` package (the tasmota PyInstaller +/// standalone binary) from `platform.json` and return the path to the +/// `esptool` executable. +/// +/// Best-effort: any failure (missing `platform.json` entry, unsupported host, +/// network error) is logged at warn level and yields `None`, so the linker's +/// existing "esptool on PATH" fallback still applies. See FastLED/fbuild#954. +async fn resolve_esptool( + platform: &fbuild_packages::library::Esp32Platform, + project_dir: &Path, +) -> Option { + let url = match platform.get_package_url("tool-esptoolpy") { + Ok(url) => url, + Err(e) => { + tracing::warn!( + "could not resolve tool-esptoolpy from platform.json: {} \ + (falling back to esptool on PATH)", + e + ); + return None; + } + }; + let esptool = fbuild_packages::library::Esptool::from_metadata_url(project_dir, &url); + match esptool.ensure_installed().await { + Ok(path) => { + tracing::info!("provisioned esptool at {}", path.display()); + Some(path) + } + Err(e) => { + tracing::warn!( + "could not provision esptool: {} (falling back to esptool on PATH)", + e + ); + None + } + } } /// Provision toolchain-* packages listed in `platform.json` other than the diff --git a/crates/fbuild-packages/src/library/esptool.rs b/crates/fbuild-packages/src/library/esptool.rs new file mode 100644 index 00000000..7f718861 --- /dev/null +++ b/crates/fbuild-packages/src/library/esptool.rs @@ -0,0 +1,282 @@ +//! `tool-esptoolpy` provisioning (FastLED/fbuild#954). +//! +//! ESP32 builds need `esptool` for the `elf2image` step that converts +//! `firmware.elf` → the flashable `firmware.bin` (and, when only a bootloader +//! ELF ships, `bootloader.bin`). Historically fbuild shelled out to an +//! `esptool` on `PATH`, which fails on a pristine machine with +//! "esptool not found — Install with: pip install esptool". This module +//! provisions esptool as a managed package instead, so no user `pip install` +//! is required. +//! +//! We provision the **PyInstaller standalone binary** from the +//! [`tasmota/esptool`](https://github.com/tasmota/esptool) releases: a single +//! self-contained executable with every Python dependency (`rich_click`, +//! `pyserial`, …) bundled inside. This deliberately avoids the pioarduino +//! `esptoolpy-vX.Y.Z.zip`, which is pure-Python *source* WITHOUT its deps and +//! therefore dies at runtime with `ModuleNotFoundError: rich_click`. The +//! standalone binary needs no Python interpreter and no network at build time. +//! +//! Flow: +//! 1. The version is taken from the pioarduino `tool-esptoolpy` metadata URL +//! (`.../esptoolpy-v5.3.0.zip` → `5.3.0`) — see [`extract_esptool_version`]. +//! 2. The host `(OS, ARCH)` maps to a tasmota platform tag +//! (`linux-amd64`, `macos-arm64`, `windows-amd64`, …). An unsupported host +//! yields an error, and the caller falls back to an `esptool` on PATH. +//! 3. `https://github.com/tasmota/esptool/releases/download/v{version}/esptool-{platform}.zip` +//! is downloaded + extracted via the shared [`PackageBase::staged_install`] +//! pattern, and the `esptool` executable is located inside it. + +use std::path::{Path, PathBuf}; + +use fbuild_core::{FbuildError, Result}; + +use crate::{CacheSubdir, PackageBase}; + +/// Managed `tool-esptoolpy` package (tasmota standalone binary). +/// +/// Constructed from the `platform.json` metadata URL (used only to extract the +/// pinned version) and resolved lazily in [`Self::ensure_installed`], which +/// returns the path to the `esptool` executable. +pub struct Esptool { + project_dir: PathBuf, + version: String, +} + +impl Esptool { + /// Create from the `platform.json`-derived `tool-esptoolpy` URL + /// (`Esp32Platform::get_package_url("tool-esptoolpy")`). Only the version + /// embedded in the URL filename is used. + pub fn from_metadata_url(project_dir: &Path, metadata_url: &str) -> Self { + Self { + project_dir: project_dir.to_path_buf(), + version: extract_esptool_version(metadata_url), + } + } + + /// Ensure the standalone esptool binary is installed and return its path. + /// The caller runs it directly as ` --chip elf2image …`. + /// + /// Cache-aware: installs via the shared [`PackageBase::staged_install`] + /// pattern, so a warm cache costs no network I/O. Returns an error on an + /// unsupported host or a missing binary, so the caller can fall back to an + /// `esptool` on PATH. + pub async fn ensure_installed(&self) -> Result { + let platform = tasmota_platform_tag().ok_or_else(|| { + FbuildError::PackageError(format!( + "no prebuilt esptool binary for {}/{}", + std::env::consts::OS, + std::env::consts::ARCH + )) + })?; + let url = format!( + "https://github.com/tasmota/esptool/releases/download/v{}/esptool-{}.zip", + self.version, platform + ); + + let base = PackageBase::new( + "tool-esptoolpy", + &self.version, + &url, + &url, + None, + CacheSubdir::Toolchains, + &self.project_dir, + ); + let install_path = base.staged_install(validate_esptool).await?; + + let bin = find_esptool_binary(&install_path).ok_or_else(|| { + FbuildError::PackageError(format!( + "esptool executable not found under {}", + install_path.display() + )) + })?; + + // The GitHub-released zips do not always preserve the executable bit; + // set it every time (idempotent, cheap) so a cached install stays + // runnable. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if let Ok(meta) = std::fs::metadata(&bin) { + let mut perms = meta.permissions(); + if perms.mode() & 0o111 == 0 { + perms.set_mode(0o755); + let _ = std::fs::set_permissions(&bin, perms); + } + } + } + + Ok(bin) + } +} + +/// Validation callback for [`PackageBase::staged_install`]: the extracted tree +/// must contain an `esptool` executable. +fn validate_esptool(dir: &Path) -> Result<()> { + if find_esptool_binary(dir).is_some() { + Ok(()) + } else { + Err(FbuildError::PackageError(format!( + "extracted esptool package has no esptool executable (in {})", + dir.display() + ))) + } +} + +/// Map the host `(OS, ARCH)` to a tasmota esptool release platform tag. +/// +/// Returns `None` for hosts without a prebuilt binary, so the caller falls +/// back to an `esptool` on PATH. +fn tasmota_platform_tag() -> Option<&'static str> { + match (std::env::consts::OS, std::env::consts::ARCH) { + ("linux", "x86_64") => Some("linux-amd64"), + ("linux", "aarch64") => Some("linux-aarch64"), + ("linux", "arm") => Some("linux-armv7"), + ("macos", "x86_64") => Some("macos-amd64"), + ("macos", "aarch64") => Some("macos-arm64"), + ("windows", "x86_64") => Some("windows-amd64"), + _ => None, + } +} + +/// Executable name for the current platform. +fn esptool_bin_name() -> &'static str { + if cfg!(windows) { + "esptool.exe" + } else { + "esptool" + } +} + +/// Locate the `esptool` executable in an extracted tree, searching the root and +/// up to two levels deep (the tasmota zip nests it under +/// `esptool-/esptool`). +fn find_esptool_binary(root: &Path) -> Option { + fn search(dir: &Path, depth: usize) -> Option { + let candidate = dir.join(esptool_bin_name()); + if candidate.is_file() { + return Some(candidate); + } + if depth == 0 { + return None; + } + if let Ok(entries) = std::fs::read_dir(dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + if let Some(found) = search(&path, depth - 1) { + return Some(found); + } + } + } + } + None + } + search(root, 2) +} + +/// Extract a version string (e.g. `"5.3.0"`) from the pioarduino +/// `tool-esptoolpy` metadata URL. The URL looks like +/// `.../releases/download/0.0.1/esptoolpy-v5.3.0.zip`, where `0.0.1` is the +/// registry release tag and the real esptool version lives in the filename — +/// so we parse the **filename**, not an earlier path segment. Falls back to +/// `"unknown"` (which then fails to resolve a release and triggers the PATH +/// fallback) rather than silently using the wrong version. +fn extract_esptool_version(url: &str) -> String { + let filename = url.rsplit('/').next().unwrap_or(url); + let stem = filename.trim_end_matches(".zip"); + let bytes = stem.as_bytes(); + let mut i = 0; + while i < bytes.len() { + if bytes[i].is_ascii_digit() { + let start = i; + while i < bytes.len() && (bytes[i].is_ascii_digit() || bytes[i] == b'.') { + i += 1; + } + let cand = stem[start..i].trim_end_matches('.'); + if cand.contains('.') { + return cand.to_string(); + } + } else { + i += 1; + } + } + "unknown".to_string() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn extract_version_from_pioarduino_metadata_url() { + // The registry release tag (0.0.1) must NOT win over the real esptool + // version embedded in the filename. + assert_eq!( + extract_esptool_version( + "https://github.com/pioarduino/registry/releases/download/0.0.1/esptoolpy-v5.3.0.zip" + ), + "5.3.0" + ); + } + + #[test] + fn extract_version_from_bare_filename() { + assert_eq!(extract_esptool_version("esptoolpy-v4.8.1.zip"), "4.8.1"); + } + + #[test] + fn extract_version_falls_back_to_unknown() { + assert_eq!( + extract_esptool_version("https://example.com/esptool.zip"), + "unknown" + ); + } + + #[test] + fn platform_tag_is_known_for_this_host_or_none() { + // Just assert the mapping is total over the match arms without panic; + // the value depends on the build host. + let tag = tasmota_platform_tag(); + if let Some(t) = tag { + assert!( + t.starts_with("linux-") || t.starts_with("macos-") || t.starts_with("windows-") + ); + } + } + + #[test] + fn find_binary_at_root() { + let tmp = tempfile::TempDir::new().unwrap(); + let root = tmp.path(); + std::fs::write(root.join(esptool_bin_name()), b"bin").unwrap(); + assert_eq!( + find_esptool_binary(root), + Some(root.join(esptool_bin_name())) + ); + } + + #[test] + fn find_binary_nested_one_level() { + let tmp = tempfile::TempDir::new().unwrap(); + let inner = tmp.path().join("esptool-linux-amd64"); + std::fs::create_dir_all(&inner).unwrap(); + std::fs::write(inner.join(esptool_bin_name()), b"bin").unwrap(); + assert_eq!( + find_esptool_binary(tmp.path()), + Some(inner.join(esptool_bin_name())) + ); + } + + #[test] + fn find_binary_missing_returns_none() { + let tmp = tempfile::TempDir::new().unwrap(); + assert_eq!(find_esptool_binary(tmp.path()), None); + } + + #[test] + fn validate_rejects_tree_without_binary() { + let tmp = tempfile::TempDir::new().unwrap(); + assert!(validate_esptool(tmp.path()).is_err()); + } +} diff --git a/crates/fbuild-packages/src/library/mod.rs b/crates/fbuild-packages/src/library/mod.rs index 3c4c24ea..71fdad40 100644 --- a/crates/fbuild-packages/src/library/mod.rs +++ b/crates/fbuild-packages/src/library/mod.rs @@ -13,6 +13,7 @@ pub mod cmsis_framework; pub mod esp32_framework; pub mod esp32_platform; pub mod esp8266_framework; +pub mod esptool; pub mod framework_library; pub mod library_compiler; pub mod library_downloader; @@ -41,6 +42,7 @@ pub use cmsis_framework::CmsisFramework; pub use esp32_framework::Esp32Framework; pub use esp32_platform::Esp32Platform; pub use esp8266_framework::Esp8266Framework; +pub use esptool::Esptool; pub use framework_library::FrameworkLibrary; pub use library_manager::LibraryResult; pub use library_spec::LibrarySpec;