Skip to content
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

feat: create venv from rust #128

Merged
merged 13 commits into from
Dec 22, 2023

Conversation

nichmor
Copy link
Collaborator

@nichmor nichmor commented Dec 19, 2023

Create python venv directly from rust.

Implementation based on built-in venv module.

I've changed a little system_python_executable implementation. which does not return original interpreter but a shim path when using pyenv. This does not work well when we later create pyenv.cfg and set home =

Closes: #83

Copy link
Contributor

@tdejager tdejager left a comment

Choose a reason for hiding this comment

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

Looking very good, 1 question and minor changes requested :)

@nichmor nichmor marked this pull request as ready for review December 19, 2023 18:17
@nichmor nichmor requested a review from tdejager December 19, 2023 18:29
@tdejager
Copy link
Contributor

I'll want to test the branch a bit tomorrow and will look at te rest of the changes, but from what I am seeing it looks pretty good!

@nichmor
Copy link
Collaborator Author

nichmor commented Dec 20, 2023

I'll want to test the branch a bit tomorrow and will look at te rest of the changes, but from what I am seeing it looks pretty good!

Thank you very much! Let me know if there is anything I can help with

Copy link
Contributor

@tdejager tdejager left a comment

Choose a reason for hiding this comment

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

Approved! Just gonna do some simple testing with it 👍

@tdejager
Copy link
Contributor

Okay looks good, merging!

@tdejager tdejager merged commit b9110f4 into prefix-dev:main Dec 22, 2023
5 checks passed
.or_else(|_| which::which("python"))
.map_err(|_| FindPythonError::NotFound)

let output = match std::process::Command::new("python3")
Copy link

@pradyunsg pradyunsg Dec 22, 2023

Choose a reason for hiding this comment

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

Not something that needed to happen in this PR but... given the move away from a few stat calls to a subprocess, consider caching the results of system_python_executable across the entire process since it's unlikely to change and you can avoid this overhead on every environment creation + bytecode compile call etc.

Copy link
Contributor

@tdejager tdejager Dec 23, 2023

Choose a reason for hiding this comment

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

Thanks @pradyunsg will make an issue (#134) out of this!

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.

Create venv purely from Rust
3 participants