Skip to content

Commit

Permalink
semver/pep508: handle wildcard exclusions (python-poetry#343)
Browse files Browse the repository at this point in the history
Prior to this change, when exporting PEP 508 strings for dependencies,
wildcard exclusion constraints like `!=2.0.*` were incorrectly
serialised as invalid PEP 508 due to how version unions were serialised.

This change allows for determining if a version union is a single
wildcard range exclusion, and if so serialise it appropriately.

Resolves: python-poetry/poetry#5470
  • Loading branch information
abn authored and DavidVujic committed Aug 31, 2022
1 parent 978627f commit 861b3a9
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 5 deletions.
8 changes: 7 additions & 1 deletion src/poetry/core/packages/dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,13 @@ def base_pep_508_name(self) -> str:

constraint = self.constraint
if isinstance(constraint, VersionUnion):
if constraint.excludes_single_version():
if (
constraint.excludes_single_version()
or constraint.excludes_single_wildcard_range()
):
# This branch is a short-circuit logic for special cases and
# avoids having to split and parse constraint again. This has
# no functional difference with the logic in the else branch.
requirement += f" ({str(constraint)})"
else:
constraints = self.pretty_constraint.split(",")
Expand Down
156 changes: 155 additions & 1 deletion src/poetry/core/semver/version_union.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,157 @@ def _ranges_for(

raise ValueError(f"Unknown VersionConstraint type {constraint}")

def _exclude_single_wildcard_range_string(self) -> str:
"""
Helper method to convert this instance into a wild card range
string.
"""
if not self.excludes_single_wildcard_range():
raise ValueError("Not a valid wildcard range")

# we assume here that since it is a single exclusion range
# that it is one of "< 2.0.0 || >= 2.1.0" or ">= 2.1.0 || < 2.0.0"
# and the one with the max is the first part
idx_order = (0, 1) if self._ranges[0].max else (1, 0)
one = self._ranges[idx_order[0]].max
two = self._ranges[idx_order[1]].min

# versions can have both semver and non semver parts
parts_one = [
one.major,
one.minor or 0,
one.patch or 0,
*list(one.non_semver_parts or []),
]
parts_two = [
two.major,
two.minor or 0,
two.patch or 0,
*list(two.non_semver_parts or []),
]

# we assume here that a wildcard range implies that the part following the
# first part that is different in the second range is the wildcard, this means
# that multiple wildcards are not supported right now.
parts = []

for idx, part in enumerate(parts_one):
parts.append(str(part))
if parts_two[idx] != part:
# since this part is different the next one is the wildcard
# for example, "< 2.0.0 || >= 2.1.0" gets us a wildcard range
# 2.0.*
parts.append("*")
break
else:
# we should not ever get here, however it is likely that poorly
# constructed metadata exists
raise ValueError("Not a valid wildcard range")

return f"!={'.'.join(parts)}"

@staticmethod
def _excludes_single_wildcard_range_check_is_valid_range(
one: VersionRangeConstraint, two: VersionRangeConstraint
) -> bool:
"""
Helper method to determine if two versions define a single wildcard range.
In cases where !=2.0.* was parsed by us, the union is of the range
<2.0.0 || >=2.1.0. In user defined ranges, precision might be different.
For example, a union <2.0 || >= 2.1.0 is still !=2.0.*. In order to
handle these cases we make sure that if precisions do not match, extra
checks are performed to validate that the constraint is a valid single
wildcard range.
"""

max_precision = max(one.max.precision, two.min.precision)

if max_precision <= 3:
# In cases where both versions have a precision less than 3,
# we can make use of the next major/minor/patch versions.
return two.min in {
one.max.next_major(),
one.max.next_minor(),
one.max.next_patch(),
}
else:
# When there are non-semver parts in one of the versions, we need to
# ensure we use zero padded version and in addition to next major/minor/
# patch versions, also check each next release for the extra parts.
from_parts = one.max.__class__.from_parts

_extras: list[list[int]] = []
_versions: list[Version] = []

for _version in [one.max, two.min]:
_extra = list(_version.non_semver_parts or [])

while len(_extra) < (max_precision - 3):
# pad zeros for extra parts to ensure precisions are equal
_extra.append(0)

# create a new release with unspecified parts padded with zeros
_padded_version: Version = from_parts(
major=_version.major,
minor=_version.minor or 0,
patch=_version.patch or 0,
extra=tuple(_extra),
)

_extras.append(_extra)
_versions.append(_padded_version)

_extra_one = _extras[0]
_padded_version_one = _versions[0]
_padded_version_two = _versions[1]

_check_versions = {
_padded_version_one.next_major(),
_padded_version_one.next_minor(),
_padded_version_one.next_patch(),
}

# for each non-semver (extra) part, bump a version
for idx in range(len(_extra_one)):
_extra = [
*_extra_one[: idx - 1],
(_extra_one[idx] + 1),
*_extra_one[idx + 1 :],
]
_check_versions.add(
from_parts(
_padded_version_one.major,
_padded_version_one.minor,
_padded_version_one.patch,
*_extra,
)
)

return _padded_version_two in _check_versions

def excludes_single_wildcard_range(self) -> bool:
from poetry.core.semver.version_range import VersionRange

if len(self._ranges) != 2:
return False

idx_order = (0, 1) if self._ranges[0].max else (1, 0)
one = self._ranges[idx_order[0]]
two = self._ranges[idx_order[1]]

is_range_exclusion = (
one.max and not one.include_max and two.min and two.include_min
)

if not is_range_exclusion:
return False

if not self._excludes_single_wildcard_range_check_is_valid_range(one, two):
return False

return isinstance(VersionRange().difference(self), VersionRange)

def excludes_single_version(self) -> bool:
from poetry.core.semver.version import Version
from poetry.core.semver.version_range import VersionRange
Expand All @@ -264,7 +415,10 @@ def __str__(self) -> str:
if self.excludes_single_version():
return f"!={VersionRange().difference(self)}"

return " || ".join([str(r) for r in self._ranges])
try:
return self._exclude_single_wildcard_range_string()
except ValueError:
return " || ".join([str(r) for r in self._ranges])

def __repr__(self) -> str:
return f"<VersionUnion {str(self)}>"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[tool.poetry]
name = "my-package"
version = "1.2.3"
description = "Some description."
authors = [
"People Everywhere <[email protected]>"
]

[tool.poetry.dependencies]
python = "^3.10"
google-api-python-client = ">=1.8,!=2.0.*"
12 changes: 12 additions & 0 deletions tests/masonry/builders/test_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,15 @@ def test_metadata_with_readme_files() -> None:
description = "\n".join([readme1.read_text(), readme2.read_text(), ""])

assert metadata.get_payload() == description


def test_metadata_with_wildcard_dependency_constraint() -> None:
test_path = (
Path(__file__).parent / "fixtures" / "with_wildcard_dependency_constraint"
)
builder = Builder(Factory().create_poetry(test_path))

metadata = Parser().parsestr(builder.get_metadata_content())

requires = metadata.get_all("Requires-Dist")
assert requires == ["google-api-python-client (>=1.8,!=2.0.*)"]
37 changes: 34 additions & 3 deletions tests/packages/test_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,18 @@ def test_to_pep_508_in_extras_parsed() -> None:
assert result == "foo[bar,baz] (>=1.23,<2.0)"


def test_to_pep_508_with_single_version_excluded() -> None:
dependency = Dependency("foo", "!=1.2.3")
@pytest.mark.parametrize(
("exclusion", "expected"),
[
("!=1.2.3", "!=1.2.3"),
("!=1.2.*", "!=1.2.*"),
("<2.0 || >=2.1", "!=2.0.*"),
],
)
def test_to_pep_508_with_excluded_versions(exclusion: str, expected: str) -> None:
dependency = Dependency("foo", exclusion)

assert dependency.to_pep_508() == "foo (!=1.2.3)"
assert dependency.to_pep_508() == f"foo ({expected})"


@pytest.mark.parametrize(
Expand Down Expand Up @@ -245,6 +253,29 @@ def test_complete_name() -> None:
["x"],
"A[x] (>=1.6.5,!=1.8.0,<3.1.0)",
),
# test single version range exclusions
("A", ">=1.8,!=2.0.*", None, "A (>=1.8,!=2.0.*)"),
("A", "!=0.0.*", None, "A (!=0.0.*)"),
("A", "!=0.1.*", None, "A (!=0.1.*)"),
("A", "!=0.*", None, "A (>=1.0.0)"),
("A", ">=1.8,!=2.*", None, "A (>=1.8,!=2.*)"),
("A", ">=1.8,!=2.*.*", None, "A (>=1.8,!=2.*)"),
("A", ">=1.8,<2.0 || >=2.1.0", None, "A (>=1.8,!=2.0.*)"),
("A", ">=1.8,<2.0.0 || >=3.0.0", None, "A (>=1.8,!=2.*)"),
("A", ">=1.8,<2.0 || >=3", None, "A (>=1.8,!=2.*)"),
("A", ">=1.8,<2 || >=2.1.0", None, "A (>=1.8,!=2.0.*)"),
("A", ">=1.8,<2 || >=2.1", None, "A (>=1.8,!=2.0.*)"),
("A", ">=1.8,!=2.0.*,!=3.0.*", None, "A (>=1.8,!=2.0.*,!=3.0.*)"),
("A", ">=1.8.0.0,<2.0.0.0 || >=2.0.1.0", None, "A (>=1.8.0.0,!=2.0.0.*)"),
("A", ">=1.8.0.0,<2 || >=2.0.1.0", None, "A (>=1.8.0.0,!=2.0.0.*)"),
# we verify that the range exclusion logic is not too eager
("A", ">=1.8,<2.0 || >=2.2.0", None, "A (>=1.8,<2.0 || >=2.2.0)"),
("A", ">=1.8,<2.0 || >=2.1.5", None, "A (>=1.8,<2.0 || >=2.1.5)"),
("A", ">=1.8.0.0,<2 || >=2.0.1.5", None, "A (>=1.8.0.0,<2 || >=2.0.1.5)"),
# non-semver version test is ignored due to existing bug in wildcard
# constraint parsing that ignores non-semver versions
# TODO: re-enable for verification once fixed
# ("A", ">=1.8.0.0,!=2.0.0.*", None, "A (>=1.8.0.0,!=2.0.0.*)"), # noqa: E800
],
)
def test_dependency_string_representation(
Expand Down

0 comments on commit 861b3a9

Please sign in to comment.