From fdfa9ef581f047e171450dc4da7f80609b3bdd19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sun, 7 Aug 2022 14:05:59 +0200 Subject: [PATCH 1/2] move export code into export plugin Co-authored-by: David Hotham --- src/poetry_plugin_export/exporter.py | 5 +- src/poetry_plugin_export/walker.py | 186 +++++++++++++++++++++++++++ 2 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 src/poetry_plugin_export/walker.py diff --git a/src/poetry_plugin_export/exporter.py b/src/poetry_plugin_export/exporter.py index 293335b..1525820 100644 --- a/src/poetry_plugin_export/exporter.py +++ b/src/poetry_plugin_export/exporter.py @@ -9,6 +9,8 @@ from poetry.core.packages.dependency_group import MAIN_GROUP from poetry.repositories.http import HTTPRepository +from poetry_plugin_export.walker import get_project_dependency_packages + if TYPE_CHECKING: from pathlib import Path @@ -80,7 +82,8 @@ def _export_requirements_txt(self, cwd: Path, output: IO | str) -> None: list(self._groups), only=True ) - for dependency_package in self._poetry.locker.get_project_dependency_packages( + for dependency_package in get_project_dependency_packages( + self._poetry.locker, project_requires=root.all_requires, project_python_marker=root.python_marker, extras=self._extras, diff --git a/src/poetry_plugin_export/walker.py b/src/poetry_plugin_export/walker.py new file mode 100644 index 0000000..f124eff --- /dev/null +++ b/src/poetry_plugin_export/walker.py @@ -0,0 +1,186 @@ +from __future__ import annotations + +from copy import deepcopy +from typing import TYPE_CHECKING + +from poetry.packages import DependencyPackage +from poetry.utils.extras import get_extra_package_names + + +if TYPE_CHECKING: + from collections.abc import Iterable + from collections.abc import Iterator + from collections.abc import Sequence + + from poetry.core.packages.dependency import Dependency + from poetry.core.packages.package import Package + from poetry.core.version.markers import BaseMarker + from poetry.packages import Locker + + +def get_project_dependency_packages( + locker: Locker, + project_requires: list[Dependency], + project_python_marker: BaseMarker | None = None, + extras: bool | Sequence[str] | None = None, +) -> Iterator[DependencyPackage]: + # Apply the project python marker to all requirements. + if project_python_marker is not None: + marked_requires: list[Dependency] = [] + for require in project_requires: + require = deepcopy(require) + require.marker = require.marker.intersect(project_python_marker) + marked_requires.append(require) + project_requires = marked_requires + + repository = locker.locked_repository() + + # Build a set of all packages required by our selected extras + extra_package_names: set[str] | None = None + + if extras is not True: + extra_package_names = set( + get_extra_package_names( + repository.packages, + locker.lock_data.get("extras", {}), + extras or (), + ) + ) + + # If a package is optional and we haven't opted in to it, do not select + selected = [] + for dependency in project_requires: + try: + package = repository.find_packages(dependency=dependency)[0] + except IndexError: + continue + + if extra_package_names is not None and ( + package.optional and package.name not in extra_package_names + ): + # a package is locked as optional, but is not activated via extras + continue + + selected.append(dependency) + + for package, dependency in get_project_dependencies( + project_requires=selected, + locked_packages=repository.packages, + ): + yield DependencyPackage(dependency=dependency, package=package) + + +def get_project_dependencies( + project_requires: list[Dependency], + locked_packages: list[Package], +) -> Iterable[tuple[Package, Dependency]]: + # group packages entries by name, this is required because requirement might use + # different constraints. + packages_by_name: dict[str, list[Package]] = {} + for pkg in locked_packages: + if pkg.name not in packages_by_name: + packages_by_name[pkg.name] = [] + packages_by_name[pkg.name].append(pkg) + + # Put higher versions first so that we prefer them. + for packages in packages_by_name.values(): + packages.sort( + key=lambda package: package.version, + reverse=True, + ) + + nested_dependencies = walk_dependencies( + dependencies=project_requires, + packages_by_name=packages_by_name, + ) + + return nested_dependencies.items() + + +def walk_dependencies( + dependencies: list[Dependency], + packages_by_name: dict[str, list[Package]], +) -> dict[Package, Dependency]: + nested_dependencies: dict[Package, Dependency] = {} + + visited: set[tuple[Dependency, BaseMarker]] = set() + while dependencies: + requirement = dependencies.pop(0) + if (requirement, requirement.marker) in visited: + continue + visited.add((requirement, requirement.marker)) + + locked_package = get_locked_package( + requirement, packages_by_name, nested_dependencies + ) + + if not locked_package: + raise RuntimeError(f"Dependency walk failed at {requirement}") + + if requirement.extras: + locked_package = locked_package.with_features(requirement.extras) + + # create dependency from locked package to retain dependency metadata + # if this is not done, we can end-up with incorrect nested dependencies + constraint = requirement.constraint + marker = requirement.marker + requirement = locked_package.to_dependency() + requirement.marker = requirement.marker.intersect(marker) + + requirement.constraint = constraint + + for require in locked_package.requires: + if require.is_optional() and not any( + require in locked_package.extras[feature] + for feature in locked_package.features + ): + continue + + require = deepcopy(require) + require.marker = require.marker.intersect( + requirement.marker.without_extras() + ) + if not require.marker.is_empty(): + dependencies.append(require) + + key = locked_package + if key not in nested_dependencies: + nested_dependencies[key] = requirement + else: + nested_dependencies[key].marker = nested_dependencies[key].marker.union( + requirement.marker + ) + + return nested_dependencies + + +def get_locked_package( + dependency: Dependency, + packages_by_name: dict[str, list[Package]], + decided: dict[Package, Dependency] | None = None, +) -> Package | None: + """ + Internal helper to identify corresponding locked package using dependency + version constraints. + """ + 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) + ] + + # If we've previously made a choice that is compatible with the current + # requirement, stick with it. + for package in packages: + old_decision = decided.get(package) + if ( + old_decision is not None + and not old_decision.marker.intersect(dependency.marker).is_empty() + ): + return package + + return next(iter(packages), None) From 3638315d476bea312d176eaa5baba88126e1d52e Mon Sep 17 00:00:00 2001 From: David Hotham Date: Thu, 28 Jul 2022 19:07:15 +0100 Subject: [PATCH 2/2] Fix complicated export case By treating different python version ranges independently, we buy the flexibility needed to make better decisions. --- poetry.lock | 20 +++--- pyproject.toml | 7 +- src/poetry_plugin_export/walker.py | 96 +++++++++++++++++++------ tests/markers.py | 11 ++- tests/test_exporter.py | 112 +++++++++++++++++++++++++++-- 5 files changed, 208 insertions(+), 38 deletions(-) diff --git a/poetry.lock b/poetry.lock index 33c7dc1..e7cb490 100644 --- a/poetry.lock +++ b/poetry.lock @@ -46,9 +46,9 @@ optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" [package.extras] -redis = ["redis (>=3.3.6,<4.0.0)"] -memcached = ["python-memcached (>=1.59,<2.0)"] msgpack = ["msgpack-python (>=0.5,<0.6)"] +memcached = ["python-memcached (>=1.59,<2.0)"] +redis = ["redis (>=3.3.6,<4.0.0)"] [[package]] name = "certifi" @@ -186,10 +186,10 @@ six = ">=1.9" webencodings = "*" [package.extras] -all = ["genshi", "chardet (>=2.2)", "lxml"] -chardet = ["chardet (>=2.2)"] -genshi = ["genshi"] lxml = ["lxml"] +genshi = ["genshi"] +chardet = ["chardet (>=2.2)"] +all = ["lxml", "chardet (>=2.2)", "genshi"] [[package]] name = "identify" @@ -244,8 +244,8 @@ optional = false python-versions = ">=3.7" [package.extras] -test = ["pytest", "pytest-trio", "pytest-asyncio (>=0.17)", "testpath", "trio", "async-timeout"] -trio = ["trio", "async-generator"] +trio = ["async-generator", "trio"] +test = ["async-timeout", "trio", "testpath", "pytest-asyncio (>=0.17)", "pytest-trio", "pytest"] [[package]] name = "keyring" @@ -373,8 +373,8 @@ python-versions = ">=3.6" importlib-metadata = {version = ">=0.12", markers = "python_version < \"3.8\""} [package.extras] -dev = ["pre-commit", "tox"] -testing = ["pytest", "pytest-benchmark"] +testing = ["pytest-benchmark", "pytest"] +dev = ["tox", "pre-commit"] [[package]] name = "poetry" @@ -693,7 +693,7 @@ testing = ["pytest (>=6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytest- [metadata] lock-version = "1.1" python-versions = "^3.7" -content-hash = "566fbff72a2130ecd300828e65c8ecca8904efd2a602a54a241b9d2281923377" +content-hash = "d2dc8653224ff8c0482b1a295e29ed597d2093af7930b60b2fbdc2dee1616973" [metadata.files] atomicwrites = [ diff --git a/pyproject.toml b/pyproject.toml index ce214ab..4854335 100644 --- 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 index f124eff..6e613fd 100644 --- 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 index 65c9dd0..8f23c3b 100644 --- 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 index 005d4de..a056fae 100644 --- 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"], [