fix(compile): rewrite backslashes on the PIO no-compile-CWD fallback path by zackees · Pull Request #912 · FastLED/fbuild

zackees · 2026-07-01T01:48:10Z

Problem

Windows PIO builds under .build/pio/<board>/ don't have a .fbuild directory upstream of the output path, so compile_cwd_from_output() returns None. That drops compile_source() into the raw to_string_lossy() fallback for the source/output args (lines 660-664 of compiler.rs before this PR), leaving backslashes intact in argv.

GCC's internal spec-file pass then interprets each \ as an escape:

-c src\sketch\AutoResearchNet.cpp
    ↓
cc1plus.exe: fatal error: srcsketchAutoResearchNet.cpp: No such file or directory

Symptomatically identical to the class of bug that #875 / #885 fixed for the compile-CWD arm — the fallback arm was just missed at the time because it wasn't a hot path for local dev.

Fix

Mirror the same replace('\', "/") guard on the fallback arm so both shapes agree on Windows.

} else {
    let source_str = source.to_string_lossy().to_string();
    let output_str = output.to_string_lossy().to_string();
    if cfg!(windows) {
        (source_str.replace('\', "/"), output_str.replace('\', "/"))
    } else {
        (source_str, output_str)
    }
};

Reproduction

Before:

$ bash compile esp32dev --examples AutoResearch
build error: build failed: compilation failed for C:\Users\niteris\dev\fastled\.build\pio\esp32dev\src\sketch\AutoResearchNet.cpp:
cc1plus.exe: fatal error: srcsketchAutoResearchNet.cpp: No such file or directory

After:

$ bash compile esp32dev --examples AutoResearch
SUCCESS: AutoResearch
All compilations completed successfully in 02m:13s

Verified on a Windows 10 box with fresh WROOM flash — device boots, RPC works, testGpioConnection([[33,34]]) returns connected: true.

Version

Bump workspace 2.3.15 → 2.3.16.

🤖 Generated with Claude Code

Implements Phase 1 of the roadmap in #618: fbuild sync reads platformio.ini, classifies every lib_deps entry per env, and writes a deterministic JSON platformio.lock next to the ini. All flags in the issue proposal land: -e <env>, --yes, --locked, --check, --dry-run, --upgrade, --upgrade-package. Multi-env prompt gates whole-project sync; --yes / -e / --check bypass it. # Scope Phase 1 is a full CLI + classification + lockfile-shape ship. Network resolution (GitHub ref -> SHA, PIO registry version -> archive URL, archive sha256) is deferred to Phase 2 per the issue's staged rollout. Every remote entry gets status: "unresolved" with the raw spec + extracted owner/name/ref captured, so Phase 2 will only add fields, never renegotiate the schema. Local sources (symlink://, file://, filesystem paths — absolute POSIX, relative ./ / ../, Windows drive letters) get status: "unlocked" and are recorded verbatim for auditability. # Files Module layout (kept as one focused directory tree, three files, no new crate — respects the monocrate rule): - crates/fbuild-cli/src/sync/mod.rs — orchestrator + SyncArgs + run_sync + prompt logic - crates/fbuild-cli/src/sync/source.rs — SourceType + LockStatus + ClassifiedDep + classify() - crates/fbuild-cli/src/sync/lockfile.rs — Lockfile struct + JSON I/O + LockDiff comparison + atomic write via fbuild_core::fs::write_atomic_sync (from #865) - crates/fbuild-cli/src/cli/sync_cmd.rs — thin CLI adapter - crates/fbuild-cli/src/cli/args.rs — new Commands::Sync variant + KNOWN_SUBCOMMANDS entry - crates/fbuild-cli/src/cli/dispatch.rs — wire the new command - crates/fbuild-cli/src/cli/mod.rs — register sync_cmd - crates/fbuild-cli/src/main.rs — register sync module - docs/sync.md — user-facing docs - docs/CLAUDE.md — index entry # Lockfile schema (v1) - envs sorted alphabetically (BTreeMap) - packages per env sorted by (name, source_type, raw) - generated_at trimmed to seconds (ISO-8601 UTC) - fields with a documented consumer only (no decorative registry metadata — per issue decision) - package records DUPLICATED under each env (per issue decision: auditability > disk size) - read() refuses to load an unrecognized version — schema evolution is versioned # TDD-first — 30+ unit tests written before implementation Written first, then wired up: - crates/fbuild-cli/src/sync/source.rs::tests — 18 tests covering every documented lib_deps shape: - Registry: bare name, name@ver, owner/name@ver, whitespace, raw preservation - GitHub: bare URL, .git suffix, #ref, case-insensitive host - Git+: URL, ref - HTTP archive: .zip, .tar.gz - Local: symlink://, file://, ./relative, ../relative, /posix-abs, C:\ backslash, D:/ forward-slash - phase1_lock_status returns Unlocked for locals, Unresolved for remotes - crates/fbuild-cli/src/sync/lockfile.rs::tests — 10 tests: - Shape smoke - Packages sorted by name - Envs sorted - Local dep -> Unlocked - Registry dep -> Unresolved in Phase 1 - JSON deterministic for same input (regardless of insertion order) - Read/write roundtrip - Version mismatch rejected - compare fresh/stale detection (new dep added, env added, version changed) - JSON ends with newline - crates/fbuild-cli/src/sync/mod.rs::tests — 11 tests: - Missing platformio.ini is error - Single env writes lockfile - --check on missing lock is failed - --check on fresh lock passes - --locked on stale lock returns LockedFailed - --dry-run writes nothing - -e <env> skips multi-env prompt - -e <bad-env> is error - Second run without changes is NoOp - exit_code matrix (Wrote/NoOp/CheckPassed/DryRun=0, CheckFailed=1, LockedFailed=2, UserCancelled=3, Error=4) - skip_multi_env_prompt matrix - ISO date formatter basic + time-of-day # Local verification - `soldr cargo check -p fbuild-cli --all-targets` — clean - `soldr cargo clippy -p fbuild-cli --all-targets -- -D warnings` — clean - `soldr cargo test -p fbuild-cli --no-run` — clean (test binaries build) - `soldr cargo fmt -p fbuild-cli` — no-op Note on test execution: my local environment (soldr silently swallows all stdout/stderr; direct-cargo hits Windows SDK linker env issues that this session has documented at length in #899) prevents me from running the test binary end-to-end today. Every test is pure enough that the compile-clean under -D warnings + the TDD-first authoring order gives strong confidence, and CI will exercise them on Linux. Closes #618

CI on macos/windows failed with clippy `-D dead-code`: - SyncArgs.upgrade + upgrade_package — Phase 2 fields, captured from the CLI now so the argv surface is stable but not yet consumed. - SyncOutcome::Wrote(PathBuf) — payload used by structured callers (tests + future --json subcommand). Clippy doesn't see the tests as consumers. Annotate with #[allow(dead_code)] at the field / enum level with a comment pointing at #618's Phase 2.

…pile-CWD fallback path Windows PIO builds under `.build/pio/<board>/` don't have a `.fbuild` component upstream of the output path, so `compile_cwd_from_output()` returns None and `compile_source()` fell through to the raw `to_string_lossy()` on the source/output paths. That leaves backslashes intact in argv, which GCC's internal spec-file pass interprets as escape characters: `-c src\sketch\AutoResearchNet.cpp` → cc1plus.exe receives `srcsketchAutoResearchNet.cpp` → "fatal error: srcsketchAutoResearchNet.cpp: No such file or directory". Symptomatically identical to the fix that landed in #875 / #885 for the compile-CWD arm; the fallback arm was missed. Mirror the same `replace('\', "/")` guard so both paths agree on Windows. Bump workspace version 2.3.15 → 2.3.16. Repro: `bash compile esp32dev --examples AutoResearch` from a FastLED checkout on Windows failed at the first .cpp compile with `srcsketchAutoResearchNet.cpp` / `srcsketchAutoResearchBle.cpp` etc. After this fix, the compile completes end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-07-01T01:48:18Z

Warning

Review limit reached

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

Next review available in: 50 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

Run ID: a92de97f-15ed-4f57-ae0c-b3a37e5f6b1a

📥 Commits

Reviewing files that changed from the base of the PR and between b0c3622 and 26bd23f.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (12)

Cargo.toml
crates/fbuild-build/src/compiler.rs
crates/fbuild-cli/src/cli/args.rs
crates/fbuild-cli/src/cli/dispatch.rs
crates/fbuild-cli/src/cli/mod.rs
crates/fbuild-cli/src/cli/sync_cmd.rs
crates/fbuild-cli/src/main.rs
crates/fbuild-cli/src/sync/lockfile.rs
crates/fbuild-cli/src/sync/mod.rs
crates/fbuild-cli/src/sync/source.rs
docs/CLAUDE.md
docs/sync.md

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/pio-compile-source-backslash

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.}

… (#3476) Lands the real-hardware `I2sPeripheralEsp32DevEsp` class that implements `II2sPeripheralEsp32Dev` against IDF v4's `driver/i2s.h` DMA + ISR machinery. Completes the peripheral surface that was scaffolded in #3473 (Stage 1). ## What Stage 2 delivers - `i2s_peripheral_esp32dev_esp.h` — declares the real-hw class paralleling the mock. Same interface, injectable through the engine's ctor exactly like the mock. - `i2s_peripheral_esp32dev_esp.cpp.hpp` — implementation using IDF v4's `i2s_driver_install()` (DMA + ISR under the hood) + `i2s_write()` for async submit + `heap_caps_malloc(MALLOC_CAP_DMA)` for DMA-capable buffer alloc. Completion callback fires synchronously after `i2s_write` (which returns once the buffer has landed in the driver's DMA queue) — Stage 3 replaces this with a real DMA-done ISR trampoline. ## Deliberately not linked yet The new `.cpp.hpp` is intentionally NOT included in the classic- ESP32 `_build.cpp.hpp` unity build. Its `#include "driver/i2s.h"` pulls in IDF's `driver_ng` framework which conflicts at boot with the legacy register access in `i2s_esp32dev.cpp.hpp` (Yves Bazin's long-standing driver), producing: E (586) ADC: CONFLICT! driver_ng is not allowed to be used with the legacy driver abort() was called at PC 0x40154717 on core 0 → boot loop. Wiring the real-hw impl requires deleting the Yves TU first — that's Stage 3 scope. In the meantime, this PR delivers the type declaration (so future PRs can `#include` the header and construct the class) plus the fully-formed .cpp.hpp waiting to be linked once the Yves cleanup lands. ## Verification - `bash compile esp32dev --examples AutoResearch` — succeeds (fbuild fix for the PIO source-path backslash regression landed separately in FastLED/fbuild#912; without that fix even master was blocked on this box). - Flashed the resulting firmware to a WROOM on COM11; device boots cleanly (no ADC conflict, no core-dump abort), emits the standard `ready` RPC event with `pinRx:19, pinTx:18, drivers:5`, responds to `testGpioConnection([[33, 34]])` with `connected: true` (matching pre-#3473 behaviour bit-for-bit). - `bash test --cpp` — host suite **344/344** pass (Stage 1's 21 engine tests still green; new real-hw impl's presence is a no-op on host builds via the `#ifdef FL_IS_ESP32` guard). - `clang-format --dry-run --Werror` clean on every touched file. ## Follow-up (Stage 3, tracked separately) - Delete `i2s_esp32dev.{h,cpp.hpp}` + `clockless_i2s_esp32.{h,cpp.hpp}` (Yves Bazin's TU) so the driver_ng conflict clears. - Include `i2s_peripheral_esp32dev_esp.cpp.hpp` in the `_build.cpp.hpp`. - Wire `BusTraits<Bus::FLEX_IO, 0>` on classic ESP32 to construct `ChannelEngineI2sEsp32Dev` with a fresh `I2sPeripheralEsp32DevEsp` peripheral. - Fold `drivers/i2s_spi/` (SPI-only Bus::FLEX_IO,0 today) into the merged engine per the parallel-IO code-review rule. - Replace the sync completion callback with an ISR trampoline. - Replace the byte-copy `packScratchBuffer()` with the parallel bit-transpose + wave8 slot expansion. - WROOM on-device WS2812B decode via `bash autoresearch --i2s`. Ref #3474. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The #618 sync module and #912's backslash-rewrite fix were combined into a single squash-merge (ecd80c7). Four clippy fixes I'd already shipped on the sync branch didn't ride along, so main's `Check macOS` run is red on `-D warnings`. Reapplying: - fbuild-serial/src/boards.rs — field-reassign after Default::default() in the 1200bps-touch upload-hint test (from #906 carryover). Use struct-literal syntax + ..Default::default(). - fbuild-cli/src/sync/source.rs — swap the closure form `.rsplit(|c| c == '/' || c == '\')` for the array-pattern form `.rsplit(['/', '\'])`. Same behavior, satisfies clippy::manual_pattern_char_comparison. - fbuild-cli/src/sync/mod.rs — fold the nested `if !prompt_multi_env(..)` into the outer condition (clippy::collapsible_if). Same behavior. - fbuild-cli/src/cli/sync_cmd.rs — add `#[allow(clippy::too_many_arguments)]` on run_sync_cmd; the 8 args mirror clap's parsed variant 1:1 and wrapping them in a struct here would just push the noise into the dispatch site. No behavior changes.

zackees and others added 3 commits June 30, 2026 18:19

zackees merged commit ecd80c7 into main Jul 1, 2026
78 of 93 checks passed

zackees mentioned this pull request Jul 1, 2026

feat(i2s-esp32dev): Stage 2 — real-hardware I2S peripheral impl (#3474) FastLED/FastLED#3476

Merged

4 tasks

fastled-project-sync Bot added this to FastLED Tracker Jul 1, 2026

fastled-project-sync Bot moved this to Triage in FastLED Tracker Jul 1, 2026

zackees mentioned this pull request Jul 1, 2026

feat(dylints): ban manual backslash-to-slash rewrite; migrate all sites to NormalizedPath::display_slash() (#911) #914

Merged

zackees mentioned this pull request Jul 5, 2026

perf(build): framework libs (~150s) recompiled per project; core cache never hydrates #952

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(compile): rewrite backslashes on the PIO no-compile-CWD fallback path - #912

fix(compile): rewrite backslashes on the PIO no-compile-CWD fallback path#912
zackees merged 3 commits into
mainfrom
fix/pio-compile-source-backslash

zackees commented Jul 1, 2026

Uh oh!

coderabbitai Bot commented Jul 1, 2026

Review limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

zackees commented Jul 1, 2026

Problem

Fix

Reproduction

Version

Uh oh!

coderabbitai Bot commented Jul 1, 2026

Review limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant