-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fixed bug in extras handling for link requirements #12392
Fixed bug in extras handling for link requirements #12392
Conversation
return base | ||
return self._make_extras_candidate(base, extras, comes_from=template) | ||
|
||
def _make_base_candidate_from_link( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_make_candidate_from_link
actually always returned a BaseCandidate
when no extras where specified. The new extras handling requires a base candidate so I extracted this part.
@uranusjr if you are available to review this that would be super helpful. |
Recently created a workflow which involves installing local extras, tested this branch and everything works fine. |
I'm preparing to do the last 23.3 patch release and I'm trying to determine if this PR fixes a regression of 23.3. To do so, I've tried to run the test added in this PR (test_new_resolver_constraint_on_link_with_extra) and it looks like it fails equally in 23.1.2 and 23.2.1. @sanderr Could it be this PR addresses something new, or that the test you added does not actually reveal the regression but something else? |
@sbidoul This is definitely a fix for a regression in 23.3. But you're right that the test case doesn't cover the regression directly. I have to admit I can't exactly recall all details, but I believe the test case covers a case that is strictly broader than the regression. Either way, I've just added a second test case that more closely resembles the scenario reported in #12373, and I've confirmed that it succeeds on both this branch and 23.2.1, while it fails on 23.3.1. |
Thank you for your work on this @sanderr. |
Fixed bug in extras handling where constraints on a package are not compatible with an explicit link candidate that also has an extra.
Closes #12372