Resolvelib 0.9.0 passes identifiers to Provider's `get_preference` method without accompanying PreferenceInformation

notatallshaw commented 1 year ago

Prerequisites

Linux-like environment
CPython 3.9 (probably works with other versions)
git
svn (because I still don't understand git sparse checkout)

Steps to reproduce

python3.9 -m venv .venv
source .venv/bin/activate
python -m pip install pip --upgrade
git clone https://github.com/pypa/pip
cd pip/src/pip/_vendor/
svn export https://github.com/sarugaku/resolvelib/tags/0.9.0/src/resolvelib --force
cd ../../../
pip install -e .
pip download flake8 hacking -d downloads

Output

ERROR: Exception:
Traceback (most recent call last):
  File ".../pip/src/pip/_internal/cli/base_command.py", line 160, in exc_logging_wrapper
    status = run_func(*args)
  File ".../pip/src/pip/_internal/cli/req_command.py", line 247, in wrapper
    return func(self, options, args)
  File ".../pip/src/pip/_internal/commands/download.py", line 138, in run
    requirement_set = resolver.resolve(reqs, check_supported_wheels=True)
  File ".../pip/src/pip/_internal/resolution/resolvelib/resolver.py", line 92, in resolve
    result = self._result = resolver.resolve(
  File ".../pip/src/pip/_vendor/resolvelib/resolvers.py", line 521, in resolve
    state = resolution.resolve(requirements, max_rounds=max_rounds)
  File ".../pip/src/pip/_vendor/resolvelib/resolvers.py", line 401, in resolve
    name = min(unsatisfied_names, key=self._get_preference)
  File ".../pip/src/pip/_vendor/resolvelib/resolvers.py", line 202, in _get_preference
    return self._p.get_preference(
  File ".../pip/src/pip/_internal/resolution/resolvelib/provider.py", line 134, in get_preference
    candidate, ireqs = zip(*lookups)
ValueError: not enough values to unpack (expected 2, got 0)

Notes

Seems to be a regression from https://github.com/sarugaku/resolvelib/pull/111

Interestingly if I patch get_preference to either prefer or not prefer these identifiers the download still completes, so not sure exactly what it happening.

pradyunsg commented 1 year ago

I have not looked at the behaviour changes closely, but it does seem intentional.

The information argument passed to AbstractProvider.get_preference may now contain empty iterators.

notatallshaw commented 1 year ago

I have not looked at the behaviour changes closely, but it does seem intentional.

The information argument passed to AbstractProvider.get_preference may now contain empty iterators.

Interesting, I guess the question is is it expected that these should be more preferred or less preferred?

It's a little frustrating that this hasn't been adapted by Pip before landing #113 as I can't cleanly test how #111 vs #113 affected real world requirements. I guess I will just retest everything once this is vendored in to Pip.

notatallshaw commented 1 year ago

I see you've landed 0.9.0 on to Pip, I'll run some tests with the updated get_preference.

pradyunsg commented 1 year ago

@notatallshaw Do you have some notes/listing of the test cases that you're trying with the resolver? If so, would you be willing to share this list?

notatallshaw commented 1 year ago

@notatallshaw Do you have some notes/listing of the test cases that you're trying with the resolver? If so, would you be willing to share this list?

Unfortunately I've never formalized it, I just ad-hoc try issues listed in the Pip issue tracker, especially ones that are still open. My testing process once I have a requirements file that is expected to be problematic is:

Create and activate a virtual environment
Install the latest public version of Pip
Convert the pip install in to a pip download so I don't pollute the virtual environment
Confirm that the pip download either takes a long time or exhibits some other issue listed
Install a dev version of Pip
Rerun test and see if the behavior has changed

If I had more time to work on this there's a lot of things I would do to improve automation and reliability of testing. The most important thing would be attempting to write upper bound constraints files so new releases wouldn't break examples of long backtracking.

For pip + resolvelib 0.9 (as is on pip main right now) I took the test cases I mentioned in this thread https://github.com/sarugaku/resolvelib/pull/113 and found that on Python 3.9 on Linux the following requirements both improved quite significantly:

darglint
flake8
flake8-bandit
flake8-black
flake8-bugbear
flake8-builtins
flake8-comprehensions
flake8-docstrings
flake8-eradicate
flake8-isort
flake8-pytest-style

And:

flake8
hacking

This one however still takes a long time backtracking, but I believe that #113 will find a resolution in a relatively short amount of time:

pywebview[qt]==3.6.2
rpaframework==15.6.0

Another problem I haven't attempted to deal with is resolvelib/pip backtracking is non-deterministic. While I've never found a situation where this directly affects whether backtracking resolves in a "short" or "long" amount of time it would certainly make automating this testing even trickier.

sarugaku / resolvelib