feat(lnk): accept the text-format .lnk alongside JSON - #1362
Conversation
Two incompatible `.lnk` formats exist in the FastLED ecosystem. fbuild
parsed only JSON, so text-format files were rejected with a confusing
"invalid .lnk JSON: expected value at line 1 column 1".
The text format is not legacy cruft — it has three implementations and is
live in the shipping product:
- fl::parse_lnk_with_metadata() src/fl/stl/url.h (C++ runtime)
- fl::asset / FL_ASSET src/fl/asset/asset.h (C++ API)
- _parse_lnk_content() ci/compiler/asset_scanner.py
`examples/AudioUrl/data/track.mp3.lnk` is committed on FastLED master in
that format and is unreadable by the tool named `lnk`.
`from_str_any()` now sniffs the first non-blank, non-comment character:
`{` routes to the JSON parser, anything else to a new text parser
matching the C++ grammar (first non-comment line is the URL, subsequent
key=value lines are metadata, unknown keys ignored for forward-compat).
`from_path()` uses it, so every existing caller gains both formats.
`sha256` stays mandatory for the text form too. The resolver caches by
content digest, so a `.lnk` without one cannot be content-addressed or
verified — weakening that to accommodate the text format would give up
the guarantee the cache is built on. The error names the file and says
how to fix it rather than failing generically.
JSON remains canonical: `fbuild lnk add` is unchanged.
Verified end to end with a real asset — text-format .lnk fetched and
sha256-verified into the blob cache, and a deliberately wrong digest
rejected without promoting the .partial. 43 lnk tests pass, 7 new.
Refs #1357, FastLED/FastLED#3986
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesLNK parsing
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR adds support for text-format .lnk files while preserving JSON handling and mandatory sha256 verification. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`examples/AudioUrl/data/track.mp3.lnk` carried no sha256, so fbuild could not verify or content-address it -- fbuild requires a digest on every `.lnk` (FastLED/fbuild#1357). Adding one to the existing URL would have pinned a digest to soundhelix.com, where any upstream re-encode breaks the build. The track is now mirrored into FastLED/assets and the `.lnk` points there, so the digest stays valid: sha256=cadd2666c5571e12fafdb21697b26aef6f196a5a9c28e708129870167679991d size =8,945,229 bytes Verified byte-identical against the origin before and after upload. The file stays in the TEXT format deliberately. I first converted it to fbuild's JSON schema and `fl_asset_asset` caught the mistake: the C++ runtime's `fl::parse_lnk` reads only the text form, so it took `{` as the URL and resolution returned host `{`. That is precisely the format split documented in FastLED/fbuild#1357, and the reason fbuild learned to read text `.lnk` files in FastLED/fbuild#1362 rather than the reverse. The test's host assertion moves with the URL, and gains a comment saying why the format cannot change here -- the next person to "tidy" this into JSON should hit the explanation before the failure. Note the released fbuild (2.5.20) predates #1362, so `fbuild lnk pull` will not read this file until the next release. The C++ runtime and the Python scanner both accept it today. bash lint clean, bash test --cpp passes, AudioUrl builds for WASM. Refs FastLED/fbuild#1357 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
`asset.h` said fbuild scans `<sketch>/data/` for `.lnk` files and emits a manifest. It does not -- `ci/compiler/asset_scanner.py` does, driven from `ci/wasm_build.py`, writing `fastled_js/asset_manifest.json`. The distinction matters because the two read different formats. fbuild's `.lnk` parser was JSON-only until FastLED/fbuild#1362; the scanner reads the text form this runtime parses, and now the JSON form too. Someone debugging a missing asset who believes fbuild produced the manifest looks in the wrong tool. Last item on FastLED/fbuild#1357 that is a plain factual fix; the remaining one there is the long-term format direction, which is a maintainer call. Refs FastLED/fbuild#1357 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem
Two incompatible
.lnkformats exist in the FastLED ecosystem, and fbuild parsed only one of them. A text-format file failed with:The text format is not legacy cruft — it has three implementations and is live in the shipping product:
src/fl/stl/url.h:274fl::parse_lnk(),:325parse_lnk_with_metadata()src/fl/asset/asset.h—FL_ASSET("data/track.mp3")ci/compiler/asset_scanner.py— emits the WASM manifestAnd
examples/AudioUrl/data/track.mp3.lnkis committed on FastLEDmasterin that format — meaning the one.lnkactually in the repo could not be read by the tool namedlnk.Change
from_str_any()sniffs the first non-blank, non-comment character:{routes to the existing JSON parser, anything else to a new text parser matching the C++ grammar — first non-comment line is the URL, subsequentkey=valuelines are metadata, unknown keys ignored for forward-compat.from_path()now uses it, so every existing caller (lnk pull,lnk check, the embed stage) gains both formats with no call-site changes. JSON remains canonical —fbuild lnk addis unchanged.sha256stays mandatoryThe text format treats the digest as optional; fbuild does not, and this PR keeps it that way. The resolver caches by content digest, so a
.lnkwithout one cannot be content-addressed or verified — relaxing that to accommodate the text format would give up the guarantee the whole cache is built on.Instead the error is made actionable:
Note this means
examples/AudioUrl/data/track.mp3.lnkstill needs a digest added — but it now gets a message saying so, instead of a JSON parse error pointing at column 1.Verification
End to end with a real asset, using a text-format
.lnk:And the integrity path, using a deliberately wrong digest — the fetch is rejected and the
.partialis never promoted:#-before-JSON rejected as malformed JSON (documenting that boundary), and JSON/text forms producing an identicalLnkFile.cargo clippy -p fbuild-toolchain --libclean.Refs #1357, FastLED/FastLED#3986
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
.lnkfiles.Bug Fixes