Skip to content

fix: find all installed python versions - #114

Closed
not-matthias wants to merge 11 commits into
mainfrom
cod-1315-statically-linked-libpythonso-adds-extra-frames-to-valgrind
Closed

not-matthias wants to merge 11 commits into
mainfrom
cod-1315-statically-linked-libpythonso-adds-extra-frames-to-valgrind

Conversation

@not-matthias

Copy link
Copy Markdown
Member

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copilot AI Sep 15, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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'")?;

Copilot uses AI. Check for mistakes.
}

fn find_system_python_paths() -> anyhow::Result<Vec<String>> {
let output = Command::new("which").args(["-a", "python"]).output()?;

Copilot AI Sep 15, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@not-matthias

Copy link
Copy Markdown
Member Author

Closing this since it was only used for testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants