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

Make BacktrackingResolver ignore extras when dropping existing constraints #1984

Merged
merged 11 commits into from
Sep 30, 2023
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.2.0
chludwig-haufe marked this conversation as resolved.
Show resolved Hide resolved
- 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 @@ -2775,6 +2775,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