-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
escape string in __VIRTUAL_ENV__ path for bash #811
Conversation
Fixes issue pypa#53 in bash and shell by using the printf %q "shell quote" formatter, which escapes as appropriate for a given environment. Falls back to quoting some characters that universally need to be escaped. Fallback is needed because there are several printf implementations, not all of which are guaranteed to have the %q formatter (though bash always should). uncompressed diff here: davidbstein/virtualenv_decompressed_scripts@0bc6629
Hi there, you need to actually commit changes for virtualenv_embedded/activate.sh as well. Did you understand not to do so from reading the documentation somewhere? If so we need to clarify it for everyone. |
Hi Matt - Completely my fault, have had a fix locally for a while and pushed it |
Hi @davidbstein upon looking further, I don't see how this solves #53 at all? The |
Regardless of whether this PR does fix the problem, would it be worth adding a test that demonstrates the issue, so we'd at least be able to see the problem via a test? |
thanks for the quick review! @Ivoz - AFAICT, #53 is caused by the bad entry in PATH, not the malformed shebang - easy_install and pip work again with this change (testing on Ubuntu and OSX - escaping the PATH fixes the issue, escaping the shebang line does not). Fixing the shebang line seems like a good idea - I'll try to dig into a good way to do that. @pfmoore - tests seem like a great idea. Will add one. |
@davidbstein so after manually applying your patch to test, I created a new virtualenv called "h a", confirm the patch is in the new virtualenv:
Yes, we've managed to escape it in the
But, We can't escape the hashbang problem:
So we've put an escape in the |
I'm pretty sure that escaping paths in I'm guessing that this is only working for you (or, appearing to work), because now that your patch with the backslash escape has made the virtualenv path invalid, when you type |
Fixes issue #53 in bash and shell only. Uses the printf
%q
"shell quote" formatter. Falls back to quoting some characters that universally need to be escaped.Fallback is needed because while bash's
printf
command has the%q
formatter, the builtin function printf may not.(edit): removed irrelevant link