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.

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

Allow pyvenv to work in existing directory #59980

Closed
stefanholek mannequin opened this issue Aug 24, 2012 · 25 comments
Closed

Allow pyvenv to work in existing directory #59980

stefanholek mannequin opened this issue Aug 24, 2012 · 25 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@stefanholek
Copy link
Mannequin

stefanholek mannequin commented Aug 24, 2012

BPO 15776
Nosy @birkenfeld, @vsajip, @pitrou, @carljm, @merwok, @asvetlov
Files
  • pyvenv.diff: Patch to initialise venv dir differently
  • allow-existing-dirs.diff
  • 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 = 'https://github.com/vsajip'
    closed_at = <Date 2012-10-11.16:23:02.524>
    created_at = <Date 2012-08-24.09:15:46.383>
    labels = ['type-feature', 'library']
    title = 'Allow pyvenv to work in existing directory'
    updated_at = <Date 2012-10-11.16:32:02.656>
    user = 'https://bugs.python.org/stefanholek'

    bugs.python.org fields:

    activity = <Date 2012-10-11.16:32:02.656>
    actor = 'asvetlov'
    assignee = 'vinay.sajip'
    closed = True
    closed_date = <Date 2012-10-11.16:23:02.524>
    closer = 'python-dev'
    components = ['Library (Lib)']
    creation = <Date 2012-08-24.09:15:46.383>
    creator = 'stefanholek'
    dependencies = []
    files = ['26983', '27384']
    hgrepos = ['150']
    issue_num = 15776
    keywords = ['patch']
    message_count = 25.0
    messages = ['168990', '168998', '169004', '169009', '169011', '169015', '169030', '169032', '169033', '169034', '169037', '169038', '169041', '169049', '169057', '169062', '169064', '169066', '169068', '169070', '169075', '169130', '171809', '172658', '172660']
    nosy_count = 8.0
    nosy_names = ['georg.brandl', 'vinay.sajip', 'pitrou', 'carljm', 'eric.araujo', 'asvetlov', 'stefanholek', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue15776'
    versions = ['Python 3.4']

    @stefanholek
    Copy link
    Mannequin Author

    stefanholek mannequin commented Aug 24, 2012

    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.

    @stefanholek stefanholek mannequin added the type-bug An unexpected behavior, bug, or error label Aug 24, 2012
    @asvetlov
    Copy link
    Contributor

    Does it mean implicit implying --upgrade option if venv dir is '.'?

    @vsajip
    Copy link
    Member

    vsajip commented Aug 24, 2012

    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.

    @asvetlov
    Copy link
    Contributor

    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

    @vsajip
    Copy link
    Member

    vsajip commented Aug 24, 2012

    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.

    @asvetlov
    Copy link
    Contributor

    Well, I see. Agree with your patch then, it is correct.

    @birkenfeld
    Copy link
    Member

    LGTM, please apply.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 24, 2012

    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'

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 24, 2012

    New changeset 0668fc196ce5 by Andrew Svetlov in branch 'default':
    Issue bpo-15776: Allow pyvenv to work in existing directory with --clean.
    http://hg.python.org/cpython/rev/0668fc196ce5

    @asvetlov
    Copy link
    Contributor

    It is bug in shutil.rmtree, right?

    @pitrou
    Copy link
    Member

    pitrou commented Aug 24, 2012

    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)

    @birkenfeld
    Copy link
    Member

    Another *perfect* example how even the most innocuous-seeming patch can be wrong.

    @asvetlov
    Copy link
    Contributor

    Good point.
    I will prepare the patch to fix this.

    @merwok
    Copy link
    Member

    merwok commented Aug 24, 2012

    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 .”.

    @stefanholek
    Copy link
    Mannequin Author

    stefanholek mannequin commented Aug 24, 2012

    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.

    @vsajip
    Copy link
    Member

    vsajip commented Aug 24, 2012

    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.

    @vsajip
    Copy link
    Member

    vsajip commented Aug 24, 2012

    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.

    @stefanholek
    Copy link
    Mannequin Author

    stefanholek mannequin commented Aug 24, 2012

    Sorry for being late. I'll make a feature request for 3.4 then.

    @merwok
    Copy link
    Member

    merwok commented Aug 24, 2012

    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.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 24, 2012

    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.

    @vsajip
    Copy link
    Member

    vsajip commented Aug 24, 2012

    Reverted.

    @vsajip vsajip added the stdlib Python modules in the Lib dir label Aug 24, 2012
    @vsajip vsajip self-assigned this Aug 24, 2012
    @vsajip vsajip added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Aug 24, 2012
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 25, 2012

    New changeset b200acb6bed4 by Andrew Svetlov in branch 'default':
    Delete Misc/NEWS record for reverted bpo-15776.
    http://hg.python.org/cpython/rev/b200acb6bed4

    @merwok
    Copy link
    Member

    merwok commented Oct 2, 2012

    LGTM.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 11, 2012

    New changeset c3c188a0325a by Vinay Sajip in branch 'default':
    Closes bpo-15776: pyvenv now works with existing directories.
    http://hg.python.org/cpython/rev/c3c188a0325a

    @python-dev python-dev mannequin closed this as completed Oct 11, 2012
    @asvetlov
    Copy link
    Contributor

    Looks good. Thank you.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants