Skip to content

Commit

Permalink
packages: fix get_python_constraint_from_marker via convert_markers f…
Browse files Browse the repository at this point in the history
…or markers whose disjunctive normal form (DNF) contains a group without marker name "python_version"

By the way, convert_markers() was simplified significantly. That was possible because markers are converted to DNF before conversion.
  • Loading branch information
radoering authored and abn committed May 9, 2022
1 parent 35e43a0 commit 7cd62df
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 60 deletions.
3 changes: 2 additions & 1 deletion src/poetry/core/packages/dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
)
from poetry.core.packages.dependency_group import MAIN_GROUP
from poetry.core.packages.specification import PackageSpecification
from poetry.core.packages.utils.utils import contains_group_without_marker
from poetry.core.semver.helpers import parse_constraint
from poetry.core.semver.version_range_constraint import VersionRangeConstraint
from poetry.core.version.markers import parse_marker
Expand Down Expand Up @@ -180,7 +181,7 @@ def marker(self, marker: str | BaseMarker) -> None:

# Recalculate python versions.
self._python_versions = "*"
if "python_version" in markers:
if not contains_group_without_marker(markers, "python_version"):
ors = []
for or_ in markers["python_version"]:
ands = []
Expand Down
86 changes: 32 additions & 54 deletions src/poetry/core/packages/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from poetry.core.semver.version_union import VersionUnion
from poetry.core.version.markers import BaseMarker

ConvertedMarkers = dict[str, list[list[tuple[str, str]]]]


BZ2_EXTENSIONS = (".tar.bz2", ".tbz")
XZ_EXTENSIONS = (".tar.xz", ".txz", ".tlz", ".tar.lz", ".tar.lzma")
Expand Down Expand Up @@ -138,67 +140,34 @@ def splitext(path: str) -> tuple[str, str]:
return base, ext


def group_markers(
markers: list[BaseMarker], or_: bool = False
) -> list[tuple[str, str, str] | list[tuple[str, str, str]]]:
def convert_markers(marker: BaseMarker) -> ConvertedMarkers:
from poetry.core.version.markers import MarkerUnion
from poetry.core.version.markers import MultiMarker
from poetry.core.version.markers import SingleMarker

groups = [[]]

for marker in markers:
if or_:
groups.append([])

if isinstance(marker, (MultiMarker, MarkerUnion)):
groups[-1].append(
group_markers(marker.markers, isinstance(marker, MarkerUnion))
)
elif isinstance(marker, SingleMarker):
lhs, op, rhs = marker.name, marker.operator, marker.value

groups[-1].append((lhs, op, rhs))

return groups


def convert_markers(marker: BaseMarker) -> dict[str, list[list[tuple[str, str]]]]:
groups = group_markers([dnf(marker)])

requirements = {}
marker = dnf(marker)
conjunctions = marker.markers if isinstance(marker, MarkerUnion) else [marker]
group_count = len(conjunctions)

def _group(
_groups: list[tuple[str, str, str] | list[tuple[str, str, str]]],
or_: bool = False,
def add_constraint(
marker_name: str, constraint: tuple[str, str], group_index: int
) -> None:
ors = {}
for group in _groups:
if isinstance(group, list):
_group(group, or_=True)
else:
variable, op, value = group
group_name = str(variable)

# python_full_version is equivalent to python_version
# for Poetry so we merge them
if group_name == "python_full_version":
group_name = "python_version"

if group_name not in requirements:
requirements[group_name] = []

if group_name not in ors:
ors[group_name] = or_

if ors[group_name] or not requirements[group_name]:
requirements[group_name].append([])

requirements[group_name][-1].append((str(op), str(value)))

ors[group_name] = False

_group(groups, or_=True)
# python_full_version is equivalent to python_version
# for Poetry so we merge them
if marker_name == "python_full_version":
marker_name = "python_version"
if marker_name not in requirements:
requirements[marker_name] = [[] for _ in range(group_count)]
requirements[marker_name][group_index].append(constraint)

for i, sub_marker in enumerate(conjunctions):
if isinstance(sub_marker, MultiMarker):
for m in sub_marker.markers:
if isinstance(m, SingleMarker):
add_constraint(m.name, (m.operator, m.value), i)
elif isinstance(sub_marker, SingleMarker):
add_constraint(sub_marker.name, (sub_marker.operator, sub_marker.value), i)

for group_name in requirements:
# remove duplicates
Expand All @@ -210,6 +179,10 @@ def _group(
return requirements


def contains_group_without_marker(markers: ConvertedMarkers, marker_name: str) -> bool:
return marker_name not in markers or [] in markers[marker_name]


def create_nested_marker(
name: str,
constraint: BaseConstraint | VersionUnion | Version | VersionConstraint,
Expand Down Expand Up @@ -315,6 +288,11 @@ def get_python_constraint_from_marker(
return EmptyConstraint()

markers = convert_markers(marker)
if contains_group_without_marker(markers, "python_version"):
# groups are in disjunctive normal form (DNF),
# an empty group means that python_version does not appear in this group,
# which means that python_version is arbitrary for this group
return VersionRange()

ors = []
for or_ in markers["python_version"]:
Expand Down
17 changes: 12 additions & 5 deletions tests/packages/test_dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,12 +324,19 @@ def test_with_constraint() -> None:
assert new.transitive_python_constraint == dependency.transitive_python_constraint


def test_marker_properly_sets_python_constraint() -> None:
@pytest.mark.parametrize(
"marker, expected",
[
('python_version >= "3.6" and python_version < "4.0"', ">=3.6,<4.0"),
('sys_platform == "linux"', "*"),
('python_version >= "3.9" or sys_platform == "linux"', "*"),
('python_version >= "3.9" and sys_platform == "linux"', ">=3.9"),
],
)
def test_marker_properly_sets_python_constraint(marker: str, expected: str) -> None:
dependency = Dependency("foo", "^1.2.3")

dependency.marker = 'python_version >= "3.6" and python_version < "4.0"' # type: ignore[assignment]

assert str(dependency.python_constraint) == ">=3.6,<4.0"
dependency.marker = marker # type: ignore[assignment]
assert str(dependency.python_constraint) == expected


def test_dependency_markers_are_the_same_as_markers() -> None:
Expand Down
18 changes: 18 additions & 0 deletions tests/packages/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,20 @@
"python_version": [[("<", "2.7")], [(">=", "3.0.0")]],
},
),
(
'python_version >= "3.9" or sys_platform == "linux"',
{
"python_version": [[(">=", "3.9")], []],
"sys_platform": [[], [("==", "linux")]],
},
),
(
'python_version >= "3.9" and sys_platform == "linux"',
{
"python_version": [[(">=", "3.9")]],
"sys_platform": [[("==", "linux")]],
},
),
],
)
def test_convert_markers(
Expand Down Expand Up @@ -108,6 +122,10 @@ def test_convert_markers(
),
# no python_version
('sys_platform == "linux"', "*"),
# no relevant python_version
('python_version >= "3.9" or sys_platform == "linux"', "*"),
# relevant python_version
('python_version >= "3.9" and sys_platform == "linux"', ">=3.9"),
],
)
def test_get_python_constraint_from_marker(marker: str, constraint: str) -> None:
Expand Down

0 comments on commit 7cd62df

Please sign in to comment.