-
Notifications
You must be signed in to change notification settings - Fork 48
Full uv path #227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Full uv path #227
Conversation
WalkthroughThe update refactors how Python and related tool executable paths are managed by introducing a global variable for the Python executable and new helper functions for resolving full paths to Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainScript
participant PythonExe
participant UV
participant Esptool
User->>MainScript: Run build/setup
MainScript->>PythonExe: Resolve Python executable path
MainScript->>UV: Use _get_uv_executable_path to resolve path
MainScript->>Esptool: Use _get_esptool_executable_path to resolve path
MainScript->>PythonExe: Run pip/uv/esptool commands via subprocess
PythonExe-->>MainScript: Return output/status
UV-->>MainScript: Return output/status
Esptool-->>MainScript: Return output/status
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
builder/main.py
(12 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Jason2866
PR: pioarduino/platform-espressif32#225
File: builder/main.py:99-108
Timestamp: 2025-07-13T20:03:29.654Z
Learning: In the ESP32 platform builder, the subprocess call to get site-packages directory in setup_python_paths() function is intentionally left without error handling because if this basic Python operation fails, it indicates a fundamental system issue that would cause the entire build process to fail anyway.
builder/main.py (1)
Learnt from: Jason2866
PR: pioarduino/platform-espressif32#225
File: builder/main.py:99-108
Timestamp: 2025-07-13T20:03:29.654Z
Learning: In the ESP32 platform builder, the subprocess call to get site-packages directory in setup_python_paths() function is intentionally left without error handling because if this basic Python operation fails, it indicates a fundamental system issue that would cause the entire build process to fail anyway.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: build (macos-15, examples/tasmota)
- GitHub Check: build (macos-15, examples/espidf-arduino-C6-ULP-blink)
- GitHub Check: build (macos-15, examples/espidf-blink)
- GitHub Check: build (windows-latest, examples/espidf-peripherals-usb)
- GitHub Check: build (windows-latest, examples/espidf-http-request)
- GitHub Check: build (macos-15, examples/arduino-blink)
- GitHub Check: build (windows-latest, examples/espidf-blink)
- GitHub Check: build (windows-latest, examples/espidf-arduino-blink)
- GitHub Check: build (windows-latest, examples/espidf-storage-sdcard)
- GitHub Check: build (windows-latest, examples/espidf-exceptions)
- GitHub Check: build (windows-latest, examples/espidf-hello-world)
- GitHub Check: build (windows-latest, examples/tasmota)
- GitHub Check: build (windows-latest, examples/arduino-matter-light)
- GitHub Check: build (windows-latest, examples/arduino-NimBLE-ext_client)
- GitHub Check: build (windows-latest, examples/arduino-wifiscan)
- GitHub Check: build (windows-latest, examples/arduino-blink)
🔇 Additional comments (8)
builder/main.py (8)
84-108
: Function refactored to use global PYTHON_EXE variable.The implementation correctly validates the Python executable and includes a timeout for the subprocess call. The lack of error handling for the subprocess is intentional based on project conventions.
111-144
: Well-designed helper function for cross-platform executable path resolution.The function correctly handles platform-specific differences and provides a sensible fallback mechanism.
146-169
: Clean wrapper functions for specific executables.Good use of the DRY principle by delegating to the generic
_get_executable_path
function.
199-236
: Correctly implements the fix for Windows uv installation issues.The implementation properly:
- Resolves the uv executable path
- Updates the path after installation
- Adds the Scripts directory to PATH on Windows
This directly addresses the PR objective.
329-367
: Function properly refactored to use global state and helper functions.The implementation correctly uses the new helper functions and returns the resolved esptool path.
371-371
: Proper handling of esptool binary path with space support.Good practice to handle paths with spaces by adding quotes when necessary.
Also applies to: 754-758
918-918
: Consistent use of global PYTHON_EXE variable.
1046-1046
: Good modernization to f-string formatting.
Description:
hopefully fixes Windows uv install issues. Adding Python virtual env scripts directory to Windows path
Checklist:
Summary by CodeRabbit
No user-facing features or interface changes were introduced in this update.