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

Make new resolver process Requires-Python before other dependencies #11398

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented Aug 21, 2022

Fix #11142. There are actually two issues; first is that the dist object in an ExplicitRequirement was being perpared too eagerly, the second is that Requires-Python should be returned before other dependencies so it is checked first.

This broke because resolvelib added a check to ensure a candidate’s dependencies are consistent. This new check made our lazy candidate preparation logic not lazy enough.

This avoid an explicit requirement (name @ url) from being prepared too
eagerly when its metadata is not required.
@uranusjr uranusjr force-pushed the new-resolver-python-before-deps-fix branch from ff4569a to 78ad423 Compare August 21, 2022 08:27
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.
@uranusjr uranusjr force-pushed the new-resolver-python-before-deps-fix branch from 78ad423 to 29ee616 Compare August 21, 2022 08:28
@ichard26
Copy link
Member

ichard26 commented May 3, 2024

Bump, what's the status on this PR? Is there some additional testing that needs to be done? If not, I can open a new one with the conflicts fixed if you'd like.

@ichard26
Copy link
Member

ichard26 commented Jul 4, 2024

I hate to be annoying, but I'd like to ask again what's blocking this PR from being ready for review. I see that there are some conflicts that need to be fixed and the patch can slightly cleaned up now that we target Python 3.8+ (re. @functools.cached_property) but otherwise this looks "ready" as-is. I can open a new PR with those changes addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Requires-Python version is not checked before downloading dependencies
2 participants