Skip to content

venv activate script would be good to show failure. #88452

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

Closed
br-kim mannequin opened this issue Jun 2, 2021 · 8 comments
Closed

venv activate script would be good to show failure. #88452

br-kim mannequin opened this issue Jun 2, 2021 · 8 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@br-kim
Copy link
Mannequin

br-kim mannequin commented Jun 2, 2021

BPO 44286
Nosy @vsajip, @br-kim

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-06-04.13:43:29.519>
created_at = <Date 2021-06-02.14:03:06.187>
labels = ['3.7', 'invalid', 'type-feature', 'library']
title = 'venv activate script would be good to show failure.'
updated_at = <Date 2021-06-04.13:43:29.518>
user = 'https://github.com/br-kim'

bugs.python.org fields:

activity = <Date 2021-06-04.13:43:29.518>
actor = 'idle947'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-04.13:43:29.519>
closer = 'idle947'
components = ['Library (Lib)']
creation = <Date 2021-06-02.14:03:06.187>
creator = 'idle947'
dependencies = []
files = []
hgrepos = []
issue_num = 44286
keywords = []
message_count = 5.0
messages = ['394909', '394997', '395002', '395074', '395083']
nosy_count = 2.0
nosy_names = ['vinay.sajip', 'idle947']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue44286'
versions = ['Python 3.7']

@br-kim
Copy link
Mannequin Author

br-kim mannequin commented Jun 2, 2021

I changed the path of the project using venv, so it didn't work properly.

I thought it worked successfully because there was a (venv) on the terminal line.

However, the __VENV_DIR__ in the

set "VIRTUAL_ENV=VENV_DIR"

in the activate script did not work properly because it was the PATH before the replacement.

How about adding a procedure to verify the __VENV_DIR__ is a valid PATH to the Activate Script?

@br-kim br-kim mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jun 2, 2021
@vsajip
Copy link
Member

vsajip commented Jun 3, 2021

venvs aren't meant to be portable (i.e. renaming the directory or moving to a new location). Scripts installed into a venv have absolute paths pointing to the location when the venv was created.

venvs should be treated as throwaway resources that can be readily recreated - so if you need a venv in a new location, just create it there and install the desired packages again.

@br-kim
Copy link
Mannequin Author

br-kim mannequin commented Jun 3, 2021

Thank you for your explanation of venv.

I understand that venv is not portable.

But I was confused because the venv was written on the left side of the terminal line and it looked like it was working.

Therefore, if venv does not work, such as if __venv_dir__ is invalid path, i thought any warning message or exception handling was necessary. Or (venv) doesn't show up.

@vsajip
Copy link
Member

vsajip commented Jun 4, 2021

i thought any warning message or exception handling was necessary. Or (venv) doesn't show up.

No, because these are just scripts created by venv and placed in the venv's directory tree. If you then move the files to a different location, they aren't engineered to notice, and so will not behave as you expect.

As venv's aren't meant to be moved, there's no particular reason to engineer for that possibility.

OK to close this issue as "not a bug"?

@br-kim
Copy link
Mannequin Author

br-kim mannequin commented Jun 4, 2021

Okay. Thank you for the detailed explanation.

@br-kim br-kim mannequin closed this as completed Jun 4, 2021
@br-kim br-kim mannequin added the invalid label Jun 4, 2021
@br-kim br-kim mannequin closed this as completed Jun 4, 2021
@br-kim br-kim mannequin added the invalid label Jun 4, 2021
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@carlwr
Copy link

carlwr commented Oct 10, 2022

I suggest re-opening this issue.

These problems seem common and elusive[1] and the fix is seemingly easy to implement.

Motivation

If an unknowing user...

  1. moves or renames the project directory, and then
  2. activates the venv

...then, the current behaviour is:

  • silently, the venv won't be activated
  • no error message or hint about this will be given
  • to the contrary, the user will believe everything is OK since the prompt changes
  • there will be all kinds of strange and hard-to-troubleshoot behaviour, e.g. the wrong (system) interpreter being used and installing packages will install them globally

(this happened for me; I came here after hours of trouble-shooting)

The desired behaviour would be:

  • print an error message
  • don't activate the venv

@vsajip:

venvs aren't meant to be portable

Apparently not, which is fine. This would be an argument for this easy fix.

Renaming or moving a source/project dir is common, and some users will expect it to be possible, believing paths are relative, as is most common for tools working on source dirs. If in doubt, it's not unlikely for a user to simply try, and then believe all is well since the venv seemingly activates successfully (the prompt changes). Often, no problems show up at first; instead subtle problems appear later, stemming from unknowingly installing to and using global packages.

Fix

It seems this fix would be easy to implement.

https://github.com/python/cpython/blob/main/Lib/venv/scripts/common/activate:

now:

[...]
# unset irrelevant variables
deactivate nondestructive

VIRTUAL_ENV="__VENV_DIR__"
export VIRTUAL_ENV
[...]

change to something like:

[...]
# unset irrelevant variables
deactivate nondestructive

if [ ! -d "$__VENV_DIR__" ]; then
    echo "Error: virtual environment directory $__VENV_DIR__ doesn't exist." >&2
else
  VIRTUAL_ENV="__VENV_DIR__"
  export VIRTUAL_ENV
  [...] # (rest of file in this else-clause)
fi

(The absolute path in __VENV_DIR__ not existing is not an absolute guarantee that the project dir wasn't moved/renamed, but would probably catch 95% of these cases. Importantly, there won't be any false positives: if no dir exists for the path in __VENV_DIR__, there's no chance that activating this path as a venv is successful.)

[1]: seemingly common and frustrating problem

Some examples of reports of this specific problem below. Most mention being misled by lack of error message and by believing the prompt changing indicated successful activation. There's sometimes a high degree of frustration, and a simple error message seemingly would have helped most of them. Several at first had no indication that the experienced problems were caused by a move/rename of the source dir.

(there are many more)

Some SO question on moving source dirs with venv, and on whether this is possible/problems that follow from it:

Notes

For reference, here are some requests to make the venv paths relative, along with explanations for why that would be a lot of work/not viable:

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 10, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 10, 2022
@vsajip
Copy link
Member

vsajip commented Oct 10, 2022

I added a warning about non-portability of venvs to the documentation. I can try and review any PRs with tests that are related to error checking in the activation scripts.

@carlwr
Copy link

carlwr commented Oct 10, 2022

Doc update: that's great.

Activation script error check:

I'm not the right person to make a PR unfortunately.

I still suggest re-opening this issue so an error check is addd eventually. Not having one has evidently caused a lot of users a lot of trouble and the fix is straight-forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants