Closed simnalamburt closed 7 years ago
I want to fix this issue right now, so I brought some plans.
@kennethreitz @nateprewitt Please review and comment. If you choose a plan, I'll send a PR right away. Thanks!
When the directory name has the whitespaces, so the name of virtualenv will.
/home/user/.local/share/virtualenvs/directory with spaces/bin/pip
And if you try to execute the pip with the format like below:
delegator.run('{0} install pip'.format(which_pip()))
The shell will understand this command like this
/home/user/.local/share/virtualenvs/directory with spaces/bin/pip install pip
^^^^
Word splitting has occurred!
Since there's no such executable named /home/user/.local/share/virtualenvs/directory
, it'll fail.
So you should double quote the path like this
delegator.run('"{0}" install pip'.format(which_pip()))
But this is the only a single pattern that I've found. There're multiple points which causes the whitespace issue, and we literally need to quote the path "wherever possible" to fix this issue. I'm not familiar with the codebase of pipenv, so I'm having trouble with quoting the all the paths. Someone need to help me out.
--- a/pipenv/project.py
+++ b/pipenv/project.py
@@ -6,6 +6,7 @@ import pipfile
import toml
import delegator
+from slugify import slugify
from requests.compat import OrderedDict
from .utils import format_toml, mkdir_p
@@ -26,7 +27,7 @@ class Project(object):
@property
def name(self):
if self._name is None:
- self._name = self.pipfile_location.split(os.sep)[-2]
+ self._name = slugify(self.pipfile_location.split(os.sep)[-2])
return self._name
@property
python-slugify
and Unidecode
. This can be undesirable for some people, since pipenv should be installed globally.PIPENV_VENV_IN_PROJECT
environmental variables are resolved.EDIT: Sorry, we can't choose this option. Please see the next comment for why.
You might think this problem can be solved with escaping paths with backslashes, but that's just temporal solution. You'll have to properly escape the all special characters like |
, ^
, &
, etc and handle the original backslash properly, which is complicated and easy to make a mistake. Double quoting is the easiest way.
(You don't want to handle this properly by your own hand)
PIPENV_VENV_IN_PROJECT
environment variable.I found that plan B cannot solve this problem.
I tried to solve this issue with both plan A and B, and plan A just worked perfect. But plan B could not solve the issue due to the bug of the pip. Please see https://github.com/pypa/pip/issues/923 for the further details.
Since there's no easy way left else plan A to solve this problem, I made a pull request with the plan A. Please comment if you have another good idea. Thanks.
Use the command below to try patched version of pipenv.
pip install git+https://github.com/simnalamburt/pipenv.git@plan-a
Closing this for the reasons discussed here in #230. Please let us know if you find more information here that will allow us to proceed with a globally workable solution. Thanks again @simnalamburt!
Reopened by https://github.com/kennethreitz/pipenv/pull/230#issuecomment-280413728
Anyone who wants an alternative design, please comment! :smile: Let's solve this problem elegantly with our collective intelligence!
Summary:
PIPENV_VENV_IN_PROJECT
environment variable.This comment is long. Dear project owner and collaborators, please read this and advice. Thanks!
At first, I'll focus on solving this problem without PIPENV_VENV_IN_PROJECT
environment variable being enabled. Because of the https://github.com/pypa/virtualenv/issues/53, there's no possible way to solve this problem without abandoning pypa/virtualenv or fixing https://github.com/pypa/virtualenv/issues/53. Personally, I do not think https://github.com/pypa/virtualenv/issues/53 is an irresolvable problem. With making a tiny native executable binary with C, we'll be able to workaround the "whitespaces in the shebang" issue. But that is an another problem and changing behavior of virtualenv will require tons of debate and thousands of user inconvenience, so I'll reserve that issue for another day. For now, I'll focus on solving this error without PIPENV_VENV_IN_PROJECT
environment.
As I said, I want to solve as many problems as possible at once if we need to introduce a backward compatibility breakage at least once. The confusion and misconception that "pipenv is unstable, and it breaks things" due to a frequent compatibility breakage is something that I don't want.
The problems related to the name of the virtualenv that I know are:
Please comment if there's an additional issue with a virtualenv's name. Let's solve all these problems at once.
Currently, we already have some "same project name" issue here. At present, those directories will be considered as same projects.
/home/user/my-project
/home/user/workspace/my-project
/home/user/another workspace/my-project
But with https://github.com/kennethreitz/pipenv/pull/230 being merged, this problem gets even worse. All those directories will be considered as same projects.
/home/user/my-project
/home/user/My,project
/home/user/MY_PROJECT
/home/user/workspace/my-project
/home/user/another workspace/my-project
/home/user/another workspace/My Project
To get rid of these "name problems" once and for all, I think we need to design a new naming scheme of virtualenv directories together.
There can be multiple solutions to solve this problem, but it differs by a "requirement of the virutualenv's name". So here's an important question. Do we have to care the readability of the virtualenv's name?
Personally, I think we don't have to. Since pipenv totally encapsulate the virtualenv, users rarely need to care the name and details of virtualenvs. They just automatically created in somewhere else the project directory, and user do not need to know the name of it since users can fully access the virtualenv using the pipenv run ~
and pipenv shell
command as a proxy. Pipenv effectively hides the virtualenv from the users.
When users enter the pipenv shell
command is the only time when users care the name of the virtualenv since the name will be prompted in the shell. But still the users won't need to type the name manually, so I think we don't need to care the name of it.
The reason that I ask such question is, it becomes much easier to solve these problems if we don't need to care the name of the virtualenv!
Hash the full project path with a cryptographic hash function. Then encode it with the URL-safe base64 or hexadecimal numeric string. And let's use it as a name of the virtualenv.
Using a cryptographic hash function to identify something is a widely accepted solution to multiple software like git. Cryptographic hash functions have Pre-image resistance, second pre-image resistance, and collision resistance. Users are not going to make a lot of virtualenvs (more than 2**32) so we're just OK to use a cryptographic hash function to identify the full project path.
I want to use BLAKE2 since it's the fastest cryptographic hash function at present and it has been included as a standard library in Python 3.6. I'll use pyblake2 package since blake2 project owner wants to transfer its name to the pyblake2 author. (I heard it from the project owner directly)
So it'll be like this.
import sys
import base64
if sys.version_info >= (3, 6):
from hashlib import blake2b
else:
from pyblake2 import blake2b
path = b'/home/Hyeon Kim/My Workspace/My Project'
base64.urlsafe_b64encode(blake2b(path, digest_size=9).digest())
# b'05AKATXSLXYA'
Now that 05AKATXSLXYA
will be the name of our virtualenv. It'll be placed at ~/.local/share/virtualenvs/05AKATXSLXYA
It's short enough and unique enough. Furthermore, it'll help pipenv to work in Windows since there'll be no more strange artifacts in the path of virtualenv. What we need to do is just detect the os and change the path to %USERPROFILE%\AppData\Local\pipenv
or somewhere.
... but what if we should care of the readability of virtualenv's name?
Instead of ~/.local/share/virtualenvs/<name of venv>
, store virtualenvs at ~/.local/share/virtualenvs/<ID of venv>/<readable name of venv>
. <ID of venv>
is a base64 or hash of virtualenv's full path, and <readable name of venv>
is a readable name of virtualenv without whitespaces. We'll have to handle the various special characters too to make pipenv function properly on multiple platforms.
Currently I don't have enough time to elaborate this solution and honestly, I don't like this solution. It may require a modification of pew or even we'll need to abandon pew.
Thank you for reading the long comment and I really want the proposal 1 to be accepted. Please review and advice. Thanks! :shipit:
I have finished implementing proposal 1. Please try it with the command below:
pip install --upgrade git+https://github.com/simnalamburt/pipenv.git@issue-228
I have tested it with Arch Linux, and OS X, and It was just fine.
send a pull request!
@kennethreitz Yes sir
i don't personally think this is the right approach, having the project virtualenv named after the project directory is important
but we can do it it in exceptional cases, perhaps, like when there's a space.
Summary
This bug occurs regardless of the value of
PIPENV_VENV_IN_PROJECT
environment variable.How to reproduce this error
Please comment if you cannot reproduce the error.
(Click to see the screenshot)
My Environment
I could reproduce the error in multiple environments. Please comment if you want more information.