Repository: theacodes/nox
Pull request #231 (2019-08-05 05:55:49)

add venv as an option for venv_backend
### Summary

This PR adds support for a different backend, `venv`. It adds a keyword argument to the `VirtualEnv` class, which defaults to `False` and is only true when the nox session has `venv_backend=True`.

It also updates the resolution of the Python path, since using `venv` alone does not fix issues when creating a venv from within a virtual environment.

The path resolution is modified to determine if a virtual environment is being used. If so, it tries to find the desired Python interpreter version in the `base_prefix` rather than anywhere on the system's PATH. Note that Windows was not tested, and probably will require some more changes.

### Test Plan

```
# Create and enter virtual environment, then
cd ~/git/nox
pip install -e .
cd ~/git/pipx
# modify pipx's noxfile.py to have `venv_backend='venv'` for the unittests session
nox -s unittests-3.6  # tests pass
```

### Additional context
> prefix: A string giving the site-specific directory prefix where the platform independent Python files are installed

https://docs.python.org/3/library/sys.html#sys.prefix

> base_prefix: Set during Python startup, before site.py is run, to the same value as prefix. If not running in a virtual environment, the values will stay the same; if site.py finds that a virtual environment is in use, the values of prefix and exec_prefix will be changed to point to the virtual environment, whereas base_prefix and base_exec_prefix will remain pointing to the base Python installation (the one which the virtual environment was created from).

https://docs.python.org/3/library/sys.html#sys.base_prefix

Tox's implementation for venv support is included as a plugin in an additional package: https://github.com/tox-dev/tox-venv/blob/master/src/tox_venv/hooks.py

--- 

fixes https://github.com/theacodes/nox/issues/199
cc @pfmoore 

theacodes: I would definitely want to make the pythonfinder stuff a separate PR, if
that's okay with y'all.

Relatedly, I'm a bit wary about taking on new dependencies in Nox. It
definitely warrants a bit of discussion.

On Mon, Aug 5, 2019 at 10:14 AM Chad Smith <notifications@github.com> wrote:

> *@cs01* commented on this pull request.
> ------------------------------
>
> In nox/virtualenv.py
> <https://github.com/theacodes/nox/pull/231#discussion_r310705860>:
>
> > @@ -174,6 +175,26 @@ def create(self):
>          return True
>
>
> +def resolve_real_python_outside_venv(desired_intepreter: str) -> str:
> +    """Return path to the real Python installation based
> +
> +    See also:
> +    https://docs.python.org/3/library/sys.html#sys.prefix
> +    https://docs.python.org/3/library/sys.html#sys.base_prefix
> +    """
>
> I replaced all the code with pythonfinder but it isn't able to find any
> installations. I'll keep trying with it. If it works it will simplify the
> code tremendously.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/theacodes/nox/pull/231?email_source=notifications&email_token=AAB5I46L4NEAV63SGVE2YHTQDBNWLA5CNFSM4IJHWZ42YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCASY7AI#discussion_r310705860>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAB5I4ZLJ2EAUPFK4QNEYVDQDBNWLANCNFSM4IJHWZ4Q>
> .
>

pfmoore: I'm completely fine with the pythonfinder thing being a separate discussion. However, getting `resolve_real_python_outside_venv` to work without it is non-trivial (the existing code doesn't support Windows at all). I'll fish out some known-working code that does the job - both tox or virtualenv have this IIRC (and I have a suspicion I contributed the code in both cases!) Let me see what I can find.

But I do think that in the long term, duplicating this same bit of code in many projects is a bad idea...
cs01: @pfmoore I'm pretty sure this is tox's: https://github.com/tox-dev/tox-venv/blob/master/src/tox_venv/hooks.py

theacodes: If we land the PR to switch to pythonfinder *first*, does this PR become
easier/less complex?

On Mon, Aug 5, 2019 at 10:35 AM Chad Smith <notifications@github.com> wrote:

> @pfmoore <https://github.com/pfmoore> I'm pretty sure this is tox's:
> https://github.com/tox-dev/tox-venv/blob/master/src/tox_venv/hooks.py
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/theacodes/nox/pull/231?email_source=notifications&email_token=AAB5I477NUF6Q4FO4AMNKDTQDBQETA5CNFSM4IJHWZ42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3SQ2DY#issuecomment-518327567>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAB5I472HOLILTZYLKOK7NLQDBQETANCNFSM4IJHWZ4Q>
> .
>

