Skip to content

helper function for resolving the site package path across platforms - #23

Merged
micahpw merged 13 commits into
mainfrom
mw/windows-fixes
Nov 13, 2025
Merged

helper function for resolving the site package path across platforms#23
micahpw merged 13 commits into
mainfrom
mw/windows-fixes

Conversation

@micahpw

@micahpw micahpw commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

This pull request aims to consolidate the cross-platform logic for finding python virtual environment related assets. I think the utils.rs file in r2x-python is a good place to put the platform specific logic for resolving paths to python packages and the python interpreter binary, given that the path structure and binary name is slightly different.

  • Finding "site-packages" path. (Added platform specific code blocks)
  • Finding specific site package. (TBD if utils.rs is a good spot for this.)
  • Finding python interpreter binary. (TODO, add a function that returns the platform specific path to the python interpreter)

Currently this branch compiles and runs most of the tests. It is currently hanging on the test python virtual environment creation.

There is a small fix for handling and propagating the errors properly. I currently have an Enum of VenvErr which could be polished more, however, they need to be mapped to BridgeError in some cases.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There error was duplicated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The main issue here is path to the site-packages directory differs between Windows and Linux/MacOS

Windows -> .venv/Lib/site-packages (Case sensitive)
Linux/MacOS -> .venv/lib/python {version}/site-packages.

Comment thread crates/r2x-python/src/utils.rs Outdated
use std::path::PathBuf;

#[derive(Debug)]
pub enum VenvErr {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Error structure needs more polish or consolidation with errors.rs

let site_packages = python_version_dir.path().join("site-packages");
// FIXME, properly handle error propagation.
let site_packages =
resolve_site_package_path(&venv_path).expect("Failed to resolve site packages");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Currently this code panics and needs to properly return and error string.

Comment thread crates/r2x-python/src/initialization.rs Outdated
})?;

let site_packages = python_version_dir.path().join(SITE_PACKAGES);
let site_packages = resolve_site_package_path(&venv_path).map_err(|e| {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Worth thinking about consolidation with BridgeError in errors.rs or differentiate between path related errors vs others.


Ok(site_packages)
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I plan on adding a "resolve_interpreter_path" function here as well unless you have an objection.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Go for it.

@micahpw
micahpw marked this pull request as ready for review November 13, 2025 05:27
@micahpw
micahpw requested a review from pesap November 13, 2025 05:28
shell: bash
run: ln -sf "$pythonLocation/bin/python" "$pythonLocation/bin/python3.12"
# Check that files are not equivalent before linking.
run: |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added checks to make sure files aren't equivalent before creating symlink.

Comment thread dist-workspace.toml
@micahpw micahpw closed this Nov 13, 2025
@micahpw micahpw reopened this Nov 13, 2025
@micahpw
micahpw merged commit 8dc1aa5 into main Nov 13, 2025
11 checks passed
@pesap pesap mentioned this pull request Nov 13, 2025
@pesap
pesap deleted the mw/windows-fixes branch November 16, 2025 00:04
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