Fix handling of markers for duplicate dependencies

poetry/console/commands/debug/resolve.py

@@ -30,6 +30,7 @@ class DebugResolveCommand(InitCommand):
     def handle(self):
         from poetry.io.null_io import NullIO
         from poetry.core.packages import ProjectPackage
+        from poetry.installation.installer import Installer
         from poetry.puzzle import Solver
         from poetry.repositories.pool import Pool
         from poetry.repositories.repository import Repository
@@ -114,10 +115,15 @@ class DebugResolveCommand(InitCommand):
             pool.add_repository(locked_repository)
 
             with package.with_python_versions(current_python_version):
+                installer = Installer(NullIO(), env, package, self.poetry.locker, pool)
                 solver = Solver(package, pool, Repository(), Repository(), NullIO())
                 ops = solver.solve()
+                installer._filter_operations(ops, Repository())
 
         for op in ops:
+            if self.option("install") and op.skipped:
+                continue
+
             pkg = op.package
             row = [
                 "<c1>{}</c1>".format(pkg.name),

poetry/installation/installer.py

@@ -2,7 +2,6 @@ from typing import List
 from typing import Union
 
 from clikit.api.io import IO
-from clikit.io import NullIO
 
 from poetry.core.packages.package import Package
 from poetry.core.semver import parse_constraint
@@ -197,33 +196,6 @@ class Installer:
             root = root.clone()
             del root.dev_requires[:]
 
-        with root.with_python_versions(
-            ".".join([str(i) for i in self._env.version_info[:3]])
-        ):
-            # We resolve again by only using the lock file
-            pool = Pool(ignore_repository_names=True)
-
-            # Making a new repo containing the packages
-            # newly resolved and the ones from the current lock file
-            repo = Repository()
-            for package in local_repo.packages + locked_repository.packages:
-                if not repo.has_package(package):
-                    repo.add_package(package)
-
-            pool.add_repository(repo)
-
-            # We whitelist all packages to be sure
-            # that the latest ones are picked up
-            whitelist = []
-            for pkg in locked_repository.packages:
-                whitelist.append(pkg.name)
-
-            solver = Solver(
-                root, pool, self._installed_repository, locked_repository, NullIO()
-            )
-
-            ops = solver.solve(use_latest=whitelist)
-
         # We need to filter operations so that packages
         # not compatible with the current system,
         # or optional and not requested, are dropped

poetry/puzzle/exceptions.py

-class CompatibilityError(Exception):
-    def __init__(self, *constraints):
-        self._constraints = list(constraints)
-
-    @property
-    def constraints(self):
-        return self._constraints
-
-
 class SolverProblemError(Exception):
     def __init__(self, error):
         self._error = error
@@ -16,3 +7,12 @@ class SolverProblemError(Exception):
     @property
     def error(self):
         return self._error
+
+
+class OverrideNeeded(Exception):
+    def __init__(self, *overrides):
+        self._overrides = overrides
+
+    @property
+    def overrides(self):
+        return self._overrides

poetry/puzzle/provider.py

@@ -46,7 +46,7 @@ from poetry.utils.inspector import Inspector
 from poetry.utils.setup_reader import SetupReader
 from poetry.utils.toml_file import TomlFile
 
-from .exceptions import CompatibilityError
+from .exceptions import OverrideNeeded
 
 
 logger = logging.getLogger(__name__)
@@ -72,6 +72,7 @@ class Provider:
         self._search_for = {}
         self._is_debugging = self._io.is_debug() or self._io.is_very_verbose()
         self._in_progress = False
+        self._overrides = {}
 
     @property
     def pool(self):  # type: () -> Pool
@@ -88,6 +89,9 @@ class Provider:
     def is_debugging(self):
         return self._is_debugging
 
+    def set_overrides(self, overrides):
+        self._overrides = overrides
+
     def name_for(self, dependency):  # type: (Dependency) -> str
         """
         Returns the name for the given dependency.
@@ -514,13 +518,28 @@ class Provider:
                         )
                     ]
 
-        dependencies = [
+        _dependencies = [
             dep
             for dep in dependencies
             if dep.name not in self.UNSAFE_PACKAGES
             and self._package.python_constraint.allows_any(dep.python_constraint)
         ]
 
+        overrides = self._overrides.get(package, {})
+        dependencies = []
+        overridden = []
+        for dep in _dependencies:
+            if dep.name in overrides:
+                if dep.name in overridden:
+                    continue
+
+                dependencies.append(overrides[dep.name])
+                overridden.append(dep.name)
+
+                continue
+
+            dependencies.append(dep)
+
         return [
             Incompatibility(
                 [Term(package.to_dependency(), True), Term(dep, False)],
@@ -554,12 +573,28 @@ class Provider:
         else:
             requires = package.requires
 
-        dependencies = [
+        _dependencies = [
             r
             for r in requires
             if self._package.python_constraint.allows_any(r.python_constraint)
+            and r.name not in self.UNSAFE_PACKAGES
         ]
 
+        overrides = self._overrides.get(package, {})
+        dependencies = []
+        overridden = []
+        for dep in _dependencies:
+            if dep.name in overrides:
+                if dep.name in overridden:
+                    continue
+
+                dependencies.append(overrides[dep.name])
+                overridden.append(dep.name)
+
+                continue
+
+            dependencies.append(dep)
+
         # Searching for duplicate dependencies
         #
         # If the duplicate dependencies have the same constraint,
@@ -651,26 +686,70 @@ class Provider:
                 continue
 
             # At this point, we raise an exception that will
-            # tell the solver to enter compatibility mode
-            # which means it will resolve for subsets
-            # Python constraints
+            # tell the solver to make new resolutions with specific overrides.
+            #
+            # For instance, if the foo (1.2.3) package has the following dependencies:
+            #   - bar (>=2.0) ; python_version >= "3.6"
+            #   - bar (<2.0) ; python_version < "3.6"
             #
-            # For instance, if our root package requires Python ~2.7 || ^3.6
-            # And we have one dependency that requires Python <3.6
-            # and the other Python >=3.6 than the solver will solve
-            # dependencies for Python >=2.7,<2.8 || >=3.4,<3.6
-            # and Python >=3.6,<4.0
-            python_constraints = []
+            # then the solver will need to make two new resolutions
+            # with the following overrides:
+            #   - {<Package foo (1.2.3): {"bar": <Dependency bar (>=2.0)>}
+            #   - {<Package foo (1.2.3): {"bar": <Dependency bar (<2.0)>}
+            markers = []
             for constraint, _deps in by_constraint.items():
-                python_constraints.append(_deps[0].python_versions)
+                markers.append(_deps[0].marker)
 
-            _deps = [str(_dep[0]) for _dep in by_constraint.values()]
+            _deps = [_dep[0] for _dep in by_constraint.values()]
             self.debug(
                 "<warning>Different requirements found for {}.</warning>".format(
-                    ", ".join(_deps[:-1]) + " and " + _deps[-1]
+                    ", ".join(
+                        "<c1>{}</c1> <fg=default>(<c2>{}</c2>)</> with markers <b>{}</b>".format(
+                            d.name,
+                            d.pretty_constraint,
+                            d.marker if not d.marker.is_any() else "*",
+                        )
+                        for d in _deps[:-1]
+                    )
+                    + " and "
+                    + "<c1>{}</c1> <fg=default>(<c2>{}</c2>)</> with markers <b>{}</b>".format(
+                        _deps[-1].name,
+                        _deps[-1].pretty_constraint,
+                        _deps[-1].marker if not _deps[-1].marker.is_any() else "*",
+                    )
                 )
             )
-            raise CompatibilityError(*python_constraints)
+
+            # We need to check if one of the duplicate dependencies
+            # has no markers. If there is one, we need to change its
+            # environment markers to the inverse of the union of the
+            # other dependencies markers.
+            # For instance, if we have the following dependencies:
+            #   - ipython
+            #   - ipython (1.2.4) ; implementation_name == "pypy"
+            #
+            # the marker for `ipython` will become `implementation_name != "pypy"`.
+            any_markers_dependencies = [d for d in _deps if d.marker.is_any()]
+            other_markers_dependencies = [d for d in _deps if not d.marker.is_any()]
+
+            if any_markers_dependencies:
+                marker = other_markers_dependencies[0].marker
+                for other_dep in other_markers_dependencies[1:]:
+                    marker = marker.union(other_dep.marker)
+
+                for i, d in enumerate(_deps):
+                    if d.marker.is_any():
+                        _deps[i].marker = marker.invert()
+
+            overrides = []
+            for _dep in _deps:
+                current_overrides = self._overrides.copy()
+                package_overrides = current_overrides.get(package, {})
+                package_overrides.update({_dep.name: _dep})
+                current_overrides.update({package: package_overrides})
+                overrides.append(current_overrides)
+
+            raise OverrideNeeded(*overrides)
 
         # Modifying dependencies as needed
         clean_dependencies = []
@@ -724,7 +803,7 @@ class Provider:
                 m2 = re.match(r"(.+?) \((.+?)\)", m.group(1))
                 if m2:
                     name = m2.group(1)
-                    version = " (<b>{}</b>)".format(m2.group(2))
+                    version = " (<c2>{}</c2>)".format(m2.group(2))
                 else:
                     name = m.group(1)
                     version = ""

poetry/puzzle/solver.py

@@ -5,13 +5,12 @@ from typing import Dict
 from typing import List
 
 from poetry.core.packages import Package
-from poetry.core.semver import parse_constraint
 from poetry.core.version.markers import AnyMarker
 from poetry.mixology import resolve_version
 from poetry.mixology.failure import SolveFailure
 from poetry.packages import DependencyPackage
 
-from .exceptions import CompatibilityError
+from .exceptions import OverrideNeeded
 from .exceptions import SolverProblemError
 from .operations import Install
 from .operations import Uninstall
@@ -28,7 +27,7 @@ class Solver:
         self._locked = locked
         self._io = io
         self._provider = Provider(self._package, self._pool, self._io)
-        self._branches = []
+        self._overrides = []
 
     def solve(self, use_latest=None):  # type: (...) -> List[Operation]
         with self._provider.progress():
@@ -36,15 +35,15 @@ class Solver:
             packages, depths = self._solve(use_latest=use_latest)
             end = time.time()
 
-            if len(self._branches) > 1:
+            if len(self._overrides) > 1:
                 self._provider.debug(
-                    "Complete version solving took {:.3f} seconds for {} branches".format(
-                        end - start, len(self._branches[1:])
+                    "Complete version solving took {:.3f} seconds with {} overrides".format(
+                        end - start, len(self._overrides)
                     )
                 )
                 self._provider.debug(
-                    "Resolved for branches: {}".format(
-                        ", ".join("({})".format(b) for b in self._branches[1:])
+                    "Resolved with overrides: {}".format(
+                        ", ".join("({})".format(b) for b in self._overrides)
                     )
                 )
 
@@ -135,42 +134,40 @@ class Solver:
             ),
         )
 
-    def solve_in_compatibility_mode(self, constraints, use_latest=None):
+    def solve_in_compatibility_mode(self, overrides, use_latest=None):
         locked = {}
         for package in self._locked.packages:
             locked[package.name] = DependencyPackage(package.to_dependency(), package)
 
         packages = []
         depths = []
-        for constraint in constraints:
-            constraint = parse_constraint(constraint)
-            intersection = constraint.intersect(self._package.python_constraint)
-
+        for override in overrides:
             self._provider.debug(
                 "<comment>Retrying dependency resolution "
-                "for Python ({}).</comment>".format(intersection)
+                "with the following overrides ({}).</comment>".format(override)
             )
-            with self._package.with_python_versions(str(intersection)):
-                _packages, _depths = self._solve(use_latest=use_latest)
-                for index, package in enumerate(_packages):
-                    if package not in packages:
-                        packages.append(package)
-                        depths.append(_depths[index])
-                        continue
-                    else:
-                        idx = packages.index(package)
-                        pkg = packages[idx]
-                        depths[idx] = max(depths[idx], _depths[index])
-                        pkg.marker = pkg.marker.union(package.marker)
+            self._provider.set_overrides(override)
+            _packages, _depths = self._solve(use_latest=use_latest)
+            for index, package in enumerate(_packages):
+                if package not in packages:
+                    packages.append(package)
+                    depths.append(_depths[index])
+                    continue
+                else:
+                    idx = packages.index(package)
+                    pkg = packages[idx]
+                    depths[idx] = max(depths[idx], _depths[index])
+                    pkg.marker = pkg.marker.union(package.marker)
 
-                        for dep in package.requires:
-                            if dep not in pkg.requires:
-                                pkg.requires.append(dep)
+                    for dep in package.requires:
+                        if dep not in pkg.requires:
+                            pkg.requires.append(dep)
 
         return packages, depths
 
     def _solve(self, use_latest=None):
-        self._branches.append(self._package.python_versions)
+        if self._provider._overrides:
+            self._overrides.append(self._provider._overrides)
 
         locked = {}
         for package in self._locked.packages:
@@ -182,10 +179,8 @@ class Solver:
             )
 
             packages = result.packages
-        except CompatibilityError as e:
-            return self.solve_in_compatibility_mode(
-                e.constraints, use_latest=use_latest
-            )
+        except OverrideNeeded as e:
+            return self.solve_in_compatibility_mode(e.overrides, use_latest=use_latest)
         except SolveFailure as e:
             raise SolverProblemError(e)
 

pyproject.toml

@@ -23,7 +23,8 @@ classifiers = [
 # Requirements
 [tool.poetry.dependencies]
 python = "~2.7 || ^3.5"
-poetry-core = "^1.0.0a5"
+
+poetry-core = "^1.0.0a6"
 cleo = "^0.8.0"
 clikit = "^0.5.1"
 requests = "^2.18"

tests/installation/fixtures/with-multiple-updates.test

@@ -22,14 +22,6 @@ optional = false
 python-versions = ">=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*"
 
 [[package]]
-name = "B"
-version = "1.1.0"
-description = ""
-category = "main"
-optional = false
-python-versions = ">=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*"
-
-[[package]]
 name = "C"
 version = "1.0"
 description = ""

tests/installation/fixtures/with-pypi-repository.test

@@ -67,7 +67,6 @@ python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*"
 [package.dependencies]
 py = ">=1.5.0"
 six = ">=1.10.0"
-setuptools = "*"
 attrs = ">=17.4.0"
 more-itertools = ">=4.0.0"
 pluggy = ">=0.5,<0.7"

tests/puzzle/test_solver.py

@@ -1060,9 +1060,6 @@ def test_solver_triggers_conflict_for_dependency_python_not_fully_compatible_wit
         solver.solve()
 
 
-@pytest.mark.skip(
-    "This is not working at the moment due to limitations in the resolver"
-)
 def test_solver_finds_compatible_package_for_dependency_python_not_fully_compatible_with_package_python(
     solver, repo, package
 ):
@@ -1928,3 +1925,37 @@ def test_solver_properly_propagates_markers(solver, repo, package):
         str(ops[0].package.marker)
         == 'python_version >= "3.6" and implementation_name != "pypy"'
     )
+
+
+def test_solver_should_not_go_into_an_infinite_loop_on_duplicate_dependencies(
+    solver, repo, package
+):
+    package.python_versions = "~2.7 || ^3.5"
+    package.add_dependency("A", "^1.0")
+
+    package_a = get_package("A", "1.0.0")
+    package_a.add_dependency("B")
+    package_a.add_dependency(
+        "B", {"version": "^1.0", "markers": "implementation_name == 'pypy'"}
+    )
+
+    package_b20 = get_package("B", "2.0.0")
+    package_b10 = get_package("B", "1.0.0")
+
+    repo.add_package(package_a)
+    repo.add_package(package_b10)
+    repo.add_package(package_b20)
+
+    ops = solver.solve()
+
+    check_solver_result(
+        ops,
+        [
+            {"job": "install", "package": package_b10},
+            {"job": "install", "package": package_b20},
+            {"job": "install", "package": package_a},
+        ],
+    )
+
+    assert 'implementation_name == "pypy"' == str(ops[0].package.marker)
+    assert 'implementation_name != "pypy"' == str(ops[1].package.marker)