Change the dependency installation order

789fcc16 · Sébastien Eustace · 04d5bdd9 · 789fcc16 · 789fcc16 · 789fcc16
Commit 789fcc16 authored Jun 18, 2018 by Sébastien Eustace
Hide whitespace changes
Inline Side-by-side

Showing with 60 additions and 40 deletions

CHANGELOG.md
+4 -0

poetry/puzzle/solver.py
+33 -16

tests/installation/test_installer.py
+2 -4

tests/puzzle/test_solver.py
+21 -20

No files found.
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@
 - Added support for `packages`, `include` and `exclude` properties.
+### Changed
+- Changed the dependency installation order, deepest dependencies are now installed first.
 ### Fixed
 - Fixed handling of duplicate dependencies with different constraints.

--- a/poetry/puzzle/solver.py
+++ b/poetry/puzzle/solver.py
@@ -28,7 +28,7 @@ class Solver:
    def solve(self, use_latest=None):  # type: (...) -> List[Operation]
        with self._provider.progress():
-            packages = self._solve(use_latest=use_latest)
+            packages, depths = self._solve(use_latest=use_latest)
        requested = self._package.all_requires
@@ -70,14 +70,14 @@ class Solver:
                operations.append(op)
-        requested_names = [r.name for r in self._package.all_requires]
        return sorted(
            operations,
            key=lambda o: (
-                1 if o.package.name in requested_names else 0,
+                o.job_type == "uninstall",
+                # Packages to be uninstalled have no depth so we default to 0
+                # since it actually doesn't matter since removals are always on top.
+                -depths[packages.index(o.package)] if o.job_type != "uninstall" else 0,
                o.package.name,
-                o.package.version,
            ),
        )
@@ -87,6 +87,7 @@ class Solver:
            locked[package.name] = package
        packages = []
+        depths = []
        for constraint in constraints:
            constraint = parse_constraint(constraint)
            intersection = constraint.intersect(self._package.python_constraint)
@@ -96,9 +97,11 @@ class Solver:
                "for Python ({}).</comment>".format(intersection)
            )
            with self._package.with_python_versions(str(intersection)):
-                for package in self._solve(use_latest=use_latest):
+                _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
                    current_package = packages[packages.index(package)]
@@ -106,7 +109,7 @@ class Solver:
                        if dep not in current_package.requires:
                            current_package.requires.append(dep)
-        return list(set(packages))
+        return packages, depths
    def _solve(self, use_latest=None):
        locked = {}
@@ -126,13 +129,14 @@ class Solver:
        except SolveFailure as e:
            raise SolverProblemError(e)
-        requested = self._package.all_requires
        graph = self._build_graph(self._package, packages)
+        depths = []
        for package in packages:
