Skip to content

Commit

Permalink
Merge pull request #1984 from chludwig-haufe/fix/issue-1977_incompati…
Browse files Browse the repository at this point in the history
…ble-constraint-with-extra

Make BacktrackingResolver ignore extras when dropping existing constraints
  • Loading branch information
chrysle committed Sep 30, 2023
2 parents 1974580 + 069382a commit 65b0d36
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 3 deletions.
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ repos:
- pip==20.3.4
- build==1.0.0
- pyproject_hooks==1.0.0
- pytest==7.4.2
- repo: https://github.com/PyCQA/bandit
rev: 1.7.5
hooks:
Expand Down
3 changes: 2 additions & 1 deletion piptools/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,8 @@ def _do_resolve(

# Collect all incompatible install requirement names
cause_ireq_names = {
key_from_req(cause.requirement) for cause in cause_exc.causes
strip_extras(key_from_req(cause.requirement))
for cause in cause_exc.causes
}

# Looks like resolution is impossible, try to fix
Expand Down
17 changes: 15 additions & 2 deletions piptools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from click.utils import LazyFile
from pip._internal.req import InstallRequirement
from pip._internal.req.constructors import install_req_from_line, parse_req_from_line
from pip._internal.resolution.resolvelib.base import Requirement as PipRequirement
from pip._internal.utils.misc import redact_auth_from_url
from pip._internal.vcs import is_url
from pip._vendor.packaging.markers import Marker
Expand Down Expand Up @@ -72,8 +73,20 @@ def key_from_ireq(ireq: InstallRequirement) -> str:
return key_from_req(ireq.req)


def key_from_req(req: InstallRequirement | Requirement) -> str:
"""Get an all-lowercase version of the requirement's name."""
def key_from_req(req: InstallRequirement | Requirement | PipRequirement) -> str:
"""
Get an all-lowercase version of the requirement's name.
**Note:** If the argument is an instance of
``pip._internal.resolution.resolvelib.base.Requirement`` (like
``pip._internal.resolution.resolvelib.requirements.SpecifierRequirement``),
then the name might include an extras specification.
Apply :py:func:`strip_extras` to the result of this function if you need
the package name only.
:param req: the requirement the key is computed for
:return: the canonical name of the requirement
"""
return str(canonicalize_name(req.name))


Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ exclude = "^tests/test_data/"
[[tool.mypy.overrides]]
module = ["tests.*"]
disallow_untyped_defs = false
disallow_incomplete_defs = false

[tool.pytest.ini_options]
addopts = [
Expand Down
133 changes: 133 additions & 0 deletions tests/test_cli_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -2779,6 +2779,139 @@ def test_cli_compile_strip_extras(runner, make_package, make_sdist, tmpdir):
assert "[more]" not in out.stderr


@pytest.mark.parametrize(
("package_specs", "constraints", "existing_reqs", "expected_reqs"),
(
(
[
{
"name": "test_package_1",
"version": "1.1",
"install_requires": ["test_package_2 ~= 1.1"],
},
{
"name": "test_package_2",
"version": "1.1",
"extras_require": {"more": "test_package_3"},
},
],
"""
test_package_1 == 1.1
""",
"""
test_package_1 == 1.0
test_package_2 == 1.0
""",
"""
test-package-1==1.1
test-package-2==1.1
""",
),
(
[
{
"name": "test_package_1",
"version": "1.1",
"install_requires": ["test_package_2[more] ~= 1.1"],
},
{
"name": "test_package_2",
"version": "1.1",
"extras_require": {"more": "test_package_3"},
},
{
"name": "test_package_3",
"version": "0.1",
},
],
"""
test_package_1 == 1.1
""",
"""
test_package_1 == 1.0
test_package_2 == 1.0
test_package_3 == 0.1
""",
"""
test-package-1==1.1
test-package-2==1.1
test-package-3==0.1
""",
),
(
[
{
"name": "test_package_1",
"version": "1.1",
"install_requires": ["test_package_2[more] ~= 1.1"],
},
{
"name": "test_package_2",
"version": "1.1",
"extras_require": {"more": "test_package_3"},
},
{
"name": "test_package_3",
"version": "0.1",
},
],
"""
test_package_1 == 1.1
""",
"""
test_package_1 == 1.0
test_package_2[more] == 1.0
test_package_3 == 0.1
""",
"""
test-package-1==1.1
test-package-2==1.1
test-package-3==0.1
""",
),
),
ids=("no-extra", "extra-stripped-from-existing", "with-extra-in-existing"),
)
def test_resolver_drops_existing_conflicting_constraint(
runner,
make_package,
make_sdist,
tmpdir,
package_specs,
constraints,
existing_reqs,
expected_reqs,
) -> None:
"""
Test that the resolver will find a solution even if some of the existing
(indirect) requirements are incompatible with the new constraints.
This must succeed even if the conflicting requirement includes some extra,
no matter whether the extra is mentioned in the existing requirements
or not (cf. `issue #1977 <https://github.com/jazzband/pip-tools/issues/1977>`_).
"""
expected_requirements = {line.strip() for line in expected_reqs.splitlines()}
dists_dir = tmpdir / "dists"

packages = [make_package(**spec) for spec in package_specs]
for pkg in packages:
make_sdist(pkg, dists_dir)

with open("requirements.txt", "w") as existing_reqs_out:
existing_reqs_out.write(dedent(existing_reqs))

with open("requirements.in", "w") as constraints_out:
constraints_out.write(dedent(constraints))

out = runner.invoke(cli, ["--strip-extras", "--find-links", str(dists_dir)])

assert out.exit_code == 0, out

with open("requirements.txt") as req_txt:
req_txt_content = req_txt.read()
assert expected_requirements.issubset(req_txt_content.splitlines())


def test_resolution_failure(runner):
"""Test resolution impossible for unknown package."""
with open("requirements.in", "w") as reqs_out:
Expand Down
44 changes: 44 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@
import sys
from pathlib import Path
from textwrap import dedent
from typing import Callable

import pip
import pytest
from click import BadOptionUsage, Context, FileError
from pip._internal.req import InstallRequirement
from pip._internal.resolution.resolvelib.requirements import SpecifierRequirement
from pip._vendor.packaging.version import Version

from piptools.scripts.compile import cli as compile_cli
Expand All @@ -29,6 +32,7 @@
is_pinned_requirement,
is_url_requirement,
key_from_ireq,
key_from_req,
lookup_table,
lookup_table_from_tuples,
override_defaults_from_config_file,
Expand Down Expand Up @@ -285,6 +289,46 @@ def test_key_from_ireq_normalization(from_line):
assert len(keys) == 1


@pytest.mark.parametrize(
("line", "expected"),
(
("build", "build"),
("cachecontrol[filecache]", "cachecontrol"),
("some-package[a-b,c_d]", "some-package"),
("other_package[a.b]", "other-package"),
),
)
def test_key_from_req_on_install_requirement(
from_line: Callable[[str], InstallRequirement],
line: str,
expected: str,
) -> None:
ireq = from_line(line)
result = key_from_req(ireq)

assert result == expected


@pytest.mark.parametrize(
("line", "expected"),
(
("build", "build"),
("cachecontrol[filecache]", "cachecontrol[filecache]"),
("some-package[a-b,c_d]", "some-package[a-b,c-d]"),
("other_package[a.b]", "other-package[a-b]"),
),
)
def test_key_from_req_on_specifier_requirement(
from_line: Callable[[str], InstallRequirement],
line: str,
expected: str,
) -> None:
req = SpecifierRequirement(from_line(line))
result = key_from_req(req)

assert result == expected


@pytest.mark.parametrize(
("line", "expected"),
(
Expand Down

0 comments on commit 65b0d36

Please sign in to comment.