fix(devcontainer): install platformio as the vscode user - #11264
fix(devcontainer): install platformio as the vscode user#11264b-anantha wants to merge 1 commit into
Conversation
pipx ran in the Dockerfile as root, before the python feature sets PIPX_HOME/PIPX_BIN_DIR.
@b-anantha, Welcome to Meshtastic!Thanks for opening your first pull request. We really appreciate it. We discuss work as a team in discord, please join us in the #firmware channel. Welcome to the team 😄 |
📝 WalkthroughWalkthroughPlatformIO installation was removed from the devcontainer image build and added to the container setup script before esptool installation. ChangesDevcontainer tooling
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.devcontainer/setup.sh:
- Around line 7-8: Update the installation commands in the setup script so a
failure from pipx install platformio immediately terminates the script and
prevents the esptool step from masking it; enable fail-fast behavior or
explicitly chain the commands with &&.
🪄 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: 6edad5cd-d382-413e-992e-a337e39f3c27
📒 Files selected for processing (2)
.devcontainer/Dockerfile.devcontainer/setup.sh
💤 Files with no reviewable changes (1)
- .devcontainer/Dockerfile
| pipx install platformio | ||
| pipx install esptool |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not mask PlatformIO installation failures.
If pipx install platformio fails, the script continues to pipx install esptool; when that succeeds, postCreateCommand returns success even though pio is missing. Make the script fail fast so container creation cannot silently complete without PlatformIO.
Proposed fix
#!/usr/bin/env sh
+set -e
git submodule update --initAlternatively, explicitly chain the installation commands with &&.
🤖 Prompt for 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.
In @.devcontainer/setup.sh around lines 7 - 8, Update the installation commands
in the setup script so a failure from pipx install platformio immediately
terminates the script and prevents the esptool step from masking it; enable
fail-fast behavior or explicitly chain the commands with &&.
Problem
The devcontainer comes up without
pioon PATH:pipx install platformioruns in the Dockerfile as root, before the devcontainers python feature setsPIPX_HOME/PIPX_BIN_DIR. pipx therefore falls back to root's defaults and installs to/root/.local/bin/pio. A directory that is mode 700 and not on thevscodeuser's PATH. The binary is in the image, just unreachable from the user the container actually runs as.Fix
Move the install to
.devcontainer/setup.sh, which runs aspostCreateCommandasvscode, after features are applied and already installs esptool the same way. It lands in/usr/local/py-utils/bin, which is on PATH.Trade-off
platformio is now fetched on container create rather than baked into the image. The alternative is keeping the Dockerfile install and setting
ENV PIPX_HOME=/usr/local/py-utils PIPX_BIN_DIR=/usr/local/py-utils/binabove it. Happy to switch if that is preferred.Testing
Created a fresh container with the devcontainer CLI:
pioresolves to/usr/local/py-utils/bin/pio(PlatformIO Core 6.1.19)/root/.local/bin/piois no longer present, confirming the Dockerfile change took effectpio run -e rak4631succeeds in that container (SUCCESS, 2:26)Before this change the same steps produced
pio: command not found.This PR touches only
.devcontainer/; no firmware code changed, so the per-device regression list is not applicable.🤝 Attestations
N/A - no firmware code is changed by this PR. The
rak4631build above was run only to confirm the toolchain works inside the container.Summary by CodeRabbit