refactor poetry export

Author: dimbleby

src/poetry/packages/locker.py

@@ -38,6 +38,7 @@
 if TYPE_CHECKING:
     from tomlkit.toml_document import TOMLDocument
 
+    from poetry.core.version.markers import BaseMarker
     from poetry.repositories import Repository
 
 logger = logging.getLogger(__name__)
@@ -204,102 +205,104 @@ def locked_repository(self, with_dev_reqs: bool = False) -> "Repository":
 
     @staticmethod
     def __get_locked_package(
-        _dependency: Dependency, packages_by_name: Dict[str, List[Package]]
+        dependency: Dependency,
+        packages_by_name: Dict[str, List[Package]],
+        decided: Optional[Dict[Package, Dependency]] = None,
     ) -> Optional[Package]:
         """
         Internal helper to identify corresponding locked package using dependency
         version constraints.
         """
-        for _package in packages_by_name.get(_dependency.name, []):
-            if _dependency.constraint.allows(_package.version):
-                return _package
-        return None
+        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)
 
     @classmethod
-    def __walk_dependency_level(
+    def __walk_dependencies(
         cls,
         dependencies: List[Dependency],
-        level: int,
-        pinned_versions: bool,
         packages_by_name: Dict[str, List[Package]],
-        project_level_dependencies: Set[str],
-        nested_dependencies: Dict[Tuple[str, str], Dependency],
-    ) -> Dict[Tuple[str, str], Dependency]:
-        if not dependencies:
-            return nested_dependencies
-
-        next_level_dependencies = []
-
-        for requirement in dependencies:
-            key = (requirement.name, requirement.pretty_constraint)
-            locked_package = cls.__get_locked_package(requirement, packages_by_name)
-
-            if locked_package:
-                # 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
-                pretty_constraint = requirement.pretty_constraint
-                marker = requirement.marker
-                requirement = locked_package.to_dependency()
-                requirement.marker = requirement.marker.intersect(marker)
+    ) -> Dict[Package, Dependency]:
+        nested_dependencies: Dict[Package, Dependency] = {}
 
-                key = (requirement.name, pretty_constraint)
-
-                if not pinned_versions:
-                    requirement.set_constraint(constraint)
+        visited: Set[Tuple[Dependency, "BaseMarker"]] = set()
+        while dependencies:
+            requirement = dependencies.pop(0)
+            if (requirement, requirement.marker) in visited:
+                continue
+            visited.add((requirement, requirement.marker))
 
-                for require in locked_package.requires:
-                    if require.marker.is_empty():
-                        require.marker = requirement.marker
-                    else:
-                        require.marker = require.marker.intersect(requirement.marker)
+            locked_package = cls.__get_locked_package(
+                requirement, packages_by_name, nested_dependencies
+            )
 
-                    require.marker = require.marker.intersect(locked_package.marker)
+            if not locked_package:
+                # Should normally be able to satisfy all requirements, but this case is
+                # permissible eg if we encounter a dev dependency when walking the
+                # non-dev dependencies.
+                continue
 
-                    if key not in nested_dependencies:
-                        next_level_dependencies.append(require)
+            # 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)
 
-            if requirement.name in project_level_dependencies and level == 0:
-                # project level dependencies take precedence
-                continue
+            requirement.set_constraint(constraint)
 
-            if not locked_package:
-                # we make a copy to avoid any side-effects
-                requirement = deepcopy(requirement)
+            for require in locked_package.requires:
+                require = deepcopy(require)
+                require.marker = require.marker.intersect(requirement.marker)
+                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 cls.__walk_dependency_level(
-            dependencies=next_level_dependencies,
-            level=level + 1,
-            pinned_versions=pinned_versions,
-            packages_by_name=packages_by_name,
-            project_level_dependencies=project_level_dependencies,
-            nested_dependencies=nested_dependencies,
-        )
+        return nested_dependencies
 
     @classmethod
     def get_project_dependencies(
         cls,
         project_requires: List[Dependency],
         locked_packages: List[Package],
-        pinned_versions: bool = False,
-        with_nested: bool = False,
-    ) -> Iterable[Dependency]:
+    ) -> Iterable[Tuple[Package, Dependency]]:
         # group packages entries by name, this is required because requirement might use
-        # different constraints
+        # different constraints.
         packages_by_name = {}
         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)
 
-        project_level_dependencies = set()
+        # 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)
+
         dependencies = []
 
         for dependency in project_requires:
@@ -311,38 +314,18 @@ def get_project_dependencies(
                     locked_package.marker
                 )
 
-                if not pinned_versions:
-                    locked_dependency.set_constraint(dependency.constraint)
+                locked_dependency.set_constraint(dependency.constraint)
 
                 dependency = locked_dependency
 
-            project_level_dependencies.add(dependency.name)
             dependencies.append(dependency)
 
