Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fail on mismatched python spec attributes #2824

Merged
merged 2 commits into from Jan 5, 2023
Merged

Fail on mismatched python spec attributes #2824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79c86c7
Fail on mismatched python spec attributes
stephenfin Jan 4, 2023
cf5ec84
Address nits in changelog
stephenfin Jan 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/changelog/2754.bugfix.rst
View file
Open in desktop
@@ -0,0 +1,2 @@
Setting ``[testenv] basepython = python3`` will no longer override the Python interpreter version requested by a factor,
such as ``py311`` - by :user:`stephenfin`.
2 changes: 1 addition & 1 deletion src/tox/tox_env/python/api.py
View file
Open in desktop
Expand Up @@ -154,7 +154,7 @@ def _validate_base_python(env_name: str, base_pythons: list[str], ignore_base_py
if any(
getattr(spec_base, key) != getattr(spec_name, key)
for key in ("implementation", "major", "minor", "micro", "architecture")
if getattr(spec_base, key) is not None and getattr(spec_name, key) is not None
if getattr(spec_name, key) is not None
):
msg = f"env name {env_name} conflicting with base python {base_python}"
if ignore_base_python_conflict:
Expand Down
1 change: 1 addition & 0 deletions tests/tox_env/python/test_python_api.py
View file
Open in desktop
Expand Up @@ -95,6 +95,7 @@ def test_base_python_env_no_conflict(env: str, base_python: list[str], ignore_co
("py3", ["py2"], ["py2"]),
("py38", ["py39"], ["py39"]),
("py38", ["py38", "py39"], ["py39"]),
("py38", ["python3"], ["python3"]),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@stephenfin stephenfin Jan 5, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

("py310", ["py38", "py39"], ["py38", "py39"]),
("py3.11.1", ["py3.11.2"], ["py3.11.2"]),
("py3-64", ["py3-32"], ["py3-32"]),
Expand Down