From bc11a40e86b2bc84b3807a9efd5dd5ad377697c5 Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 10:55:22 -0700 Subject: [PATCH 1/3] feat(lnk): write build-time blob pointers as `.fetch` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.lnk` served two roles with different consumers, formats and guarantees: FastLED's runtime asset link (plain text, read on the MCU by `fl::parse_lnk`, sha256 optional) and fbuild's build-time blob pointer (JSON, read on the build host, sha256 mandatory because the resolver caches by content). Sharing one extension read as one concept, so people normalized toward whichever form they met first — converting a runtime link to fbuild's JSON schema looked like tidying, and `fl::parse_lnk` then took `{` as the URL (FastLED/FastLED#4012). A test caught that one; a sketch author gets a silently wrong URL. Per FastLED/fbuild#1369 the roles get distinct extensions rather than converged formats — converging on JSON means carrying a JSON parser on the MCU for no user-visible benefit, and converging on text means giving up the mandatory digest the content-addressed cache depends on. `.lnk` keeps the runtime role. Build-time pointers are `.fetch`: it names what the build does with the file, and `foo.bin.fetch` cannot be misread as being the blob the way `foo.bin.blob` can. Three places stripped `".lnk"` by hand and would each have refused a `.fetch` *after* recognizing it — `strip_pointer_extension` is now the one definition, next to `is_blob_pointer` (which replaces `has_lnk_extension`, whose name would no longer be true). The e2e suite proves both halves: the main pipeline run uses `.fetch`, and the sha-mismatch run deliberately keeps `.lnk` so back-compat is demonstrated rather than asserted in a comment. The scanner's malformed-file warning now distinguishes the two. A `.lnk` that will not parse as JSON is far more likely a runtime asset link — not fbuild's to resolve — than a corrupt pointer, so calling it "malformed" was telling the user their correct file was broken. `.fetch` is unambiguously ours and keeps the blunt wording. Out of scope, per the issue's own open questions: the `fbuild lnk` subcommand name, a deprecation window for `.lnk` reads, and whether `assets.json` folds into this form. Co-Authored-By: Claude Opus 5 (1M context) --- agents/docs/commands-reference.md | 2 +- .../src/esp32/orchestrator/embed_stage.rs | 2 +- crates/fbuild-cli/src/cli/args.rs | 26 ++++--- crates/fbuild-cli/src/cli/lnk.rs | 28 +++++-- crates/fbuild-packages/tests/lnk_e2e.rs | 12 ++- crates/fbuild-toolchain/src/lnk/README.md | 37 +++++++-- crates/fbuild-toolchain/src/lnk/embed.rs | 78 +++++++++++++++---- .../fbuild-toolchain/src/lnk/materialize.rs | 37 ++++++++- crates/fbuild-toolchain/src/lnk/mod.rs | 5 +- crates/fbuild-toolchain/src/lnk/scanner.rs | 44 +++++++++-- docs/reference/cli.md | 6 +- 11 files changed, 226 insertions(+), 51 deletions(-) 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..113b9d40b 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs @@ -21,7 +21,7 @@ fn expand_embed_entries( } 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(), 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..3eb8cca92 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 ] ``` 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` From 8baffd5195ff1c6cc68c30ff0b8bf354089db936 Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 12:24:49 -0700 Subject: [PATCH 2/3] fix(lnk): refuse colliding blob pointers, and finish the doc migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on FastLED/fbuild#1369. A materialized target is named after the pointer's blob, so two pointers whose blob names match land on one path: the second overwrites the first and both embed entries end up holding the second blob's bytes. Firmware then carries the wrong asset with nothing to indicate it. That is pre-existing for two `.lnk` in different directories, but `.fetch` adds the case where `logo.bin.fetch` and `logo.bin.lnk` sit in the *same* directory — which is exactly what a half-finished migration looks like, and `fbuild lnk add` will happily produce it next to an existing `.lnk`. The resolver now refuses, naming both entries and the path they collide on. Confirmed by a test that fails on the previous code, where the collision resolves silently. Also: the "disk cache unavailable" error said `.lnk entries` on a branch that now takes both spellings, and the README's later `embed_files` example and integration notes still presented `.lnk` as the current format while the introduction called it legacy. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/esp32/orchestrator/embed_stage.rs | 80 ++++++++++++++++++- crates/fbuild-toolchain/src/lnk/README.md | 16 ++-- 2 files changed, 85 insertions(+), 11 deletions(-) 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 113b9d40b..c8abb99d1 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,6 +16,12 @@ 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. + 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) @@ -24,10 +31,17 @@ fn expand_embed_entries( 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)?; + if let Some(first) = claimed.insert(materialized.target_path.clone(), 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 +51,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 +166,64 @@ 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}" + ); + } } diff --git a/crates/fbuild-toolchain/src/lnk/README.md b/crates/fbuild-toolchain/src/lnk/README.md index 3eb8cca92..be6f580fd 100644 --- a/crates/fbuild-toolchain/src/lnk/README.md +++ b/crates/fbuild-toolchain/src/lnk/README.md @@ -97,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) @@ -107,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 @@ -134,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` | @@ -143,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?** From 495148534aebc288ea30c4df93de7f2c4657bf92 Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 15:44:10 -0700 Subject: [PATCH 3/3] fix(lnk): compare materialized targets case-insensitively where the OS does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on FastLED/fbuild#1369. The collision guard keyed claimed targets by `PathBuf` equality. Windows and macOS fold case, so `logo.bin.fetch` and `LOGO.bin.lnk` materialize to one file while comparing unequal as paths — the guard let through exactly the collision it was written to catch, on the platforms where it actually happens. Keyed by `fbuild_core::path::normalize_for_key` now, which is the existing shared representation for this: it strips Windows extended-length prefixes, normalizes separators, and folds case only on case-insensitive platforms. On Linux the two names remain distinct files and are still allowed. The new test is gated on a case-insensitive host and verified both ways — with lexical keying it fails, with the comparison key it passes. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/esp32/orchestrator/embed_stage.rs | 68 ++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) 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 c8abb99d1..1c10f2558 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/embed_stage.rs @@ -21,7 +21,12 @@ fn expand_embed_entries( // 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. - let mut claimed: std::collections::HashMap = std::collections::HashMap::new(); + // 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) @@ -36,7 +41,8 @@ fn expand_embed_entries( ) })?; let materialized = fbuild_packages::lnk::materialize_lnk_entry(&p, lnk_dir, cache)?; - if let Some(first) = claimed.insert(materialized.target_path.clone(), entry.clone()) { + 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() @@ -226,4 +232,62 @@ mod tests { "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}"); + } }