From 38a5c846ca58abd0a312efe325fc211b62c995f6 Mon Sep 17 00:00:00 2001 From: zackees Date: Mon, 6 Jul 2026 06:30:38 -0700 Subject: [PATCH] feat(deploy): provision esptool as a managed standalone binary (#954) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ESP32 builds ran esptool from PATH for the elf2image step (firmware.elf → firmware.bin), so a pristine machine failed after a fully successful compile+link with 'esptool not found — pip install esptool'. fbuild manages toolchains/frameworks/libraries itself; esptool was the one required tool a user still had to install by hand. Provision it as a managed package. platform.json's tool-esptoolpy points at a pure-Python source zip that is NOT self-contained (needs rich_click, click, pyserial, …) — running it dies with ModuleNotFoundError. Instead download the PyInstaller STANDALONE binary from tasmota/esptool releases for the current platform (esptool-linux-amd64.zip, esptool-macos-arm64.zip, esptool-windows-amd64.zip, …), which bundles esptool + every dependency into one executable — zero Python setup. The version tracks the pioarduino tool-esptoolpy URL. Provisioning runs concurrently with the toolchain + framework (tokio::join!), is best-effort (a resolution/download miss logs a warning and the linker falls back to esptool on PATH), and the resolved binary gets the exec bit on unix. Both the firmware and bootloader elf2image paths route through one shared esptool_elf2image_argv helper. Validated in the profiling container with esptool REMOVED from PATH: the NightDriver demo build provisions the tasmota binary and produces firmware.bin (753 KB), exit 0. clippy -D warnings clean; fbuild-build (718) + fbuild-packages (460) lib tests pass. Fixes #954. Co-Authored-By: Claude Fable 5 --- crates/fbuild-build/src/esp32/esp32_linker.rs | 71 ++++- .../src/esp32/orchestrator/boot_artifacts.rs | 19 +- .../src/esp32/orchestrator/build.rs | 4 +- .../src/esp32/orchestrator/packages.rs | 61 +++- crates/fbuild-packages/src/library/esptool.rs | 282 ++++++++++++++++++ crates/fbuild-packages/src/library/mod.rs | 2 + 6 files changed, 410 insertions(+), 29 deletions(-) create mode 100644 crates/fbuild-packages/src/library/esptool.rs 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;