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

Allow virtual env creation in dir with leading dash #2415

merged 4 commits into from Jun 26, 2018


Copy link

@dnoetzel dnoetzel commented Jun 25, 2018

Following feedback from PR #2399, I decided to close that PR and go in a new direction. Instead of sanitizing the virtualenv name, I fixed how pew is called. However, I chose to keep the command-line invocation and not use pew's mkvirtualenv() function. This was done to keep the codepaths similar, since virtualenv needs to continue to be called from the command line. (virtualenv does not support supplying a different version of Python in create_environment(); it opens a subprocess to achieve that.)

The test is also a much better representation of the problem, albeit more complex.

Fixes #439

dnoetzel added 3 commits Jun 25, 2018
Invoke pew with a double dash separator ("--"), to make it clear that
the virtualenv name is a positional argument. Since the virtualenv
name includes all or at least the beginning of the directory name,
trying to create a virtualenv in a directory with a leading dash in
its name will cause pew to erroneously parse the virtualenv name as an
optional argument. Adding the separator causes the virtualenv name to
be parsed correctly.

Fixes #439
Copy link

@uranusjr uranusjr commented Jun 25, 2018

This looks like a good middle road to me.

Copy link

@techalchemy techalchemy commented Jun 26, 2018

Thanks for the PR!

@techalchemy techalchemy merged commit 9ec6b74 into pypa:master Jun 26, 2018
2 checks passed
2 checks passed
buildkite/pipenv Build #542 passed (6 minutes, 11 seconds)
continuous-integration/appveyor/pr AppVeyor build succeeded
@dnoetzel dnoetzel deleted the dnoetzel:bugfix-439 branch Jun 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants