feat: added venv_location option to specify virtualenv location

[wpk-nist-gov]

This PR adds the option venv_location to specify the location for virtualenv creation and a per session basis, and is a follow up to #753.

The use case is quickly defining development environments:

@nox.session(venv_location=".venv")
def dev(session):
    ....

[henryiii]

This could also be used by power users to have multiple sessions share a single venv. Nicely combines with better logic on venv reuse. :)

[wpk-nist-gov]

Yes, I was thinking the same thing! I've used a similar (not supported) feature in tox in the past. I'll play around with this possible functionality...

[cjolowicz]

Does this allow arbitrary path traversal?

[wpk-nist-gov]

Does this allow arbitrary path traversal?

Sorry @cjolowicz, I'm not sure what you mean.

If you mean can you place environments anywhere, the answer is yes. So you could do the following:

@nox.session(venv_location=os.path.expanduser("~/venvs/project-venv"))
def dev(session):
    ....

To place a dev environment at the location "~/venv/project-venv".

Just pushed a commit that wraps venv_location with os.path.expanduser to automatically expand "~/", so the above call to os.path.expanduser is unneeded...

[henryiii]

wrap venv_location with os.path.expanuser

Do we do that anywhere else? Any reason not to let a user make it explicit? Path.home() would work, for example.

[wpk-nist-gov]

The call to os.pathexpanduser was just to catch an edge case. Not necessarily needed. But it also will covert pathlib.Path objects to strings correctly. Most other paths in the code at some point get passed through os.path.join or similar which typically ensures they are path-like strings.

When I was working on this, I resisted the urge to convert string paths to pathlib.Path objects, as nox uses string paths and the os.path module exclusively. Maybe a future PR is in store converting to pathlib...

For now, I'll make a change allowing os.PathLike objects for venv_location

[cjolowicz]

If you mean can you place environments anywhere, the answer is yes.

That is what I meant, thanks. Absolute paths won't be very portable and may be surprising behavior when you run Nox on somebody else's project. Why do you think this should be possible?

---

How does the venv_location feature work for parameterized sessions including sessions with multiple Pythons?

[cjolowicz]

Are we concerned about users trying something like venv_location="tests"? They might expect this to land under .nox but it would create files within their test suite.

[henryiii]

How would this interact with parametrization? I could see it potentially being very useful to allow a parametrized test to reuse the same environment, but some cases that would be bad, especially if Python was parametrized over (the most common one!). Would it make sense to allow .format substitution in the path, providing the parametrized values? Edit: I didn't read the edited comment above, sorry for repeating it.

[wpk-nist-gov]

That is what I meant, thanks. Absolute paths won't be very portable and may be surprising behavior when you run Nox on somebody else's project. Why do you think this should be possible?

I'm fine restricting venv_location to be below the current directory, if that's the consensus? I don't have strong feelings on the matter.

How does the venv_location feature work for parameterized sessions including sessions with multiple Pythons?

Combining with the comment from @henryiii, I like the idea of using .format, so you could use something like venv_location="my-env-{python}". But I'm hesitant to add this functionality in general. In my opinion, you should use the default behavior for these cases. Maybe for now, raise an error if using venv_location with parametrize? I'm not sure how to do this...

Are we concerned about users trying something like venv_location="tests"? They might expect this to land under .nox but it would create files within their test suite.

Yes, I agree this could be an issue. How about a check like: if venv_location exists, it must contains {venv_location}/pyvenv.cfg or {venv_location}/conda-meta?

(EDIT: I'm not sure there's a general solution to testing for a venv)

[henryiii]

Does {venv_location} contain pyvenv.cfg on Windows? I'm not sure it does, for at least one of venv/virtualenv.

[wpk-nist-gov]

Does {venv_location} contain pyvenv.cfg on Windows? I'm not sure it does, for at least one of venv/virtualenv.

No clue. Is there a general way to test if a directory is venv/virtualenv/conda-env?

Sorry, accidentally closed PR. Reopened.

[cjolowicz]

A pyvenv.cfg with a home key is always present in a virtual environment, CPython needs it to build sys.path. It doesn't depend on the tool that creates it or the platform.

[henryiii]

That's what I thought, but some of the tests are disabled on Windows looking for it (and virutalenv < 20 didn't add one it seems from comments).

[wpk-nist-gov]

A pyvenv.cfg with a home key is always present in a virtual environment, CPython needs it to build sys.path. It doesn't depend on the tool that creates it or the platform.

In this case, should I add a check that, if venv_location exists, that it is a virtualenv?

Just an FYI, using tox devenv my-location (the partial inspiration for venv_location) will place environments anywhere (it expands "~/" to the home directory), and doesn't check the specified directory (will overwrite/delete/etc)...

[cjolowicz]

In this case, should I add a check that, if venv_location exists, that it is a virtualenv?

Yes, I think this is a good safeguard.

I'm fine restricting venv_location to be below the current directory, if that's the consensus? I don't have strong feelings on the matter.

Either that or just requiring a relative path. I'm not opposed to relaxing this restriction later if people come up with good use cases.

Maybe for now, raise an error if using venv_location with parametrize? I'm not sure how to do this...

Yes, I'd prefer this solution, also for sessions with multiple Pythons. Let's see some real world usage of this feature before we add something like templating to it. One approach would be to check this in Manifest.make_session (similar to how we warn about backend="none" but resulting in a hard error instead of a warning). Another approach would be to error out when we create the environment. I'm not sure which is best.

[wpk-nist-gov]

The latest commit raises an error if trying to pass multiple pythons to a session with venv_location. It also ignores (with warning) any --extra-python or --force-python flags, although I'm thinking it should handle the case where passing a single value of --force-python.

TODO

• [ ] Make venv_location always relative. And hopefully fix the bug in windows CI...
• [ ] Add check for venv before overwriting directory
• [ ] Proper handling of --force-python if single valued.
• [ ] Possible addition. Add --devenv flag that the could be used to direct any session to a user defined venv_location