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.

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

Handle version conflicts between deps and package deps #2386

Closed
daneah opened this issue Mar 24, 2022 · 14 comments · Fixed by #2888
Closed

Handle version conflicts between deps and package deps #2386

daneah opened this issue Mar 24, 2022 · 14 comments · Fixed by #2888
Assignees
Labels
area:testenv-creation feature:new something does not exist yet, but should

Comments

@daneah
Copy link

daneah commented Mar 24, 2022

When using tox to run tests for a Python package and installing testing dependencies through deps, there is potential for the package's install_requires dependency resolution to result in a package version that doesn't match the constraint in deps.

Because deps is explicitly defined, it feels like it should take precedence (get installed after the package?) or at the very least tox should warn about this situation.

I ran into this when testing a package against two Django versions:

# setup.cfg

...

[options]
install_requires =
    # a dependency that specifies Django>=1.11,<4

[tox:tox]
envlist = py{39,310}-django{3.2,4}
isolated_build = True

[testenv]
deps =
    pytest
    pytest-cov
    pytest-django
    pytest-randomly
    django3.2: Django>=3.2,<3.3
    django4: Django>=4.0,<5

...

The package's install_requires ultimately had an abstract dependency who needed Django>=1.11,<4, so even for the envs with the django4 factor, Django==3.2.12 was being installed. The only real knowledge of this fact was gotten by reading the output list of installed packages, and the lack of a failure I expected to see in Django 4.

I was almost inclined to call this a bug, but I have to imagine there are some design decisions at play here that I'm not aware of.

@daneah daneah added the feature:new something does not exist yet, but should label Mar 24, 2022
@jugmac00
Copy link
Member

jugmac00 commented Mar 24, 2022

The way it currently works is outlined in the documentation:
https://tox.wiki/en/latest/config.html#conf-deps

Switching the order, ie installing the dependencies after the package would certainly break for quite some users, as I have seen configurations where deps was used to make up for missing install_requires for the packages under test.

I do see your issue, but I do not see a straight forward solution to it.

@gaborbernat
Copy link
Member

Because deps is explicitly defined, it feels like it should take precedence (get installed after the package?) or at the very least tox should warn about this situation.

From the POV of tox both are explicitly defined at same level. The problem is to find out when there's a dependency conflict is a hard job. You need an entire solver for that. tox is unlikely to take on that gigantic task. I'd rather make the package installation take on as additional arguments the deps part and then let pip find the issue and warn about it. Not sure how much that would break people, but in version 3 we're definitely not changing the current behaviour. However for v4 I can see an argument.

@gaborbernat gaborbernat added this to the 4.0 milestone Mar 24, 2022
@gaborbernat gaborbernat changed the title Provide a warning when a dependency specified in deps is overwritten by package dependencies Handle version conflicts between deps and package deps Mar 24, 2022
@daneah
Copy link
Author

daneah commented Mar 24, 2022

@jugmac00 @gaborbernat thanks, both!

I had the same thought about pushing this problem to pip to handle, but wasn't sure how readily that could happen with the tox architecture. I definitely see that doing it that way would be a breaking change, so waiting for tox4 makes plenty of sense.

@gaborbernat gaborbernat modified the milestones: 4.0, 4.1 Nov 20, 2022
@masenf
Copy link
Collaborator

masenf commented Jan 18, 2023

So tox 4 is well and out, and this is still an issue, and likely dupe of much older bug #513.

But there is a way to solve this long-standing issue without an actual tox change! 🎉

Example

tox.ini

A tox file that tests the example package with multiple versions of pytest

[tox]
envlist =
  py3-pytest4
  py3-pytest5
  py3-pytest6
  py3-pytest7

[testenv]
deps =
    pytest4: pytest ~= 4.0
    pytest5: pytest ~= 5.0
    pytest6: pytest ~= 6.0
    pytest7: pytest ~= 7.0
commands = pip freeze

pyproject.toml

This is just a dummy package that depends on a range of pytest versions, similar to the scenario described in this bug, the project itself constrains the deps that it can run with. Using the tox.ini above, a user would expect that py3-pytest4 and py3-pytest7 environments should FAIL because they are incompatible with the example package. Instead, it works fine, because the package deps override the tox deps.

[project]
name = "toxfile"
description = "example package that depends on pytest >5, <7"
version = "0.1"
dependencies = [
    "pytest >5, < 7",
]

[build-system]
build-backend = "flit_core.buildapi"
requires = ["flit_core >=3.2,<4"]

toxfile.py

With the magic of inline plugins, we can make tox enforce our listed deps as constraints when installing the package_deps.

"""
toxfile.py: tox-dev/tox#2386 fixer

MIT License
Copyright (c) 2023 Masen Furer
"""
from typing import Any

from tox.plugin import impl
from tox.tox_env.api import ToxEnv
from tox.tox_env.python.virtual_env.runner import VirtualEnvRunner


@impl
def tox_on_install(tox_env: ToxEnv, arguments: Any, section: str, of_type: str) -> None:
    """
    Before installing:
      * save environment deps to a constraints file
      * apply constraints file to package_deps
    """
    constraints_file = tox_env.env_dir / "constraints.txt"

    if of_type == "deps" and isinstance(tox_env, VirtualEnvRunner):
        constraints_file.write_text("\n".join(arguments.lines()))

    if of_type == "package":
        constraints_file_dep = f"-c{constraints_file}"
        for package in arguments:
            getattr(package, "deps", []).append(constraints_file_dep)

How does it look? 👀

Before (empty toxfile.py)

atomicwrites==1.4.1
attrs==22.2.0
iniconfig==2.0.0
more-itertools==9.0.0
packaging==23.0
pluggy==0.13.1
py==1.11.0
pytest==6.2.5                <-- ☠ unexpected for pytest4 factor
six==1.16.0
toml==0.10.2
toxfile @ file:///git/repro/tox-2386/.tox/.tmp/package/1/toxfile-0.1.tar.gz
wcwidth==0.2.6
py3-pytest4: OK ✔ in 8.56 seconds
attrs==22.2.0
more-itertools==9.0.0
packaging==23.0
pluggy==0.13.1
py==1.11.0
pytest==5.4.3
toxfile @ file:///git/repro/tox-2386/.tox/.tmp/package/2/toxfile-0.1.tar.gz
wcwidth==0.2.6
py3-pytest5: OK ✔ in 5.29 seconds
attrs==22.2.0
iniconfig==2.0.0
packaging==23.0
pluggy==1.0.0
py==1.11.0
pytest==6.2.5
toml==0.10.2
toxfile @ file:///git/repro/tox-2386/.tox/.tmp/package/3/toxfile-0.1.tar.gz
py3-pytest6: OK ✔ in 5.51 seconds
attrs==22.2.0
exceptiongroup==1.1.0
iniconfig==2.0.0
packaging==23.0
pluggy==1.0.0
py==1.11.0
pytest==6.2.5                <-- ☠ unexpected for pytest7 factor
toml==0.10.2
tomli==2.0.1
toxfile @ file:///git/repro/tox-2386/.tox/.tmp/package/4/toxfile-0.1.tar.gz
  py3-pytest4: OK (8.56=setup[8.12]+cmd[0.44] seconds)
  py3-pytest5: OK (5.29=setup[4.85]+cmd[0.44] seconds)
  py3-pytest6: OK (5.51=setup[5.07]+cmd[0.45] seconds)
  py3-pytest7: OK (6.04=setup[5.59]+cmd[0.45] seconds)
  congratulations :) (26.05 seconds)

After

py3-pytest4: install_deps> python -I -m pip install 'pytest~=4.0'
.pkg: install_requires> python -I -m pip install 'flit_core<4,>=3.2'
.pkg: _optional_hooks> python /home/mfurer/.pytools/lib/python3.8/site-packages/pyproject_api/_backend.py True flit_core.buildapi
.pkg: get_requires_for_build_sdist> python /home/mfurer/.pytools/lib/python3.8/site-packages/pyproject_api/_backend.py True flit_core.buildapi
.pkg: build_sdist> python /home/mfurer/.pytools/lib/python3.8/site-packages/pyproject_api/_backend.py True flit_core.buildapi
py3-pytest4: install_package_deps> python -I -m pip install -c/git/repro/tox-2386/.tox/py3-pytest4/constraints.txt 'pytest<7,>5'
Looking in indexes: https://artifactory-ha.west.isilon.com/artifactory/api/pypi/pypi-repo/simple

The conflict is caused by:
    The user requested pytest<7 and >5
    The user requested (constraint) pytest~=4.0

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict

ERROR: Cannot install pytest<7 and >5 because these package versions have conflicting dependencies.
ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts

py3-pytest4: exit 1 (0.71 seconds) /git/repro/tox-2386> python -I -m pip install -c/git/repro/tox-2386/.tox/py3-pytest4/constraints.txt 'pytest<7,>5' pid=1333688
py3-pytest4: FAIL ✖ in 5.45 seconds
py3-pytest5: install_deps> python -I -m pip install 'pytest~=5.0'
py3-pytest5: install_package_deps> python -I -m pip install -c/git/repro/tox-2386/.tox/py3-pytest5/constraints.txt 'pytest<7,>5'
py3-pytest5: install_package> python -I -m pip install --force-reinstall --no-deps /git/repro/tox-2386/.tox/.tmp/package/2/toxfile-0.1.tar.gz
py3-pytest5: commands[0]> pip freeze
attrs==22.2.0
more-itertools==9.0.0
packaging==23.0
pluggy==0.13.1
py==1.11.0
pytest==5.4.3
toxfile @ file:///git/repro/tox-2386/.tox/.tmp/package/2/toxfile-0.1.tar.gz
wcwidth==0.2.6
py3-pytest5: OK ✔ in 5.3 seconds
py3-pytest6: install_deps> python -I -m pip install 'pytest~=6.0'
py3-pytest6: install_package_deps> python -I -m pip install -c/git/repro/tox-2386/.tox/py3-pytest6/constraints.txt 'pytest<7,>5'
py3-pytest6: install_package> python -I -m pip install --force-reinstall --no-deps /git/repro/tox-2386/.tox/.tmp/package/3/toxfile-0.1.tar.gz
py3-pytest6: commands[0]> pip freeze
attrs==22.2.0
iniconfig==2.0.0
packaging==23.0
pluggy==1.0.0
py==1.11.0
pytest==6.2.5
toml==0.10.2
toxfile @ file:///git/repro/tox-2386/.tox/.tmp/package/3/toxfile-0.1.tar.gz
py3-pytest6: OK ✔ in 5.45 seconds
py3-pytest7: install_deps> python -I -m pip install 'pytest~=7.0'
py3-pytest7: install_package_deps> python -I -m pip install -c/git/repro/tox-2386/.tox/py3-pytest7/constraints.txt 'pytest<7,>5'
Looking in indexes: https://artifactory-ha.west.isilon.com/artifactory/api/pypi/pypi-repo/simple

The conflict is caused by:
    The user requested pytest<7 and >5
    The user requested (constraint) pytest~=7.0

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict

ERROR: Cannot install pytest<7 and >5 because these package versions have conflicting dependencies.
ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts

