fix(esp32): add FBUILD_ESPTOOL_PATH and stop misreporting provisioning failures (#1220)

[zackees]

Closes#1220.

Both asks from the issue, plus the misleading error message it called out.

1. FBUILD_ESPTOOL_PATH

esptool was the only provisioned tool without an FBUILD_*_PATH override, alongside the seven that have one (FBUILD_WCHISP_PATH, FBUILD_PROBE_RS_PATH, …). That matters more than it looks: FBUILD_ is the only prefix that survives the daemon's env_clear (crates/fbuild-cli/src/daemon_client.rs:1045), so it's the only escape hatch that reliably reaches the daemon. During the #1217 outage there was no supported way to point fbuild at a known-good esptool.

Resolution order in Esptool::ensure_installed:

FBUILD_ESPTOOL_PATH	Behavior
unset / empty / whitespace	provision normally
names a file	use it, skip provisioning entirely
set but names nothing (or a directory)	hard error

The last row is deliberate. Silently falling back to provisioning would defeat the escape hatch — the user set it precisely because provisioning is what's broken, so a typo has to be loud. It's checked before platform.json is consulted, so a metadata failure can't preempt the override on the way to it.

2. Provisioning failure is now visible where it happens

resolve_esptool logged a tracing::warn! and returned None. In #1217 the real fault — a 404 on .../download/vunknown/esptool-linux-amd64.zip — was invisible at default verbosity, and the build ran ~3 more minutes before dying at elf2image.

It now logs at error level with the three things you need to diagnose it: the metadata URL, the parsed version, and the download URL that was actually tried. Esptool::version() / Esptool::download_url() are new accessors for exactly that.

3. The elf2image error no longer says "pip install esptool"

The old message was one string for two different faults, and it was wrong in the case that actually happened: esptool 5.1.0 was installed in #1217, it just wasn't on the daemon's PATH.

• esptool was provisioned but won't launch → names the executable; does not claim provisioning failed, because it didn't.
• provisioning failed and the PATH fallback found nothing → points at the earlier error line for the URL/version, and at FBUILD_ESPTOOL_PATH as the fix.

The pip install advice is gone from both. It was doubly wrong for the daemon, whose scrubbed PATH may not see a user pip install at all.

Tests

9 new unit tests, all passing:

• override_unset_provisions_normally, override_pointing_at_a_file_wins, override_pointing_nowhere_is_an_error_not_a_fallback, override_pointing_at_a_directory_is_an_error, empty_override_is_treated_as_unset
• download_url_reports_the_parsed_version (the vunknown shape from regression (2.5.4): ESP32 elf2image dies on bare esptool — version parser never resolves pioarduino URL, and the PATH fallback it relies on broke #1217), download_url_matches_the_url_ensure_installed_uses
• esptool_spawn_failure_never_recommends_pip_install, esptool_spawn_failure_distinguishes_provisioned_from_fallback

Env-var tests serialize on a mutex and restore the prior value, matching the LPC21ISP_ENV_LOCK pattern already in fbuild-deploy.

Verified locally: soldr cargo test -p fbuild-library -p fbuild-build-esp --lib green, soldr cargo clippy -p fbuild-library -p fbuild-build-esp --all-targets -- -D warnings clean, soldr cargo fmt --all applied.

Scope

Does not touch #1219 (daemon binding tool resolution to spawn-time PATH) — that's the reason the fallback failed rather than merely being slow, and it stays independently open.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added support for configuring a custom esptool executable through FBUILD_ESPTOOL_PATH.
  • Validated custom paths can bypass automatic esptool provisioning.
  • Added clearer access to esptool version and download information.

• Bug Fixes

  • Invalid executable overrides now produce a clear, actionable error.
  • Provisioning failures now include relevant download details before falling back to PATH-based esptool.
  • Launch failures distinguish between configured overrides and PATH discovery, without suggesting unnecessary installation steps.

[coderabbitai]

Warning

Review limit reached

@zackees, you've reached your PR review limit, so we couldn't start this review.

Next review available in:25 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 95091e3e-2042-4198-999a-5796df1817fa

📥 Commits

Reviewing files that changed from the base of the PR and between 4a56f24 and 44680fa.

📒 Files selected for processing (2)

• crates/fbuild-build-esp/src/esp32/esp32_linker.rs
• crates/fbuild-library/src/library/esptool.rs

📝 Walkthrough

Walkthrough

The change adds FBUILD_ESPTOOL_PATH override support, validates override paths, preserves recoverable provisioning fallback, and adds distinct diagnostics for provisioned and PATH-based esptool launch failures.

Changes

Esptool override flow

Layer / File(s)	Summary
Override resolution and provisioning metadata crates/fbuild-library/src/library/esptool.rs, crates/fbuild-library/src/library/mod.rs	The library resolves and validates FBUILD_ESPTOOL_PATH, exposes version and download URL accessors, validates host platforms, and tests override cases.
Orchestrator resolution and fallback crates/fbuild-build-esp/src/esp32/orchestrator/packages.rs	The orchestrator returns valid overrides directly, treats invalid overrides as fatal, and logs recoverable provisioning failures before falling back to esptool on PATH.
Firmware spawn diagnostics crates/fbuild-build-esp/src/esp32/esp32_linker.rs	Firmware conversion uses distinct diagnostics for provisioned executable failures and PATH fallback failures. Regression tests verify the messages and underlying errors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• esptool: provisioning failure degrades silently to PATH, and there is no FBUILD_ESPTOOL_PATH override #1220 — Directly covers FBUILD_ESPTOOL_PATH support and actionable provisioning and fallback diagnostics.

Possibly related PRs

• FastLED/fbuild#988 — Extends the managed-esptool integration with override handling and failure diagnostics.
• FastLED/fbuild#991 — Directly relates to esptool provisioning and the NormalizedPath resolution flow.
• FastLED/fbuild#1036 — Directly modifies esptool provisioning and validation in the same library module.

Sequence Diagram(s)

sequenceDiagram
participant convert_firmware
participant resolve_esptool
participant esptool_path_override
participant Esptool
convert_firmware->>resolve_esptool: request esptool executable
resolve_esptool->>esptool_path_override: check FBUILD_ESPTOOL_PATH
esptool_path_override-->>resolve_esptool: validated override or no override
resolve_esptool->>Esptool: provision managed package when no override exists
Esptool-->>resolve_esptool: installed path or recoverable failure
resolve_esptool-->>convert_firmware: executable path or PATH fallback
convert_firmware->>convert_firmware: format branch-specific spawn diagnostic

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: adding FBUILD_ESPTOOL_PATH support and correcting provisioning failure diagnostics.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1220-esptool-override-and-diagnostics

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fbuild-build-esp/src/esp32/esp32_linker.rs`:
- Around line 92-124: Update esptool_spawn_failure_message for the Some(bin)
branch so it uses neutral wording such as “selected esptool executable could not
be launched,” covering both provisioned binaries and FBUILD_ESPTOOL_PATH
overrides. Keep the executable path, override guidance, and error details
unchanged, and preserve the distinct None PATH-fallback message.
In `@crates/fbuild-library/src/library/esptool.rs`:
- Around line 587-594: Extend the test empty_override_is_treated_as_unset to
also pass a whitespace-only override such as " \t" through with_env_override and
assert that esptool_path_override resolves to None, preserving the existing
empty-value assertion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3db03502-86ae-4148-b556-8a00f4fc48c1

📥 Commits

Reviewing files that changed from the base of the PR and between c81221d and 4a56f24.

📒 Files selected for processing (4)

• crates/fbuild-build-esp/src/esp32/esp32_linker.rs
• crates/fbuild-build-esp/src/esp32/orchestrator/packages.rs
• crates/fbuild-library/src/library/esptool.rs
• crates/fbuild-library/src/library/mod.rs