[BUG] --no-reuse-existing-virtualenvs doesn't seem to work

[henryiii]

Describe the bug

Adding --no-reuse-existing-virtualenvs does not seem to ignore the cache, and instead just reuses the existing environments anyway. Happened on the previous version, and then did a brew upgrade, and 2021.10.1 also has this problem. Unless I misunderstand this option, pretty sure it's broken.

How to reproduce

@nox.session(reuse_venv=True)
def type(session):
    session.install('mypy')
    session.run('mypy')

(simplified slightly, should be unimportant)

$ nox --no-reuse-existing-virtualenvs -s type
nox > Running session type
nox > Re-using existing virtual environment at .nox/type.

Expected behavior
The existing virtual environment should not have been reused. (It really was, not just visually; I was trying to remove an item from the install list, but it remained installed).

[cjolowicz]

Thanks for reporting this. This appears to be the culprit:

https://github.com/theacodes/nox/blob/73d14b11e6f45d3269544811ddcb0000875ce312/nox/sessions.py#L580-L582

Judging from a quick look at the history, --no-reuse-existing-virtualenvs only ever worked with respect to a global nox.options setting, not with respect to a per-session reuse_venv setting.

It would be nice to support this use case as well. (Could we check is False to determine if it was specified explicitly? IIUC the setting is None by default, so this should work.)

[henryiii]

So the behavior could be:

• If either is None, use the other one.
• If either is False, don't reuse venvs.
• Otherwise, reuse if one or both are True.

local	global	Behavior
Unset	Unset	Don't reuse
False	Unset	Don't reuse
True	Unset	Reuse
Unset	False	Don't reuse
False	False	Don't reuse
True	False	Don't reuse1
Unset	True	Reuse
False	True	Don't reuse2
True	True	Reuse

Footnotes

1. This is currently Reuse, which makes it impossible to globally force a clean run. ↩

2. I think this is Reuse, and maybe it should be. Not sure. ↩

[cjolowicz]

Hmm.. Ideally we'd do the following (but it's more work):

1. Use the command-line option if it's set (either way).
2. Use the per-session setting from the noxfile, if it's not None.
3. Use the global nox.options setting from the noxfile, if it's not None.
4. Don't reuse.

So this would split your True/False row in two cases, depending on where the global setting comes from.

Consider this noxfile:

import nox
nox.options.reuse_existing_virtualenvs = False
@nox.session(reuse_venvs=True):
def test(session):
    pass

I'd be surprised if the test session was not reused.

On the other hand, I'd expect nox --no-reuse-existing-virtualenvs --session test to disable reuse.

[cjolowicz]

And, for completeness, your False/True row would also be split:

import nox
nox.options.reuse_existing_virtualenvs = True
@nox.session(reuse_venvs=False):
def test(session):
    pass

So this would not reuse test by default, but you could run nox -rs test to override.

[henryiii]

There is a very easy workaround for the above, though - don't set nox.options.reuse_existing_virtualenvs = False (or set it to none). This gives you a quick way to drop a line in (nox.options.reuse_existing_virtualenvs = False) to override all reuse_venvs to False (otherwise, why would you add a line like that?)

Same is true if you set reuse_venvs=False, I don't think if you explicitly state don't want a venv reused you want it suddenly being used. I think the goal of the global setting is to make the "not specified" ones True, not everything True. (that was what I was thinking, not necessarily right)

But I see your point.

[theacodes]

This behavior is definitely ambiguous if we're just using True/False. What do y'all think using "yes", "no", "always" and "never"?

So:

• Command-line --reuse=yes reuses envs for all sessions except ones marked "never".
• Command-line --reuse=no (or unset) only reuses sessions marked "always".
• Command-line --reuse=always forces every session to re-use its env.
• Command-line --reuse=never forces every session to re-create its env.

The noxfile.py option nox.options.reuse_existing_virtualenvs should behave the same as the other noxfile.py options - it provides a default for the command-line arg if it isn't specified.

[cjolowicz]

Yes, I think that solves it nicely!

[henryiii]

I think this is what you meant, but just to be clear:
I'd leave the session option as reuse_venv=True/False(/None, implicit), then:

• Command-line --reuse=yes reuses envs for all sessions except ones with reuse_venv=False.
• Command-line --reuse=no (or unset) only reuses sessions with reuse_venv=True.
• Command-line --reuse=always forces every session to re-use its env.
• Command-line --reuse=never forces every session to re-create its env.

I think that's the proposal?

[henryiii]

And then some deprecation/removal of nox.options.reuse_existing_virtualenvs, maybe being mapped to nox.options.reuse for a bit?

[theacodes]

@henryiii that doesn't cover the same set of behaviors I described.

Also, we can keep the existing long name- I was just being brief. If y'all wanna shorten it to reuse with the old name being an alias go ahead.

[henryiii]

I do like --reuse-venv=<stuff> or --reuse=<stuff> better, as the current name is very cumbersome and will be even longer when =<stuff> is added. Also, changing it to an option from a flag is not backward-compatible, --no-reuse_existing_virtualenvs won't work anymore, etc. So seems a good time to simplify a bit. I like --reuse-venv=<stuff>, similar to reuse_venv in sessions, personally.

I feel like allowing the common line option to be more expressive than the local setting makes this a bit simpler, but still covers the main uses. Passing a string on the command line is more natural than in a function call, and it would be nice to only have a change to the command line usage, and maybe a single global line, rather than changing and complicating all session definitions.

[henryiii]

Also, @nox.session(reuse_venvs="always"): and @nox.session(reuse_venvs="never"): would be invalid, they can be overridden by the command line option, so they aren't "always" and "never".

[theacodes]

Passing a string on the command line is more natural than in a function call

There is lots of precedence for this in existing Python code.

I still think using True/False for the decorator leads to ambiguity, and especially if the command-line/global option is a string it feels odd to have the per-session one be a bool. It also is strange to have a value be valid for global options but not for a per-session option.

Since this describes a specific behavior and not a simple on/off flag, I lean towards using "yes"/"no"/"always"/"never. We must retain backwards compatibility, and the easiest way to do that is have True map to "yes" and False/None/unset map to "no".

[henryiii]

There is lots of precedence for this in existing Python code.

This is fine when it's needed. I'm aware of Literals, python's string comparisons for short strings, etc. My worry is about ambiguity and over-specification. Also, there's also lots of bad precedence too. ;)

False and None are useful different behaviors. reuse_venvs=None means you didn't pass it, and might not care about it being reused. reuse_venvs=False means you explicitly don't want this to be reused normally. One pattern could be:

session.options.reuse_venv = "yes"  (could be mapped to True?)

@session(reuse_venv=False)
def test...

@session
def docs...

@session
def lint...

...

That is, this is switching the "default" to reuse, but on the small number of sessions that build something that should not be reused, then that can be specified. (most of my files are likely this way, most venvs are reusable) For the string version, that now has to be:

...
@session(reuse_venv="never") # False would not work, would be mapped to no - and 'yes' wins over 'no'.
def test...

Hoever, passing --reuse-venv=always would cause the session marked reuse_venv="never" to be reused; this to me was much less surprising if it was reuse_venv=False. Having multiple never and always specifications is ambiguous and confusing, IMO. How would a user know that session.options.reuse_venv = "never" will override a local reuse_venv="always"? I would expect the most local thing to win, not the global setting. But if reuse_venv is True, now it's clear that "always" will win.

Up to you, just my thoughts.

[henryiii]

If you wanted strings for reuse_venv, "default", "no", and "yes" would be identical to "no", "never", and "yes"/"always" (there is no difference between "yes" and "always" when set locally, by the way). But if you have different literals, and three of them, with these meanings, why not just stay with None/True/False?

[theacodes]

Alright, I think I understand what you're getting at:

• Without anything passed on the command-line, sessions marked True will be reused - this is the default behavior and matches the intended current behavior.
• Command-line --reuse (True) reuses envs for all sessions except ones marked False. This matches the intended current behavior (that isn't working correctly).
• Command-line --reuse=always forces every session to re-use its env, and is not a valid option for local specification.
• Command-line --reuse=never forces every session to re-create its env, and is not a valid option for local specification.

If that's the case, that works for me.

[johnthagen]

I think I am running into this problem. I have a noxfile.py that looks like:

import nox
from nox_poetry import Session, session

nox.options.reuse_existing_virtualenvs = True

...

@session(reuse_venv=False)
def licenses(s: Session) -> None:
    s.install(".", "pip-licenses")
    s.run("pip-licenses", *s.posargs)

I expected that nox -s licenses would always re-create the venv since I am locally setting reuse_venv=False, but instead every time:

$ nox -s licenses
nox > Running session licenses
nox > Re-using existing virtual environment at .nox\licenses.
...

[q0w]

@johnthagen because of that (False or True)

nox/nox/sessions.py

Lines 718 to 720 in b15909e

	reuse_existing = (
	self.func.reuse_venv or self.global_config.reuse_existing_virtualenvs
	)