classification
Title: venv activation scripts erroneously check if __VENV_PROMPT__ is defined
Type: behavior Stage: resolved
Components: Library (Lib) Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, mental, vinay.sajip
Priority: normal Keywords: patch

Created on 2019-07-23 19:17 by brett.cannon, last changed 2019-08-22 17:52 by brett.cannon. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 14932 closed mental, 2019-07-24 11:07
Messages (4)
msg348346 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-07-23 19:17
If you look at https://github.com/python/cpython/blob/master/Lib/venv/__init__.py#L112-L113 you will see that the prompt context for a virtual environment is always set. Then if you look at the string substitutions for the activation scripts you will also see that the string substitution for __VENV_PROMPT__ is thus always done.

This is an issue as it means that for the fish and bash activation scripts they have erroneous checks for custom prompts. I.e. https://github.com/python/cpython/blob/master/Lib/venv/scripts/posix/activate.fish#L55 for fish and https://github.com/python/cpython/blob/master/Lib/venv/scripts/common/activate#L57 for bash will never fail, halting any other further checks unless one creates a custom EnvBuilder class which will make sure the prompt context stays empty all the way to string substitution. This impacts the colouring of the prompt in fish as that's only done if there is no default prompt (which is never true).
msg348376 - (view) Author: (mental) * Date: 2019-07-24 09:32
Brett, Vinay: mind if I handle this one? I'm looking for a good first issue to tackle.

In terms of solutions I propose adding short circuiting logic to the checks for custom prompts to test against the default case `({context.env_name})`
msg348380 - (view) Author: (mental) * Date: 2019-07-24 11:10
I've added a PR (#1492) since it was a simple fix.

Please feel free to reject if this issue is reserved or some other fatal issue with the solution is found.

Otherwise I'd love a review and a double check (mainly paranoia from not wanting to mess up).
msg350219 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-08-22 17:52
Fixed by issue37663.
History
Date User Action Args
2019-08-22 17:52:26brett.cannonsetstatus: open -> closed
resolution: fixed
messages: + msg350219

stage: patch review -> resolved
2019-07-24 11:10:15mentalsetmessages: + msg348380
2019-07-24 11:07:18mentalsetkeywords: + patch
stage: patch review
pull_requests: + pull_request14703
2019-07-24 09:32:54mentalsetnosy: + mental
messages: + msg348376
2019-07-23 20:41:48brett.cannonlinkissue37663 dependencies
2019-07-23 19:17:07brett.cannoncreate