Fail on mismatched python spec attributes

[stephenfin]

The following is intended to be a correct tox configuration:

[tox]
min_version = 4.1
env_list = py{38,39,310,311},docs

[testenv]
base_python = python3.8

[testenv:docs]
commands =
  sphinx-build ...

The goal of this is to use python3.8 (i.e. the value of [testenv] base_python) for all environments except those with specific pyXX factors in them. This helps eliminate issues where testenvs that do not specify a pyXX factor run with different Python versions in different environments.

An earlier bug, #477, prevented us from doing this. Due to #477, the interpreter version indicated by [testenv] base_python (or rather [testenv] basepython - the underscore-separated variant was only introduced in tox 4) would override the value indicated by the pyXX factor. This was resolved with a PR, #841, which started warning users if they were unintentionally running against the wrong interpreter and introduced the ignore_basepython_conflict value to opt into the correct behavior.

Unfortunately, this has been partially broken in the move to tox 4. While the above configuration still works, the following no longer does:

[tox]
min_version = 4.1
env_list = py{38,39,310,311},docs

[testenv]
base_python = python3

[testenv:docs]
commands =
  sphinx-build ...

This configuration was common back during the move from Python 2 to Python 3. It ensured that python3 was used for all testenvs that did not request an explicit Python version via a factor or their own base_python version. While it's no longer necessary, it is still quite common. Currently, running with this configuration will result in python3 being used for every environment, rather than e.g. python3.8 for a testenv with the py38 factor. This is clearly incorrect. This issue stems from an errant bit of logic. When comparing interpreter versions as part of the _validate_base_python, we ignore various attributes if they are unset on either [testenv] base_python or the pyXX factor. This allows e.g. python3 to match python3.8, since the minor version is unset for the [testenv] base_python value, python3. The fix is simple: while we can ignore unset attributes for factor-derived versions (to allow a py3 factor to work with e.g. base_python = python3.8), we should insist on them for base_python-derived versions.

Closes: #2754

[stephenfin]

Docs failures don't appear to be due to this...

Edit: It looks like Sphinx 6.1.0 is the culprit. Works fine on 6.0.1.

Edit: And it's this commit in Sphinx that's causing an incompatibility with the sphinx_argparse_cli extension. Apparently that's not setting the source attribute on the nodes it creates. If I comment out the directive in docs/cli_interface.rst then the build mostly succeeds (or rather fails but only due to missing missing labels).

[gaborbernat]

Wrap at 120 and add - by :user:`x`.

[gaborbernat]

The following is intended to be a correct tox configuration:

[tox]
min_version = 4.1
env_list = py{38,39,310,311},docs

[testenv]
base_python = python3.8

[testenv:docs]
commands =
  sphinx-build ...

The goal of this is to use python3.8 (i.e. the value of [testenv] base_python) for all environments except those with specific pyXX factors in them. This helps eliminate issues where testenvs that do not specify a pyXX factor run with different Python versions in different environments.

Strong disagree. This is not a good way to express this. This config should hard fail for all envs expect py38.

[gaborbernat]

This helps eliminate issues where testenvs that do not specify a pyXX factor run with different Python versions in different environments.

If those other envs care about a python version, they should specify so in their section; e.g. here, docs should set it to 3.8. Not setting it means I'm happy with any Python.

[stephenfin]

The following is intended to be a correct tox configuration:

[tox]
min_version = 4.1
env_list = py{38,39,310,311},docs

[testenv]
base_python = python3.8

[testenv:docs]
commands =
  sphinx-build ...

The goal of this is to use python3.8 (i.e. the value of [testenv] base_python) for all environments except those with specific pyXX factors in them. This helps eliminate issues where testenvs that do not specify a pyXX factor run with different Python versions in different environments.

Strong disagree. This is not a good way to express this. This config should hard fail for all envs expect py38.

Regarding the hard fail, currently tox does not hard fail with this configuration and it does the wrong thing (using python3 for all environments regardless of the version inferred from the factor). With this change, it does hard fail unless the user has explicitly set ignore_base_python_conflict = true. So I think what we're discussing here isn't actually this change itself, since this fixes an obvious bug.

Regarding whether this is a good way to express this, requiring users duplicate base_python in every section makes base_python a special-case, no? For everything else, the idea is that [testenv] allows you to set "default" values that you can later override (or extend) in [testenv:foo] sections. tox itself does this for various things like command:

tox/tox.ini

Lines 27 to 35 in af4b558

	commands =
	pytest {posargs: \
	--junitxml {toxworkdir}{/}junit.{envname}.xml --cov {envsitepackagesdir}{/}tox --cov {toxinidir}{/}tests \
	--cov-config={toxinidir}{/}pyproject.toml --no-cov-on-fail --cov-report term-missing:skip-covered --cov-context=test \
	--cov-report html:{envtmpdir}{/}htmlcov \
	--cov-report xml:{toxworkdir}{/}coverage.{envname}.xml \
	-n={env:PYTEST_XDIST_AUTO_NUM_WORKERS:auto} \
	tests --durations 5 --run-integration}
	diff-cover --compare-branch {env:DIFF_AGAINST:origin/main} {toxworkdir}{/}coverage.{envname}.xml

tox/tox.ini

Lines 47 to 49 in af4b558

	commands =
	pre-commit run --all-files --show-diff-on-failure {posargs}
	python -c 'print(r"hint: run {envbindir}{/}pre-commit install to add checks as pre-commit hook")'

My question is why can we do this for things like commands, deps, or passenv etc. but we should not be able to do it for base_python?

[gaborbernat]

But that makes base_python a special-case? For everything else, the idea is that [testenv] allows you to set "default" values that you can later override (or extend) in [testenv:foo] sections. tox itself does this for various things like command:

It's not a special case. Follows the same rule.

[testenv]
base_python = python3.8
[testenv:py39]
base_python = python3.9 

Would work. Where you're mistaken is that py38 only sets base_python to py38 if at no point is base_python specified. If you set it at testenv or [testenv:py38] level that no longer activates.

[gaborbernat]

This shouldn't conflict, but py27 and python3 should 🤔

[stephenfin]

Both should conflict. python3 on my system == python3.11.1. That is decidedly not python3.8, which is what I intended. I should be exploding on that combination - which I do (unless the user explicitly said not to, via ignore_base_python_conflict)

[gaborbernat]

You're miss understanding. base_python is not an executable match. It is a specification. And the specification states Python 3, without resolving it any further.

[stephenfin]

That may be the case, but providing a specification of python3 (e.g. base_python = python3) causes the python3 executable to be used. For example, using a tox.ini like this:

[tox]
minversion = 3.1
envlist = py{37,38,39,310,311}

[testenv]
basepython = python3

If I run tox -e py38, the py38 venv is using python3.11 :

❯ source .tox/py38/bin/activate
❯ python --version
Python 3.11.1

No warnings. Nothing. That's got to be incorrect behaviour, surely?

[gaborbernat]

Indeed, so we have two options:

• for py38 python3 spec should conflict and hard fail,
• hard fail once we resolve python3 and see its something else.

I'm inclined to go for option 1.

[stephenfin]

Agree. Option 1 is the way to go. Option 2 will result in different behaviour depending on the environment making tox less deterministic. That's A Bad Thing ™️

Without this change, the py38 venv uses whatever python3 is on my host (python3.11). With this change, things hard fail as expected.

❯ tox -e py38
py38: failed with env name py38 conflicting with base python python3
  py38: FAIL code 1 (0.00 seconds)
  evaluation failed :( (0.08 seconds)

So it seems I have implemented option 1, yes?

[gaborbernat]

What happens for env py3, py, magic? Does that still work without this failure?

[stephenfin]

It passes, as expected.

❯ tox -e py3
py3: install_deps> python -I -m pip install -r /tmp/tox-issue-2717/test-requirements.txt
.pkg: _optional_hooks> python /tmp/tox-venv/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_sdist> python /tmp/tox-venv/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: prepare_metadata_for_build_wheel> python /tmp/tox-venv/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_sdist> python /tmp/tox-venv/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
py3: install_package_deps> python -I -m pip install requests
py3: install_package> python -I -m pip install --force-reinstall --no-deps /tmp/tox-issue-2717/.tox/.tmp/package/11/foo-0.0.0.tar.gz
.pkg: _exit> python /tmp/tox-venv/lib/python3.11/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  py3: OK (4.39 seconds)
  congratulations :) (4.42 seconds)

[stephenfin]

🥳 Thanks. Happy to have gotten this resolved 😄

[gaborbernat]

Sorry for taking a while for me to understand 😆