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

Rework backtracking and state management #62

Merged
merged 5 commits into from
Nov 27, 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: 2 additions & 0 deletions news/62.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a state management bug that causes the resolver to enter an infinite loop
in certain backtracking cases.
135 changes: 83 additions & 52 deletions src/resolvelib/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,15 @@ def merged_with(self, provider, requirement, parent):
raise RequirementsConflicted(criterion)
return criterion

def excluded_of(self, candidate):
"""Build a new instance from this, but excluding specified candidate.
def excluded_of(self, candidates):
"""Build a new instance from this, but excluding specified candidates.

Returns the new instance, or None if we still have no valid candidates.
"""
cands = self.candidates.excluding(candidate)
cands = self.candidates.excluding(candidates)
if not cands:
return None
incompats = list(self.incompatibilities)
incompats.append(candidate)
incompats = self.incompatibilities + candidates
return type(self)(cands, list(self.information), incompats)


Expand Down Expand Up @@ -158,15 +157,11 @@ def _push_new_state(self):
This new state will be used to hold resolution results of the next
coming round.
"""
try:
base = self._states[-1]
except IndexError:
state = State(mapping=collections.OrderedDict(), criteria={})
else:
state = State(
mapping=base.mapping.copy(),
criteria=base.criteria.copy(),
)
base = self._states[-1]
state = State(
mapping=base.mapping.copy(),
criteria=base.criteria.copy(),
)
self._states.append(state)

def _merge_into_criterion(self, requirement, parent):
Expand Down Expand Up @@ -239,59 +234,92 @@ def _attempt_to_pin_criterion(self, name, criterion):
return causes

def _backtrack(self):
# Drop the current state, it's known not to work.
del self._states[-1]

# We need at least 2 states here:
# (a) One to backtrack to.
# (b) One to restore state (a) to its state prior to candidate-pinning,
# so we can pin another one instead.
"""Perform backtracking.

When we enter here, the stack is like this::

[ state Z ]
[ state Y ]
[ state X ]
.... earlier states are irrelevant.

1. No pins worked for Z, so it does not have a pin.
2. We want to reset state Y to unpinned, and pin another candidate.
3. State X holds what state Y was before the pin, but does not
have the incompatibility information gathered in state Y.

Each iteration of the loop will:

1. Discard Z.
2. Discard Y but remember its incompatibility information gathered
previously, and the failure we're dealing with right now.
3. Push a new state Y' based on X, and apply the incompatibility
information from Y to Y'.
4a. If this causes Y' to conflict, we need to backtrack again. Make Y'
the new Z and go back to step 2.
4b. If the incompatibilites apply cleanly, end backtracking.
"""
while len(self._states) >= 3:
# Remove the state that triggered backtracking.
del self._states[-1]

# Retrieve the last candidate pin and known incompatibilities.
broken_state = self._states.pop()
name, candidate = broken_state.mapping.popitem()
incompatibilities_from_broken = [
(k, v.incompatibilities)
for k, v in broken_state.criteria.items()
]

while len(self._states) >= 2:
# Retract the last candidate pin.
prev_state = self._states.pop()
try:
name, candidate = prev_state.mapping.popitem()
except KeyError:
continue
self._r.backtracking(candidate)

# Create a new state to work on, with the newly known not-working
# candidate excluded.
# Create a new state from the last known-to-work one, and apply
# the previously gathered incompatibility information.
self._push_new_state()
for k, incompatibilities in incompatibilities_from_broken:
try:
crit = self.state.criteria[k]
except KeyError:
continue
self.state.criteria[k] = crit.excluded_of(incompatibilities)

# Mark the retracted candidate as incompatible.
criterion = self.state.criteria[name].excluded_of(candidate)
if criterion is None:
# This state still does not work. Try the still previous state.
del self._states[-1]
continue
self.state.criteria[name] = criterion
# Mark the newly known incompatibility.
criterion = self.state.criteria[name].excluded_of([candidate])

return True
# It works! Let's work on this new state.
if criterion:
self.state.criteria[name] = criterion
return True

# State does not work after adding the new incompatibility
# information. Try the still previous state.

# No way to backtrack anymore.
return False

def resolve(self, requirements, max_rounds):
if self._states:
raise RuntimeError("already resolved")

self._push_new_state()
self._r.starting()

