-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
First stab at implementing the resolver provider using pip internals #7799
Conversation
They probably don't need to be global, and this fixes complaints from Flake8 for unused imports.
Get them ready to work as InstallRequirement wrappers. I'm not entirely sure about the interface yet (should find_matches and get_dependencies live somewhere else?) but that can change if needed.
…to provider-integration
The IReq *must* have a inner packaging Requirement because the non-prepared variant is only possible when passing in a URL. Those IReq instances should be wrapped in a DirectRequirement instead.
@pradyunsg I'm not sure why? It's a bunch of new files - how would it be reasonable to split those up? As it stands, it doesn't even link into the main code in pip. As and when we integrate it, then we may well need to look at doing that integration in an incremental manner (@uranusjr discussed this briefly on Tuesday) but for just putting the new resolvelib objects in place, I think a single PR is fine. |
Maybe eventually we want to merge the resolver implementation as smaller PRs, likely in the steps like:
But we’ll need to get a working implementation first to know what the necessary refactorings are. This PR is more geared into that. |
@uranusjr That sounds about right to me. |
This reverts commit 8b86746.
Got the test closer to working again. It still fails with
which I think is a genuine failure, in the sense that it's a conflict of assumptions. It may be that it's the test not creating the correct "stuff", or it may be that the assertion isn't correct - I can't tell right now. @uranusjr I've committed this as is so you can take a look if you have a moment - it might be obvious to you what's wrong. |
It seems I misunderstood how |
Removing the assertion gives me a new error, probably caused by the cloned
|
Hello! I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the |
I’m going to go ahead and close this PR since is has served its purposes :) Continuing the discussion on the split-up PRs. |
I'm deleting the corresponding branch for this PR now. In the off chance someone wants to look it up, the current ref points to: 5265557 |
Migrated from #7789, different branch for collaboration.
TODO: