Skip to content
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.

Sign up for GitHub

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

Jump to bottom

virtualenvwrapper plugin fail to deactivate virtualenv when parent directory is a git repository #4603

Closed
gled-rs opened this issue Nov 11, 2015 · 6 comments · Fixed by #5663
Closed

virtualenvwrapper plugin fail to deactivate virtualenv when parent directory is a git repository #4603

gled-rs opened this issue Nov 11, 2015 · 6 comments · Fixed by #5663
Labels
Area: plugin Issue or PR related to a plugin

Comments

@gled-rs
Copy link

gled-rs commented Nov 11, 2015

Consider the following directory layout:

/home/src
/home/src/gitrepo => git repository
/home/src/gitrepo/adirwithvirtualenv => a repository with a virtualenv ( either a .venv or else ).

When going from adirwithvirtualenv to gitrepo ( cd .. ), deactivate is not called because $ENV_NAME is filled with the name of the directory, as it is a git repository.
@gled-rs
Copy link
Author

gled-rs commented Nov 11, 2015

Pull request: #4604

@apjanke
Copy link
Contributor

apjanke commented Nov 12, 2015

I think this is intended behavior. Git repos, even without venv-specific customizations, are considered to be project roots, so when this happens, it should switch you from the adirwithvirtualenv venv to the gitrepo venv. I think just calling workon without deactivate first will accomplish that. This change looks like it would end up fully deactivating the venvs in the example case. (Is that what it's intended to do? Or should deactivate just be called before activating the new environment?)

Have a look at the discussion in #4122, #4110, and #3918. There was a while when virtualenvwrapper did not consider plain git repos to be project roots, but this was a behavior accidentally introduced by #3918.

Maybe you'd want to add a setting that controls whether ordinary git repos are considered project roots? At any case, we should probably run this by the other virtualenvwrapper users before merging, since they've already had some unexpected behavior changes this year.

gled-rs added a commit to gled-rs/oh-my-zsh that referenced this issue Nov 12, 2015
@gled-rs
virtualenvwrapper: add the option DEACTIVATE_VENV_ASAP to cope with i…
acb0a82 
…ssue ohmyzsh#4603
@gled-rs
Copy link
Author

gled-rs commented Nov 12, 2015

In my case, gitrepo is not a virtualenv. Hence the annoying behaviour.

But yes, it may be a change for some people, thanks for the headsup !

Pull request has been updated to add an option DEACTIVATE_VENV_ASAP to control that behaviour.

Without setting that option, the behaviour is the same as previously.

@apjanke
Copy link
Contributor

apjanke commented Nov 14, 2015

Cool.

Maybe name it VENV_DEACTIVATE_ASAP so it's prefixed with the plugin name? We're starting to get a lot of config variables floating around, and that might help keep it organized.

@jaytavares jaytavares mentioned this issue Jan 26, 2017
Closed
@jaytavares
Copy link
Contributor

What's the status of this issue? It would appear that this is a problem not only when the parent directory is a git repo but any time the destination directory of the cd command is. (see referenced issue above)

@jaytavares jaytavares mentioned this issue Jan 26, 2017
Closed
@jaytavares
Copy link
Contributor

jaytavares commented Jan 26, 2017 •
edited

I just submitted a PR (#5817) with my stab at this issue. I took a different approach which also resolves another issue (#5816). Any thoughts are much appreciated.

@jaytavares jaytavares mentioned this issue Mar 22, 2018
Merged
@domsekotill domsekotill mentioned this issue May 26, 2018
Merged
@mcornella mcornella added the Area: plugin Issue or PR related to a plugin label Mar 12, 2020
robbyrussell added a commit that referenced this issue Jun 12, 2021
@domsekotill @robbyrussell
fix(virtualenvwrapper): several changes for checking git directory, i…
94ea7b4 
…ncluding fixes (#5663)

* Test only for the presence of a .git directory in virtualenvwrapper

Instead of using both $(git rev-parse --show-toplevel) and a check for
a .git directory, use just the latter. As well as being redundant
the former does not work quite so well when using multiple worktrees;
each worktree will be treated as a separate project.

* Unset ENV_NAME & deactivate if no virtualenv found

This addresses #4603 without breaking current behaviour (where current
behaviour is correct).

When changing directories, if there is no environment matching
ENV_NAME, ENV_NAME is emptied and deactivate called if there is a
current environment active (based on CD_VIRTUAL_ENV).

* Use path comparison not string comparison for paths

This will solve part of issue #4255 where WORKON_HOME is defined with a
trailing slash or not normalised in some way, as well as instances
where symlinks are used, and any other instances where constructed
paths don't exactly match even though they go to the same file.

Co-authored-by: Robby Russell <robby@planetargon.com>
shlomif pushed a commit to shlomif/oh-my-zsh that referenced this issue Jun 20, 2021
@domsekotill @robbyrussell @shlomif
fix(virtualenvwrapper): several changes for checking git directory, i…
8fa25c7 
…ncluding fixes (ohmyzsh#5663)

* Test only for the presence of a .git directory in virtualenvwrapper

Instead of using both $(git rev-parse --show-toplevel) and a check for
a .git directory, use just the latter. As well as being redundant
the former does not work quite so well when using multiple worktrees;
each worktree will be treated as a separate project.

* Unset ENV_NAME & deactivate if no virtualenv found

This addresses ohmyzsh#4603 without breaking current behaviour (where current
behaviour is correct).

When changing directories, if there is no environment matching
ENV_NAME, ENV_NAME is emptied and deactivate called if there is a
current environment active (based on CD_VIRTUAL_ENV).

* Use path comparison not string comparison for paths

This will solve part of issue ohmyzsh#4255 where WORKON_HOME is defined with a
trailing slash or not normalised in some way, as well as instances
where symlinks are used, and any other instances where constructed
paths don't exactly match even though they go to the same file.

Co-authored-by: Robby Russell <robby@planetargon.com>
grantstephens pushed a commit to grantstephens/oh-my-zsh that referenced this issue Jun 23, 2021
@domsekotill @robbyrussell @grantstephens
fix(virtualenvwrapper): several changes for checking git directory, i…
c710a91 
…ncluding fixes (ohmyzsh#5663)

* Test only for the presence of a .git directory in virtualenvwrapper

Instead of using both $(git rev-parse --show-toplevel) and a check for
a .git directory, use just the latter. As well as being redundant
the former does not work quite so well when using multiple worktrees;
each worktree will be treated as a separate project.

* Unset ENV_NAME & deactivate if no virtualenv found

This addresses ohmyzsh#4603 without breaking current behaviour (where current
behaviour is correct).

When changing directories, if there is no environment matching
ENV_NAME, ENV_NAME is emptied and deactivate called if there is a
current environment active (based on CD_VIRTUAL_ENV).

* Use path comparison not string comparison for paths

This will solve part of issue ohmyzsh#4255 where WORKON_HOME is defined with a
trailing slash or not normalised in some way, as well as instances
where symlinks are used, and any other instances where constructed
paths don't exactly match even though they go to the same file.

Co-authored-by: Robby Russell <robby@planetargon.com>
dilfish pushed a commit to dilfish/oh-my-zsh that referenced this issue Aug 9, 2021
@domsekotill @robbyrussell
fix(virtualenvwrapper): several changes for checking git directory, i…
4e2b84f 
…ncluding fixes (ohmyzsh#5663)

* Test only for the presence of a .git directory in virtualenvwrapper

Instead of using both $(git rev-parse --show-toplevel) and a check for
a .git directory, use just the latter. As well as being redundant
the former does not work quite so well when using multiple worktrees;
each worktree will be treated as a separate project.

* Unset ENV_NAME & deactivate if no virtualenv found

This addresses ohmyzsh#4603 without breaking current behaviour (where current
behaviour is correct).

When changing directories, if there is no environment matching
ENV_NAME, ENV_NAME is emptied and deactivate called if there is a
current environment active (based on CD_VIRTUAL_ENV).

* Use path comparison not string comparison for paths

This will solve part of issue ohmyzsh#4255 where WORKON_HOME is defined with a
trailing slash or not normalised in some way, as well as instances
where symlinks are used, and any other instances where constructed
paths don't exactly match even though they go to the same file.

Co-authored-by: Robby Russell <robby@planetargon.com>
tinogomes pushed a commit to tinogomes/ohmyzsh that referenced this issue Sep 24, 2021
@domsekotill @robbyrussell @tinogomes
fix(virtualenvwrapper): several changes for checking git directory, i…
de84e42 
…ncluding fixes (ohmyzsh#5663)

* Test only for the presence of a .git directory in virtualenvwrapper

Instead of using both $(git rev-parse --show-toplevel) and a check for
a .git directory, use just the latter. As well as being redundant
the former does not work quite so well when using multiple worktrees;
each worktree will be treated as a separate project.

* Unset ENV_NAME & deactivate if no virtualenv found

This addresses ohmyzsh#4603 without breaking current behaviour (where current
behaviour is correct).

When changing directories, if there is no environment matching
ENV_NAME, ENV_NAME is emptied and deactivate called if there is a
current environment active (based on CD_VIRTUAL_ENV).

* Use path comparison not string comparison for paths

This will solve part of issue ohmyzsh#4255 where WORKON_HOME is defined with a
trailing slash or not normalised in some way, as well as instances
where symlinks are used, and any other instances where constructed
paths don't exactly match even though they go to the same file.

Co-authored-by: Robby Russell <robby@planetargon.com>
sudeeshjohn pushed a commit to sudeeshjohn/oh-my-zsh that referenced this issue Nov 10, 2021
@domsekotill @robbyrussell @sudeeshjohn
fix(virtualenvwrapper): several changes for checking git directory, i…
4caffee 
…ncluding fixes (ohmyzsh#5663)

* Test only for the presence of a .git directory in virtualenvwrapper

Instead of using both $(git rev-parse --show-toplevel) and a check for
a .git directory, use just the latter. As well as being redundant
the former does not work quite so well when using multiple worktrees;
each worktree will be treated as a separate project.

* Unset ENV_NAME & deactivate if no virtualenv found

This addresses ohmyzsh#4603 without breaking current behaviour (where current
behaviour is correct).

When changing directories, if there is no environment matching
ENV_NAME, ENV_NAME is emptied and deactivate called if there is a
current environment active (based on CD_VIRTUAL_ENV).

* Use path comparison not string comparison for paths

This will solve part of issue ohmyzsh#4255 where WORKON_HOME is defined with a
trailing slash or not normalised in some way, as well as instances
where symlinks are used, and any other instances where constructed
paths don't exactly match even though they go to the same file.

Co-authored-by: Robby Russell <robby@planetargon.com>
tekniklr pushed a commit to tekniklr/oh-my-zsh that referenced this issue Sep 6, 2022
@domsekotill @robbyrussell
fix(virtualenvwrapper): several changes for checking git directory, i…
4f7ad5d 
…ncluding fixes (ohmyzsh#5663)

* Test only for the presence of a .git directory in virtualenvwrapper

Instead of using both $(git rev-parse --show-toplevel) and a check for
a .git directory, use just the latter. As well as being redundant
the former does not work quite so well when using multiple worktrees;
each worktree will be treated as a separate project.

* Unset ENV_NAME & deactivate if no virtualenv found

This addresses ohmyzsh#4603 without breaking current behaviour (where current
behaviour is correct).

When changing directories, if there is no environment matching
ENV_NAME, ENV_NAME is emptied and deactivate called if there is a
current environment active (based on CD_VIRTUAL_ENV).

* Use path comparison not string comparison for paths

This will solve part of issue ohmyzsh#4255 where WORKON_HOME is defined with a
trailing slash or not normalised in some way, as well as instances
where symlinks are used, and any other instances where constructed
paths don't exactly match even though they go to the same file.

Co-authored-by: Robby Russell <robby@planetargon.com>
nbaronov pushed a commit to nbaronov/oh-my-zsh that referenced this issue Aug 3, 2023
@domsekotill @robbyrussell @nbaronov
fix(virtualenvwrapper): several changes for checking git directory, i…
610523f 
…ncluding fixes (ohmyzsh#5663)

* Test only for the presence of a .git directory in virtualenvwrapper

Instead of using both $(git rev-parse --show-toplevel) and a check for
a .git directory, use just the latter. As well as being redundant
the former does not work quite so well when using multiple worktrees;
each worktree will be treated as a separate project.

* Unset ENV_NAME & deactivate if no virtualenv found

This addresses ohmyzsh#4603 without breaking current behaviour (where current
behaviour is correct).

When changing directories, if there is no environment matching
ENV_NAME, ENV_NAME is emptied and deactivate called if there is a
current environment active (based on CD_VIRTUAL_ENV).

* Use path comparison not string comparison for paths

This will solve part of issue ohmyzsh#4255 where WORKON_HOME is defined with a
trailing slash or not normalised in some way, as well as instances
where symlinks are used, and any other instances where constructed
paths don't exactly match even though they go to the same file.

Co-authored-by: Robby Russell <robby@planetargon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area: plugin Issue or PR related to a plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

virtualenvwrapper: several changes, including fixes
4 participants
@jaytavares @mcornella @apjanke @gled-rs