classification
Title: Allow pyvenv to work in existing directory
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: vinay.sajip Nosy List: asvetlov, carljm, eric.araujo, georg.brandl, pitrou, python-dev, stefanholek, vinay.sajip
Priority: normal Keywords: patch

Created on 2012-08-24 09:15 by stefanholek, last changed 2012-10-11 16:32 by asvetlov. This issue is now closed.

Files
File name Uploaded Description Edit
pyvenv.diff vinay.sajip, 2012-08-24 12:17 Patch to initialise venv dir differently review
allow-existing-dirs.diff vinay.sajip, 2012-10-02 11:30 review
Repositories containing patches
http://hg.python.org/sandbox/vsajip#fix15776
Messages (25)
msg168990 - (view) Author: Stefan Holek (stefanholek) Date: 2012-08-24 09:15
With virtualenv I can do

    $ virtualenv .

but with pyvenv I get

    $pyvenv .
    Error: Directory exists: /Users/stefan/sandbox/foo

Please allow pyvenv to apply to existing directories.
msg168998 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-24 11:31
Does it mean implicit implying --upgrade option if venv dir is '.'?
msg169004 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-08-24 12:17
Running

pyvenv --clear .

should work, but doesn't because of the way the venv directory is initialised - a shutil.rmtree() call is used. This can cause problems on Windows (current directory is regarded as open and so cannot be deleted) and also on Posix, because the inode changes and you get "file not found" errors because e.g. the shell has pointers to the old inode and the venv files are added to the new inode.

The attached patch should work, as it deletes the venv directory contents rather than the venv directory itself. I'll just check with Georg that it's OK to commit this - I don't see any problem, as it's a bug-fix rather than a new feature, but I think it best to run it past him.
msg169009 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-24 12:42
I don't like current --clear behavior, it's really useless because now venv just deletes everything from virtual environment.
You lose not only virtual env but files from your project also.
virtualenv cleans only <env>/Lib directory, that's much better.

I think venv --clear should cleanup everything which it creates (I mean bin/include/lib for Posix and Scripts/Include/Lib for Windows). Not sure about pyvenv.cfg
msg169011 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-08-24 12:50
Venvs should be regarded as throwaway; there is really no reason to add other files (e.g. project files) to venvs.

Two common patterns are:

1. Use a single place for all venvs (virtualenvwrapper does this)
2. Use a venv in a subdirectory of a project directory

The current implementation follows PEP 405, and the functionality was discussed on python-dev before being finalised.
msg169015 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-24 13:03
Well, I see. Agree with your patch then, it is correct.
msg169030 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-08-24 15:54
LGTM, please apply.
msg169032 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-08-24 15:58
os.path.isdir("foo") will return True if "foo" is a symlink to a directory, and then shutil.rmtree("foo") will fail:

>>> os.path.isdir("foo")
True
>>> os.path.islink("foo")
True
>>> shutil.rmtree("foo")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/antoine/opt/lib/python3.3/shutil.py", line 456, in rmtree
    "Not a directory: '{}'".format(path))
NotADirectoryError: [Errno 20] Not a directory: 'foo'
msg169033 - (view) Author: Roundup Robot (python-dev) Date: 2012-08-24 16:00
New changeset 0668fc196ce5 by Andrew Svetlov in branch 'default':
Issue #15776: Allow pyvenv to work in existing directory with --clean.
http://hg.python.org/cpython/rev/0668fc196ce5
msg169034 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-24 16:03
It is bug in shutil.rmtree, right?
msg169037 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-08-24 16:08
> It is bug in shutil.rmtree, right?

Read the documentation:

 shutil.rmtree(path, ignore_errors=False, onerror=None)

    Delete an entire directory tree; path must point to a directory (but
not a symbolic link to a directory)
msg169038 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-08-24 16:10
Another *perfect* example how even the most innocuous-seeming patch can be wrong.
msg169041 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-08-24 16:20
Good point.
I will prepare the patch to fix this.
msg169049 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-08-24 17:00
> Two common patterns are:
>
> 1. Use a single place for all venvs (virtualenvwrapper does this)
> 2. Use a venv in a subdirectory of a project directory

I’m a recent virtualenv user, but before I became a virtualenvwrapper fan I used to create venvs in the project top-level directory (typically a Mercurial clone root), using “virtualenv .”.
msg169057 - (view) Author: Stefan Holek (stefanholek) Date: 2012-08-24 17:49
Hm. What I am actually after is to "bless" an existing directory – source files and all – with a virtualenv (or pyvenv). I am not interested in the command deleting anything from anywhere, why thank you.

Workflow:

    $ git clone git@github.com:stefanholek/foo
    $ cd foo
    $ virtualenv .
    $ ./bin/python setup.py develop
    $ ./bin/python setup.py -q test

This is how I use virtualenv at the moment and I'd rather not lose that ability. Thanks.
msg169062 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-08-24 18:17
> This is how I use virtualenv at the moment and I'd rather not lose that ability. Thanks.

