From 7bde8dfaad5e0013c16d8ea654156a788a1af57b Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 7 Nov 2026 15:24:08 -0800 Subject: [PATCH] fix(esp32): use the published toolchain slice Two bugs made the per-chip toolchain slices inert in real builds: 1. `slice_shard` matched only the manifest name `toolchain-xtensa-esp-elf`, but `Esp32Toolchain` names its package `esp32-xtensa-gcc`, so the shard was never fetched and every cold ESP32 build downloaded the whole 176 MB archive. 2. After the slice download, `install` re-verified the archive against the package checksum, which pins the WHOLE archive -- so every slice install died with `expected da31f36d..., got 36a9f0e7...`. Both were hidden by tests that force the slice and pass `checksum: None`. Fixed by aliasing the toolchain package name in `slice_shard` and by having `acquire_archive` report whether the archive is a slice, so the whole-archive pin applies only to the whole archive. Real ESP32-S3 cold-cache build: 106 MB slice, chip-scoped 603 MB install dir, no whole archive, build succeeds. --- .../fbuild-packages-fetch/src/chip_slice.rs | 62 +++++++++------- .../src/chip_slice_tests.rs | 72 ++++++++++++++++++- crates/fbuild-packages-fetch/src/lib.rs | 8 ++- crates/fbuild-packages-fetch/src/variants.rs | 32 ++++++++- 4 files changed, 144 insertions(+), 30 deletions(-) diff --git a/crates/fbuild-packages-fetch/src/chip_slice.rs b/crates/fbuild-packages-fetch/src/chip_slice.rs index c0bc2562e..1f26775e2 100644 --- a/crates/fbuild-packages-fetch/src/chip_slice.rs +++ b/crates/fbuild-packages-fetch/src/chip_slice.rs @@ -79,35 +79,47 @@ impl PackageBase { } } - /// The archive to install: a published slice for this chip when the mirror - /// lists one, else the artifact's own URL. - pub(crate) async fn acquire_archive(&self, dir: &Path) -> fbuild_core::Result { + /// The archive to install, and whether it is a chip slice rather than the + /// whole artifact. + /// + /// The caller must know: `self.checksum` pins the WHOLE archive, so a slice + /// — already verified against its own published digest in + /// [`Self::download_slice`] — must not be re-checked against it. Doing so + /// failed every slice install with `expected , got ` + /// (FastLED/fbuild#1944). + pub(crate) async fn acquire_archive( + &self, + dir: &Path, + ) -> fbuild_core::Result<(NormalizedPath, bool)> { match self.published_slice().await { - Some(slice) => self.download_slice(&slice, dir).await, - None => { - tracing::info!("downloading {} v{}", self.name, self.version); - let name = self.name.clone(); - let version = self.version.clone(); - downloader::download_file_with_progress( - &self.url, - dir, - &mut |progress: &downloader::DownloadProgress| { - install_status::publish_install_status(install_status::status( - &name, - Some(&version), - InstallPhase::Downloading, - InstallRole::Installer, - progress.format_message(), - None::, - )); - }, - ) - .await - .map(NormalizedPath::new) - } + Some(slice) => Ok((self.download_slice(&slice, dir).await?, true)), + None => Ok((self.download_whole(dir).await?, false)), } } + /// The whole artifact, from its own URL. + async fn download_whole(&self, dir: &Path) -> fbuild_core::Result { + tracing::info!("downloading {} v{}", self.name, self.version); + let name = self.name.clone(); + let version = self.version.clone(); + downloader::download_file_with_progress( + &self.url, + dir, + &mut |progress: &downloader::DownloadProgress| { + install_status::publish_install_status(install_status::status( + &name, + Some(&version), + InstallPhase::Downloading, + InstallRole::Installer, + progress.format_message(), + None::, + )); + }, + ) + .await + .map(NormalizedPath::new) + } + async fn published_slice(&self) -> Option { #[cfg(test)] if let Some(slice) = &self.forced_slice { diff --git a/crates/fbuild-packages-fetch/src/chip_slice_tests.rs b/crates/fbuild-packages-fetch/src/chip_slice_tests.rs index 3bccec52c..6f6806d65 100644 --- a/crates/fbuild-packages-fetch/src/chip_slice_tests.rs +++ b/crates/fbuild-packages-fetch/src/chip_slice_tests.rs @@ -72,12 +72,18 @@ async fn serve(bodies: Vec<(&'static str, Vec)>) -> (String, Arc PackageBase { + base_pinned(tmp, cache_root, url, None) +} + +/// `checksum` is the vendor's whole-archive pin, exactly as the real +/// `Esp32Toolchain` carries it. +fn base_pinned(tmp: &Path, cache_root: &Path, url: &str, checksum: Option<&str>) -> PackageBase { PackageBase::with_cache_root( "esp32-xtensa-gcc", "14.2.0_20241119", url, "toolchain-xtensa-esp-elf", - None, + checksum, CacheSubdir::Toolchains, tmp, cache_root, @@ -141,6 +147,70 @@ async fn slice_installs_into_its_own_chip_path_and_never_fetches_the_whole_archi ); } +/// The package's `checksum` pins the WHOLE archive. A slice carries its own +/// digest, so re-checking it against the vendor pin failed every real slice +/// install with `expected , got ` -- invisible to the +/// tests above because they never set a checksum (FastLED/fbuild#1944). +#[tokio::test] +async fn a_slice_is_not_checked_against_the_whole_archive_checksum() { + let payload = tar_zst(&[("bin/xtensa-esp-elf-gcc", b"#!/bin/sh\n")]); + let (url, hits) = serve(vec![ + ("/slice.tar.zst", payload.clone()), + ("/vendor.tar.zst", vec![b'X'; 4096]), + ]) + .await; + let tmp = tempfile::TempDir::new().expect("tempdir"); + let cache_root = tmp.path().join("cache"); + let whole_sha = sha256_hex(b"the whole archive these bytes are not"); + let mut base = base_pinned( + tmp.path(), + &cache_root, + &format!("{url}/vendor.tar.zst"), + Some(&whole_sha), + ); + assert_eq!(base.checksum.as_deref(), Some(whole_sha.as_str())); + base.forced_slice = Some(SliceFetch { + url: format!("{url}/slice.tar.zst"), + sha256: sha256_hex(&payload), + size: payload.len() as u64, + }); + let base = base.with_slice_variant(CHIP); + + let installed = base + .staged_install(validate_gcc) + .await + .expect("a verified slice must not be re-checked against the whole archive"); + + assert!(installed.as_path().is_dir()); + assert_eq!( + hits.lock().expect("hits").clone(), + vec!["/slice.tar.zst".to_string()] + ); +} + +#[tokio::test] +async fn a_whole_archive_is_still_checked_against_its_pin() { + let (url, hits) = serve(vec![("/vendor.tar.zst", vec![b'X'; 4096])]).await; + let tmp = tempfile::TempDir::new().expect("tempdir"); + let cache_root = tmp.path().join("cache"); + let base = base_pinned( + tmp.path(), + &cache_root, + &format!("{url}/vendor.tar.zst"), + Some(&sha256_hex(b"not those bytes")), + ); + + let error = base + .staged_install(validate_gcc) + .await + .expect_err("a whole archive must still be verified"); + assert!(error.to_string().contains("checksum mismatch"), "{error}"); + assert_eq!( + hits.lock().expect("hits").clone(), + vec!["/vendor.tar.zst".to_string()] + ); +} + #[tokio::test] async fn corrupt_slice_fails_instead_of_downloading_the_whole_archive() { let payload = tar_zst(&[("bin/xtensa-esp-elf-gcc", b"#!/bin/sh\n")]); diff --git a/crates/fbuild-packages-fetch/src/lib.rs b/crates/fbuild-packages-fetch/src/lib.rs index 604a621db..fd91c4e4f 100644 --- a/crates/fbuild-packages-fetch/src/lib.rs +++ b/crates/fbuild-packages-fetch/src/lib.rs @@ -526,10 +526,12 @@ impl PackageBase { })?; // Download (a published chip slice when one exists) with progress. - let archive_path = self.acquire_archive(&staging_path).await?; + let (archive_path, from_slice) = self.acquire_archive(&staging_path).await?; - // Verify checksum - if let Some(ref expected) = self.checksum { + // Verify checksum. A slice was already verified against its own + // published digest; `self.checksum` pins the whole archive, so applying + // it here rejects every slice (FastLED/fbuild#1944). + if let Some(expected) = self.checksum.as_ref().filter(|_| !from_slice) { install_status::publish_install_status(install_status::status( &self.name, Some(&self.version), diff --git a/crates/fbuild-packages-fetch/src/variants.rs b/crates/fbuild-packages-fetch/src/variants.rs index 5a79194bd..22cf20bca 100644 --- a/crates/fbuild-packages-fetch/src/variants.rs +++ b/crates/fbuild-packages-fetch/src/variants.rs @@ -15,6 +15,17 @@ const LIBS: &str = "framework-arduinoespressif32-libs"; /// because `lib/` is the bulk of the 660 MB tree (FastLED/fbuild#1944). const XTENSA_TOOLCHAIN: &str = "toolchain-xtensa-esp-elf"; +/// The SAME artifact under the name `Esp32Toolchain` gives it +/// (`crates/fbuild-toolchain/src/toolchain/esp32.rs`, `new`/`from_resolved`). +/// +/// The shard is looked up by URL, so the name only selects the owner -- but a +/// name the map does not know means "not sliced", silently. Publishing the +/// slices while this alias was missing made every ESP32-S3 build download the +/// whole 176 MB unified archive: the `@esp-toolchain` shard was never even +/// fetched, because the toolchain package is called `esp32-xtensa-gcc` +/// (FastLED/fbuild#1944). +const XTENSA_TOOLCHAIN_PACKAGE: &str = "esp32-xtensa-gcc"; + /// Manifest shard that lists the slices for `name`, when the artifact is /// published as per-chip slices at all. /// @@ -24,7 +35,7 @@ const XTENSA_TOOLCHAIN: &str = "toolchain-xtensa-esp-elf"; fn slice_shard(name: &str) -> Option<&'static str> { match name { LIBS => Some("@esp-libs"), - XTENSA_TOOLCHAIN => Some("@esp-toolchain"), + XTENSA_TOOLCHAIN | XTENSA_TOOLCHAIN_PACKAGE => Some("@esp-toolchain"), _ => None, } } @@ -327,6 +338,25 @@ mod tests { assert_eq!(slice_shard("platform-espressif32"), None); } + /// Both names this artifact is constructed under must reach the shard. + /// + /// `esp32-xtensa-gcc` is what `Esp32Toolchain::new`/`from_resolved` pass to + /// `PackageBase` in `crates/fbuild-toolchain/src/toolchain/esp32.rs`; the + /// manifest name is what the slicer/publisher use. Looking the shard up by + /// URL hides the difference until a real build silently downloads the whole + /// archive (FastLED/fbuild#1944). + #[test] + fn every_name_the_esp_orchestrator_builds_resolves_to_the_toolchain_shard() { + for name in [XTENSA_TOOLCHAIN, "esp32-xtensa-gcc"] { + assert_eq!(slice_shard(name), Some("@esp-toolchain"), "{name}"); + } + // The RISC-V toolchain keeps the whole archive under either of its + // names: its multilibs are ISA variants, not chips. + for name in ["toolchain-riscv32-esp", "esp32-riscv-gcc"] { + assert_eq!(slice_shard(name), None, "{name}"); + } + } + #[test] fn mcu_normalization_preserves_engineering_sample_directory_suffix() { for (mcu, chip) in [