-        if not with_nested:
-            # return only with project level dependencies
-            return dependencies
-
-        nested_dependencies = cls.__walk_dependency_level(
+        nested_dependencies = cls.__walk_dependencies(
             dependencies=dependencies,
-            level=0,
-            pinned_versions=pinned_versions,
             packages_by_name=packages_by_name,
-            project_level_dependencies=project_level_dependencies,
-            nested_dependencies={},
         )
 
-        # Merge same dependencies using marker union
-        for requirement in dependencies:
-            key = (requirement.name, requirement.pretty_constraint)
-            if key not in nested_dependencies:
-                nested_dependencies[key] = requirement
-            else:
-                nested_dependencies[key].marker = nested_dependencies[key].marker.union(
-                    requirement.marker
-                )
-
-        return sorted(nested_dependencies.values(), key=lambda x: x.name.lower())
+        return nested_dependencies.items()
 
     def get_project_dependency_packages(
         self,
@@ -382,16 +365,10 @@ def get_project_dependency_packages(
 
             selected.append(dependency)
 
-        for dependency in self.get_project_dependencies(
+        for package, dependency in self.get_project_dependencies(
             project_requires=selected,
             locked_packages=repository.packages,
-            with_nested=True,
         ):
-            try:
-                package = repository.find_packages(dependency=dependency)[0]
-            except IndexError:
-                continue
-
             for extra in dependency.extras:
                 package.requires_extras.append(extra)
 

src/poetry/utils/exporter.py

@@ -1,6 +1,6 @@
-import itertools
 import urllib.parse
 
+from copy import deepcopy
 from typing import TYPE_CHECKING
 from typing import Optional
 from typing import Sequence
@@ -70,21 +70,25 @@ def _export_requirements_txt(
         content = ""
         dependency_lines = set()
 
-        for package, groups in itertools.groupby(
-            self._poetry.locker.get_project_dependency_packages(
-                project_requires=self._poetry.package.all_requires,
-                dev=dev,
-                extras=extras,
-            ),
-            lambda dependency_package: dependency_package.package,
+        # Get project dependencies, and add the project-wide marker to them.
+        groups = ["dev"] if dev else []
+        root_package = self._poetry.package.with_dependency_groups(groups)
+        project_requires = []
+        for require in root_package.all_requires:
+            require = deepcopy(require)
+            require.marker = require.marker.intersect(
+                root_package.python_marker
+            )
+            project_requires.append(require)
+
+        for dependency_package in self._poetry.locker.get_project_dependency_packages(
+            project_requires=project_requires,
+            dev=dev,
+            extras=extras,
         ):
             line = ""
-            dependency_packages = list(groups)
-            dependency = dependency_packages[0].dependency
-            marker = dependency.marker
-            for dep_package in dependency_packages[1:]:
-                marker = marker.union(dep_package.dependency.marker)
-            dependency.marker = marker
+            dependency = dependency_package.dependency
+            package = dependency_package.package
 
             if package.develop:
                 line += "-e "

tests/console/commands/test_export.py

@@ -82,7 +82,9 @@ def _export_requirements(tester: "CommandTester", poetry: "Poetry") -> None:
     assert poetry.locker.lock.exists()
 
     expected = """\
-foo==1.0.0
+foo==1.0.0 ;\
+ python_version >= "2.7" and python_version < "2.8" or\
+ python_version >= "3.4" and python_version < "4.0"
 """
 
     assert content == expected
@@ -111,7 +113,9 @@ def test_export_fails_on_invalid_format(tester: "CommandTester", do_lock: None):
 def test_export_prints_to_stdout_by_default(tester: "CommandTester", do_lock: None):
     tester.execute("--format requirements.txt")
     expected = """\
-foo==1.0.0
+foo==1.0.0 ;\
+ python_version >= "2.7" and python_version < "2.8" or\
+ python_version >= "3.4" and python_version < "4.0"
 """
     assert tester.io.fetch_output() == expected
 
@@ -121,16 +125,22 @@ def test_export_uses_requirements_txt_format_by_default(
 ):
     tester.execute()
     expected = """\
-foo==1.0.0
+foo==1.0.0 ;\
+ python_version >= "2.7" and python_version < "2.8" or\
+ python_version >= "3.4" and python_version < "4.0"
 """
     assert tester.io.fetch_output() == expected
 
 
 def test_export_includes_extras_by_flag(tester: "CommandTester", do_lock: None):
     tester.execute("--format requirements.txt --extras feature_bar")
     expected = """\
-bar==1.1.0
-foo==1.0.0
+bar==1.1.0 ;\
+ python_version >= "2.7" and python_version < "2.8" or\
+ python_version >= "3.4" and python_version < "4.0"
+foo==1.0.0 ;\
+ python_version >= "2.7" and python_version < "2.8" or\
+ python_version >= "3.4" and python_version < "4.0"
 """
     assert tester.io.fetch_output() == expected