diff --git a/agents/docs/commands-reference.md b/agents/docs/commands-reference.md index 748a17034..8a2253da2 100644 --- a/agents/docs/commands-reference.md +++ b/agents/docs/commands-reference.md @@ -51,7 +51,7 @@ help text). | `fbuild show` | Show daemon logs or other introspection. | `fbuild help show` | | `fbuild device` | List / inspect connected devices the daemon knows about. | `fbuild help device` | | `fbuild purge` | Purge cached packages — full purge or LRU-only via `--gc`. | `fbuild help purge` | -| `fbuild lnk` | Manage `.lnk` resource pointers (fetch / verify / add). | `fbuild help lnk` | +| `fbuild lnk` | Manage `.fetch` blob pointers (fetch / verify / add). `.lnk` is still read for pointers written before FastLED/fbuild#1369; FastLED's runtime `.lnk` asset links are a different format and are skipped. | `fbuild help lnk` | ## Serial-port introspection (FastLED/fbuild#686) diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs index c82394c9d..1c10f2558 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs @@ -1,4 +1,5 @@ -//! Wrap `process_embed_files` with `.lnk` resolution + objcopy target selection. +//! Wrap `process_embed_files` with blob-pointer resolution + objcopy target +//! selection. use std::path::{Path, PathBuf}; @@ -15,19 +16,38 @@ fn expand_embed_entries( lnk_leases: &mut Vec, ) -> Result> { let mut out = Vec::with_capacity(entries.len()); + // A materialized target is named after the pointer's blob, so two + // pointers with the same blob name land on one path — the second + // overwrites the first and both embed entries end up holding the second + // blob's bytes. Refuse instead: a wrong asset embedded in firmware is + // not something the user can see went wrong. + // Keyed by `normalize_for_key`, not by `PathBuf` equality. Windows and + // macOS are case-insensitive, so `logo.bin.fetch` and `LOGO.bin.lnk` + // produce lexically distinct targets that are the *same file* — which is + // exactly the collision this guard exists to catch, and the one a plain + // comparison lets through. + let mut claimed: std::collections::HashMap = std::collections::HashMap::new(); for entry in entries { let p = if Path::new(entry).is_absolute() { PathBuf::from(entry) } else { project_dir.join(entry) }; - if fbuild_packages::lnk::has_lnk_extension(&p) { + if fbuild_packages::lnk::is_blob_pointer(&p) { let cache = lnk_cache.ok_or_else(|| { fbuild_core::FbuildError::PackageError( - "disk cache unavailable; cannot resolve .lnk entries".to_string(), + "disk cache unavailable; cannot resolve blob-pointer (.fetch/.lnk) entries" + .to_string(), ) })?; let materialized = fbuild_packages::lnk::materialize_lnk_entry(&p, lnk_dir, cache)?; + let claim_key = fbuild_core::path::normalize_for_key(&materialized.target_path); + if let Some(first) = claimed.insert(claim_key, entry.clone()) { + return Err(fbuild_core::FbuildError::PackageError(format!( + "embed entries `{first}` and `{entry}` both materialize to {} — blob pointers are named after the blob they point at, so two of them cannot share one. Rename one, or drop the stale pointer if this is a leftover `.lnk` beside its `.fetch` replacement (FastLED/fbuild#1369).", + materialized.target_path.display() + ))); + } out.push(materialized.target_path.to_string_lossy().into_owned()); lnk_leases.push(materialized); } else { @@ -37,7 +57,7 @@ fn expand_embed_entries( Ok(out) } -/// Resolve `.lnk` entries in `embed_files`/`embed_txtfiles` against the disk +/// Resolve blob-pointer entries in `embed_files`/`embed_txtfiles` against the disk /// cache, then convert each entry into a linkable ELF object. Returns the /// list of object files to be appended to the sketch link set. #[allow(clippy::too_many_arguments)] @@ -152,4 +172,122 @@ mod tests { "the cache lease must release when embed processing ends" ); } + + /// Two pointers whose blob names match materialize to one path, because + /// the target is derived from the file name alone. The second silently + /// replaced the first and both embed entries then pointed at the same + /// bytes. + /// + /// Pre-existing for two `.lnk` in different directories; FastLED/fbuild + /// #1369 adds the case where `foo.bin.fetch` and `foo.bin.lnk` sit in the + /// *same* directory, which is exactly what a half-finished migration + /// looks like. Silence is the wrong answer either way. + #[test] + fn colliding_blob_names_are_refused_rather_than_silently_overwritten() { + let cache_root = tempfile::tempdir().unwrap(); + let cache = fbuild_packages::DiskCache::open_at(cache_root.path()).unwrap(); + let project = tempfile::tempdir().unwrap(); + + let write_pointer = |rel: &str, body: &[u8]| { + let sha = format!("{:x}", Sha256::digest(body)); + let url = format!("https://localhost.invalid/{rel}"); + let archive_dir = cache.archive_dir(Kind::LnkBlobs, &url, &sha); + std::fs::create_dir_all(&archive_dir).unwrap(); + let blob_path = archive_dir.join("blob.bin"); + std::fs::write(&blob_path, body).unwrap(); + cache + .record_archive( + Kind::LnkBlobs, + &url, + &sha, + &blob_path.to_string_lossy(), + body.len() as i64, + &sha, + ) + .unwrap(); + let path = project.path().join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write( + &path, + format!(r#"{{"v":1,"url":"{url}","sha256":"{sha}"}}"#), + ) + .unwrap(); + }; + write_pointer("logo.bin.fetch", b"the fetch blob"); + write_pointer("logo.bin.lnk", b"the legacy blob"); + + let mut leases = Vec::new(); + let error = expand_embed_entries( + &["logo.bin.fetch".to_string(), "logo.bin.lnk".to_string()], + project.path(), + &project.path().join("build/lnk"), + Some(&cache), + &mut leases, + ) + .expect_err("two pointers cannot share one materialized path"); + let message = error.to_string(); + assert!(message.contains("logo.bin"), "{message}"); + assert!( + message.contains("logo.bin.fetch") && message.contains("logo.bin.lnk"), + "the error must name both pointers, or it is unactionable: {message}" + ); + } + + /// FastLED/fbuild#1369 review: on Windows and macOS the filesystem folds + /// case, so `logo.bin.fetch` and `LOGO.bin.lnk` materialize to one file + /// while comparing unequal as paths. Keying the guard lexically let + /// exactly the collision it was written to catch slip through — on the + /// platforms where it actually happens. + #[test] + fn blob_names_differing_only_by_case_collide_on_case_insensitive_hosts() { + if !fbuild_core::platform::host::is_windows() && !fbuild_core::platform::host::is_macos() { + return; // case-sensitive host: these really are two distinct files + } + + let cache_root = tempfile::tempdir().unwrap(); + let cache = fbuild_packages::DiskCache::open_at(cache_root.path()).unwrap(); + let project = tempfile::tempdir().unwrap(); + + let write_pointer = |rel: &str, body: &[u8]| { + let sha = format!("{:x}", Sha256::digest(body)); + let url = format!("https://localhost.invalid/{rel}"); + let archive_dir = cache.archive_dir(Kind::LnkBlobs, &url, &sha); + std::fs::create_dir_all(&archive_dir).unwrap(); + let blob_path = archive_dir.join("blob.bin"); + std::fs::write(&blob_path, body).unwrap(); + cache + .record_archive( + Kind::LnkBlobs, + &url, + &sha, + &blob_path.to_string_lossy(), + body.len() as i64, + &sha, + ) + .unwrap(); + let path = project.path().join(rel); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write( + &path, + format!(r#"{{"v":1,"url":"{url}","sha256":"{sha}"}}"#), + ) + .unwrap(); + }; + write_pointer("data/logo.bin.fetch", b"the fetch blob"); + write_pointer("assets/LOGO.bin.lnk", b"the legacy blob"); + + let mut leases = Vec::new(); + let error = expand_embed_entries( + &[ + "data/logo.bin.fetch".to_string(), + "assets/LOGO.bin.lnk".to_string(), + ], + project.path(), + &project.path().join("build/lnk"), + Some(&cache), + &mut leases, + ) + .expect_err("case-folded names name one file on this host"); + assert!(error.to_string().contains("materialize to"), "{error}"); + } } diff --git a/crates/fbuild-cli/src/cli/args.rs b/crates/fbuild-cli/src/cli/args.rs index 550b189ed..581fe6262 100644 --- a/crates/fbuild-cli/src/cli/args.rs +++ b/crates/fbuild-cli/src/cli/args.rs @@ -620,12 +620,17 @@ pub enum Commands { #[arg(short = 'm', long)] matcher: Option, }, - /// Manage `.lnk` resource pointers (fetch / verify / add). + /// Manage `.fetch` blob pointers (fetch / verify / add). /// - /// `.lnk` files are tiny JSON manifests checked into source control + /// `.fetch` files are tiny JSON manifests checked into source control /// that point at remote binary blobs (sha256-verified). At build time /// fbuild downloads + caches them; this command lets you operate on /// them outside of a build. + /// + /// `.lnk` is still read for pointers written before the split + /// (FastLED/fbuild#1369). Note that FastLED's *runtime* asset links are + /// also `.lnk` but a different, plain-text format read on-device by + /// `fl::parse_lnk` — those are not fbuild's and are skipped. Lnk { #[command(subcommand)] action: LnkAction, @@ -771,26 +776,29 @@ pub enum Commands { /// Subcommands for `fbuild lnk`. #[derive(Subcommand)] pub enum LnkAction { - /// Walk the current dir (or a project root) and fetch every `.lnk` - /// referenced blob into the disk cache. Cache hits are no-ops. + /// Walk the current dir (or a project root) and fetch every + /// blob-pointer-referenced blob into the disk cache. Cache hits are + /// no-ops. Pull { /// Project root to scan. Defaults to the current directory. project_dir: Option, }, - /// Verify every `.lnk` blob in the cache matches its sha256, without - /// touching the network. Reports mismatches; exits non-zero on any. + /// Verify every pointed-at blob in the cache matches its sha256, + /// without touching the network. Reports mismatches; exits non-zero on + /// any. Check { /// Project root to scan. Defaults to the current directory. project_dir: Option, }, - /// Download a URL once, compute its sha256, and write a new `.lnk` + /// Download a URL once, compute its sha256, and write a new `.fetch` /// JSON pointing at it. Useful for adding new resources without /// hand-editing JSON. Add { /// URL to download. url: String, - /// Where to write the `.lnk` file. Defaults to the URL's basename - /// + `.lnk` in the current directory. + /// Where to write the pointer. Defaults to the URL's basename with + /// a `.fetch` suffix, in the current directory; an explicit path is + /// used exactly as given. #[arg(short = 'o', long)] output: Option, }, diff --git a/crates/fbuild-cli/src/cli/lnk.rs b/crates/fbuild-cli/src/cli/lnk.rs index 3d6f712ed..afff68fc0 100644 --- a/crates/fbuild-cli/src/cli/lnk.rs +++ b/crates/fbuild-cli/src/cli/lnk.rs @@ -1,8 +1,8 @@ //! `fbuild lnk` subcommands. //! -//! - `pull` — scan + fetch every .lnk's blob into the disk cache +//! - `pull` — scan + fetch every blob pointer's blob into the disk cache //! - `check` — verify every cached blob's sha256 (no network) -//! - `add` — fetch a URL once, hash it, write a new .lnk pointing at it +//! - `add` — fetch a URL once, hash it, write a new `.fetch` pointing at it use crate::output; @@ -36,7 +36,10 @@ pub async fn run_lnk( let root = resolve_root(project_dir, top_level_project_dir); let discovered = scan_for_lnk(&root)?; if discovered.is_empty() { - output::result(format!("no .lnk files found under {}", root.display())); + output::result(format!( + "no blob pointers (.fetch/.lnk) found under {}", + root.display() + )); return Ok(()); } let cache = open_cache()?; @@ -73,7 +76,10 @@ pub async fn run_lnk( let root = resolve_root(project_dir, top_level_project_dir); let discovered = scan_for_lnk(&root)?; if discovered.is_empty() { - output::result(format!("no .lnk files found under {}", root.display())); + output::result(format!( + "no blob pointers (.fetch/.lnk) found under {}", + root.display() + )); return Ok(()); } let cache = open_cache()?; @@ -150,9 +156,19 @@ pub async fn run_lnk( // Determine output path before downloading so we fail early on a // bad output spec. let basename = url.rsplit('/').next().unwrap_or("blob"); + // FastLED/fbuild#1369: newly written blob pointers get `.fetch`, + // which is unambiguously fbuild's. `.lnk` stays FastLED's runtime + // asset link — a different format with a different consumer, and + // conflating the two produced a silently wrong URL on-device + // (FastLED/FastLED#4012). An explicit `--output` is honored as + // typed: the user naming a file is not the user asking to be + // corrected. let output_path = match output_arg { Some(p) => PathBuf::from(p), - None => PathBuf::from(format!("{basename}.lnk")), + None => PathBuf::from(format!( + "{basename}.{}", + fbuild_packages::lnk::BLOB_POINTER_EXTENSION + )), }; if let Some(parent) = output_path.parent() { if !parent.as_os_str().is_empty() { @@ -201,7 +217,7 @@ pub async fn run_lnk( }); let pretty = serde_json::to_string_pretty(&json).map_err(|e| { fbuild_core::FbuildError::PackageError(format!( - "failed to serialize .lnk JSON: {e}" + "failed to serialize blob-pointer JSON: {e}" )) })?; let mut f = std::fs::File::create(&output_path).map_err(|e| { diff --git a/crates/fbuild-packages/tests/lnk_e2e.rs b/crates/fbuild-packages/tests/lnk_e2e.rs index 90b015699..ba8e028cc 100644 --- a/crates/fbuild-packages/tests/lnk_e2e.rs +++ b/crates/fbuild-packages/tests/lnk_e2e.rs @@ -116,13 +116,16 @@ async fn lnk_pipeline_e2e_fetches_verifies_and_materializes() { spawn_test_server(vec![("asset.bin".to_string(), blob_bytes.clone())]).await; let url = format!("http://127.0.0.1:{port}/asset.bin"); - // Set up a project tree with one .lnk pointing at our test server. + // Set up a project tree with one blob pointer aimed at our test + // server. `.fetch` is what `fbuild lnk add` writes as of + // FastLED/fbuild#1369; the legacy `.lnk` spelling gets its own + // end-to-end run below. let work = tempdir(); let src_root = work.path().join("src"); let build_dir = work.path().join("build/resources"); let cache_dir = work.path().join("cache"); - let lnk_path = src_root.join("data/asset.bin.lnk"); + let lnk_path = src_root.join("data/asset.bin.fetch"); std::fs::create_dir_all(lnk_path.parent().unwrap()).unwrap(); let lnk_json = format!( r#"{{"v":1,"url":"{url}","sha256":"{blob_sha}","size":{}}}"#, @@ -134,7 +137,7 @@ async fn lnk_pipeline_e2e_fetches_verifies_and_materializes() { // Scan finds the lnk. let discovered = scan_for_lnk(&src_root).unwrap(); - assert_eq!(discovered.len(), 1, "scanner should find the one .lnk"); + assert_eq!(discovered.len(), 1, "scanner should find the one pointer"); assert_eq!(discovered[0].lnk.sha256, blob_sha); // Materialize fetches + verifies + writes into the build tree. @@ -179,6 +182,9 @@ async fn lnk_pipeline_rejects_sha_mismatch() { let build_dir = work.path().join("build"); let cache_dir = work.path().join("cache"); + // Deliberately the legacy `.lnk` spelling: pointers written before + // FastLED/fbuild#1369 must keep resolving, and this run is what + // proves it rather than a comment claiming so. let lnk_path = src_root.join("x.bin.lnk"); std::fs::create_dir_all(&src_root).unwrap(); std::fs::write( diff --git a/crates/fbuild-toolchain/src/lnk/README.md b/crates/fbuild-toolchain/src/lnk/README.md index 496869e7e..be6f580fd 100644 --- a/crates/fbuild-toolchain/src/lnk/README.md +++ b/crates/fbuild-toolchain/src/lnk/README.md @@ -1,13 +1,40 @@ -# `.lnk` resource pointers +# `.fetch` blob pointers Tiny JSON manifests checked into source control that point at remote binary blobs. At build time fbuild fetches them, verifies the sha256, caches them in the shared two-phase disk cache, and materializes them next to where the -`.lnk` would have been (in the build tree, not the source tree). +pointer would have been (in the build tree, not the source tree). The intent: keep the source repo small, keep binary assets out of git history, but have them appear as if they were always there during builds. +## The extension, and the one it is not + +`fbuild lnk add` writes `.fetch`. `.lnk` is still read, so pointers written +before FastLED/fbuild#1369 keep working — only the default for newly +written ones moved. + +The split exists because `.lnk` was serving two unrelated roles: + +| | runtime asset link | build-time blob pointer | +| --- | --- | --- | +| extension | `.lnk` | `.fetch` | +| parsed by | `fl::parse_lnk` (C++, on the MCU) | fbuild (Rust, on the build host) | +| format | text: URL line + `key=value` | JSON: `{v, url, sha256, size, extract}` | +| `sha256` | optional, unenforced | **required** — the cache is content-addressed | +| written by | hand | `fbuild lnk add` | + +They are not competing drafts of one format. The runtime form has to parse +on an MCU, where a JSON parser is not worth carrying; the build-time form +needs a mandatory digest because the resolver caches by content. + +Sharing one extension read as one concept, so people normalized toward +whichever they met first. That happened: converting a runtime link to this +JSON schema looked like tidying, and `fl::parse_lnk` then took `{` as the +URL (FastLED/FastLED#4012). Distinct extensions make the mistake +unexpressible. A `.lnk` that does not parse as JSON is therefore reported as +"probably a runtime asset link", not as a malformed blob pointer. + ## Format (v1) ```json @@ -36,7 +63,7 @@ escape hatch. ```text source tree: build tree: - foo.bin.lnk ─────► resources/foo.bin + foo.bin.fetch ─────► resources/foo.bin │ ▲ │ scan + parse │ hardlink (or copy) ▼ │ @@ -56,13 +83,13 @@ the materialized file as if it had been in the source tree all along. ## CLI ```bash -# Fetch every .lnk-referenced blob into the disk cache +# Fetch every pointer-referenced blob into the disk cache fbuild lnk pull [] # Verify every cached blob matches its sha256 (no network) fbuild lnk check [] -# One-shot: download a URL, hash it, write a new .lnk +# One-shot: download a URL, hash it, write a new .fetch fbuild lnk add [-o ] ``` @@ -70,7 +97,7 @@ fbuild lnk add [-o ] **fbuild side** — uses the existing `DiskCache` with `Kind::LnkBlobs`. Cache key: `(LnkBlobs, url, sha256)`. The sha256 in the "version" slot guarantees -that flipping the `.lnk`'s sha256 forces a refetch. +that flipping the pointer's sha256 forces a refetch. - LRU eviction via `disk_cache::gc` - Lease-aware GC reaping (active builds pin their blobs) @@ -80,26 +107,26 @@ that flipping the `.lnk`'s sha256 forces a refetch. materialized blob (e.g. `objcopy` invoked by the esp32 orchestrator) already hashes its inputs as part of the cache key. Because the blob's on-disk content is byte-identical to its sha256, the cache key changes -whenever the `.lnk`'s sha256 changes. Composition is automatic. +whenever the pointer's sha256 changes. Composition is automatic. ## Integration with `embed_files` PlatformIO `board_build.embed_files` and `board_build.embed_txtfiles` -entries can mix plain paths with `.lnk` pointers: +entries can mix plain paths with blob pointers: ```ini [env:demo] board_build.embed_files = site/dist/index.html.gz ; plain file in source tree - assets/large_blob.bin.lnk ; resolved at build time + assets/large_blob.bin.fetch ; resolved at build time board_build.embed_txtfiles = config/timezones.json ``` -The esp32 orchestrator pre-resolves any `.lnk` entries through +The esp32 orchestrator pre-resolves any blob-pointer entries through `materialize_lnk_entry` before passing them to `process_embed_files`. The -materialized path is what reaches `objcopy`. The original `.lnk` file is +materialized path is what reaches `objcopy`. The original pointer file is not visible to downstream tooling. ## Module map @@ -107,7 +134,7 @@ not visible to downstream tooling. | File | What | |------|------| | `format.rs` | `LnkFile` struct, JSON parser, validation | -| `scanner.rs` | `scan_for_lnk(root)` — walk a tree, collect parsed `.lnk`s | +| `scanner.rs` | `scan_for_lnk(root)` — walk a tree, collect parsed pointers (`.fetch` and `.lnk`) | | `resolver.rs` | `resolve(lnk, cache)` — cache hit / miss + download + verify | | `materialize.rs` | `materialize_one` / `materialize_all` — write blob into build tree | | `embed.rs` | `expand_lnk_entries` / `materialize_lnk_entry` — glue for `embed_files` | @@ -116,7 +143,7 @@ not visible to downstream tooling. **Can I use git LFS instead?** You can — git LFS is orthogonal. But that pulls every blob on every -clone. `.lnk` lets you fetch only what a build actually consumes, with +clone. A blob pointer lets you fetch only what a build actually consumes, with content-addressable cache sharing across projects on the same machine. **Why mandatory sha256?** diff --git a/crates/fbuild-toolchain/src/lnk/embed.rs b/crates/fbuild-toolchain/src/lnk/embed.rs index 5380b1932..e379faf4f 100644 --- a/crates/fbuild-toolchain/src/lnk/embed.rs +++ b/crates/fbuild-toolchain/src/lnk/embed.rs @@ -41,7 +41,7 @@ where let mut out = Vec::with_capacity(entries.len()); for entry in entries { let entry_path = make_absolute(entry, project_dir); - if has_lnk_extension(&entry_path) { + if is_blob_pointer(&entry_path) { let resolved = resolver(&entry_path)?; out.push(resolved); } else { @@ -51,10 +51,39 @@ where Ok(out) } -/// Whether the given path's filename ends in `.lnk` (case-sensitive, -/// matching the convention of the rest of the module). -pub fn has_lnk_extension(path: &Path) -> bool { - path.extension().and_then(|e| e.to_str()) == Some("lnk") +/// The extension `fbuild lnk add` writes for a build-time blob pointer. +/// +/// FastLED/fbuild#1369: `.lnk` used to serve two roles with different +/// consumers, formats and guarantees — this JSON blob pointer (parsed by +/// fbuild on the build host, sha256 mandatory) and FastLED's runtime asset +/// link (parsed by `fl::parse_lnk` on the MCU, plain text). Sharing one +/// extension read as one concept, and someone "tidied" a runtime link into +/// this schema; `fl::parse_lnk` then took `{` as the URL +/// (FastLED/FastLED#4012). Distinct extensions make that unexpressible. +pub const BLOB_POINTER_EXTENSION: &str = "fetch"; + +/// The extension blob pointers used to carry, still accepted on read. +/// +/// Files written before #1369 keep working; only the *default* for newly +/// written ones moved. A `.lnk` that fails to parse as JSON is far more +/// likely a runtime asset link than a corrupt blob pointer, which is why +/// the scanner's diagnostic distinguishes the two. +pub const LEGACY_BLOB_POINTER_EXTENSION: &str = "lnk"; + +/// Whether the path names a build-time blob pointer, in either spelling +/// (case-sensitive, matching the convention of the rest of the module). +pub fn is_blob_pointer(path: &Path) -> bool { + matches!( + path.extension().and_then(|e| e.to_str()), + Some(BLOB_POINTER_EXTENSION) | Some(LEGACY_BLOB_POINTER_EXTENSION) + ) +} + +/// Strip whichever blob-pointer extension `name` carries, yielding the name +/// of the blob it points at. `None` if it carries neither. +pub fn strip_pointer_extension(name: &str) -> Option<&str> { + name.strip_suffix(&format!(".{BLOB_POINTER_EXTENSION}")) + .or_else(|| name.strip_suffix(&format!(".{LEGACY_BLOB_POINTER_EXTENSION}"))) } fn make_absolute(entry: &str, project_dir: &Path) -> PathBuf { @@ -86,9 +115,9 @@ pub fn materialize_lnk_entry( .ok_or_else(|| { FbuildError::PackageError(format!("invalid lnk path: {}", lnk_path.display())) })?; - let stripped = basename.strip_suffix(".lnk").ok_or_else(|| { + let stripped = strip_pointer_extension(basename).ok_or_else(|| { FbuildError::PackageError(format!( - "lnk path does not end in .lnk: {}", + "blob pointer does not end in .{BLOB_POINTER_EXTENSION} or .{LEGACY_BLOB_POINTER_EXTENSION}: {}", lnk_path.display() )) })?; @@ -170,12 +199,35 @@ mod tests { assert!(err.contains("simulated fetch failure"), "got: {err}"); } + /// FastLED/fbuild#1369: `.fetch` is the build-time blob pointer. It must + /// be recognized everywhere `.lnk` is, or an ESP32 `embed_files` entry + /// written by `fbuild lnk add` reaches objcopy as a literal JSON file. + #[test] + fn fetch_extension_is_a_blob_pointer() { + assert!(is_blob_pointer(Path::new("foo.fetch"))); + assert!(is_blob_pointer(Path::new("path/to/foo.bin.fetch"))); + assert!(!is_blob_pointer(Path::new("foo.fetch.bak"))); + } + + /// The suffix strip must follow the extension, or a `.fetch` entry is + /// rejected with "does not end in .lnk" — the pointer is recognized and + /// then refused, which is worse than not recognizing it. + #[test] + fn blob_pointer_basename_strips_whichever_extension_it_has() { + assert_eq!( + strip_pointer_extension("asset.bin.fetch"), + Some("asset.bin") + ); + assert_eq!(strip_pointer_extension("asset.bin.lnk"), Some("asset.bin")); + assert_eq!(strip_pointer_extension("asset.bin"), None); + } + #[test] - fn has_lnk_extension_handles_dotted_paths() { - assert!(has_lnk_extension(Path::new("foo.lnk"))); - assert!(has_lnk_extension(Path::new("path/to/foo.bin.lnk"))); - assert!(!has_lnk_extension(Path::new("foo.lnk.bak"))); - assert!(!has_lnk_extension(Path::new("foo"))); - assert!(!has_lnk_extension(Path::new("foo.bin"))); + fn legacy_lnk_extension_is_still_a_blob_pointer() { + assert!(is_blob_pointer(Path::new("foo.lnk"))); + assert!(is_blob_pointer(Path::new("path/to/foo.bin.lnk"))); + assert!(!is_blob_pointer(Path::new("foo.lnk.bak"))); + assert!(!is_blob_pointer(Path::new("foo"))); + assert!(!is_blob_pointer(Path::new("foo.bin"))); } } diff --git a/crates/fbuild-toolchain/src/lnk/materialize.rs b/crates/fbuild-toolchain/src/lnk/materialize.rs index 8a10202d2..930c33e68 100644 --- a/crates/fbuild-toolchain/src/lnk/materialize.rs +++ b/crates/fbuild-toolchain/src/lnk/materialize.rs @@ -184,8 +184,13 @@ fn strip_lnk_suffix(rel: &Path) -> Result { let file_name = rel.file_name().and_then(|n| n.to_str()).ok_or_else(|| { FbuildError::PackageError(format!("cannot decode lnk file name: {}", rel.display())) })?; - let stripped = file_name.strip_suffix(".lnk").ok_or_else(|| { - FbuildError::PackageError(format!("lnk path does not end in .lnk: {}", rel.display())) + let stripped = super::strip_pointer_extension(file_name).ok_or_else(|| { + FbuildError::PackageError(format!( + "blob pointer does not end in .{} or .{}: {}", + super::BLOB_POINTER_EXTENSION, + super::LEGACY_BLOB_POINTER_EXTENSION, + rel.display() + )) })?; Ok(rel .parent() @@ -277,6 +282,32 @@ mod tests { assert_eq!(target, Path::new("/build/foo.bin")); } + /// FastLED/fbuild#1369: the target path drops whichever pointer + /// extension the file carries. Missing this is not a silent no-op — + /// `target_path_for` hard-errors, so a `.fetch` the scanner just found + /// would be refused at materialize time. + #[test] + fn target_path_strips_either_pointer_extension() { + assert_eq!( + target_path_for( + Path::new("/repo/data/asset.bin.fetch"), + Path::new("/repo"), + Path::new("/build") + ) + .unwrap(), + Path::new("/build/data/asset.bin") + ); + assert_eq!( + target_path_for( + Path::new("/repo/data/asset.bin.lnk"), + Path::new("/repo"), + Path::new("/build") + ) + .unwrap(), + Path::new("/build/data/asset.bin") + ); + } + #[test] fn target_path_rejects_non_lnk_suffix() { let err = target_path_for( @@ -286,7 +317,7 @@ mod tests { ) .unwrap_err() .to_string(); - assert!(err.contains("does not end in .lnk"), "got: {err}"); + assert!(err.contains("does not end in .fetch or .lnk"), "got: {err}"); } #[test] diff --git a/crates/fbuild-toolchain/src/lnk/mod.rs b/crates/fbuild-toolchain/src/lnk/mod.rs index f3c8f46e5..b433022c0 100644 --- a/crates/fbuild-toolchain/src/lnk/mod.rs +++ b/crates/fbuild-toolchain/src/lnk/mod.rs @@ -28,7 +28,10 @@ pub mod materialize; pub mod resolver; pub mod scanner; -pub use embed::{expand_lnk_entries, has_lnk_extension, materialize_lnk_entry}; +pub use embed::{ + BLOB_POINTER_EXTENSION, LEGACY_BLOB_POINTER_EXTENSION, expand_lnk_entries, is_blob_pointer, + materialize_lnk_entry, strip_pointer_extension, +}; pub use format::{ExtractMode, LnkFile}; pub use materialize::{MaterializedLnk, materialize_all, materialize_one}; pub use resolver::{ResolvedBlob, resolve}; diff --git a/crates/fbuild-toolchain/src/lnk/scanner.rs b/crates/fbuild-toolchain/src/lnk/scanner.rs index 3152fb321..928a74d6b 100644 --- a/crates/fbuild-toolchain/src/lnk/scanner.rs +++ b/crates/fbuild-toolchain/src/lnk/scanner.rs @@ -42,7 +42,7 @@ pub fn scan_for_lnk(root: &Path) -> Result> { continue; } let path = entry.path(); - if path.extension().and_then(|s| s.to_str()) != Some("lnk") { + if !super::is_blob_pointer(path) { continue; } match LnkFile::from_path(path) { @@ -51,11 +51,27 @@ pub fn scan_for_lnk(root: &Path) -> Result> { lnk, }), Err(e) => { - warn!( - path = %path.display(), - error = %e, - "skipping malformed .lnk file" - ); + // FastLED/fbuild#1369: a `.lnk` that will not parse as JSON + // is more likely FastLED's runtime asset link — plain text, + // read on the MCU by `fl::parse_lnk`, and none of fbuild's + // business — than a corrupt blob pointer. Say so, instead of + // calling someone's correct file malformed. A `.fetch` is + // unambiguously ours, so that one keeps the blunt wording. + if path.extension().and_then(|s| s.to_str()) + == Some(super::LEGACY_BLOB_POINTER_EXTENSION) + { + warn!( + path = %path.display(), + error = %e, + "skipping .lnk that is not fbuild's JSON blob-pointer format; if this is a runtime asset link consumed by fl::parse_lnk, it is not fbuild's to resolve and this warning is expected" + ); + } else { + warn!( + path = %path.display(), + error = %e, + "skipping malformed blob pointer" + ); + } } } } @@ -132,6 +148,22 @@ mod tests { assert_eq!(found[0].lnk.url, "https://x/g.bin"); } + /// FastLED/fbuild#1369: the scanner must find what `fbuild lnk add` + /// now writes, and must go on finding what it wrote before. + #[test] + fn finds_both_pointer_extensions_in_one_tree() { + let dir = tempfile::tempdir().unwrap(); + write_valid_lnk(&dir.path().join("new.bin.fetch"), "https://x/new.bin"); + write_valid_lnk(&dir.path().join("old.bin.lnk"), "https://x/old.bin"); + let mut found: Vec = scan_for_lnk(dir.path()) + .unwrap() + .into_iter() + .map(|d| d.lnk.url) + .collect(); + found.sort(); + assert_eq!(found, vec!["https://x/new.bin", "https://x/old.bin"]); + } + #[test] fn directory_with_lnk_extension_is_ignored() { let dir = tempfile::tempdir().unwrap(); diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 49052a14a..18c4d468f 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -166,9 +166,9 @@ known limitations. | `fbuild clang-tidy` | Run clang-tidy against project sources. | | `fbuild iwyu` | Run include-what-you-use analysis. | | `fbuild clang-query` | Run a clang-query matcher. | -| `fbuild lnk pull` | Fetch `.lnk` resource blobs into the cache. | -| `fbuild lnk check` | Verify cached `.lnk` resources. | -| `fbuild lnk add ` | Create a `.lnk` manifest for a remote blob. | +| `fbuild lnk pull` | Fetch every blob pointed at by a `.fetch` (or legacy `.lnk`) into the cache. | +| `fbuild lnk check` | Verify cached blobs still match their recorded sha256. | +| `fbuild lnk add ` | Create a `.fetch` manifest for a remote blob (FastLED/fbuild#1369). | | `fbuild mcp` | Start the MCP server for AI assistant integration. | ### `fbuild ide`