classification
Title: Misuse of $ in activate.fish of venv
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: brett.cannon, erdban fed, karolyi, python-dev, tinchester, vinay.sajip, xiang.zhang, 鄭景文
Priority: normal Keywords:

Created on 2016-03-29 12:46 by 鄭景文, last changed 2016-07-01 12:14 by karolyi. This issue is now closed.

Files
File name Uploaded Description Edit
screenshot.png 鄭景文, 2016-03-29 12:46 error message
Messages (18)
msg262609 - (view) Author: 鄭景文 (鄭景文) Date: 2016-03-29 12:46
I use venv module to make a virtual environment in cpython3.6.
After it, I try to activate it with fish, but it give me an error, like the screenshot. 
I open the activate.fish and remove the $ sign in line 58 and 59, then the activate file works. 
I open the activate.fish of py3.5 venv, there isn't a $ sign in line 58 and 59, too. 
However, there is a bug issue which add the $ in front of the prompt. 
I'm not sure how this happen, but I think there should not be a $ there.
msg262620 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2016-03-29 18:49
The change you mention was in response to Issue #26348. The patch supplied there introduced the $ characters you say aren't required. I'm not a fish user, but it would seem that "$__VENV_PROMPT__" would resolve to the *value* of the environment variable __VENV_PROMPT__, whereas "__VENV_PROMPT__" would just resolve to the literal string "__VENV_PROMPT__" which doesn't seem like what is wanted. Can you explain the reasoning by which you assert that the $ isn't wanted?
msg262748 - (view) Author: 鄭景文 (鄭景文) Date: 2016-04-01 17:10
First, I use python 3.5.1 to create virtual environment. It works fine with fish. There is no $ in the activate.fish file.
Second, after I removed the $, It works fine, both in ubuntu and arch linux.

I guess it is because the $ is for bash not for fish, the activate.fish didn't work.
These are why I assert the $ is unwanted.
msg262917 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2016-04-05 17:55
I've asked the person who sent in the patch for #26348 to comment on this issue.
msg264328 - (view) Author: Tin Tvrtković (tinchester) Date: 2016-04-26 21:03
I'm getting this exact issue on Python 3.5 (xenial system installation).

All my existing venvs (from before my upgrade to xenial) work. Here's the diff:


> diff .venv/bin/activate.fish ../wrapt/.venv/bin/activate.fish 
35c35
< set -gx VIRTUAL_ENV "/home/tin/pg/hypothesis/.venv"
---
> set -gx VIRTUAL_ENV "/home/tin/pg/wrapt/.venv"
58,59c58,59
<         if test -n "$(.venv) "
<             printf "%s%s%s" "$(.venv) " (set_color normal) (_old_fish_prompt)
---
>         if test -n "(.venv) "
>             printf "%s%s%s" "(.venv) " (set_color normal) (_old_fish_prompt)

The added dollar signs are the issue. Removing them fixes the problem.

For the record:

> fish --version
fish, version 2.2.0

And here's the actual error message, the same as in the png:

> . .venv/bin/activate.fish 
$(...) is not supported. In fish, please use '(.venv)'.
.venv/bin/activate.fish (line 58):         if test -n "$(.venv) "
                                                       ^
from sourcing file .venv/bin/activate.fish
	called on line 151 of file /usr/share/fish/config.fish

in function “.”
	called on standard input

source: Error while reading file “.venv/bin/activate.fish”
msg264330 - (view) Author: Tin Tvrtković (tinchester) Date: 2016-04-26 21:07
Also I will add I've been using fish for a long time and have never been affected by #26348.
msg264383 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-04-27 17:44
The patch introduced by issue26348 should be reverted. The __VENV_PROMPT__ is meant to be a placeholder. For example, when you create a virtual environment named venv-test, the __VENV_PROPMT__ will be replace by (venv-test). If the patch is applied, it will be $(venv-test), treated as an expression and then cause a failure.
msg269610 - (view) Author: erdban fed (erdban fed) Date: 2016-06-30 17:46
This bug shipped in 3.5.2.  It should've been a blocker.
msg269612 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-06-30 17:52
I get the same feeling. Add Larry.
msg269615 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-30 18:43
New changeset 8d7bde14d7a4 by Brett Cannon in branch 'default':
Merge from 3.5 for issue #26664
https://hg.python.org/cpython/rev/8d7bde14d7a4
msg269616 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-06-30 18:46
Two things.

One, sorry for not getting to this before 3.5.1 got out. It simply slipped off my radar thanks to various other issues that I've been dealing with. It's obviously now fixed.

Two, this is not a release blocker. Python itself continues to function properly with this bug and it doesn't prevent you from working with a venv under fish (but I recognize it's an annoyance).
msg269618 - (view) Author: erdban fed (erdban fed) Date: 2016-06-30 18:56
> Two, this is not a release blocker. Python itself continues to function properly with this bug and it doesn't prevent you from working with a venv under fish (but I recognize it's an annoyance).