# Initialize the root state.
self._states = [State(mapping=collections.OrderedDict(), criteria={})]
for r in requirements:
try:
name, crit = self._merge_into_criterion(r, parent=None)
except RequirementsConflicted as e:
raise ResolutionImpossible(e.criterion.information)
self.state.criteria[name] = crit

self._r.starting()
# The root state is saved as a sentinel so the first ever pin can have
# something to backtrack to if it fails. The root state is basically
# pinning the virtual "root" package in the graph.
self._push_new_state()

for round_index in range(max_rounds):
self._r.starting_round(round_index)

self._push_new_state()
curr = self.state

unsatisfied_criterion_items = [
item
for item in self.state.criteria.items()
Expand All @@ -300,8 +328,7 @@ def resolve(self, requirements, max_rounds):

# All criteria are accounted for. Nothing more to pin, we are done!
if not unsatisfied_criterion_items:
del self._states[-1]
self._r.ending(curr)
self._r.ending(self.state)
return self.state

# Choose the most preferred unpinned criterion to try.
Expand All @@ -311,16 +338,20 @@ def resolve(self, requirements, max_rounds):
)
failure_causes = self._attempt_to_pin_criterion(name, criterion)

# Backtrack if pinning fails.
if failure_causes:
result = self._backtrack()
if not result:
causes = [
i for crit in failure_causes for i in crit.information
]
# Backtrack if pinning fails. The backtrack process puts us in
# an unpinned state, so we can work on it in the next round.
success = self._backtrack()

# Dead ends everywhere. Give up.
if not success:
causes = [i for c in failure_causes for i in c.information]
raise ResolutionImpossible(causes)
else:
# Pinning was successful. Push a new state to do another pin.
self._push_new_state()

self._r.ending_round(round_index, curr)
self._r.ending_round(round_index, self.state)

raise ResolutionTooDeep(max_rounds)

Expand Down
18 changes: 12 additions & 6 deletions src/resolvelib/structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ class _FactoryIterableView(object):
def __init__(self, factory):
self._factory = factory

def __repr__(self):
return "{}({})".format(type(self).__name__, list(self._factory()))

def __bool__(self):
try:
next(self._factory())
Expand All @@ -95,11 +98,11 @@ def for_preference(self):
"""Provide an candidate iterable for `get_preference()`"""
return self._factory()

def excluding(self, candidate):
"""Create a new `Candidates` instance excluding `candidate`."""
def excluding(self, candidates):
"""Create a new instance excluding specified candidates."""

def factory():
return (c for c in self._factory() if c != candidate)
return (c for c in self._factory() if c not in candidates)

return type(self)(factory)

Expand All @@ -114,6 +117,9 @@ class _SequenceIterableView(object):
def __init__(self, sequence):
self._sequence = sequence

def __repr__(self):
return "{}({})".format(type(self).__name__, self._sequence)

def __bool__(self):
return bool(self._sequence)

Expand All @@ -129,9 +135,9 @@ def for_preference(self):
"""Provide an candidate iterable for `get_preference()`"""
return self._sequence

def excluding(self, candidate):
"""Create a new instance excluding `candidate`."""
return type(self)([c for c in self._sequence if c != candidate])
def excluding(self, candidates):
"""Create a new instance excluding specified candidates."""
return type(self)([c for c in self._sequence if c not in candidates])


def build_iter_view(matches):
Expand Down
27 changes: 27 additions & 0 deletions tests/functional/python/inputs/case/cheroot.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"#": "See pypa/pip#9011 for more information",
"index": "pypi-2020-11-17-cheroot",
"requested": [
"coverage==5.3",
"pytest-mock>=1.11.0",
"pytest-sugar>=0.9.3",
"pytest-testmon<1.0.0"
],
"resolved": {
"atomicwrites": "1.4.0",
"attrs": "20.3.0",
"coverage": "5.3",
"more-itertools": "8.6.0",
"packaging": "20.4",
"pluggy": "0.13.1",
"py": "1.9.0",
"pyparsing": "2.4.7",
"pytest": "3.10.1",
"pytest-mock": "3.2.0",
"pytest-sugar": "0.9.4",
"pytest-testmon": "0.9.13",
"setuptools": "50.3.2",
"six": "1.15.0",
"termcolor": "1.1.0"
}
}
Loading