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

Use creator venv when creating virtual environments #5376

Merged
merged 2 commits into from
Sep 25, 2022
Merged

Conversation

matteius
Copy link
Member

@matteius matteius commented Sep 24, 2022

I did not realize how broken pipenv is out of the box on the latest ubuntu until I upgraded. It has to do with it defaulting to posix_local when creating a virtualenv, which has different paths for the python binaries. I initially tried preferring posix_local for the paths but it had the side effect of trying to install everything to local site packages.

Risks: The venv creator should be available on all systems, but the documentation points out:

venv - this delegates the creation process towards the venv module, as described in PEP 405. This is only available on Python interpreters having version 3.5 or later, and also has the downside that virtualenv must create a process to invoke that module (unless virtualenv is installed in the system python), which can be an expensive operation (especially true on Windows).

@matteius matteius changed the title Prefer posix_local if its available over posix_prefix. Use creator venv when creating virtual environments Sep 25, 2022
@matteius matteius requested a review from oz123 September 25, 2022 10:02
@@ -903,6 +903,7 @@ def do_create_virtualenv(project, python=None, site_packages=None, pypi_mirror=N
Path(sys.executable).absolute().as_posix(),
"-m",
"virtualenv",
"--creator=venv",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we simplify this to: -m venv I hope we are not using any specific features of virtualenv which aren't found in venv.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that Ubuntu does not ship venv as part of the system python so this might not solve the issue...

Copy link
Member Author

Choose a reason for hiding this comment

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

This change did solve the issue for my ubuntu 22.04 -- I am not sure if that -m venv is equivalent to this, do you know?

Copy link
Contributor

Choose a reason for hiding this comment

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

The built in module venv is a trimmed down version of virtualenv: https://virtualenv.pypa.io/en/latest/.
We can't use this approach immediately anyway because it only supports one python version.

To use this approach, we will have to detect the python version by ourselves. That's because you can only do:

python3.X -m venv ...

Instead of:

python -m virtualenv --python ....

@@ -903,6 +903,7 @@ def do_create_virtualenv(project, python=None, site_packages=None, pypi_mirror=N
Path(sys.executable).absolute().as_posix(),
"-m",
"virtualenv",
"--creator=venv",
Copy link
Contributor

Choose a reason for hiding this comment

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

The built in module venv is a trimmed down version of virtualenv: https://virtualenv.pypa.io/en/latest/.
We can't use this approach immediately anyway because it only supports one python version.

To use this approach, we will have to detect the python version by ourselves. That's because you can only do:

python3.X -m venv ...

Instead of:

python -m virtualenv --python ....

@oz123 oz123 merged commit 2fa19e5 into main Sep 25, 2022
@oz123 oz123 deleted the sysconfig-path-bug branch September 25, 2022 18:27
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