fix: find all installed python versions#114
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors Python object detection for Valgrind to find all installed Python versions instead of relying on a single system Python. The change replaces the previous approach that used Python's sysconfig module with a more comprehensive method that searches for Python installations through both uv (Python version manager) and system paths.
- Replaces single Python detection with comprehensive multi-version discovery
- Adds support for uv-managed Python installations alongside system Python
- Implements filesystem-based library discovery instead of relying on Python's sysconfig
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let output = output.unwrap(); | ||
|
|
||
| let json_output = String::from_utf8_lossy(&output.stdout); | ||
| let json: serde_json::Value = serde_json::from_str(&json_output).unwrap_or_default(); |
There was a problem hiding this comment.
Using unwrap_or_default() on JSON parsing will silently return an empty JSON value for invalid JSON, which could mask parsing errors. Consider using proper error handling with context() to provide meaningful error messages when JSON parsing fails.
| let json: serde_json::Value = serde_json::from_str(&json_output).unwrap_or_default(); | |
| let json: serde_json::Value = serde_json::from_str(&json_output) | |
| .context("Failed to parse JSON output from 'uv python list'")?; |
| } | ||
|
|
||
| fn find_system_python_paths() -> anyhow::Result<Vec<String>> { | ||
| let output = Command::new("which").args(["-a", "python"]).output()?; |
There was a problem hiding this comment.
Using which command is not portable across all systems (not available on Windows). Consider using a more portable approach or add platform-specific handling for cross-platform compatibility.
|
Closing this since it was only used for testing. |
1 participant
No description provided.