Rewrite virtualenv #697
Rewrite virtualenv #697
Conversation
…s the system implementation as an argument.
…something wrong with the output: on 2.7 they are unicode literals).
…n absolute (in the builder's `__init__`)
…windows the subprocess must have the SYSTEMROOT env var, otherwise it cannot load DLLs.
|
Sorry to come to this discussion late. Can I ask - what is the plan here? Would it be worth making this rewrite into a temporary pip package such as |
|
Oh sorry - I see now that you already have a That branch (not this one) also solved my Windows virtualenv woes on Python 3.5 (as well as working on 2.7. 3.3 3.4). I was getting this error on Python 3.5: I wasn't getting this error : #796 - but perhaps the rewrite fixes that too? |
The rewrite fixes that because it will use the |
I have published that as an experiment. In retrospect I think it's a bad idea since the legacy virtualenv will still be installed if you install that. It's like a last one wins situation, and in some cases it's hard to predict which one will run. |
|
I wonder if there is a way forward here. Is there general agreement that this direction is the way forward? Is it possible that the risk from accepting so much code without much review is outweighed by the cost of maintaining the older code-base? Would it be worth biting the bullet and agreeing on a fork to a project with another name, to get wider testing? |
|
Hello! As part of an effort to ease the contribution process and adopt a more standard workflow pip has switched to doing development on the If you do nothing, this Pull Request will be automatically closed by @BrownTruck since it cannot be merged. If this pull request is still valid, please rebase it against If you choose to rebase/merge and resubmit this Pull Request, here is an example message that you can copy and paste: |
|
This Pull Request was closed because it cannot be automatically reparented to the Please feel free to re-open it or re-submit it if it is still valid and you have rebased it onto |
|
Is there still interest in this? |
|
I still think this is a good idea, but I think the tooling landscape has changed sufficiently to make it worthwhile to reassess whether an in-place rewrite is still preferable to the alternative option of introducing a second venv management library:
The UX difference between the two approaches is essentially as follows:
Putting it like that, I think it's clear that despite the potential compatibility risks, the in-place rewrite is likely still the better approach, as it offers the clearest long-term path to switching entirely to PEP 405 based virtual environments as various other projects are able to start dropping support for Python 2.7. The backwards compatibility risks could then be mitigated by:
|
|
I think that sounds like a really good plan and will be happy to help getting this tested with tox in the beta stage. |
|
One thought I had, which I've been meaning to try when I get time, was to simply check at the start of virtualenv, whether the request was one that could be satisfied by the core venv library (basically that means there's no This is basically an alternative implementation strategy for @ncoghlan's "Option 1", with a lot less complexity than @dstufft's approach, at the cost of a significantly higher risk that we break backward compatibility (albeit only in ways that can be argued as "implementation details" now that venv exists). Treating this less as "rewrite virtualenv to use the core machinery" and more like "make virtualenv a frontend for core venv, with a fallback to the old code where needed" seems to me to be lower-risk in the long term. Thoughts? |
|
I personally would still like to finish the rewrite... I just haven't been able to make time for virtualenv lately with Warehouse chewing up most of my OSS time. I'm hoping that once I get that launched I'll have more time for this... but I can't promise that. So that being said, if someone wants to do something else, please don't feel like you need to wait on me. |
|
I'll try to take a look at this as part of PyCon 2018 sprints. |
|
A unified interface for remote execution and local directory sync/sharing would be nice. Vagrant, multipass, minikube, snapcraft, tox - they could benefit from it. |
This Pull Request was previously #691 however I've moved the rewrite branch to pypa/virtualenv to make it more official.
This is a major change to the virtualenv code base, and as thus I want to get some sign off and opinions from the other @pypa/virtualenv-developers. I don't consider this idea a forgone conclusion without input from the other core developers.
This rewrites the entirety of virtualenv, this is a "dangerous" thing to do which risks breaking things that have been working and generally complete rewrites are considered a bad idea. However while attempting to fix errors like #694, #671, #666, #650, #672, #687, #674, #648, etc I ran into situation where I believe it's not feasible to fix all of the issues with virtualenv given the current methods it employs. In particular I believe that the way
-pis implemented makes it extremely fragile if not impossible to work accurately in all situations.In particular I believe there are a number of issues, most of which stem from architectural issues with virtualenv itself. The (incomplete) list of issues are:
-pfunctionality to restart the script, which is likely installed into one Python'ssite-packagesdirectory, seems to be fundamentally flawed in a way where it's not reasonable to fix it in all situations.virtualenv.pymodule attempts to run as a single file, however it can't actually do that because it requires thevirtualenv_supportdirectory to exist alongside it. I believe that it's saner and easier to have a single directoryvirtualenv/directory which contains everything that needs to exist for an install to work. This also removes giant base64 encoded blobs from needing to be maintained inside of thevirtualenv.pyfile.site.pythat we maintain inside of virtualenv. However thissite.pymisses features from later versions of Python which means that there are subtle differences in how Python behaves from within a virtual environment and without.In contrast the rewrite is structured in a different way.
In order to still support the
-poption without needing to implement the hackish "restart in target interpreter" logic, the rewrite instead (for the "legacy" isolation method) inspects the target environment for any values it needs. It does this by running a subprocess which will shove all of the required values into a dictionary, and then dump that as json. When the virtualenv script pulls these values out it then loads the json and then has all of the values it needs in order to construct the virtual environment without needing to execute itself with the target interpreter.Given that newer versions of Python has built in isolation the rewrite wants to take advantage of that. It will detect if the target Python has the
venvmodule, and if they do it will execute a short script (via-c) that will do the actual creation of the virtual environment (without any scripts or tools installed). This will also allow alternative implementations (like PyPy) to implement the isolation themselves so that they can ensure that isolation works correctly in their interpreter without needing to maintain a set of instructions inside of virtualenv for handling their interpreter.The rewrite also attempts to start fresh with none of the additional hacks. This will mean that we'll likely be broken in certain situations and we'll need to determine how the old virtualenv solved that problem and reapply any additional hacks. However this time we can hopefully ensure that we document/comment what those hacks are and why we're doing it. We'll also ideally write a good set of tests for those hacks so that if we ever remove them in the future we'll have a test suite that will tell us if we broke anything by removing them. Another benefit is that since the hacks are (almost?) always related to the legacy isolation mechanism they can all be quarantined to the
virtualenv.builders.legacy.LegacyBuilderclass instead of leaking all over the code base.The new code base is also implemented in a much more modular way. Instead of a ball of spaghetti where the implementation of isolation is spread all over the place, the new code uses "Builder" classes which handle creating the actual environments. In addition (thanks to @ionelmc) it utilizes a "flavor" system which abstracts away the differences between Windows and Posix in the builder code.
In addition to the better (imo) layout of the code base, the new code base is also going to mandate that there is 100% unit test coverage at all times. This will help ensure that we continue to work through refactoring and the like in the future.
No matter which isolation method is in use, virtualenv itself will install it's own scripts and install it's own tools (pip and setuptools). This is so we can have consistent scripts and tool versions no matter which isolation mechanism is in play. It also makes it easier to get newer versions of the tooling and scripts without requiring people to update their entire Python interpreter. In other words if the venv isolation mechanism is available, we will only use it to create the isolated environment and nothing else.
The high level overview of what the major differences in capabilities are:
--relocatableno longer is supported as it never worked very well and I don't believe it's possible to actually make it work in all cases. It appears the major use case for this is to avoid re-compiling software, for which I believe Wheels are a better solution.site.pyfile in the rewrite allows us to copy a lot less files over than previously required. Almost the entire size of an empty virtual environment in the new rewrite is the executable files, which are required to be copied no matter what.--distributeoption is gone.--no-pipand--no-setuptoolsoptions now operate independently of each other. In addition there are now--setuptoolsand--pipoptions which will negate the--no-*options if they were given previously.sys.real_prefixwe havesys.base_prefixandsys.base_exec_prefixin order to match what the venv module does.virtualenv-X.Yentry points which didn't work correctly with Wheel anyways and are unneeded since it doesn't really matter what version of Python virtualenv is installed with.setup.pyand will always use the script wrappers in that situation.That major things left to do are:
There is an open question about how best to handle the transition. If we just merge this and release it then we risk regressing behavior for a lot of people since this is essentially brand new untested code. In the previous Pull Request @ncoghlan suggested releasing it on PyPI as something like virtualenv-rewrite for a period of time and asking people to switch to using that instead. I think that's a good idea and that's how I would plan the migration to go I think.
Summary of outstanding issues that get fixed/closed:
Summary of outstanding pull requests that get fixed/closed:
I am pretty sure that this fixes more issues and pull requests than I've listed here. However I don't feel like going through every single issue and determining which ones are still valid when they would require me to actually attempt to reproduce the issues instead of simply looking at the issue and determining if the new code can handle that situation.