fix(esp8266): fetch the core from a release asset so submodules come with it - #1398
Merged
Conversation
…with it Closes #1380. The ESP8266 core was fetched from `github.com/esp8266/Arduino/archive/refs/tags/3.1.2.tar.gz` — GitHub's auto-generated source archive, which omits submodules by design. The directories are created; the contents are not. `esp8266/Arduino` keeps five: `libraries/LittleFS/lib/littlefs`, `libraries/SoftwareSerial`, `libraries/ESP8266SdFat`, `tools/sdk/lwip2/builder`, and `tools/sdk/ssl/bearssl`. So any sketch reaching `<LittleFS.h>` failed inside the core's own header: LittleFS.h:38:10: fatal error: ../lib/littlefs/lfs.h: No such file The failure is not guardable from the consumer side, which is what makes it nastier than a missing package: `__has_include(<LittleFS.h>)` passes, because the header is present and only the thing it includes is absent. FastLED's esp8266 badge went red on master for this. The release asset published on the tag bundles the submodule contents. Verified against 3.1.2 before switching: 47 entries under `libraries/LittleFS/lib/littlefs`, `lfs.h` among them. The archive root also changes (`Arduino-3.1.2/` -> `esp8266-3.1.2/`), which is fine — `find_framework_root` discovers the root by looking for `cores/` rather than by name — and `.zip` already dispatches to `extract_zip` on extension. The test asserts the URL *shape*, not the string. A future version bump must not drift back to `/archive/refs/`, and that is the easy mistake to make since every other core in this crate uses that form. ## Other cores The issue asks whether others are exposed. Checked each core URL's repo for `.gitmodules` at its pinned tag: esp8266/Arduino@3.1.2 5 submodules <- this bug adafruit/ArduinoCore-samd@1.7.16 2 submodules, both under libraries/ (Adafruit_TinyUSB_Arduino, Adafruit_ZeroDMA) SiliconLabs/arduino@2.2.0 1 submodule (extra/core-api) ArduinoCore-avr, -mbed, -API, ATTinyCore none samd carries the same latent failure — a sketch including TinyUSB or ZeroDMA would break identically. Not fixed here: it needs its own verification that a bundling artifact exists, and bundling this into an esp8266 fix would make both harder to review. Reported on the issue instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
…test
The four-space-indented sample in the `ESP8266_FRAMEWORK_URL` doc comment is
a Markdown code block, so rustdoc treated it as Rust and tried to compile
`LittleFS.h:38:10: fatal error: ...`:
error: expected one of `!` or `::`, found `.`
Fenced as ```text.
Missed locally because I verified with `cargo test --lib`, which does not run
doctests. `--doc` does, and now passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 5, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1380.
The fix
The ESP8266 core was fetched from
github.com/esp8266/Arduino/archive/refs/tags/3.1.2.tar.gz— GitHub's auto-generated source archive, which omits submodules by design. The directories are created; the contents are not. Exactly the empty-lib/littlefs/the issue reported.Switched to the release asset on the same tag, which bundles them.
Verified before switching rather than assuming — downloaded
esp8266-3.1.2.zipand inspected it:Two things that change and are fine:
Arduino-3.1.2/→esp8266-3.1.2/.find_framework_rootlocates the root by looking for acores/subdirectory, not by name..tar.gz→.zip.extractor.rsalready dispatches on extension toextract_zip.Why this one is worse than a normal missing package
__has_include(<LittleFS.h>)passes in the broken state — the header is present, only the thing it includes is missing. So the failure lands inside the core's own header, past every capability guard a consumer could write. There is no preprocessor test that detects it.The test asserts shape, not the string
A future version bump must not drift back to
/archive/refs/. That is the easy mistake, because every other core in this crate uses that URL form — so the guard checks the form, names the five submodules that would silently empty out, and pins that the asset URL tracks the version const.Other cores — the issue asks, so I checked
Queried each core URL's repo for
.gitmodulesat its pinned tag:esp8266/Arduino@3.1.2adafruit/ArduinoCore-samd@1.7.16libraries/Adafruit_TinyUSB_Arduino,libraries/Adafruit_ZeroDMASiliconLabs/arduino@2.2.0extra/core-apiArduinoCore-avr,-mbed,-API,ATTinyCoresamd carries the same latent failure. Both of its submodules are under
libraries/, so a sketch including TinyUSB or ZeroDMA headers breaks identically and just as unguardably.Not fixed here. It needs its own verification that a bundling artifact actually exists for that core (I have not confirmed one does), and folding an unverified second core into an esp8266 fix would make both harder to review. Reported on the issue as a follow-up.
SiliconLabs/arduino'sextra/core-apimay or may not be on an include path — unknown, and I did not want to guess.Verification
soldr cargo test -p fbuild-library --lib esp8266— 4 passed-D warnings— cleanWhat I have not done is a real ESP8266 build against the new package — that needs the toolchain download, and the board build will exercise it in CI.