Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve dependency resolution and markers handling #2361

Merged
merged 1 commit into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@ jobs:

- name: Run pytest
shell: bash
run: poetry run pytest -q tests
run: poetry run pytest -v tests
10 changes: 3 additions & 7 deletions poetry/console/commands/debug/resolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ class DebugResolveCommand(InitCommand):
loggers = ["poetry.repositories.pypi_repository"]

def handle(self):
from poetry.core.packages.project_package import ProjectPackage
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
Expand Down Expand Up @@ -106,19 +105,16 @@ def handle(self):

if self.option("install"):
env = EnvManager(self.poetry).get()
current_python_version = ".".join(str(v) for v in env.version_info)
pool = Pool()
locked_repository = Repository()
for op in ops:
locked_repository.add_package(op.package)

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())
solver = Solver(package, pool, Repository(), Repository(), NullIO())
with solver.use_environment(env):
ops = solver.solve()
installer._filter_operations(ops, Repository())

for op in ops:
if self.option("install") and op.skipped:
Expand Down
46 changes: 34 additions & 12 deletions poetry/installation/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

from clikit.api.io import IO

from poetry.core.packages.package import Package
from poetry.core.semver import parse_constraint
from poetry.core.packages.project_package import ProjectPackage
from poetry.io.null_io import NullIO
from poetry.packages import Locker
from poetry.puzzle import Solver
from poetry.puzzle.operations import Install
Expand All @@ -26,7 +26,7 @@ def __init__(
self,
io, # type: IO
env,
package, # type: Package
package, # type: ProjectPackage
locker, # type: Locker
pool, # type: Pool
installed=None, # type: (Union[InstalledRepository, None])
Expand Down Expand Up @@ -196,6 +196,37 @@ def _do_install(self, local_repo):
root = root.clone()
del root.dev_requires[:]

if self._io.is_verbose():
self._io.write_line("")
self._io.write_line(
"<info>Finding the necessary packages for the current system</>"
)

# 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()
)

with solver.use_environment(self._env):
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
Expand Down Expand Up @@ -411,15 +442,6 @@ def _filter_operations(
if op.job_type == "uninstall":
continue

current_python = parse_constraint(
".".join(str(v) for v in self._env.version_info[:3])
)
if not package.python_constraint.allows(
current_python
) or not self._env.is_valid_for_marker(package.marker):
op.skip("Not needed for the current environment")
continue

if self._update:
extras = {}
for extra, deps in self._package.extras.items():
Expand Down
7 changes: 7 additions & 0 deletions poetry/mixology/version_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,13 @@ def _get_locked(self, dependency): # type: (Dependency) -> Union[Package, None]
if dependency.extras:
locked.requires_extras = dependency.extras

if not dependency.transitive_marker.without_extras().is_any():
marker_intersection = dependency.transitive_marker.without_extras().intersect(
locked.dependency.marker.without_extras()
)
if not marker_intersection.is_empty():
locked.dependency.transitive_marker = marker_intersection

return locked

def _log(self, text):
Expand Down
9 changes: 4 additions & 5 deletions poetry/packages/locker.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,16 +244,17 @@ def _dump_package(self, package): # type: (Package) -> dict
if dependency.pretty_name not in dependencies:
dependencies[dependency.pretty_name] = []

constraint = {"version": str(dependency.pretty_constraint)}
constraint = inline_table()
constraint["version"] = str(dependency.pretty_constraint)

if dependency.extras:
constraint["extras"] = sorted(dependency.extras)

if dependency.is_optional():
constraint["optional"] = True

if not dependency.python_constraint.is_any():
constraint["python"] = str(dependency.python_constraint)
if not dependency.marker.is_any():
constraint["markers"] = str(dependency.marker)

dependencies[dependency.pretty_name].append(constraint)

Expand All @@ -274,8 +275,6 @@ def _dump_package(self, package): # type: (Package) -> dict
"python-versions": package.python_versions,
"files": sorted(package.files, key=lambda x: x["file"]),
}
if not package.marker.is_any():
data["marker"] = str(package.marker)

if package.extras:
extras = {}
Expand Down
30 changes: 16 additions & 14 deletions poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from poetry.utils._compat import OrderedDict
from poetry.utils._compat import Path
from poetry.utils._compat import urlparse
from poetry.utils.env import Env
from poetry.utils.helpers import download_file
from poetry.utils.helpers import safe_rmtree
from poetry.utils.helpers import temporary_directory
Expand All @@ -52,10 +53,13 @@ class Provider:

UNSAFE_PACKAGES = {"setuptools", "distribute", "pip"}

