pypa/pip

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:

  1. A fairly "independent" RequirementPreparer.prepare to get an AbstractDistribution from InstallRequirement unconditionally (/cc @chrahunt)
  2. A sorted list of applicable_candidates, as part of BestCandidateResult (/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 into RequirementPreparer and InstallationCandidate.populate_link

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:

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. :)

With #7328, #7331 and #7332, I think all this got done. For all I understand, we're properly unblocked to do this now. :)

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.