Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds optional UV_CACHE_DIR support to Python dependency and esptool installation flows. The uv_cache_dir parameter is propagated through install_python_deps, install_esptool, and _install_esptool_from_tl_install functions, with UV_CACHE_DIR environment variable set in all uv subprocess calls and cache directory defaulting to build_dir/.uv-cache. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai Are there negative side effects to expect by moving the uv cache folder in the pio build directory? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🌐 Web query: 💡 Result: For Astral’s Default cache location
To print the active path, run: “Shared cache” behavior (what that means in practice)
Quick controls
Sources: [1] [2] [3]
Negative side effects:
Positive trade-offs (your intended benefits):
Recommendation:The trade-off makes sense for your use case (recovering from GitHub connectivity issues), but users with multiple projects will see increased disk usage and slower clean builds. You might consider:
Does this align with your expectations, or would you like to reconsider the approach? 🧠 Learnings used |
|
@coderabbitai Do analyze the effects now. uv cache folder is moved into folder |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1375
Effects of
|
| Location | Shared across projects? | Survives clean? | Isolated recovery? |
|---|---|---|---|
System default (~/.cache/uv) |
✅ All projects | ✅ Yes | ❌ Affects all uv usage |
.platformio/.cache/uv (your PR) |
✅ All PlatformIO projects | ✅ Yes | ✅ PlatformIO only |
| Per-project build dir | ❌ No | ❌ No | ✅ Per-project |
Verdict:
This is an excellent middle ground! You get:
- All the benefits of shared caching (speed, disk efficiency)
- Isolation for recovery scenarios (your original goal)
- Persistence across project operations
The only users who might see slight disk increase are those who actively use uv for non-PlatformIO projects, but that's a reasonable trade-off for the isolation benefits you're targeting.
This approach resolves my earlier concerns - the implementation is well-designed! 🎯
🧠 Learnings used
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 268
File: builder/penv_setup.py:87-96
Timestamp: 2025-08-26T14:52:29.044Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to trust system executables when falling back to PATH-based command resolution (like "uv"), rather than implementing additional validation checks for security concerns.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 281
File: builder/frameworks/component_manager.py:1047-1054
Timestamp: 2025-09-04T15:27:18.112Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers not to add guard checks for missing arduino_libs_mcu in backup operations, preferring to let the operation proceed rather than silently skip when the directory might be missing.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 263
File: platform.py:80-84
Timestamp: 2025-08-20T14:13:13.842Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers to always override IDF_TOOLS_PATH environment variable with PlatformIO's core directory, rather than respecting any existing user-defined IDF_TOOLS_PATH values. This ensures consistent ESP32 tool management within PlatformIO.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 371
File: builder/spiffsgen.py:620-629
Timestamp: 2025-12-27T16:33:38.710Z
Learning: In the pioarduino/platform-espressif32 project, when extracting SPIFFS files in builder/spiffsgen.py, Jason2866 prefers to check if rel_path is empty after lstrip('/') and skip such files with a warning message, rather than attempting to handle empty parent directories in os.makedirs calls. This treats empty paths as invalid/corrupted data.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 296
File: builder/penv_setup.py:612-647
Timestamp: 2025-09-23T16:55:39.788Z
Learning: In the pioarduino/platform-espressif32 project, Jason2866 prefers no fallback mechanisms in penv setup functions. The penv virtual environment setup must work properly and should crash immediately rather than falling back to host Python environments, to avoid using "unknown env" configurations.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-23T12:35:35.508Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 has explicitly instructed to NEVER create issues or PRs outside of his repositories (pioarduino/*). All issue and PR creation must be strictly limited to his own repositories only, never in upstream or other repositories.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 296
File: builder/main.py:49-50
Timestamp: 2025-09-22T14:47:08.855Z
Learning: For the pioarduino/platform-espressif32 project, Jason2866 prefers that critical setup operations like platform.setup_python_env should crash immediately if they fail rather than having fallback mechanisms, as this ensures problems are detected early and clearly rather than being masked.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 0
File: :0-0
Timestamp: 2025-09-22T10:59:14.425Z
Learning: In the pioarduino/platform-espressif32 project, the penv setup ensures that uv gets installed in the venv (penv), so additional safeguards for uv availability are not needed.
Learnt from: Jason2866
Repo: pioarduino/platform-espressif32 PR: 255
File: builder/penv_setup.py:265-275
Timestamp: 2025-08-10T16:59:15.533Z
Learning: In the ESP32 platform builder's penv_setup.py, the esptool_repo_path used in subprocess calls comes from PlatformIO's internal package management system (platform.get_package_dir()) which is a trusted source. Command injection concerns are not applicable here since this is build infrastructure code with controlled inputs, not user-facing code accepting arbitrary input.
|
@swoboda1337 your 2 cents please. Thx! If you dont see any road blocks i will merge. |
.platformio/.cache folder to clean up
.platformio/.cache folder to clean up.platformio/.cache folder for easy clean up
Clarify the docstring for the uv_cache_dir argument.
Yeah I don't think it can hurt. Makes it easier for users to clean everything. |
Upgrade uv from 0.6.14 to 0.10.1 to pick up the fix for HTTP/2 connection reset retry handling (astral-sh/uv#15675, released in 0.8.16). Also set UV_HTTP_RETRIES=10 (default 3) to better handle transient network errors during PlatformIO penv bootstrap. Remove the UV_CACHE_DIR override since pioarduino now handles this upstream (pioarduino/platform-espressif32#386). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Description:
to try to recover better from broken install attempts when Github is not reachable and broken? leftovers are stored in uv cache.
Checklist:
Summary by CodeRabbit