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

With python3 use default venv module. #71

Closed
wants to merge 2 commits into from

Conversation

marcaurele
Copy link

If the venv module is found, use it to create the virtual env, otherwise fallback to use virtualenv. An new paramater --virtualenv is available to force the use of virtualenv.

It fixes the problem on OSX #70

@RonnyPfannschmidt
Copy link
Contributor

looks good, any idea if/how we can automatically test this in a sensible manner?

@marcaurele
Copy link
Author

Yeah, there're 2 improvements I would like to add, but haven't found the
time yet:

  • find a better test to see if it's using python3 instead of simply trying
    to invoke the module;
  • add tests for python2 & python3.

On Friday, 4 March 2016, Ronny Pfannschmidt [email protected]
wrote:

looks good, any idea if/how we can automatically test this in a sensible
manner?


Reply to this email directly or view it on GitHub
#71 (comment).

@marcaurele
Copy link
Author

I tried a few things but I didn't find an approach that would make it better than the way I'm doing it now, simply by trying to invoke the module. It's tricky to invoke python from a python script to get its version correctly, and it doesn't make the code any shorter/better in my point of view.

@mikek
Copy link

mikek commented Sep 11, 2016

Note, this also requires a few minor modifications in get-pipsi.py.

The issue also allows to avoid installing virtualenv to system-wide site-packages (or to the Python version installed by pyenv, for example).

@marcaurele
Copy link
Author

@mikek I updated get-pipsi.py to use p3 venv module too.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
If the venv module is found, use it to create the virtual env, otherwise
fallback to use virtualenv. An new paramater --virtualenv is available
to force the use of virtualenv.

Fixes mitsuhiko#70

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@@ -45,6 +45,15 @@ def command_exists(cmd):
return False


def is_python3():
try:
Copy link

Choose a reason for hiding this comment

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

Why not use return sys.version_info > (3, 0, 0)

Copy link
Contributor

Choose a reason for hiding this comment

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

because python may not be sys.executable ^^

Copy link

Choose a reason for hiding this comment

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

Makes sense, thanks.

@mmerickel
Copy link
Contributor

This PR should be closed in favor of #76 and #91.

Unless the -p option to pipsi is removed, this PR makes no sense as the stdlib venv module does not allow creating virtualenvs for alternate python interpreters. It's best to just continue depending on the virtualenv package after pipsi is installed. #91 will allow pipsi itself to be installed using venv in the get-pipsi.py script.

@marcaurele marcaurele deleted the feature/py3venv branch December 4, 2017 16:35
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.

None yet

5 participants