def __init__(self, package, pool, io): # type: (Package, Pool, Any) -> None
def __init__(
self, package, pool, io, env=None
): # type: (Package, Pool, Any, Optional[Env]) -> None
self._package = package
self._pool = pool
self._io = io
self._env = env
self._python_constraint = package.python_constraint
self._search_for = {}
self._is_debugging = self._io.is_debug() or self._io.is_very_verbose()
Expand All @@ -66,25 +70,21 @@ def __init__(self, package, pool, io): # type: (Package, Pool, Any) -> None
def pool(self): # type: () -> Pool
return self._pool

@property
def name_for_explicit_dependency_source(self): # type: () -> str
return "pyproject.toml"

@property
def name_for_locking_dependency_source(self): # type: () -> str
return "poetry.lock"

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.
"""
return dependency.name
@contextmanager
def use_environment(self, env): # type: (Env) -> Provider
original_env = self._env

self._env = env

yield self

self._env = original_env

def search_for(self, dependency): # type: (Dependency) -> List[Package]
"""
Expand Down Expand Up @@ -371,6 +371,7 @@ def incompatibilities_for(
for dep in dependencies
if dep.name not in self.UNSAFE_PACKAGES
and self._package.python_constraint.allows_any(dep.python_constraint)
and (not self._env or dep.marker.validate(self._env.marker_env))
]

overrides = self._overrides.get(package, {})
Expand Down Expand Up @@ -426,6 +427,7 @@ def complete_package(
for r in requires
if self._package.python_constraint.allows_any(r.python_constraint)
and r.name not in self.UNSAFE_PACKAGES
and (not self._env or r.marker.validate(self._env.marker_env))
]

overrides = self._overrides.get(package, {})
Expand Down
51 changes: 14 additions & 37 deletions poetry/puzzle/solver.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import time

from contextlib import contextmanager
from typing import Any
from typing import Dict
from typing import List

from poetry.core.packages import Package
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 poetry.utils.env import Env

from .exceptions import OverrideNeeded
from .exceptions import SolverProblemError
Expand All @@ -29,6 +30,15 @@ def __init__(self, package, pool, installed, locked, io):
self._provider = Provider(self._package, self._pool, self._io)
self._overrides = []

@property
def provider(self): # type: () -> Provider
return self._provider

@contextmanager
def use_environment(self, env): # type: (Env) -> None
with self.provider.use_environment(env):
yield

def solve(self, use_latest=None): # type: (...) -> List[Operation]
with self._provider.progress():
start = time.time()
Expand Down Expand Up @@ -157,7 +167,6 @@ def solve_in_compatibility_mode(self, overrides, use_latest=None):
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:
Expand Down Expand Up @@ -189,18 +198,10 @@ def _solve(self, use_latest=None):
depths = []
final_packages = []
for package in packages:
category, optional, marker, depth = self._get_tags_for_package(
package, graph
)

if marker is None:
marker = AnyMarker()
if marker.is_empty():
continue
category, optional, depth = self._get_tags_for_package(package, graph)

package.category = category
package.optional = optional
package.marker = marker

depths.append(depth)
final_packages.append(package)
Expand All @@ -213,25 +214,15 @@ def _build_graph(
if not previous:
category = "dev"
optional = True
marker = package.marker
else:
category = dep.category
optional = dep.is_optional() and not dep.is_activated()
intersection = (
previous["marker"]
.without_extras()
.intersect(previous_dep.transitive_marker.without_extras())
)
intersection = intersection.intersect(package.marker.without_extras())

marker = intersection

childrens = [] # type: List[Dict[str, Any]]
graph = {
"name": package.name,
"category": category,
"optional": optional,
"marker": marker,
"children": childrens,
}

Expand Down Expand Up @@ -290,9 +281,6 @@ def _build_graph(
child_graph["optional"] = True

if existing:
existing["marker"] = existing["marker"].union(
child_graph["marker"]
)
continue

childrens.append(child_graph)
Expand All @@ -302,27 +290,23 @@ def _build_graph(
def _get_tags_for_package(self, package, graph, depth=0):
categories = ["dev"]
optionals = [True]
markers = []
_depths = [0]

children = graph["children"]
for child in children:
if child["name"] == package.name:
category = child["category"]
optional = child["optional"]
marker = child["marker"]
_depths.append(depth)
else:
(category, optional, marker, _depth) = self._get_tags_for_package(
(category, optional, _depth) = self._get_tags_for_package(
package, child, depth=depth + 1
)

_depths.append(_depth)

categories.append(category)
optionals.append(optional)
if marker is not None:
markers.append(marker)

if "main" in categories:
category = "main"
Expand All @@ -333,11 +317,4 @@ def _get_tags_for_package(self, package, graph, depth=0):

depth = max(*(_depths + [0]))

if not markers:
marker = None
else:
marker = markers[0]
for m in markers[1:]:
marker = marker.union(m)

return category, optional, marker, depth
return category, optional, depth
Loading