Commit f75b1cb7 by Sébastien Eustace

Fix dependency resolution

parent 7e4668ea
...@@ -36,7 +36,7 @@ class SpecificationProvider: ...@@ -36,7 +36,7 @@ class SpecificationProvider:
def dependencies_for(self, specification: Any) -> List[Any]: def dependencies_for(self, specification: Any) -> List[Any]:
""" """
Returns the dependencies of `specification`. Returns the dependencies of specification.
""" """
return [] return []
......
...@@ -26,7 +26,7 @@ class UI: ...@@ -26,7 +26,7 @@ class UI:
def after_resolution(self) -> None: def after_resolution(self) -> None:
self.output.write('') self.output.write('')
def debug(self, depth, message) -> None: def debug(self, message, depth) -> None:
if self.is_debugging(): if self.is_debugging():
debug_info = str(message) debug_info = str(message)
debug_info = '\n'.join([ debug_info = '\n'.join([
......
...@@ -44,7 +44,7 @@ class VersionConflict(ResolverError): ...@@ -44,7 +44,7 @@ class VersionConflict(ResolverError):
for conflicting in flat_map( for conflicting in flat_map(
list(conflicts.values()), lambda x: x.requirements list(conflicts.values()), lambda x: x.requirements
): ):
for source, conflict_requirements in conflicting: for source, conflict_requirements in conflicting.items():
for c in conflict_requirements: for c in conflict_requirements:
pairs.append((c, source)) pairs.append((c, source))
......
...@@ -50,7 +50,10 @@ class AddEdgeNoCircular(Action): ...@@ -50,7 +50,10 @@ class AddEdgeNoCircular(Action):
""" """
:type elements: list :type elements: list
""" """
try:
index = elements.index(element) index = elements.index(element)
except ValueError:
return
if index != -1: if index != -1:
elements.pop(index) elements.pop(index)
from .action import Action from .action import Action
from .vertex import Vertex from .vertex import Vertex
_NULL = object()
class AddVertex(Action): class AddVertex(Action):
...@@ -20,7 +22,7 @@ class AddVertex(Action): ...@@ -20,7 +22,7 @@ class AddVertex(Action):
self._name = name self._name = name
self._payload = payload self._payload = payload
self._root = root self._root = root
self._existing_payload = None self._existing_payload = _NULL
self._existing_root = None self._existing_root = None
@property @property
...@@ -56,7 +58,7 @@ class AddVertex(Action): ...@@ -56,7 +58,7 @@ class AddVertex(Action):
return vertex return vertex
def down(self, graph): def down(self, graph):
if self._existing_payload is not None: if self._existing_payload is not _NULL:
vertex = graph.vertices[self._name] vertex = graph.vertices[self._name]
vertex.payload = self._existing_payload vertex.payload = self._existing_payload
vertex.root = self._existing_root vertex.root = self._existing_root
......
...@@ -50,7 +50,10 @@ class DeleteEdge(Action): ...@@ -50,7 +50,10 @@ class DeleteEdge(Action):
""" """
:type elements: list :type elements: list
""" """
try:
index = elements.index(element) index = elements.index(element)
except ValueError:
return
if index != -1: if index != -1:
elements.pop(index) elements.pop(index)
...@@ -62,7 +62,7 @@ class Vertex: ...@@ -62,7 +62,7 @@ class Vertex:
continue continue
vertices.add(vertex) vertices.add(vertex)
vertex._recursive_predecessors(vertices) vertex._recursive_successors(vertices)
return vertices return vertices
......
...@@ -11,3 +11,6 @@ class PossibilitySet: ...@@ -11,3 +11,6 @@ class PossibilitySet:
def __str__(self): def __str__(self):
return '[{}]'.format(', '.join([repr(p) for p in self.possibilities])) return '[{}]'.format(', '.join([repr(p) for p in self.possibilities]))
def __repr__(self):
return f'<PossibilitySet {str(self)}>'
import logging import logging
from copy import copy from copy import copy
from time import time from datetime import datetime
from typing import Any from typing import Any
from typing import List from typing import List
...@@ -36,8 +36,10 @@ class Resolution: ...@@ -36,8 +36,10 @@ class Resolution:
self._requested = requested self._requested = requested
self._original_requested = copy(requested) self._original_requested = copy(requested)
self._base = base self._base = base
self._states = [ResolutionState.empty()] self._states = []
self._iteration_counter = 0 self._iteration_counter = 0
self._progress_rate = 0.33
self._iteration_rate = None
self._parents_of = {} self._parents_of = {}
self._started_at = None self._started_at = None
...@@ -73,9 +75,14 @@ class Resolution: ...@@ -73,9 +75,14 @@ class Resolution:
if not self.state.requirement and not self.state.requirements: if not self.state.requirement and not self.state.requirements:
break break
# TODO: indicate progress self._indicate_progress()
if hasattr(self.state, 'pop_possibility_state'): if hasattr(self.state, 'pop_possibility_state'):
s = self.state.pop_possiblity_state() self._debug(
f'Creating possibility state for '
f'{str(self.state.requirement)} '
f'({len(self.state.possibilities)} remaining)'
)
s = self.state.pop_possibility_state()
if s: if s:
self._states.append(s) self._states.append(s)
self.activated.tag(s) self.activated.tag(s)
...@@ -90,11 +97,14 @@ class Resolution: ...@@ -90,11 +97,14 @@ class Resolution:
""" """
Set up the resolution process. Set up the resolution process.
""" """
self._started_at = time() self._started_at = datetime.now()
self._handle_missing_or_push_dependency_state(self._initial_state()) self._handle_missing_or_push_dependency_state(self._initial_state())
logger.debug('Starting resolution') self._debug(
f'Starting resolution ({self._started_at})\n'
f'Requested dependencies: {self._original_requested}'
)
self._ui.before_resolution() self._ui.before_resolution()
...@@ -124,12 +134,13 @@ class Resolution: ...@@ -124,12 +134,13 @@ class Resolution:
""" """
Ends the resolution process Ends the resolution process
""" """
elapsed = (datetime.now() - self._started_at).total_seconds()
self._ui.after_resolution() self._ui.after_resolution()
logger.debug( self._debug(
'Finished resolution ({} steps) in {:.3f} seconds'.format( f'Finished resolution ({self._iteration_counter} steps) '
self._iteration_counter, time() - self._started_at f'in {elapsed:.3f} seconds'
)
) )
def _process_topmost_state(self) -> None: def _process_topmost_state(self) -> None:
...@@ -151,6 +162,7 @@ class Resolution: ...@@ -151,6 +162,7 @@ class Resolution:
""" """
The current possibility that the resolution is trying. The current possibility that the resolution is trying.
""" """
if self.state.possibilities:
return self.state.possibilities[-1] return self.state.possibilities[-1]
@property @property
...@@ -213,17 +225,21 @@ class Resolution: ...@@ -213,17 +225,21 @@ class Resolution:
unwind_options = self.state.unused_unwind_options unwind_options = self.state.unused_unwind_options
self._debug( self._debug(
self.state.depth,
'Unwinding for conflict: ' 'Unwinding for conflict: '
'{} to {}'.format( '{} to {}'.format(
self.state.requirement, str(self.state.requirement),
details_for_unwind.state_index // 2 details_for_unwind.state_index // 2
) ),
self.state.depth
) )
conflicts = self.state.conflicts conflicts = self.state.conflicts
sliced_states = self._states[details_for_unwind.state_index + 1:] sliced_states = self._states[details_for_unwind.state_index + 1:]
if details_for_unwind.state_index == -1:
self._states = []
else:
self._states = self._states[:details_for_unwind.state_index] self._states = self._states[:details_for_unwind.state_index]
self._raise_error_unless_state(conflicts) self._raise_error_unless_state(conflicts)
if sliced_states: if sliced_states:
self.activated.rewind_to( self.activated.rewind_to(
...@@ -251,7 +267,7 @@ class Resolution: ...@@ -251,7 +267,7 @@ class Resolution:
return return
errors = [c.underlying_error errors = [c.underlying_error
for c in conflicts for c in conflicts.values()
if c.underlying_error is not None] if c.underlying_error is not None]
if errors: if errors:
error = errors[0] error = errors[0]
...@@ -484,12 +500,11 @@ class Resolution: ...@@ -484,12 +500,11 @@ class Resolution:
return satisfied return satisfied
def _filter_possibilities_for_parent_unwind(self, unwind_details): def _filter_possibilities_for_parent_unwind(self,
unwind_details: UnwindDetails):
""" """
Filter a state's possibilities to remove any that would (eventually) Filter a state's possibilities to remove any that would (eventually)
the requirements in the conflict we've just rewound from. the requirements in the conflict we've just rewound from.
:type unwind_details: UnwindDetails
""" """
unwinds_to_state = [ unwinds_to_state = [
uw uw
...@@ -569,7 +584,10 @@ class Resolution: ...@@ -569,7 +584,10 @@ class Resolution:
# If all the requirements together don't filter out all possibilities, # If all the requirements together don't filter out all possibilities,
# then the only two requirements we need to consider are the initial one # then the only two requirements we need to consider are the initial one
# (where the dependency's version was first chosen) and the last # (where the dependency's version was first chosen) and the last
if self._binding_requirement_in_set(None, possible_binding_requirements, possibilities): if self._binding_requirement_in_set(
None, possible_binding_requirements,
possibilities
):
return list(filter(None, [ return list(filter(None, [
conflict.requirement, conflict.requirement,
self._requirement_for_existing_name( self._requirement_for_existing_name(
...@@ -699,19 +717,29 @@ class Resolution: ...@@ -699,19 +717,29 @@ class Resolution:
return tree return tree
def _debug(self, depth, message): def _indicate_progress(self):
self._ui.debug(depth, message) self._iteration_counter += 1
progress_rate = self._ui.progress_rate or self._progress_rate
if self._iteration_rate is None:
if (datetime.now() - self._started_at).total_seconds() >= progress_rate:
self._iteration_rate = self._iteration_counter
if self._iteration_rate and (self._iteration_counter % self._iteration_rate) == 0:
self._ui.indicate_progress()
def _debug(self, message, depth=0):
self._ui.debug(message, depth)
def _attempt_to_activate(self): def _attempt_to_activate(self):
self._debug( self._debug(
f'Attempting to activate {str(self.possibility)}',
self.state.depth, self.state.depth,
'Attempting to activate {}'.format(self.possibility)
) )
existing_vertex = self.activated.vertex_named(self.state.name) existing_vertex = self.activated.vertex_named(self.state.name)
if existing_vertex.payload: if existing_vertex.payload:
self._debug( self._debug(
self.state.depth, 'Found existing spec ({})'.format(existing_vertex.payload),
'Found existing spec ({})'.format(existing_vertex.payload) self.state.depth
) )
self._attempt_to_filter_existing_spec(existing_vertex) self._attempt_to_filter_existing_spec(existing_vertex)
else: else:
...@@ -747,6 +775,10 @@ class Resolution: ...@@ -747,6 +775,10 @@ class Resolution:
self._push_state_for_requirements(new_requirements, False) self._push_state_for_requirements(new_requirements, False)
else: else:
self._create_conflict() self._create_conflict()
self._debug(
f'Unsatisfied by existing spec ({str(vertex.payload)})',
self.state.depth
)
self._unwind_for_conflict() self._unwind_for_conflict()
def _filtered_possibility_set(self, vertex): def _filtered_possibility_set(self, vertex):
...@@ -770,13 +802,22 @@ class Resolution: ...@@ -770,13 +802,22 @@ class Resolution:
if self.state.name in self.state.conflicts: if self.state.name in self.state.conflicts:
del self.state.conflicts[self.name] del self.state.conflicts[self.name]
self.activated.set_payload(self.name, self.possibility) self._debug(
f'Activated {self.state.name} at {str(self.possibility)}',
self.state.depth
)
self.activated.set_payload(self.state.name, self.possibility)
self._require_nested_dependencies_for(self.possibility) self._require_nested_dependencies_for(self.possibility)
def _require_nested_dependencies_for(self, possibility_set): def _require_nested_dependencies_for(self, possibility_set):
nested_dependencies = self._provider.dependencies_for( nested_dependencies = self._provider.dependencies_for(
possibility_set.latest_version possibility_set.latest_version
) )
self._debug(
f'Requiring nested dependencies '
f'({", ".join([str(d) for d in nested_dependencies])})',
self.state.depth
)
for d in nested_dependencies: for d in nested_dependencies:
self.activated.add_child_vertex( self.activated.add_child_vertex(
......
...@@ -41,6 +41,9 @@ class ResolutionState: ...@@ -41,6 +41,9 @@ class ResolutionState:
def empty(cls): def empty(cls):
return cls(None, [], DependencyGraph(), None, None, 0, {}, []) return cls(None, [], DependencyGraph(), None, None, 0, {}, [])
def __repr__(self):
return f'<{self.__class__.__name__} {self._name}>'
class PossibilityState(ResolutionState): class PossibilityState(ResolutionState):
...@@ -49,16 +52,16 @@ class PossibilityState(ResolutionState): ...@@ -49,16 +52,16 @@ class PossibilityState(ResolutionState):
class DependencyState(ResolutionState): class DependencyState(ResolutionState):
def pop_possiblity_state(self): def pop_possibility_state(self):
state = PossibilityState( state = PossibilityState(
self._name, self._name,
copy(self._requirements), copy(self._requirements),
self._activated, self._activated,
self._requirement, self._requirement,
[self._possibilities.pop()], [self.possibilities.pop() if self.possibilities else None],
self._depth + 1, self._depth + 1,
copy(self._conflicts), copy(self.conflicts),
copy(self._unused_unwind_options) copy(self.unused_unwind_options)
) )
state.activated.tag(state) state.activated.tag(state)
......
from collections import namedtuple
class UnwindDetails: class UnwindDetails:
def __init__(self, def __init__(self,
...@@ -37,8 +40,12 @@ class UnwindDetails: ...@@ -37,8 +40,12 @@ class UnwindDetails:
if self._sub_dependencies_to_avoid is None: if self._sub_dependencies_to_avoid is None:
self._sub_dependencies_to_avoid = [] self._sub_dependencies_to_avoid = []
for tree in self.requirement_trees: for tree in self.requirement_trees:
try:
index = tree.index(self.state_requirement) index = tree.index(self.state_requirement)
if index and tree[index + 1] is not None: except ValueError:
continue
if tree[index + 1] is not None:
self._sub_dependencies_to_avoid.append(tree[index + 1]) self._sub_dependencies_to_avoid.append(tree[index + 1])
return self._sub_dependencies_to_avoid return self._sub_dependencies_to_avoid
...@@ -89,3 +96,6 @@ class UnwindDetails: ...@@ -89,3 +96,6 @@ class UnwindDetails:
return NotImplemented return NotImplemented
return self.state_index >= other.state_index return self.state_index >= other.state_index
def __hash__(self):
return hash((id(self), self.state_index, self.state_requirement))
{
"name": "resolves an empty list of dependencies",
"requested": {
},
"base": [],
"resolved": [
],
"conflicts": []
}
{
"name": "yields conflicts if a child dependency is not resolved",
"index": "unresolvable_child",
"requested": {
"chef_app_error": "*"
},
"base": [],
"resolved": [],
"conflicts": [
"json"
]
}
{
"json": [
{
"name": "json",
"version": "1.8.0",
"dependencies": {
}
}
],
"chef": [
{
"name": "chef",
"version": "10.26",
"dependencies": {
"json": "<= 1.7.7, >= 1.4.4"
}
}
],
"berkshelf": [
{
"name": "berkshelf",
"version": "2.0.7",
"dependencies": {
"json": ">= 1.7.7"
}
}
],
"chef_app_error": [
{
"name": "chef_app_error",
"version": "1.0.0",
"dependencies": {
"berkshelf": "~2.0",
"chef": "~10.26"
}
}
]
}
...@@ -4,7 +4,9 @@ import pytest ...@@ -4,7 +4,9 @@ import pytest
from poetry.mixology import DependencyGraph from poetry.mixology import DependencyGraph
from poetry.mixology import Resolver from poetry.mixology import Resolver
from poetry.mixology.exceptions import CircularDependencyError
from poetry.mixology.exceptions import ResolverError from poetry.mixology.exceptions import ResolverError
from poetry.mixology.exceptions import VersionConflict
from poetry.packages import Dependency from poetry.packages import Dependency
from .index import Index from .index import Index
...@@ -120,6 +122,7 @@ def assert_graph(dg, result): ...@@ -120,6 +122,7 @@ def assert_graph(dg, result):
@pytest.mark.parametrize( @pytest.mark.parametrize(
'fixture', 'fixture',
[ [
'empty',
'simple', 'simple',
'simple_with_base', 'simple_with_base',
'simple_with_dependencies', 'simple_with_dependencies',
...@@ -132,3 +135,27 @@ def test_resolver(fixture): ...@@ -132,3 +135,27 @@ def test_resolver(fixture):
dg = resolver.resolve(c.requested, base=c.base) dg = resolver.resolve(c.requested, base=c.base)
assert_graph(dg, c.result) assert_graph(dg, c.result)
@pytest.mark.parametrize(
'fixture',
[
'circular',
'unresolvable_child'
]
)
def test_resolver_fail(fixture):
c = case(fixture)
resolver = Resolver(c.index, UI())
with pytest.raises(ResolverError) as e:
resolver.resolve(c.requested, base=c.base)
names = []
e = e.value
if isinstance(e, CircularDependencyError):
names = [d.name for d in e.dependencies]
elif isinstance(e, VersionConflict):
names = [n for n in e.conflicts.keys()]
assert sorted(names) == sorted(c.conflicts)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment