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.

Sign up for GitHub

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

Jump to bottom

Resolvelib: process Requires-Python before other dependencies #13160

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Resolvelib: process Requires-Python before other dependencies #13160

wants to merge 3 commits into from

Conversation

ichard26
Copy link
Member

Revived version of #11398.

Closes #11398.
Fixes #13146.
Fixes #11142.

@ichard26 ichard26 added C: dependency resolution About choosing which dependencies to install type: performance Commands take too long to run labels Jan 12, 2025
@ichard26 ichard26 requested a review from uranusjr January 12, 2025 18:14
@ichard26 ichard26 force-pushed the bug/requires-python branch from 8362096 to b3b468f Compare January 12, 2025 18:14
@ichard26 ichard26 marked this pull request as draft January 12, 2025 18:24
@ichard26
Copy link
Member Author

Ah, other tests are failing. That's why this was a draft earlier. Will take a look sometime later.

@pradyunsg
Copy link
Member

Was this rebased on main?

@ichard26
Copy link
Member Author

image

A little out of date, but new enough where the test suite is up to date.

ichard26 and others added 3 commits January 29, 2025 10:46
@ichard26 @uranusjr
Fix silently broken requires-python test
0538477 
Co-authored-by: Tzu-ping Chung <[email protected]>
@uranusjr @ichard26
Emit Requires-Python dependency first
e21141a 
This makes the resolver always inspect Requires-Python first when
checking a candidate's consistency, ensuring that no other candidates
are prepared if the Requires-Python check fails.

Co-authored-by: Richard Si <[email protected]>
@ichard26
Return dependencies lazily in resolvelib provider
7b6e3b6 
While ideally we wouldn't prepare any candidates when not necessary, pip
has grown a lot of metadata checks (for reporting bad metadata, skipping
candidates with unsupported legacy metadata, etc.) so it's not really
feasible to stop preparing the candidate upon creation. However, we can
create the candidates one-by-one as they're processed instead of all
dependencies at once.

This is necessary so the resolver can process requires-python first
without processing other dependencies.

Are there potential side-effects...? Probably. A test suite run didn't
flag anything though.
@ichard26 ichard26 force-pushed the bug/requires-python branch from b3b468f to 7b6e3b6 Compare January 29, 2025 15:47
@ichard26
Copy link
Member Author

ichard26 commented Jan 29, 2025 •
edited
Loading

So ce6d8a4 made candidate preparation as lazy as possible, only preparing the candidate once its dist attribute was accessed. This is incompatible with the various metadata checks pip has grown, but we do need to avoid preparing other dependencies until at least the requires-python requirement has been fully processed.

It turns out the resolvelib provider API lets us return dependencies lazily using a generator, so I did that. While candidates are still prepared upon creation, the difference is that the candidates are prepared in sequential order, instead of all of the dependency candidates being prepared at all once before being processed.

This probably has some side-effects (although a local test suite run did pass) but I'm not very familiar with resolvelib or preparation, so I need some input from someone else who knows this logic well. I'd appreciate your thoughts @uranusjr !

@ichard26 ichard26 marked this pull request as ready for review January 29, 2025 15:59
@notatallshaw
Copy link
Member

I will review this next week, and run a wide range of scenarios locally.

@ichard26 ichard26 changed the title Make new resolver process Requires-Python before other dependencies Resolvelib: process Requires-Python before other dependencies Jan 29, 2025
@notatallshaw notatallshaw self-requested a review January 30, 2025 13:07
@ichard26 ichard26 added this to the 25.1 milestone Feb 4, 2025
@notatallshaw
Copy link
Member

notatallshaw commented Feb 8, 2025 •
edited
Loading

This looks good to me, but I will note a minor performance issue it creates, which we should fix after it lands and before a release.

Specifically it forces the processing of RequiresPython in the resolution step, where seemingly it was being skipped over before. I've not walked through the full code path to understand why it has this effect exactly, but you can observe on Python 3.10 with this command (I suggest running it once before to populate that cache):

