perf(deploy): make espflash a build-time-optional cargo feature#139
perf(deploy): make espflash a build-time-optional cargo feature#139
Conversation
The espflash crate (pulled in by PRs #74, #106, and the FLASH_MD5SUM verify path) ships with ~30 transitive deps (strum, deku, miette, esp-idf-part, ...). Those deps were being pulled into every fbuild build whether or not the operator ever set FBUILD_USE_ESPFLASH_*. Make espflash and md-5 optional in fbuild-deploy, introduce an `espflash-native` cargo feature that turns them on, and forward the feature from fbuild-daemon. Gate the runtime toggles (`with_native_*` builders, env-var check helpers, call sites) behind the same cfg, so the struct layout, helper fns, and daemon routing only pay their cost when the feature is enabled. Runtime behaviour when the feature IS enabled is unchanged: the FBUILD_USE_ESPFLASH_VERIFY / FBUILD_USE_ESPFLASH_WRITE env vars still control the native-vs-esptool choice. Runtime behaviour when the feature is disabled (the new default): FBUILD_USE_ESPFLASH_* are ignored; esptool subprocess is the only path. Operators who relied on those env vars on a stock build need to rebuild with `--features fbuild-daemon/espflash-native`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 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 |
Summary
Touches issue #66 but not in the originally-requested direction. The
espflash-based verify+write migration already landed in PRs #74 / #106 — the remaining observable cost is that every fbuild build pullsespflash(and its ~30 transitive deps:strum,deku,miette,esp-idf-part, ...) whether or not the operator ever setsFBUILD_USE_ESPFLASH_*.This PR walks that back:
espflash+md-5become optional infbuild-deploy, and a newespflash-nativecargo feature onfbuild-deploy+fbuild-daemongates them in.Change
Default build (new behaviour):
espflash,md-5, and their transitive deps are not compiled.esp32_native.rsis not compiled.FBUILD_USE_ESPFLASH_VERIFY/FBUILD_USE_ESPFLASH_WRITEare ignored; the daemon always takes the esptool-subprocess path.--features fbuild-daemon/espflash-native.--features espflash-nativebuild:main. Env vars still pick native-vs-esptool.Feature gates (what is cfg-gated)
fbuild-deploy:pub mod esp32_native;Esp32Deployer::{use_native_verify, use_native_write}struct fields;Esp32Deployer::{with_native_verify, with_native_write}builders;try_verify_deployment_native,try_deploy_nativehelper fns. Native-path dispatch arms inside the public methods are also#[cfg(feature = "espflash-native")].fbuild-daemon:native_verify_enabled/native_write_enabledhelper fns, plus the sole call site that wireswith_native_{verify,write}.Quality gates (verified locally on Windows 10)
uv run cargo clippy --workspace --all-targets -- -D warnings— clean (default, no feature).uv run cargo clippy --workspace --all-targets --features fbuild-daemon/espflash-native -- -D warnings— clean.uv run cargo test -p fbuild-deploy— 36 pass, 0 fail, 8 ignored.uv run cargo fmt --all --check— clean.Scope / Non-goals
Test plan
🤖 Generated with Claude Code