Skip to content

fix(esp8266): honor PlatformIO registry platform pins - #1532

Merged
zackees merged 2 commits into
mainfrom
fix/esp8266-registry-1496
Sep 28, 2026
Merged

zackees merged 2 commits into
mainfrom
fix/esp8266-registry-1496

Conversation

@zackees

@zackees zackees commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

Closes #1496

What changed

  • Resolve pinned espressif8266 platform aliases through the shared PlatformIO registry resolver, then select the manifest-derived Arduino framework and host-specific Xtensa toolchain with verified SHA-256 payloads.
  • Preserve framework archive overrides and reject unavailable or unsupported aliases instead of using the fixed 3.1.2 framework.
  • Report requested and concrete resolved platform/package versions in build output and include those identities in the fast-path fingerprint.
  • Keep fbuild install --check and --dry-run offline: use cached metadata/manifest or report would-fetch.
  • Add an offline fbuild-core fixture for the published 4.0.1 manifest, plus adapter and real firmware-build tests.

Validation

  • RED: explicit toolchain pin selected the fixed 3.2.0-gcc10.3 package before this change.
  • GREEN: espressif8266@4.0.1 built nodemcuv2 firmware with toolchain-xtensa@2.100300.220621 (GCC 10.3.0) and framework-arduinoespressif8266@3.30002.0; the installed core_version.h declares Arduino ESP8266 3.0.2.
  • Offline resolver fixture: 3 tests passed, including exact URL/checksum, alias forms, cache identity, and unavailable versions.
  • Engine/ESP8266 unit suites passed; full-workspace Clippy with -D warnings and rustfmt check passed.
  • Local pre-push review clean after adding a cache-only resolution path.

Summary by CodeRabbit

  • New Features
    • ESP8266 builds now resolve platform, framework, and toolchain versions from registry pins, including manifest requirements and explicit package overrides.
    • ESP8266 provisioning can use cached registry metadata and installed packages when running offline. If required data is unavailable, the build indicates which packages would need to be fetched.
  • Bug Fixes
    • Version-pin warnings now account for ESP8266 platform and package selections.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: FastLED/fbuild/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ff88a5b6-8e48-4507-aff3-e6ecf6597d46

📥 Commits

Reviewing files that changed from the base of the PR and between 9d5bfba and 90aa517.

📒 Files selected for processing (9)
  • .github/workflows/dylint.yml
  • crates/fbuild-build-engine/src/package_override.rs
  • crates/fbuild-build-engine/src/pipeline/context.rs
  • crates/fbuild-build-esp/src/esp8266/mod.rs
  • crates/fbuild-build-esp/src/esp8266/orchestrator.rs
  • crates/fbuild-config/src/platform_packages.rs
  • crates/fbuild-core/tests/platformio_esp8266_resolution.rs
  • crates/fbuild-toolchain/src/toolchain/esp8266.rs
  • tests/platform/esp8266/platformio.ini

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

ESP8266 platform and package registry pins now resolve through online and cache-only paths. Provisioning uses the selected mode, and build logs and fingerprints include requested and resolved package identities. The Dylint workflow also installs rustfmt for its pinned nightly.

Changes

ESP8266 registry pin resolution

Layer / File(s) Summary
Shared registry resolution
crates/fbuild-build-engine/src/package_override.rs, crates/fbuild-core/tests/platformio_esp8266_resolution.rs
The shared resolver supports online fetching and cache-only resolution. Offline resolution returns no result when required cached data is unavailable. Tests cover platform aliases, manifest package requirements, explicit framework pins, and unavailable versions or hosts.
ESP8266 package selection
crates/fbuild-build-esp/src/esp8266/orchestrator.rs, crates/fbuild-build-engine/src/pipeline/context.rs, crates/fbuild-config/src/platform_packages.rs, crates/fbuild-toolchain/src/toolchain/esp8266.rs
ESP8266 package selection applies registry overrides before environment overrides and defaults. Ignored-pin checks recognize ESP8266 platform and package pins. The toolchain can apply a package override.
Provisioning and build identity
crates/fbuild-build-esp/src/esp8266/*, tests/platform/esp8266/platformio.ini
Provisioning selects online or offline package lookup. Build logging and fast-path fingerprints include requested and resolved platform and package identities. Tests cover package overrides and the platform 4.0.1 build fixture.

Dylint rustfmt setup

Layer / File(s) Summary
Install rustfmt for the Dylint nightly
.github/workflows/dylint.yml
The workflow installs rustfmt for the pinned nightly before the library formatting check.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant PlatformSupport as Esp8266PlatformSupport
  participant Orchestrator as ESP8266 orchestrator
  participant Resolver as resolve_registry_overrides
  participant RegistryCache as Registry cache
  PlatformSupport->>Orchestrator: select online or offline package lookup
  Orchestrator->>Resolver: resolve platform package overrides
  Resolver->>RegistryCache: read metadata and package payloads
  RegistryCache-->>Resolver: return cached data
  Resolver-->>Orchestrator: return overrides or unavailable result
  Orchestrator-->>PlatformSupport: return package pair or no pair
Loading

Merge Risk: ⚪ Minimal · up to 90aa5

No concrete issue is established that should prevent merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 90aa5

The change makes pinned builds select the intended packages and includes checks on downloaded packages. A pre-existing cache-reuse weakness warrants hardening, but the available evidence does not establish that this change materially increases its security exposure.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A selected package affects the local build host through its toolchain and the resulting firmware through its framework; the examined change does not establish a new service or tenant-wide entrypoint.

Security Findings and Attack Paths

  • observed — The unchanged shared installer trusts an existing installation directory without rechecking its completion marker or payload. The changed online ESP8266 resolver can reach that path, but the evidence does not establish a materially worsened attacker-controlled route to the cache.

Trust Boundaries and Controls

  • observed — Registry metadata supplies package locations and digests; fresh installations verify downloaded archives, while explicit non-registry package overrides retain precedence.

Hardening Proposals

  • proposed — Align online existing-install reuse with the cache-only completion check, and define when reused build inputs must be revalidated after cache restore or corruption.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The change to .github/workflows/dylint.yml only installs rustfmt for the Dylint toolchain. The PR objective in #1496 concerns ESP8266 PlatformIO registry resolution and package selection. The work… Remove the unrelated .github/workflows/dylint.yml change, or provide a direct requirement that this PR must change the Dylint toolchain setup.
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 7 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: ESP8266 now honors PlatformIO registry platform pins.
Linked Issues check ✅ Passed The PR satisfies the coding requirements in #1496. The shared resolver handles short and owner-qualified ESP8266 aliases, resolves the 4.0.1 platform manifest, and selects framework 3.30002.0 and tool…
Full details: Out of Scope Changes check

Explanation

The change to .github/workflows/dylint.yml only installs rustfmt for the Dylint toolchain. The PR objective in #1496 concerns ESP8266 PlatformIO registry resolution and package selection. The workflow change has no demonstrated connection to that objective.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 7 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zackees
zackees merged commit 11c2b8a into main Sep 28, 2026
18 checks passed
@zackees
zackees deleted the fix/esp8266-registry-1496 branch September 28, 2026 00:20
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

fix(esp8266): honor registry version pins

1 participant