Refactoring needed to bring in resolvelib's Resolver abstraction
pradyunsg opened this issue · 5 comments
I spent some time today, taking a look at where we are, w.r.t. bringing in resolvelib into pip, as part of #6536, for fixing #988.
I think there's only 2 main things remaining:
- A fairly "independent"
RequirementPreparer.prepare
to get anAbstractDistribution
fromInstallRequirement
unconditionally (/cc @chrahunt) - A sorted list of
applicable_candidates
, as part ofBestCandidateResult
(/cc @cjerdonek)
I think I have a good idea for how to approach both of these changes. I'll elaborate on them in dedicated comments below.
AFAICT, the main way that dependency resolution is entangled with metadata generation today is (1). To decouple them, my plan is:
use_user_site
into RequirementPreparer- move the logic for
require_hashes
into preparer- make it a public attribute that we'd access in
Resolver._get_abstract_dist_for
- Currently,
Resolver
computes and passes it intoRequirementPreparer
andInstallationCandidate.populate_link
- make it a public attribute that we'd access in
This would eliminate the cyclic dependency between Resolver
and RequirementPreparer
(RequirementPreparer
won't require arguments held as attributes in Resolver
).
I don't think we'd be able to directly use this new prepare
method in our existing resolver, but it'd be handy to have when prototyping, so I think we should add it anyway. :)
For self-reference later, this is what the prepare method looks like:
def prepare(self, candidate):
if candidate.editable:
return self.prepare_editable_requirement(candidate)
if candidate.link:
return self.prepare_linked_requirement(candidate)
if candidate.already_installed:
return self.prepare_installed_requirement(candidate)
assert False, "should never reach here"
s/candidate/requirement/
For (2), the fix can be pretty targeted:
- Change
CandidateEvaluator.get_applicable_candidates
to sort applicable candidates.- best_candidate would be
BestCandidateResult.applicable_candidates[-1]
- it's at the end because that's how resolvelib's abstractions are defined.
- best_candidate would be
applicable_candidates
is not used anywhere outside of package_finder.py
and I don't think any of the uses within package_finder.py
expect it to be ordered or anything.
As an aside, we should change InstallationCandidate.project: Any
-> InstallationCandidate.name: str
.
move the logic for
require_hashes
into preparer
@chrahunt and I discussed this on voice call.
We figured out that the best place to do this would be in populate_requirement_set
. There's already a comment there from a past @pradyunsg about how that method tweaks require_hashes
and this change would make it explicit how all that works. :)
As a note for posterity: as Paul mentioned in a meeting several weeks ago, we chose resolvelib as the least bad choice with the least blockers to overcome. Resolvelib API is the most straightforward. And we can plug something in later if we change our mind.