Well, changing it to do that means changing functionality, which is disallowed at this point in the release process.
msg169064 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-08-24 18:20
> I used to create venvs in the project top-level directory (typically a Mercurial clone root), using “virtualenv .”

I've used option 2 for this, using e.g. "virtualenv env" in the project directory.
msg169066 - (view) Author: Stefan Holek (stefanholek) Date: 2012-08-24 18:26
Sorry for being late. I'll make a feature request for 3.4 then.
msg169068 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-08-24 18:47
> Well, changing it to do that means changing functionality, which is disallowed at this point in the release process.
I think people used to “virtualenv .” can see this as a regression between virtualenv and pyvenv, but if the PEP did not list that use case then it’s too late.  I’d suggest we even back out the “pyvenv --clear .” commit, which did not address the needs of the “virtualenv .” users.
msg169070 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-08-24 18:54
Le vendredi 24 août 2012 à 18:47 +0000, Éric Araujo a écrit :
> Éric Araujo added the comment:
> 
> > Well, changing it to do that means changing functionality, which is
> disallowed at this point in the release process.
> I think people used to “virtualenv .” can see this as a regression
> between virtualenv and pyvenv, but if the PEP did not list that use
> case then it’s too late.  I’d suggest we even back out the “pyvenv
> --clear .” commit, which did not address the needs of the
> “virtualenv .” users.

Agreed with Eric. The feature could be added correctly in 3.4.
msg169075 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2012-08-24 19:14
Reverted.
msg169130 - (view) Author: Roundup Robot (python-dev) Date: 2012-08-25 11:22
New changeset b200acb6bed4 by Andrew Svetlov in branch 'default':
Delete Misc/NEWS record for reverted #15776.
http://hg.python.org/cpython/rev/b200acb6bed4
msg171809 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-10-02 15:13
LGTM.
msg172658 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-11 16:23
New changeset c3c188a0325a by Vinay Sajip in branch 'default':
Closes #15776: pyvenv now works with existing directories.
http://hg.python.org/cpython/rev/c3c188a0325a
msg172660 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-11 16:32
Looks good. Thank you.
History
Date User Action Args
2012-10-11 16:32:02asvetlovsetmessages: + msg172660
2012-10-11 16:23:02python-devsetstatus: open -> closed
resolution: fixed
messages: + msg172658

stage: patch review -> resolved
2012-10-02 15:13:33eric.araujosetpriority: critical -> normal

messages: + msg171809
stage: needs patch -> patch review
2012-10-02 11:30:37vinay.sajipsetfiles: + allow-existing-dirs.diff
2012-10-02 11:29:56vinay.sajipsethgrepos: + hgrepo150
2012-08-30 13:54:36vinay.sajipsetnosy: + carljm
2012-08-25 11:22:22python-devsetmessages: + msg169130
2012-08-25 11:20:22asvetlovsetpriority: release blocker -> critical
stage: needs patch
2012-08-24 19:14:08vinay.sajipsetversions: - Python 3.3
messages: + msg169075

assignee: vinay.sajip
components: + Library (Lib), - None
type: behavior -> enhancement
2012-08-24 18:54:02pitrousetmessages: + msg169070
2012-08-24 18:47:06eric.araujosetmessages: + msg169068
2012-08-24 18:26:12stefanholeksetmessages: + msg169066
2012-08-24 18:20:33vinay.sajipsetmessages: + msg169064
2012-08-24 18:17:39vinay.sajipsetmessages: + msg169062
2012-08-24 17:49:16stefanholeksetmessages: + msg169057
2012-08-24 17:00:03eric.araujosetnosy: + eric.araujo
messages: + msg169049
2012-08-24 16:20:01asvetlovsetmessages: + msg169041
2012-08-24 16:19:32georg.brandlsetpriority: normal -> release blocker
2012-08-24 16:10:56georg.brandlsetmessages: + msg169038
2012-08-24 16:08:45pitrousetmessages: + msg169037
2012-08-24 16:03:52asvetlovsetmessages: + msg169034
2012-08-24 16:00:28python-devsetnosy: + python-dev
messages: + msg169033
2012-08-24 15:58:19pitrousetnosy: + pitrou
messages: + msg169032
2012-08-24 15:54:33georg.brandlsetmessages: + msg169030
2012-08-24 13:03:03asvetlovsetmessages: + msg169015
2012-08-24 12:50:58vinay.sajipsetmessages: + msg169011
2012-08-24 12:42:53asvetlovsetmessages: + msg169009
2012-08-24 12:17:44vinay.sajipsetfiles: + pyvenv.diff

nosy: + georg.brandl
messages: + msg169004

keywords: + patch
2012-08-24 11:31:40asvetlovsetnosy: + asvetlov
messages: + msg168998
2012-08-24 09:17:22ned.deilysetnosy: + vinay.sajip
2012-08-24 09:15:46stefanholekcreate