py3-pytest7: exit 1 (0.73 seconds) /git/repro/tox-2386> python -I -m pip install -c/git/repro/tox-2386/.tox/py3-pytest7/constraints.txt 'pytest<7,>5' pid=1333816
.pkg: _exit> python /home/mfurer/.pytools/lib/python3.8/site-packages/pyproject_api/_backend.py True flit_core.buildapi
  py3-pytest4: FAIL code 1 (5.45 seconds)
  py3-pytest5: OK (5.30=setup[4.86]+cmd[0.44] seconds)
  py3-pytest6: OK (5.45=setup[4.98]+cmd[0.46] seconds)
  py3-pytest7: FAIL code 1 (3.14 seconds)
  evaluation failed :( (19.99 seconds)

Not sure the best way to proceed @gaborbernat, is this a kind of workflow that tox should support internally? If so, it should be trivial to implement. If not, then we can document it and leave it to a plugin to implement.

@gaborbernat
Copy link
Member

I wouldn't be against supporting this behavior part of the core.

@masenf masenf self-assigned this Jan 19, 2023
masenf added a commit to masenf/tox that referenced this issue Jan 23, 2023
Create a constraints file based on the listed or installed ``deps`` and use it
during ``install_package_deps`` to ensure the package dependencies do not
override the environment's dependencies.

Adds new config keys:
  * ``constrain_package_deps`` - chicken switch to go back to the legacy
    behavior of allowing the package deps to override the testenv deps
  * ``use_frozen_constraints`` - a more strict mode, enforced that the package
    deps do not conflict in any way with the set of packages installed by
    ``deps``

Fix tox-dev#2386
masenf added a commit to masenf/tox that referenced this issue Jan 23, 2023
Create a constraints file based on the listed or installed ``deps`` and use it
during ``install_package_deps`` to ensure the package dependencies do not
override the environment's dependencies.

Adds new config keys:
  * ``constrain_package_deps`` - chicken switch to go back to the legacy
    behavior of allowing the package deps to override the testenv deps
  * ``use_frozen_constraints`` - a more strict mode, enforced that the package
    deps do not conflict in any way with the set of packages installed by
    ``deps``

Fix tox-dev#2386
masenf added a commit to masenf/tox that referenced this issue Jan 23, 2023
Create a constraints file based on the listed or installed ``deps`` and use it
during ``install_package_deps`` to ensure the package dependencies do not
override the environment's dependencies.

Adds new config keys:
  * ``constrain_package_deps`` - chicken switch to go back to the legacy
    behavior of allowing the package deps to override the testenv deps
  * ``use_frozen_constraints`` - a more strict mode, enforced that the package
    deps do not conflict in any way with the set of packages installed by
    ``deps``

Fix tox-dev#2386
masenf added a commit to masenf/tox that referenced this issue Jan 23, 2023
Create a constraints file based on the listed or installed ``deps`` and use it
during ``install_package_deps`` to ensure the package dependencies do not
override the environment's dependencies.

Adds new config keys:
  * ``constrain_package_deps`` - chicken switch to go back to the legacy
    behavior of allowing the package deps to override the testenv deps
  * ``use_frozen_constraints`` - a more strict mode, enforced that the package
    deps do not conflict in any way with the set of packages installed by
    ``deps``

Fix tox-dev#2386
@tvalentyn
Copy link

Is it possible to install environment deps and package_deps in the same pip install command and have pip handle dependency resolution? If not, what would be a disadvantage providing this capability?

@gaborbernat
Copy link
Member

gaborbernat commented Apr 5, 2024

Well this could be possible but it would probably also break some people who are relying to install dependencies in the first invocation that will be build dependencies for the second one.

@tvalentyn
Copy link

tvalentyn commented Apr 5, 2024

We could make it configurable perhaps via install_command to allow various customizations, for example

install_command = {envbindir}/python {envbindir}/pip install {opts} {packages} {package_deps}
install_command = {envbindir}/python {envbindir}/pip install {opts} {packages} {package_build_deps} {package_deps}
install_command = {envbindir}/python {envbindir}/pip install {opts} {packages} {package} (packages becomes overloaded ,but it currently means test environment packages.

If build dependencies are declared in pyproject.toml, i believe they are already installed separately.

@gaborbernat
Copy link
Member

That would not work due to the way this tool is designed. Furthermore pip used in various situations not just installing the package dependencies, so changing the install command is out of the question and wrong approach.

@tvalentyn
Copy link

i see; would there be a better mechanism to install the package-under-test together with dependencies of a test environment?

I seems like a reasonable setup to me. when I use my package in conjunction with other packages, I would install them together, either by using a requirements file or by installing both in the same pip install command.

When tox installs test environment packages before my package, we are actually testing a slightly different environment. As others pointed out, the followup installation of package-under-test can affect the test environment deps in an undesirable way.

@gaborbernat
Copy link
Member

Any reason you can just add your dependencies into the package dependencies, For example inside an extra group and you need to put it inside deps for tox?

@tvalentyn
Copy link

tvalentyn commented Apr 5, 2024

our use case is compatibility testing. We do use [test] extras, but they add a range, for example 'pyarrow>=3.0.0,<12.0.0'; then, we test several pyarrow versions in tox suites. Having several extras for this purpose seems cumbersome.

@gaborbernat
Copy link
Member

I feel like for that use case you want to use constraint files that you can inject to environment variables in each specific environment instead of using deps.

@tvalentyn
Copy link

Are you referring to the suggestion by masenf@ or there is a simpler example of what that could look like?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:testenv-creation feature:new something does not exist yet, but should
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants