From acb2336621055c3e7ab845b3c5c3ae049b0735ce Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Mon, 2 May 2022 15:58:10 -0500 Subject: [PATCH] semver/pep508: handle wildcard exclusions 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 serialsed. 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 --- src/poetry/core/packages/dependency.py | 8 +- src/poetry/core/semver/version_union.py | 155 +++++++++++++++++- .../my_package/__init__.py | 0 .../pyproject.toml | 11 ++ tests/masonry/builders/test_builder.py | 12 ++ tests/packages/test_dependency.py | 37 ++++- 6 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 tests/masonry/builders/fixtures/with_wildcard_dependency_constraint/my_package/__init__.py create mode 100644 tests/masonry/builders/fixtures/with_wildcard_dependency_constraint/pyproject.toml diff --git a/src/poetry/core/packages/dependency.py b/src/poetry/core/packages/dependency.py index 1beac6cdc..d3c6698c4 100644 --- a/src/poetry/core/packages/dependency.py +++ b/src/poetry/core/packages/dependency.py @@ -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(",") diff --git a/src/poetry/core/semver/version_union.py b/src/poetry/core/semver/version_union.py index 8e54742c2..d98c9fed2 100644 --- a/src/poetry/core/semver/version_union.py +++ b/src/poetry/core/semver/version_union.py @@ -238,6 +238,156 @@ def _ranges_for( raise ValueError(f"Unknown VersionConstraint type {constraint}") + def _exclude_single_wildcard_range_string(self) -> str: + """ + Helper method to convert + """ + 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 @@ -264,7 +414,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"" diff --git a/tests/masonry/builders/fixtures/with_wildcard_dependency_constraint/my_package/__init__.py b/tests/masonry/builders/fixtures/with_wildcard_dependency_constraint/my_package/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/masonry/builders/fixtures/with_wildcard_dependency_constraint/pyproject.toml b/tests/masonry/builders/fixtures/with_wildcard_dependency_constraint/pyproject.toml new file mode 100644 index 000000000..662684076 --- /dev/null +++ b/tests/masonry/builders/fixtures/with_wildcard_dependency_constraint/pyproject.toml @@ -0,0 +1,11 @@ +[tool.poetry] +name = "my-package" +version = "1.2.3" +description = "Some description." +authors = [ + "People Everywhere " +] + +[tool.poetry.dependencies] +python = "^3.10" +google-api-python-client = ">=1.8,!=2.0.*" diff --git a/tests/masonry/builders/test_builder.py b/tests/masonry/builders/test_builder.py index 22a42f63f..6e3db706d 100644 --- a/tests/masonry/builders/test_builder.py +++ b/tests/masonry/builders/test_builder.py @@ -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.*)"] diff --git a/tests/packages/test_dependency.py b/tests/packages/test_dependency.py index 90cb1ff24..00751a025 100644 --- a/tests/packages/test_dependency.py +++ b/tests/packages/test_dependency.py @@ -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( @@ -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(