PIP_RESOLVER_DEBUG=1 python -m pip install --dry-run --ignore-installed --progress-bar off --disable-pip-version-check "numpy>=2.1,<2.2" "numba<=0.60,>0.1"

The diff between main and this branch:

13c13
< Reporter.pinning(<pip._internal.resolution.resolvelib.candidates.RequiresPythonCandidate object at 0x7f8acdaa8d90>)
---
> Reporter.pinning(<pip._internal.resolution.resolvelib.candidates.RequiresPythonCandidate object at 0x7f31ec46ca60>)
15a16
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.9'), LinkCandidate('https://files.pythonhosted.org/packages/79/58/cb4ac5b8f7ec64200460aef1fed88258fb872ceef504ab1f989d2ff0f684/numba-0.60.0-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.9)'))
22a24
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.9'), LinkCandidate('https://files.pythonhosted.org/packages/f6/2d/f8cdcf325c8fbdfff911607d184e28eb7c94ca5c4760d7f149323404778a/numba-0.59.1-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.9)'))
29a32
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.9'), LinkCandidate('https://files.pythonhosted.org/packages/73/d5/d359cece32302442c8ea9742b1324c4eda689fd54281eb3144f520c81f6d/numba-0.59.0-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.9)'))
33a37
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.8'), LinkCandidate('https://files.pythonhosted.org/packages/ed/13/b66627125b35f2987bd9872cf028b5e1e1ffcbc8d1e182ac4e84eed3998f/numba-0.58.1-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.8)'))
40a45
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.8'), LinkCandidate('https://files.pythonhosted.org/packages/e7/69/d228b38ffb70858d74538bdfe5aa18c7640b7f07840239690985b3a94009/numba-0.58.0-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.8)'))
44a50
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.8'), LinkCandidate('https://files.pythonhosted.org/packages/aa/9d/e93ddc139fcd5b7201bcbdd1ac9c76534aac043fbbdb86ab4bd2e7aebae5/numba-0.57.1-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.8)'))
51a58
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.8'), LinkCandidate('https://files.pythonhosted.org/packages/32/ea/e839410833fe25207a1d2a0e88fa39eccfd8f61fdc4b790a58ca34189b38/numba-0.57.0-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.8)'))
55a63
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.7'), LinkCandidate('https://files.pythonhosted.org/packages/58/a4/859605be01d9979fecde5e94ed6662d9a85853f9849f396d9a84455f4846/numba-0.56.4-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.7)'))
62a71
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.7'), LinkCandidate('https://files.pythonhosted.org/packages/6b/42/c0eda9d494d33f0856b3b2184fd8369203ea583a1c33a96482f1be0353d5/numba-0.56.3-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.7)'))
66a76
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.7'), LinkCandidate('https://files.pythonhosted.org/packages/27/d5/96074b4885826b802b4a2052e81230cebc54efeae72efcbc850b059dbb06/numba-0.56.2-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.7)'))
70a81
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.7'), LinkCandidate('https://files.pythonhosted.org/packages/d8/0d/70d77ace7ced25f94c6ab3a1c4fac3bcdfb780b66f6446c7d503c83ea861/numba-0.56.0-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.7)'))
74a86
> Reporter.adding_requirement(RequiresPythonRequirement('<3.11,>=3.7'), LinkCandidate('https://files.pythonhosted.org/packages/ce/16/d897665f7b1968b795abc7bc15e084d9f89b3609381dd83d8e59d4be7e37/numba-0.55.2-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.7,<3.11)'))
81a94
> Reporter.adding_requirement(RequiresPythonRequirement('<3.11,>=3.7'), LinkCandidate('https://files.pythonhosted.org/packages/54/81/765a642e80efa4040207c5c3979ef736c21f2444d31a9dff06b8bd97fe8d/numba-0.55.1-1-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.7,<3.11)'))
85a99
> Reporter.adding_requirement(RequiresPythonRequirement('<3.11,>=3.7'), LinkCandidate('https://files.pythonhosted.org/packages/52/af/74c349582ebd37bafef5870b909a1f32968d06fe1378f48980b8cb782228/numba-0.55.0-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.whl (from https://pypi.org/simple/numba/) (requires-python:>=3.7,<3.11)'))
97a112
> Reporter.adding_requirement(RequiresPythonRequirement('>=3.6'), LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)'))
111d125
< Reporter.adding_requirement(RequiresPythonRequirement('>=3.6'), LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)'))
123c137
< Reporter.ending(State(mapping=OrderedDict([('numpy', LinkCandidate('https://files.pythonhosted.org/packages/05/db/5d9c91b2e1e2e72be1369278f696356d44975befcae830daf2e667dcb54f/numpy-2.1.3-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (from https://pypi.org/simple/numpy/) (requires-python:>=3.10)')), ('<Python from Requires-Python>', <pip._internal.resolution.resolvelib.candidates.RequiresPythonCandidate object at 0x7f8acdaa8d90>), ('numba', LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)')), ('llvmlite', LinkCandidate('https://files.pythonhosted.org/packages/0b/96/07bfa93a103fb9e3e9ae7f9f7c6687ae714aee66b6f3000da3fad71e0aa2/llvmlite-0.34.0.tar.gz (from https://pypi.org/simple/llvmlite/) (requires-python:>=3.6)')), ('setuptools', LinkCandidate('https://files.pythonhosted.org/packages/69/8a/b9dc7678803429e4a3bc9ba462fa3dd9066824d3c607490235c6a796be5a/setuptools-75.8.0-py3-none-any.whl (from https://pypi.org/simple/setuptools/) (requires-python:>=3.9)'))]), criteria={'numpy': Criterion((SpecifierRequirement('numpy<2.2,>=2.1'), via=None), (SpecifierRequirement('numpy>=1.15'), via=LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)'))), 'numba': Criterion((SpecifierRequirement('numba<=0.60,>0.1'), via=None)), '<Python from Requires-Python>': Criterion((RequiresPythonRequirement('>=3.10'), via=LinkCandidate('https://files.pythonhosted.org/packages/05/db/5d9c91b2e1e2e72be1369278f696356d44975befcae830daf2e667dcb54f/numpy-2.1.3-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (from https://pypi.org/simple/numpy/) (requires-python:>=3.10)')), (RequiresPythonRequirement('>=3.6'), via=LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)')), (RequiresPythonRequirement('>=3.6'), via=LinkCandidate('https://files.pythonhosted.org/packages/0b/96/07bfa93a103fb9e3e9ae7f9f7c6687ae714aee66b6f3000da3fad71e0aa2/llvmlite-0.34.0.tar.gz (from https://pypi.org/simple/llvmlite/) (requires-python:>=3.6)')), (RequiresPythonRequirement('>=3.9'), via=LinkCandidate('https://files.pythonhosted.org/packages/69/8a/b9dc7678803429e4a3bc9ba462fa3dd9066824d3c607490235c6a796be5a/setuptools-75.8.0-py3-none-any.whl (from https://pypi.org/simple/setuptools/) (requires-python:>=3.9)'))), 'llvmlite': Criterion((SpecifierRequirement('llvmlite<0.35,>=0.34.0.dev0'), via=LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)'))), 'setuptools': Criterion((SpecifierRequirement('setuptools'), via=LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)')))}, backtrack_causes=[]))
---
> Reporter.ending(State(mapping=OrderedDict([('numpy', LinkCandidate('https://files.pythonhosted.org/packages/05/db/5d9c91b2e1e2e72be1369278f696356d44975befcae830daf2e667dcb54f/numpy-2.1.3-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (from https://pypi.org/simple/numpy/) (requires-python:>=3.10)')), ('<Python from Requires-Python>', <pip._internal.resolution.resolvelib.candidates.RequiresPythonCandidate object at 0x7f31ec46ca60>), ('numba', LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)')), ('llvmlite', LinkCandidate('https://files.pythonhosted.org/packages/0b/96/07bfa93a103fb9e3e9ae7f9f7c6687ae714aee66b6f3000da3fad71e0aa2/llvmlite-0.34.0.tar.gz (from https://pypi.org/simple/llvmlite/) (requires-python:>=3.6)')), ('setuptools', LinkCandidate('https://files.pythonhosted.org/packages/69/8a/b9dc7678803429e4a3bc9ba462fa3dd9066824d3c607490235c6a796be5a/setuptools-75.8.0-py3-none-any.whl (from https://pypi.org/simple/setuptools/) (requires-python:>=3.9)'))]), criteria={'numpy': Criterion((SpecifierRequirement('numpy<2.2,>=2.1'), via=None), (SpecifierRequirement('numpy>=1.15'), via=LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)'))), 'numba': Criterion((SpecifierRequirement('numba<=0.60,>0.1'), via=None)), '<Python from Requires-Python>': Criterion((RequiresPythonRequirement('>=3.10'), via=LinkCandidate('https://files.pythonhosted.org/packages/05/db/5d9c91b2e1e2e72be1369278f696356d44975befcae830daf2e667dcb54f/numpy-2.1.3-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (from https://pypi.org/simple/numpy/) (requires-python:>=3.10)')), (RequiresPythonRequirement('>=3.6'), via=LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)')), (RequiresPythonRequirement('>=3.6'), via=LinkCandidate('https://files.pythonhosted.org/packages/0b/96/07bfa93a103fb9e3e9ae7f9f7c6687ae714aee66b6f3000da3fad71e0aa2/llvmlite-0.34.0.tar.gz (from https://pypi.org/simple/llvmlite/) (requires-python:>=3.6)')), (RequiresPythonRequirement('>=3.9'), via=LinkCandidate('https://files.pythonhosted.org/packages/69/8a/b9dc7678803429e4a3bc9ba462fa3dd9066824d3c607490235c6a796be5a/setuptools-75.8.0-py3-none-any.whl (from https://pypi.org/simple/setuptools/) (requires-python:>=3.9)'))), 'llvmlite': Criterion((SpecifierRequirement('llvmlite<0.35,>=0.34.0.dev0'), via=LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)'))), 'setuptools': Criterion((SpecifierRequirement('setuptools'), via=LinkCandidate('https://files.pythonhosted.org/packages/5e/81/6fd1dd064bcf71a79da109e8966a39e2da61d68bf0bd1e0839fa997f8c41/numba-0.51.2.tar.gz (from https://pypi.org/simple/numba/) (requires-python:>=3.6)')))}, backtrack_causes=[]))

This additional requirement unfortunately has a greater than constant performance impact when in heavy backtracking situations, because get_preference is called for number of unsatisfied requirements which in turn calls is_backtrack_cause which loops over every backtrack causes.

We could special case requires python requirements in get_preference to not call is_backtrack_cause, but we can make an even better optimization with resolvelib 1.1.0 and implement the new provider method narrow_requirement_selection and return only the requires python requirement, and this will skip calling get_preference altogether.

@notatallshaw notatallshaw mentioned this pull request Feb 9, 2025
Merged
@ichard26
Copy link
Member Author

Thank you for taking a look! I'll wait for other maintainers to chime in. At the very least, I want someone else to "desk check" and confirm that this won't shouldn't break something horribly. (My track record isn't great, lately.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notatallshaw notatallshaw notatallshaw approved these changes

@uranusjr uranusjr Awaiting requested review from uranusjr

Assignees
No one assigned
Labels
bot:chronographer:provided C: dependency resolution About choosing which dependencies to install type: performance Commands take too long to run
Projects
None yet
Milestone
25.1
4 participants
@ichard26 @pradyunsg @notatallshaw @uranusjr