diff --git a/crates/fbuild-library/src/library/ch32v_core.rs b/crates/fbuild-library/src/library/ch32v_core.rs index 3aca544af..4c5e4daee 100644 --- a/crates/fbuild-library/src/library/ch32v_core.rs +++ b/crates/fbuild-library/src/library/ch32v_core.rs @@ -10,6 +10,15 @@ use crate::{CacheSubdir, Framework, PackageBase, PackageInfo}; const CH32V_CORE_VERSION: &str = "1.0.4+d767162.ch32l103"; const CH32V_CORE_URL: &str = "https://github.com/openwch/arduino_core_ch32/archive/d76716239cdf8a084a5045c3dfd3151b3f69eeec.tar.gz"; +// The core declares `libraries/Adafruit_TinyUSB_Arduino` as a submodule, and +// GitHub's source archive ships it empty, which the unpack-time check (#1401) +// rejects. openwch publishes no archive that bundles it, and the pin above +// postdates every release, so the submodule is fetched on its own at the +// commit the core's gitlink records (FastLED/fbuild#1420). +const TINYUSB_SUBMODULE_PATH: &str = "libraries/Adafruit_TinyUSB_Arduino"; +const TINYUSB_URL: &str = "https://github.com/adafruit/Adafruit_TinyUSB_Arduino/archive/1f9da4918f2c05441a9bfc3866a5b9cc03f2da62.tar.gz"; +const TINYUSB_SHA256: &str = "e9ddbe3ac62adc402b33c1015bac0825c0b0bd1bad9a9e13d89183623b164080"; + /// OpenWCH CH32V Arduino core framework manager. pub struct Ch32vCores { base: PackageBase, @@ -27,7 +36,8 @@ impl Ch32vCores { None, CacheSubdir::Platforms, project_dir, - ), + ) + .with_submodule_source(TINYUSB_SUBMODULE_PATH, TINYUSB_URL, TINYUSB_SHA256), install_dir: None, } } @@ -64,7 +74,8 @@ impl Ch32vCores { CacheSubdir::Platforms, project_dir, cache_root, - ), + ) + .with_submodule_source(TINYUSB_SUBMODULE_PATH, TINYUSB_URL, TINYUSB_SHA256), install_dir: None, } } @@ -214,6 +225,17 @@ mod tests { assert_eq!(find_core_root(tmp.path()), nested); } + /// The TinyUSB pin must stay on the path the core's `.gitmodules` declares + /// and the commit its gitlink records at `d767162`. Moving the core pin + /// means re-reading both from the new commit, not keeping these. + #[test] + fn test_tinyusb_pin_matches_the_core_gitlink() { + assert!(CH32V_CORE_URL.contains("d76716239cdf8a084a5045c3dfd3151b3f69eeec")); + assert_eq!(TINYUSB_SUBMODULE_PATH, "libraries/Adafruit_TinyUSB_Arduino"); + assert!(TINYUSB_URL.contains("/archive/1f9da4918f2c05441a9bfc3866a5b9cc03f2da62.")); + assert_eq!(TINYUSB_SHA256.len(), 64); + } + #[test] fn test_validate_missing_cores() { let tmp = tempfile::TempDir::new().unwrap(); diff --git a/crates/fbuild-library/src/library/silabs_core.rs b/crates/fbuild-library/src/library/silabs_core.rs index 10af63320..416be1d29 100644 --- a/crates/fbuild-library/src/library/silabs_core.rs +++ b/crates/fbuild-library/src/library/silabs_core.rs @@ -11,6 +11,15 @@ const SILABS_CORE_VERSION: &str = "2.2.0"; const SILABS_CORE_URL: &str = "https://github.com/SiliconLabs/arduino/archive/refs/tags/2.2.0.tar.gz"; +// The core declares `extra/core-api` as a submodule, and the source archive +// ships it empty. Nothing reads it: `ensure_arduino_api()` installs +// ArduinoCore-API into `cores/silabs/api` after every install. The release +// asset that bundles it is 448 MB, so the directory is declared expected-empty +// instead of fetched (FastLED/fbuild#1421). +const CORE_API_SUBMODULE_PATH: &str = "extra/core-api"; +const CORE_API_SUBMODULE_REASON: &str = + "ArduinoCore-API is installed into cores/silabs/api by ensure_arduino_api()"; + /// Silicon Labs Arduino core framework manager. pub struct SilabsCores { base: PackageBase, @@ -28,7 +37,8 @@ impl SilabsCores { None, CacheSubdir::Platforms, project_dir, - ), + ) + .expect_empty_submodule(CORE_API_SUBMODULE_PATH, CORE_API_SUBMODULE_REASON), install_dir: None, } } @@ -65,7 +75,8 @@ impl SilabsCores { CacheSubdir::Platforms, project_dir, cache_root, - ), + ) + .expect_empty_submodule(CORE_API_SUBMODULE_PATH, CORE_API_SUBMODULE_REASON), install_dir: None, } } diff --git a/crates/fbuild-packages-fetch/src/lib.rs b/crates/fbuild-packages-fetch/src/lib.rs index 9d544c8b1..bfde7cdcc 100644 --- a/crates/fbuild-packages-fetch/src/lib.rs +++ b/crates/fbuild-packages-fetch/src/lib.rs @@ -169,6 +169,9 @@ pub struct PackageBase { pub cache_subdir: CacheSubdir, /// Optional DiskCache for LRU tracking. Best-effort: `None` if SQLite open fails. disk_cache: Option, + /// How declared submodules are handled at unpack (FastLED/fbuild#1420, + /// #1422). Empty for almost every package. + submodules: submodules::SubmodulePlan, } /// Which cache subdirectory to use. @@ -208,6 +211,7 @@ impl PackageBase { cache, cache_subdir, disk_cache, + submodules: submodules::SubmodulePlan::default(), } } @@ -233,6 +237,7 @@ impl PackageBase { cache: Cache::with_cache_root(project_dir, cache_root), cache_subdir, disk_cache, + submodules: submodules::SubmodulePlan::default(), } } @@ -248,6 +253,9 @@ impl PackageBase { /// `checksum: None` skips sha256 verification — consumer-trusted, which is /// the right policy for `platform_packages` overrides (#681, sibling of #663). /// + /// The submodule plan is dropped too: it describes the *default* archive, + /// and an override's commit may declare different submodules. + /// /// Emits a single INFO log so the override is visible in build scrollback /// across every framework package, without each orchestrator having to /// remember to log it themselves. @@ -257,6 +265,7 @@ impl PackageBase { self.cache_key = ovr.url; self.version = ovr.version; self.checksum = ovr.checksum; + self.submodules = submodules::SubmodulePlan::default(); self } @@ -433,16 +442,9 @@ impl PackageBase { // A core whose archive dropped its submodules extracts to something // that looks complete. Catch it here rather than letting the compiler // report a missing header from inside the core (FastLED/fbuild#1380, - // #1400). Checked against the extracted root and one level down, - // since most archives nest under a single version directory. - for root in submodule_scan_roots(&staging_path) { - let empty = submodules::find_empty_submodules(&root); - if !empty.is_empty() { - return Err(fbuild_core::FbuildError::PackageError( - submodules::empty_submodule_error(&self.name, &self.url, &empty), - )); - } - } + // #1400), after filling or excusing what the package's plan covers. + submodules::prepare_submodules(&self.name, &self.url, &staging_path, &self.submodules) + .await?; // Validate validate(&staging_path)?; @@ -940,21 +942,3 @@ mod package_override_tests { ); } } - -/// Where to look for a `.gitmodules` after extraction. -/// -/// Archives usually nest everything under one directory named for the -/// version (`esp8266-3.1.2/`), so the repo root is one level down from the -/// staging dir — but not always. Checking both costs one `read_dir`. -fn submodule_scan_roots(staging: &Path) -> Vec { - let mut roots = vec![staging.to_path_buf()]; - if let Ok(entries) = std::fs::read_dir(staging) { - for entry in entries.flatten() { - let path = entry.path(); - if path.is_dir() { - roots.push(path); - } - } - } - roots -} diff --git a/crates/fbuild-packages-fetch/src/submodules.rs b/crates/fbuild-packages-fetch/src/submodules.rs index 94c99ae86..1d1c09ec0 100644 --- a/crates/fbuild-packages-fetch/src/submodules.rs +++ b/crates/fbuild-packages-fetch/src/submodules.rs @@ -20,11 +20,20 @@ //! The archive carries `.gitmodules` even when it drops the submodule //! contents, which is what makes this cheap to catch: the file states exactly //! which directories are supposed to be non-empty. +//! +//! Not every core can switch to an archive that bundles its submodules. A +//! [`SubmodulePlan`] covers those: a pinned source fills a submodule during +//! install (FastLED/fbuild#1420), and an expected-empty entry records one whose +//! contents fbuild supplies another way (FastLED/fbuild#1421, #1422). Both are +//! checked against `.gitmodules`, so a plan that no longer matches upstream +//! fails instead of rotting. use std::path::Path; use fbuild_core::path::NormalizedPath; +use crate::PackageBase; + /// A declared submodule whose directory came out of the archive empty. #[derive(Debug, Clone, PartialEq, Eq)] pub struct EmptySubmodule { @@ -115,6 +124,229 @@ pub fn empty_submodule_error(package: &str, url: &str, empty: &[EmptySubmodule]) ) } +/// Contents for a declared submodule, fetched from a pinned archive. +/// +/// For a core whose upstream publishes no archive that bundles its +/// submodules, so the URL swap FastLED/fbuild#1380 made for esp8266 is not +/// available. `ch32v-core` is the case: openwch ships no release assets, and +/// the pinned commit postdates every release (FastLED/fbuild#1420). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SubmoduleSource { + /// Path as written in `.gitmodules`, relative to the repo root. + pub path: String, + /// Archive of the submodule at the commit the parent's gitlink records. + pub url: String, + /// SHA-256 of that archive. + pub sha256: String, +} + +/// A declared submodule left empty on purpose. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ExpectedEmpty { + /// Path as written in `.gitmodules`, relative to the repo root. + pub path: String, + /// How fbuild supplies the contents instead. + pub reason: String, +} + +/// How a package's declared submodules are handled at unpack. Empty for +/// almost every package. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct SubmodulePlan { + pub sources: Vec, + pub expected_empty: Vec, +} + +impl SubmodulePlan { + fn is_empty(&self) -> bool { + self.sources.is_empty() && self.expected_empty.is_empty() + } +} + +impl PackageBase { + /// Fill a declared submodule from a pinned archive during install, for a + /// core whose upstream publishes no archive that bundles its submodules + /// (FastLED/fbuild#1420). + pub fn with_submodule_source(mut self, path: &str, url: &str, sha256: &str) -> Self { + self.submodules.sources.push(SubmoduleSource { + path: path.to_string(), + url: url.to_string(), + sha256: sha256.to_string(), + }); + self + } + + /// Accept a declared submodule that extracts empty because fbuild supplies + /// its contents another way. `reason` says how (FastLED/fbuild#1422). + pub fn expect_empty_submodule(mut self, path: &str, reason: &str) -> Self { + self.submodules.expected_empty.push(ExpectedEmpty { + path: path.to_string(), + reason: reason.to_string(), + }); + self + } +} + +/// Apply `plan` to a freshly extracted package, then reject any declared +/// submodule that is still empty. +/// +/// Checked against the extracted root and one level down, since most +/// archives nest under a single version directory (`esp8266-3.1.2/`). +pub async fn prepare_submodules( + package: &str, + url: &str, + staging: &Path, + plan: &SubmodulePlan, +) -> fbuild_core::Result<()> { + let children = std::fs::read_dir(staging) + .into_iter() + .flatten() + .flatten() + .map(|entry| entry.path()) + .filter(|path| path.is_dir()); + let roots: Vec<_> = std::iter::once(staging.to_path_buf()) + .chain(children) + .collect(); + + let mut applied = false; + for root in &roots { + let Ok(text) = std::fs::read_to_string(root.join(".gitmodules")) else { + continue; + }; + let declared = declared_submodule_paths(&text); + check_plan(package, root, &declared, plan)?; + populate(package, root, &plan.sources).await?; + applied = true; + + let empty = unexpected_empty_submodules(root, plan); + if !empty.is_empty() { + return Err(fbuild_core::FbuildError::PackageError( + empty_submodule_error(package, url, &empty), + )); + } + for entry in &plan.expected_empty { + tracing::debug!( + "{package}: submodule {} left empty: {}", + entry.path, + entry.reason + ); + } + } + + if !plan.is_empty() && !applied { + return Err(fbuild_core::FbuildError::PackageError(format!( + "{package} has a submodule plan, but its archive has no .gitmodules. \ + The plan is stale and should be removed." + ))); + } + Ok(()) +} + +/// Every plan entry must name a path `.gitmodules` declares and that +/// extracted empty; anything else means upstream moved on. +fn check_plan( + package: &str, + root: &Path, + declared: &[String], + plan: &SubmodulePlan, +) -> fbuild_core::Result<()> { + let sources = plan + .sources + .iter() + .map(|s| (s.path.as_str(), "pinned source")); + let expected = plan + .expected_empty + .iter() + .map(|e| (e.path.as_str(), "expected empty")); + for (path, kind) in sources.chain(expected) { + if !declared.iter().any(|d| d == path) { + return Err(fbuild_core::FbuildError::PackageError(format!( + "{package} lists submodule `{path}` ({kind}), but its .gitmodules \ + does not declare that path. The entry is stale: remove it, or \ + move it to the path upstream uses now." + ))); + } + if !is_empty_dir(&root.join(path)) { + return Err(fbuild_core::FbuildError::PackageError(format!( + "{package} lists submodule `{path}` ({kind}), but the archive did \ + not ship that directory empty. If upstream now bundles it, the \ + entry is stale and should be removed." + ))); + } + } + Ok(()) +} + +/// Empty submodules the plan does not excuse. +fn unexpected_empty_submodules(root: &Path, plan: &SubmodulePlan) -> Vec { + find_empty_submodules(root) + .into_iter() + .filter(|empty| { + !plan + .expected_empty + .iter() + .any(|expected| expected.path == empty.declared_path) + }) + .collect() +} + +/// Fill each pinned submodule under `root` from its archive. +async fn populate( + package: &str, + root: &Path, + sources: &[SubmoduleSource], +) -> fbuild_core::Result<()> { + for source in sources { + let dest = root.join(&source.path); + let work = dest.with_file_name(format!( + "{}.fbuild-fetch", + dest.file_name().unwrap_or_default().to_string_lossy() + )); + let _ = std::fs::remove_dir_all(&work); + std::fs::create_dir_all(&work)?; + let fetched = fetch_into(source, &work, &dest).await; + let _ = std::fs::remove_dir_all(&work); + fetched.map_err(|e| { + fbuild_core::FbuildError::PackageError(format!( + "{package}: fetching submodule `{}` from {} failed: {e}", + source.path, source.url + )) + })?; + tracing::info!( + "{package}: populated submodule {} from {}", + source.path, + source.url + ); + } + Ok(()) +} + +async fn fetch_into(source: &SubmoduleSource, work: &Path, dest: &Path) -> fbuild_core::Result<()> { + let archive = crate::downloader::download_file(&source.url, work).await?; + crate::downloader::verify_checksum(&archive, &source.sha256)?; + let extracted = work.join("extracted"); + std::fs::create_dir_all(&extracted)?; + crate::extractor::extract(&archive, &extracted)?; + move_archive_contents(&extracted, dest)?; + Ok(()) +} + +/// Move an extracted archive's contents into `dest`, looking through the +/// single top-level directory GitHub archives wrap everything in +/// (`Adafruit_TinyUSB_Arduino-/`). +fn move_archive_contents(extracted: &Path, dest: &Path) -> std::io::Result<()> { + let entries = std::fs::read_dir(extracted)?.collect::>>()?; + let top = match entries.as_slice() { + [only] if only.file_type()?.is_dir() => only.path(), + _ => extracted.to_path_buf(), + }; + for entry in std::fs::read_dir(&top)? { + let entry = entry?; + std::fs::rename(entry.path(), dest.join(entry.file_name()))?; + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -134,6 +366,35 @@ mod tests { \turl = https://github.com/plerup/espsoftwareserial.git "; + const CH32V_GITMODULES: &str = "\ +[submodule \"libraries/Adafruit_TinyUSB_Arduino\"] +\tpath = libraries/Adafruit_TinyUSB_Arduino +\turl = https://github.com/adafruit/Adafruit_TinyUSB_Arduino.git +"; + + const TINYUSB: &str = "libraries/Adafruit_TinyUSB_Arduino"; + + fn tinyusb_plan() -> SubmodulePlan { + SubmodulePlan { + sources: vec![SubmoduleSource { + path: TINYUSB.to_string(), + url: "https://example.invalid/tinyusb.tar.gz".to_string(), + sha256: "0".repeat(64), + }], + expected_empty: Vec::new(), + } + } + + fn expect_empty(path: &str) -> SubmodulePlan { + SubmodulePlan { + sources: Vec::new(), + expected_empty: vec![ExpectedEmpty { + path: path.to_string(), + reason: "supplied another way".to_string(), + }], + } + } + #[test] fn declared_paths_are_read_from_gitmodules() { assert_eq!( @@ -207,6 +468,127 @@ mod tests { assert!(find_empty_submodules(tmp.path()).is_empty()); } + #[test] + fn plan_entries_for_declared_empty_submodules_are_accepted() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::create_dir_all(tmp.path().join(TINYUSB)).unwrap(); + let declared = declared_submodule_paths(CH32V_GITMODULES); + check_plan("ch32v-core", tmp.path(), &declared, &tinyusb_plan()).unwrap(); + check_plan("ch32v-core", tmp.path(), &declared, &expect_empty(TINYUSB)).unwrap(); + } + + /// Upstream dropping or moving the submodule must not leave an entry that + /// silently does nothing. + #[test] + fn a_plan_entry_for_an_undeclared_path_is_stale() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::create_dir_all(tmp.path().join(TINYUSB)).unwrap(); + let err = check_plan("ch32v-core", tmp.path(), &[], &tinyusb_plan()) + .unwrap_err() + .to_string(); + assert!(err.contains("does not declare"), "{err}"); + } + + /// Upstream starting to bundle the contents must neither be overwritten + /// by an older pin nor excused by an outdated expected-empty entry. + #[test] + fn a_plan_entry_for_a_populated_submodule_is_stale() { + let tmp = tempfile::TempDir::new().unwrap(); + write( + tmp.path(), + "libraries/Adafruit_TinyUSB_Arduino/src/Adafruit_TinyUSB.h", + "// header", + ); + let declared = declared_submodule_paths(CH32V_GITMODULES); + for plan in [tinyusb_plan(), expect_empty(TINYUSB)] { + let err = check_plan("ch32v-core", tmp.path(), &declared, &plan) + .unwrap_err() + .to_string(); + assert!(err.contains("did not ship that directory empty"), "{err}"); + } + } + + /// An expected-empty entry excuses only its own path, so the #1380 case + /// stays caught next to it. + #[test] + fn expected_empty_submodules_excuse_only_their_own_path() { + let tmp = tempfile::TempDir::new().unwrap(); + let root = tmp.path(); + write(root, ".gitmodules", ESP8266_GITMODULES); + std::fs::create_dir_all(root.join("libraries/LittleFS/lib/littlefs")).unwrap(); + std::fs::create_dir_all(root.join("libraries/SoftwareSerial")).unwrap(); + + let found = unexpected_empty_submodules(root, &expect_empty("libraries/SoftwareSerial")); + let paths: Vec<&str> = found.iter().map(|e| e.declared_path.as_str()).collect(); + assert_eq!(paths, vec!["libraries/LittleFS/lib/littlefs"]); + } + + /// The plan describes the default archive; an override's commit may + /// declare different submodules. + #[test] + fn an_override_drops_the_submodule_plan() { + let tmp = tempfile::TempDir::new().unwrap(); + let base = PackageBase::with_cache_root( + "ch32v-core", + "1.0.4", + "https://example.invalid/core.tar.gz", + "https://example.invalid/core.tar.gz", + None, + crate::CacheSubdir::Platforms, + tmp.path(), + &tmp.path().join("cache"), + ) + .with_submodule_source(TINYUSB, "https://example.invalid/t.tar.gz", "0") + .expect_empty_submodule("extra/core-api", "supplied another way"); + assert!(!base.submodules.is_empty()); + + let overridden = base.with_override(fbuild_config::PackageOverride { + url: "https://example.invalid/other.tar.gz".to_string(), + version: "1.0.4+gabc".to_string(), + checksum: None, + }); + assert!(overridden.submodules.is_empty()); + } + + #[test] + fn archive_contents_are_moved_out_of_the_wrapper_directory() { + let tmp = tempfile::TempDir::new().unwrap(); + let extracted = tmp.path().join("extracted"); + write( + &extracted, + "Adafruit_TinyUSB_Arduino-1f9da49/src/Adafruit_TinyUSB.h", + "// header", + ); + write( + &extracted, + "Adafruit_TinyUSB_Arduino-1f9da49/library.properties", + "name=TinyUSB", + ); + let dest = tmp.path().join(TINYUSB); + std::fs::create_dir_all(&dest).unwrap(); + + move_archive_contents(&extracted, &dest).unwrap(); + + assert!(dest.join("src/Adafruit_TinyUSB.h").is_file()); + assert!(dest.join("library.properties").is_file()); + assert!(!dest.join("Adafruit_TinyUSB_Arduino-1f9da49").exists()); + } + + #[test] + fn archive_contents_without_a_wrapper_are_moved_as_is() { + let tmp = tempfile::TempDir::new().unwrap(); + let extracted = tmp.path().join("extracted"); + write(&extracted, "src/Adafruit_TinyUSB.h", "// header"); + write(&extracted, "library.properties", "name=TinyUSB"); + let dest = tmp.path().join("dest"); + std::fs::create_dir_all(&dest).unwrap(); + + move_archive_contents(&extracted, &dest).unwrap(); + + assert!(dest.join("src/Adafruit_TinyUSB.h").is_file()); + assert!(dest.join("library.properties").is_file()); + } + #[test] fn the_error_names_the_directories_and_the_archive_kind() { let empty = vec![EmptySubmodule {