-            category, optional, python, platform = self._get_tags_for_package(
+            category, optional, python, platform, depth = self._get_tags_for_package(
                package, graph
            )
+            depths.append(depth)
            package.category = category
            package.optional = optional
@@ -147,7 +151,7 @@ class Solver:
            package.requirements = requirements
-        return packages
+        return packages, depths
    def _build_graph(
        self, package, packages, previous=None, previous_dep=None, dep=None
@@ -188,13 +192,15 @@ class Solver:
        for dependency in package.all_requires:
            if dependency.is_optional():
-                if not package.is_root() and (not dep or not dep.extras):
+                if not package.is_root() and (
+                    not previous_dep or not previous_dep.extras
+                ):
                    continue
                is_activated = False
                for group, extras in package.extras.items():
                    if dep:
-                        extras = dep.extras
+                        extras = previous_dep.extras
                    elif package.is_root():
                        extras = package.extras
                    else:
@@ -213,7 +219,10 @@ class Solver:
                    # we merge the requirements
                    existing = None
                    for child in graph["children"]:
-                        if child["name"] == pkg.name:
+                        if (
+                            child["name"] == pkg.name
+                            and child["category"] == dependency.category
+                        ):
                            existing = child
                            continue
@@ -233,26 +242,32 @@ class Solver:
        return graph
-    def _get_tags_for_package(self, package, graph):
+    def _get_tags_for_package(self, package, graph, depth=0):
        categories = ["dev"]
        optionals = [True]
        python_versions = []
        platforms = []
+        _depths = [0]
        children = graph["children"]
+        found = False
        for child in children:
            if child["name"] == package.name:
                category = child["category"]
                optional = child["optional"]
                python_version = child["python_version"]
                platform = child["platform"]
+                _depths.append(depth)
            else:
                (
                    category,
                    optional,
                    python_version,
                    platform,
-                ) = self._get_tags_for_package(package, child)
+                    _depth,
+                ) = self._get_tags_for_package(package, child, depth=depth + 1)
+                _depths.append(_depth)
            categories.append(category)
            optionals.append(optional)
@@ -300,4 +315,6 @@ class Solver:
                elif current.matches(previous):
                    platform = constraint
-        return category, optional, python_version, platform
+        depth = max(*(_depths + [0]))
+        return category, optional, python_version, platform, depth
--- a/tests/installation/test_installer.py
+++ b/tests/installation/test_installer.py
@@ -846,8 +846,8 @@ def test_run_install_duplicate_dependencies_different_constraints(
    installs = installer.installer.installs
    assert len(installs) == 3
-    assert installs[0] == package_b10
+    assert installs[0] == package_c12
-    assert installs[1] == package_c12
+    assert installs[1] == package_b10
    assert installs[2] == package_a
    updates = installer.installer.updates
@@ -1013,8 +1013,6 @@ def test_run_update_uninstalls_after_removal_transient_dependency(
    installer.update(True)
    installer.run()
-    print(locker.written_data)
    installs = installer.installer.installs
    assert len(installs) == 0
    updates = installer.installer.updates

--- a/tests/puzzle/test_solver.py
+++ b/tests/puzzle/test_solver.py
@@ -194,8 +194,8 @@ def test_install_with_deps_in_order(solver, repo, package):
        ops,
        [
            {"job": "install", "package": package_a},
-            {"job": "install", "package": package_b},
            {"job": "install", "package": package_c},
+            {"job": "install", "package": package_b},
        ],
    )
@@ -342,6 +342,7 @@ def test_solver_fails_if_mismatch_root_python_versions(solver, repo, package):
 def test_solver_solves_optional_and_compatible_packages(solver, repo, package):
    package.python_versions = "^3.4"
+    package.extras["foo"] = [get_dependency("B")]
    package.add_dependency("A", {"version": "*", "python": "~3.5"})
    package.add_dependency("B", {"version": "*", "optional": True})
@@ -528,8 +529,8 @@ def test_solver_sub_dependencies_with_requirements(solver, repo, package):
    check_solver_result(
        ops,
        [
-            {"job": "install", "package": package_c},
            {"job": "install", "package": package_d},
+            {"job": "install", "package": package_c},
            {"job": "install", "package": package_a},
            {"job": "install", "package": package_b},
        ],
@@ -570,25 +571,25 @@ def test_solver_sub_dependencies_with_requirements_complex(solver, repo, package
    check_solver_result(
        ops,
        [
-            {"job": "install", "package": package_d},
            {"job": "install", "package": package_e},
            {"job": "install", "package": package_f},
-            {"job": "install", "package": package_a},
            {"job": "install", "package": package_b},
+            {"job": "install", "package": package_d},
+            {"job": "install", "package": package_a},
            {"job": "install", "package": package_c},
        ],
    )
-    op = ops[0]
+    op = ops[3]  # d
    assert op.package.requirements == {"python": "<4.0"}
-    op = ops[1]
+    op = ops[0]  # e
    assert op.package.requirements == {"platform": "win32", "python": "<5.0"}
-    op = ops[2]
+    op = ops[1]  # f
    assert op.package.requirements == {"python": "<5.0"}
-    op = ops[4]
+    op = ops[4]  # a
    assert op.package.requirements == {"python": "<5.0"}
@@ -641,16 +642,16 @@ def test_solver_with_dependency_in_both_main_and_dev_dependencies(
    check_solver_result(
        ops,
        [
+            {"job": "install", "package": package_d},
            {"job": "install", "package": package_b},
            {"job": "install", "package": package_c},
-            {"job": "install", "package": package_d},
            {"job": "install", "package": package_a},
        ],
    )
-    b = ops[0].package
+    b = ops[1].package
-    c = ops[1].package
+    c = ops[2].package
-    d = ops[2].package
+    d = ops[0].package
    a = ops[3].package
    assert d.category == "dev"
@@ -693,17 +694,17 @@ def test_solver_with_dependency_in_both_main_and_dev_dependencies_with_one_more_
        ops,
        [
            {"job": "install", "package": package_b},
-            {"job": "install", "package": package_c},
            {"job": "install", "package": package_d},
            {"job": "install", "package": package_a},
+            {"job": "install", "package": package_c},
            {"job": "install", "package": package_e},
        ],
    )
    b = ops[0].package
-    c = ops[1].package
+    c = ops[3].package
-    d = ops[2].package
+    d = ops[1].package
-    a = ops[3].package
+    a = ops[2].package
    e = ops[4].package
    assert d.category == "dev"
@@ -872,16 +873,16 @@ def test_solver_duplicate_dependencies_sub_dependencies(solver, repo, package):
    check_solver_result(
        ops,
        [
-            {"job": "install", "package": package_b10},
-            {"job": "install", "package": package_b20},
            {"job": "install", "package": package_c12},
            {"job": "install", "package": package_c15},
+            {"job": "install", "package": package_b10},
+            {"job": "install", "package": package_b20},
            {"job": "install", "package": package_a},
        ],
    )
-    op = ops[0]
+    op = ops[2]
    assert op.package.requirements == {"python": "<3.4"}
-    op = ops[1]
+    op = ops[3]
    assert op.package.requirements == {"python": ">=3.4"}