cs01: @theacodes Assuming pythonfinder does what I think it does, yes. 

This PR is really doing two things: adding the venv_backend option of venv, and updating Python resolution to make venv creation from within a virtual environment work. It makes sense to review the two changes separately, I can separate it out.
pfmoore: > I'm pretty sure this is tox's

I'm not sure that's all of it. A key part is `tox_get_python_executable`, which is defined in the `tox.interpreters` subpackage). To get the available interpreters on Windows you need to look in the registry (PEP 514).

As a simpler workaround, you can let the launcher do the work for you, and call `py -X.Y -c "import sys; print(sys.executable)"` in a subprocess, but you need to be careful with things like setting `PYTHONIOENCODING` so that you support full Unicode paths.

The reason I'm in favour of a library solution is that it's really tricky to get all the edge cases correct. Of course, users can always just supply the full executable path, so it's not like it's critical, but it's not a good UX if the software can't find a Python installation you know is there.

> updating Python resolution to make venv creation from within a virtual environment work

Yeah, mixing virtualenvs and venvs is really messy. At this point I *think* you can safely create a virtualenv from within an active venv (see the discussions around https://github.com/pypa/virtualenv/pull/1345 for the ugly details) but I don't think creating a venv from within an active virtual environment (either venv or virtualenv) is even supported. So getting this fixed would be good (for general reliability, even if there's no specific issue it's causing right now).
pfmoore: Something like the following should work for Windows, for what it's worth. Lightly tested, it works on my PC but I only have one Python version installed, so I couldn't test much...

```python
import os
import subprocess

def get_python(major, minor=None):
    env = os.environ.copy()
    env['PYTHONIOENCODING'] = "utf-8"
    version = "-" + major
    if minor:
        version += "." + minor
    try:
        proc = subprocess.run(
            ["py", version, "-c", "import sys; print(sys.executable)"],
            env=env,
            capture_output=True
        )
    except Exception:
        # Just say we couldn't find anything if we get any sort of error
        return None
    if proc.returncode != 0:
        return None
    return proc.stdout.decode("utf-8").strip()

if __name__ == '__main__':
    import sys
    print(get_python(*sys.argv[1:]))
```
pfmoore: Whoa! I just looked at the list of dependencies for pythonfinder:

```
install_requires =
    attrs
    cached-property
    click
    crayons
    six
    packaging
    vistir[spinner]>=0.4
```

That's *way* too much for the job it's doing. Installing it pulled in 15 other packages, for a total of about 14M of data.

At this point, I'd say let's *not* use pythonfinder. That's just silly :-(
cs01: > Whoa! I just looked at the list of dependencies for pythonfinder:
> 
> ```
> install_requires =
>     attrs
>     cached-property
>     click
>     crayons
>     six
>     packaging
>     vistir[spinner]>=0.4
> ```
> 
> That's _way_ too much for the job it's doing. Installing it pulled in 15 other packages, for a total of about 14M of data.
> 
> At this point, I'd say let's _not_ use pythonfinder. That's just silly :-(

Just FYI I filed an issue in pythonfinder on this: https://github.com/sarugaku/pythonfinder/issues/78
cs01: Got the tests passing :tada: .

So to summarize, this PR adds support for `venv` as an alternate backend in a nox session. It does not modify the logic when resolving the interpreter, which is required to create a new venv from within a venv. That can be done in a different PR, possibly with `pythonfinder`, depending on how https://github.com/sarugaku/pythonfinder/issues/78 evolves. 

Patch:
diff --git a/docs/index.rst b/docs/index.rst
index 1984d08..dd61195 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -52,6 +52,7 @@ Nox is lucky to have several wonderful projects that use it and provide feedback
 
 - `Bezier <https://github.com/dhermes/bezier>`__
 - `gapic-generator-python <https://github.com/googleapis/gapic-generator-python>`__
+- `gdbgui <https://github.com/cs01/gdbgui>`__
 - `Google Assistant SDK <https://github.com/googlesamples/assistant-sdk-python>`__
 - `google-cloud-python <https://github.com/googlecloudplatform/google-cloud-python>`__
 - `google-resumable-media-python <https://github.com/GoogleCloudPlatform/google-resumable-media-python>`__
diff --git a/nox/sessions.py b/nox/sessions.py
index 2affcf7..ccf5993 100644
--- a/nox/sessions.py
+++ b/nox/sessions.py
@@ -375,9 +375,16 @@ class SessionRunner:
             self.venv = CondaEnv(
                 path, interpreter=self.func.python, reuse_existing=reuse_existing
             )
+        elif self.func.venv_backend == "venv":
+            self.venv = VirtualEnv(
+                path,
+                interpreter=self.func.python,
+                reuse_existing=reuse_existing,
+                venv=True,
+            )
         else:
             raise ValueError(
-                "Expected venv_backend one of ('virtualenv', 'conda'), but got '{}'.".format(
+                "Expected venv_backend one of ('virtualenv', 'conda', 'venv'), but got '{}'.".format(
                     self.func.venv_backend
                 )
             )
diff --git a/nox/virtualenv.py b/nox/virtualenv.py
index 9063b2b..836b642 100644
--- a/nox/virtualenv.py
+++ b/nox/virtualenv.py
@@ -196,12 +196,13 @@ class VirtualEnv(ProcessEnv):
 
     is_sandboxed = True
 
-    def __init__(self, location, interpreter=None, reuse_existing=False):
+    def __init__(self, location, interpreter=None, reuse_existing=False, *, venv=False):
         self.location_name = location
         self.location = os.path.abspath(location)
         self.interpreter = interpreter
         self._resolved = None
         self.reuse_existing = reuse_existing
+        self.venv_or_virtualenv = "venv" if venv else "virtualenv"
         super(VirtualEnv, self).__init__()
 
     _clean_location = _clean_location
@@ -275,21 +276,27 @@ class VirtualEnv(ProcessEnv):
             return os.path.join(self.location, "bin")
 
     def create(self):
-        """Create the virtualenv."""
+        """Create the virtualenv or venv."""
         if not self._clean_location():
             logger.debug(
-                "Re-using existing virtualenv at {}.".format(self.location_name)
+                "Re-using existing virtual environment at {}.".format(
+                    self.location_name
+                )
             )
             return False
 
-        cmd = [sys.executable, "-m", "virtualenv", self.location]
-
-        if self.interpreter:
-            cmd.extend(["-p", self._resolved_interpreter])
+        if self.venv_or_virtualenv == "virtualenv":
+            cmd = [sys.executable, "-m", "virtualenv", self.location]
+            if self.interpreter:
+                cmd.extend(["-p", self._resolved_interpreter])
+        else:
+            cmd = [self._resolved_interpreter, "-m", "venv", self.location]
 
         logger.info(
-            "Creating virtualenv using {} in {}".format(
-                os.path.basename(self._resolved_interpreter), self.location_name
+            "Creating virtual environment ({}) using {} in {}".format(
+                self.venv_or_virtualenv,
+                os.path.basename(self._resolved_interpreter),
+                self.location_name,
             )
         )
         nox.command.run(cmd, silent=True, log=False)


Test patch:
diff --git a/tests/test_sessions.py b/tests/test_sessions.py
index 0b9559a..dbe72bb 100644
--- a/tests/test_sessions.py
+++ b/tests/test_sessions.py
@@ -514,6 +514,7 @@ class TestSessionRunner:
                 "virtualenv",
                 nox.virtualenv.VirtualEnv,
             ),
+            ("nox.virtualenv.VirtualEnv.create", "venv", nox.virtualenv.VirtualEnv),
             ("nox.virtualenv.CondaEnv.create", "conda", nox.virtualenv.CondaEnv),
         ],
     )
diff --git a/tests/test_virtualenv.py b/tests/test_virtualenv.py
index 04296e2..61b246d 100644
--- a/tests/test_virtualenv.py
+++ b/tests/test_virtualenv.py
@@ -117,6 +117,7 @@ def test_constructor_defaults(make_one):
     assert venv.location
     assert venv.interpreter is None
     assert venv.reuse_existing is False
+    assert venv.venv_or_virtualenv == "virtualenv"
 
 
 @pytest.mark.skipif(IS_WINDOWS, reason="Not testing multiple interpreters on Windows.")
@@ -213,6 +214,11 @@ def test_create(make_one):
     assert dir_.join("test.txt").check()
 
 
+def test_create_venv_backend(make_one):
+    venv, dir_ = make_one(venv=True)
+    venv.create()
+
+
 @pytest.mark.skipif(IS_WINDOWS, reason="Not testing multiple interpreters on Windows.")
 def test_create_interpreter(make_one):
     venv, dir_ = make_one(interpreter="python3")