Fix complicated export case

dimbleby · radoering · commit 503010c956ee · 2022-08-07T14:22:05.000+02:00
By treating different python version ranges independently, we buy the
flexibility needed to make better decisions.
diff --git a/poetry.lock b/poetry.lock
diff --git a/pyproject.toml b/pyproject.toml
@@ -17,6 +17,7 @@ include = [
 [tool.poetry.dependencies]
 python = "^3.7"
 poetry = "^1.2.0b3"
+poetry-core = "^1.1.0b3"
 
 [tool.poetry.dev-dependencies]
 pre-commit = "^2.18"
@@ -43,7 +44,11 @@ use_parentheses = true
 [tool.mypy]
 namespace_packages = true
 show_error_codes = true
-enable_error_code = ["ignore-without-code"]
+enable_error_code = [
+    "ignore-without-code",
+    "redundant-expr",
+    "truthy-bool",
+]
 strict = true
 files = ["src", "tests"]
 exclude = ["^tests/fixtures/"]
diff --git a/src/poetry_plugin_export/walker.py b/src/poetry_plugin_export/walker.py
@@ -1,8 +1,10 @@
 from __future__ import annotations
 
-from copy import deepcopy
 from typing import TYPE_CHECKING
 
+from poetry.core.semver.util import constraint_regions
+from poetry.core.version.markers import AnyMarker
+from poetry.core.version.markers import SingleMarker
 from poetry.packages import DependencyPackage
 from poetry.utils.extras import get_extra_package_names
 
@@ -18,6 +20,33 @@
     from poetry.packages import Locker
 
 
+def get_python_version_region_markers(packages: list[Package]) -> list[BaseMarker]:
+    markers = []
+
+    regions = constraint_regions([package.python_constraint for package in packages])
+    for region in regions:
+        marker: BaseMarker = AnyMarker()
+        if region.min is not None:
+            min_operator = ">=" if region.include_min else ">"
+            marker_name = (
+                "python_full_version" if region.min.precision > 2 else "python_version"
+            )
+            lo = SingleMarker(marker_name, f"{min_operator} {region.min}")
+            marker = marker.intersect(lo)
+
+        if region.max is not None:
+            max_operator = "<=" if region.include_max else "<"
+            marker_name = (
+                "python_full_version" if region.max.precision > 2 else "python_version"
+            )
+            hi = SingleMarker(marker_name, f"{max_operator} {region.max}")
+            marker = marker.intersect(hi)
+
+        markers.append(marker)
+
+    return markers
+
+
 def get_project_dependency_packages(
     locker: Locker,
     project_requires: list[Dependency],
@@ -28,7 +57,7 @@ def get_project_dependency_packages(
     if project_python_marker is not None:
         marked_requires: list[Dependency] = []
         for require in project_requires:
-            require = deepcopy(require)
+            require = require.clone()
             require.marker = require.marker.intersect(project_python_marker)
             marked_requires.append(require)
         project_requires = marked_requires
@@ -136,12 +165,23 @@ def walk_dependencies(
             ):
                 continue
 
-            require = deepcopy(require)
-            require.marker = require.marker.intersect(
-                requirement.marker.without_extras()
-            )
-            if not require.marker.is_empty():
-                dependencies.append(require)
+            base_marker = require.marker.intersect(requirement.marker.without_extras())
+
+            if not base_marker.is_empty():
+                # So as to give ourselves enough flexibility in choosing a solution,
+                # we need to split the world up into the python version ranges that
+                # this package might care about.
+                #
+                # We create a marker for all of the possible regions, and add a
+                # requirement for each separately.
+                candidates = packages_by_name.get(require.name, [])
+                region_markers = get_python_version_region_markers(candidates)
+                for region_marker in region_markers:
+                    marker = region_marker.intersect(base_marker)
+                    if not marker.is_empty():
+                        require2 = require.clone()
+                        require2.marker = marker
+                        dependencies.append(require2)
 
         key = locked_package
         if key not in nested_dependencies:
@@ -165,22 +205,38 @@ def get_locked_package(
     """
     decided = decided or {}
 
-    # Get the packages that are consistent with this dependency.
-    packages = [
-        package
-        for package in packages_by_name.get(dependency.name, [])
-        if package.python_constraint.allows_all(dependency.python_constraint)
-        and dependency.constraint.allows(package.version)
-    ]
+    candidates = packages_by_name.get(dependency.name, [])
 
-    # If we've previously made a choice that is compatible with the current
-    # requirement, stick with it.
-    for package in packages:
+    # If we've previously chosen a version of this package that is compatible with
+    # the current requirement, we are forced to stick with it.  (Else we end up with
+    # different versions of the same package at the same time.)
+    overlapping_candidates = set()
+    for package in candidates:
         old_decision = decided.get(package)
         if (
             old_decision is not None
             and not old_decision.marker.intersect(dependency.marker).is_empty()
         ):
-            return package
+            overlapping_candidates.add(package)
+
+    # If we have more than one overlapping candidate, we've run into trouble.
+    if len(overlapping_candidates) > 1:
+        return None
+
+    # Get the packages that are consistent with this dependency.
+    compatible_candidates = [
+        package
+        for package in candidates
+        if package.python_constraint.allows_all(dependency.python_constraint)
+        and dependency.constraint.allows(package.version)
+    ]
+
+    # If we have an overlapping candidate, we must use it.
+    if overlapping_candidates:
+        compatible_candidates = [
+            package
+            for package in compatible_candidates
+            if package in overlapping_candidates
+        ]
 
-    return next(iter(packages), None)
+    return next(iter(compatible_candidates), None)
diff --git a/tests/markers.py b/tests/markers.py
@@ -8,16 +8,21 @@
 MARKER_LINUX = parse_marker('sys_platform == "linux"')
 MARKER_DARWIN = parse_marker('sys_platform == "darwin"')
 
+MARKER_CPYTHON = parse_marker('implementation_name == "cpython"')
+
 MARKER_PY27 = parse_marker('python_version >= "2.7" and python_version < "2.8"')
 
 MARKER_PY36 = parse_marker('python_version >= "3.6" and python_version < "4.0"')
 MARKER_PY36_38 = parse_marker('python_version >= "3.6" and python_version < "3.8"')
+MARKER_PY36_PY362 = parse_marker(
+    'python_version >= "3.6" and python_full_version < "3.6.2"'
+)
+MARKER_PY362_PY40 = parse_marker(
+    'python_full_version >= "3.6.2" and python_version < "4.0"'
+)
 MARKER_PY36_ONLY = parse_marker('python_version >= "3.6" and python_version < "3.7"')
 
 MARKER_PY37 = parse_marker('python_version >= "3.7" and python_version < "4.0"')
-MARKER_PY37_PY400 = parse_marker(
-    'python_version >= "3.7" and python_full_version < "4.0.0"'
-)
 
 MARKER_PY = MARKER_PY27.union(MARKER_PY36)
 
diff --git a/tests/test_exporter.py b/tests/test_exporter.py
@@ -17,12 +17,15 @@
 from poetry.repositories.legacy_repository import LegacyRepository
 
 from poetry_plugin_export.exporter import Exporter
+from tests.markers import MARKER_CPYTHON
 from tests.markers import MARKER_PY
 from tests.markers import MARKER_PY27
 from tests.markers import MARKER_PY36
 from tests.markers import MARKER_PY36_38
 from tests.markers import MARKER_PY36_ONLY
+from tests.markers import MARKER_PY36_PY362
 from tests.markers import MARKER_PY37
+from tests.markers import MARKER_PY362_PY40
 from tests.markers import MARKER_PY_DARWIN
 from tests.markers import MARKER_PY_LINUX
 from tests.markers import MARKER_PY_WIN32
@@ -2019,10 +2022,10 @@ def test_exporter_doesnt_confuse_repeated_packages(
     expected = f"""\
 celery==5.1.2 ; {MARKER_PY36_ONLY}
 celery==5.2.3 ; {MARKER_PY37}
-click-didyoumean==0.0.3 ; {MARKER_PY36_ONLY}
-click-didyoumean==0.3.0 ; {MARKER_PY37}
-click-plugins==1.1.1 ; {MARKER_PY36_ONLY.union(MARKER_PY37)}
-click==7.1.2 ; {MARKER_PY36_ONLY}
+click-didyoumean==0.0.3 ; {MARKER_PY36_PY362}
+click-didyoumean==0.3.0 ; {MARKER_PY362_PY40}
+click-plugins==1.1.1 ; {MARKER_PY36}
+click==7.1.2 ; python_version < "3.7" and python_version >= "3.6"
 click==8.0.3 ; {MARKER_PY37}
 """
 
@@ -2141,6 +2144,107 @@ def test_exporter_handles_extras_next_to_non_extras(
     assert io.fetch_output() == expected
 
 
+def test_exporter_handles_overlapping_python_versions(
+    tmp_dir: str, poetry: Poetry
+) -> None:
+    # Testcase derived from
+    # https://github.com/python-poetry/poetry-plugin-export/issues/32.
+    poetry.locker.mock_lock_data(  # type: ignore[attr-defined]
+        {
+            "package": [
+                {
+                    "name": "ipython",
+                    "python-versions": ">=3.6",
+                    "version": "7.16.3",
+                    "category": "main",
+                    "optional": False,
+                    "dependencies": {},
+                },
+                {
+                    "name": "ipython",
+                    "python-versions": ">=3.7",
+                    "version": "7.34.0",
+                    "category": "main",
+                    "optional": False,
+                    "dependencies": {},
+                },
+                {
+                    "name": "slash",
+                    "python-versions": ">=3.6.*",
+                    "version": "1.13.0",
+                    "category": "main",
+                    "optional": False,
+                    "dependencies": {
+                        "ipython": [
+                            {
+                                "version": "*",
+                                "markers": (
+                                    'python_version >= "3.6" and implementation_name !='
+                                    ' "pypy"'
+                                ),
+                            },
+                            {
+                                "version": "<7.17.0",
+                                "markers": (
+                                    'python_version < "3.6" and implementation_name !='
+                                    ' "pypy"'
+                                ),
+                            },
+                        ],
+                    },
+                },
+            ],
+            "metadata": {
+                "lock-version": "1.1",
+                "python-versions": "^3.6",
+                "content-hash": (
+                    "832b13a88e5020c27cbcd95faa577bf0dbf054a65c023b45dc9442b640d414e6"
+                ),
+                "hashes": {
+                    "ipython": [],
+                    "slash": [],
+                },
+            },
+        }
+    )
+    root = poetry.package.with_dependency_groups([], only=True)
+    root.python_versions = "^3.6"
+    root.add_dependency(
+        Factory.create_dependency(
+            name="ipython",
+            constraint={"version": "*", "python": "~3.6"},
+        )
+    )
+    root.add_dependency(
+        Factory.create_dependency(
+            name="ipython",
+            constraint={"version": "^7.17", "python": "^3.7"},
+        )
+    )
+    root.add_dependency(
+        Factory.create_dependency(
+            name="slash",
+            constraint={
+                "version": "^1.12",
+                "markers": "implementation_name == 'cpython'",
+            },
+        )
+    )
+    poetry._package = root
+
+    exporter = Exporter(poetry)
+    io = BufferedIO()
+    exporter.export("requirements.txt", Path(tmp_dir), io)
+
+    expected = f"""\
+ipython==7.16.3 ; {MARKER_PY36_ONLY}
+ipython==7.34.0 ; {MARKER_PY37}
+slash==1.13.0 ; {MARKER_PY36} and {MARKER_CPYTHON}
+"""
+
+    assert io.fetch_output() == expected
+
+
 @pytest.mark.parametrize(
     ["with_extras", "expected"],
     [