If you can't load the venv, I think that pretty definitely prevents you from working with one.  I'll now have to edit the activation script file manually every time I create a new venv until 3.6 is released.  Or revert to 3.5.1.
msg269620 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-06-30 19:24
Activation doesn't "load" the Python interpreter in the venv; all it does is tweak shell variables to put the interpreter in venv early on PATH. I personally never use the activate.* scripts and simply directly execute the Python interpreter, e.g. venv/bin/python. The only other thing the activation script does that potentially changes semantics is the setting of $PYTHONHOME which can do manually if you need that specific bit of functionality.

You have to realize that there are over 5,000 issues open on bugs.python.org. We are all volunteers working on Python and we do our best to get through issues, but we just don't get to everything on the timeframe people prefer. I realize this issue is important to you, but basically every reported issue is important to someone so every time we prioritize anything over something else we are upsetting someone somewhere.
msg269625 - (view) Author: Tin Tvrtković (tinchester) Date: 2016-06-30 19:54
Thanks for dealing with this, Brett, your efforts are appreciated. :)

On Thu, Jun 30, 2016 at 9:24 PM Brett Cannon <report@bugs.python.org> wrote:

>
> Brett Cannon added the comment:
>
> Activation doesn't "load" the Python interpreter in the venv; all it does
> is tweak shell variables to put the interpreter in venv early on PATH. I
> personally never use the activate.* scripts and simply directly execute the
> Python interpreter, e.g. venv/bin/python. The only other thing the
> activation script does that potentially changes semantics is the setting of
> $PYTHONHOME which can do manually if you need that specific bit of
> functionality.
>
> You have to realize that there are over 5,000 issues open on
> bugs.python.org. We are all volunteers working on Python and we do our
> best to get through issues, but we just don't get to everything on the
> timeframe people prefer. I realize this issue is important to you, but
> basically every reported issue is important to someone so every time we
> prioritize anything over something else we are upsetting someone somewhere.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue26664>
> _______________________________________
>
msg269627 - (view) Author: erdban fed (erdban fed) Date: 2016-06-30 20:20
> Activation doesn't "load" the Python interpreter in the venv; all it does is tweak shell variables to put the interpreter in venv early on PATH.

I'm aware.

> You have to realize that ...

I know it's often a thankless task and that mistakes can and do happen.  Not prioritising a two-click fix to a regression that was spotted several months in advance would - in my book - count as such a mistake.  Okay, you were preoccupied with other things - that's fine.  That's a valid excuse.  And if you disagree with the importance of it, that's also fine; I'm not gonna argue my case further.
msg269628 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-06-30 20:37
I wish it was a two-click fix. It's actually a fix-in-3.5-then-test-then-update-Misc/NEWS-then-commit-then-merge-then-verify-again-then-commit-then-push-then-update-the-issue (IOW it's about 8 separate steps).
msg269639 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-07-01 02:03
Since the bug is fixed, we can also close issue26348.
msg269661 - (view) Author: László Károlyi (karolyi) Date: 2016-07-01 12:14
+1 here, broken on homebrew cpython > 3.5.1 (a.k.a. 3.5.2)
History
Date User Action Args
2016-07-01 12:14:45karolyisetnosy: + karolyi
messages: + msg269661
2016-07-01 02:03:11xiang.zhangsetmessages: + msg269639
2016-06-30 20:37:13brett.cannonsetmessages: + msg269628
2016-06-30 20:20:35erdban fedsetmessages: + msg269627
2016-06-30 19:54:05tinchestersetmessages: + msg269625
2016-06-30 19:24:50brett.cannonsetmessages: + msg269620
2016-06-30 18:56:16erdban fedsetmessages: + msg269618
2016-06-30 18:46:36brett.cannonsetstatus: open -> closed

nosy: - larry
messages: + msg269616

resolution: fixed
stage: resolved
2016-06-30 18:43:33python-devsetnosy: + python-dev
messages: + msg269615
2016-06-30 18:33:29brett.cannonsettitle: find a bug in activate.fish of venv of cpython3.6 -> Misuse of $ in activate.fish of venv
2016-06-30 17:52:58xiang.zhangsetnosy: + larry
messages: + msg269612
2016-06-30 17:46:40erdban fedsetnosy: + erdban fed
messages: + msg269610
2016-04-27 19:47:29brett.cannonsetassignee: brett.cannon
2016-04-27 17:44:18xiang.zhangsetnosy: + xiang.zhang
messages: + msg264383
2016-04-26 21:07:26tinchestersetmessages: + msg264330
2016-04-26 21:05:17tinchestersetversions: + Python 3.5
2016-04-26 21:03:47tinchestersetnosy: + tinchester
messages: + msg264328
2016-04-05 18:36:35brett.cannonsetnosy: + brett.cannon
2016-04-05 17:55:15vinay.sajipsetmessages: + msg262917
2016-04-01 17:10:59鄭景文setmessages: + msg262748
2016-03-29 18:49:21vinay.sajipsetmessages: + msg262620
2016-03-29 12:51:34SilentGhostsetnosy: + vinay.sajip
2016-03-29 12:48:22鄭景文settype: behavior
2016-03-29 12:47:24鄭景文setcomponents: + Library (Lib)
versions: + Python 3.6
2016-03-29 12:46:25鄭景文create