Merge pull request #7910 from uranusjr/resolver-refactor-factory

Second round of resolver refactor factory

src/pip/_internal/resolution/resolvelib/base.py

@@ -44,7 +44,7 @@ class Candidate(object):
         raise NotImplementedError("Override in subclass")
 
     def get_dependencies(self):
-        # type: () -> Sequence[InstallRequirement]
+        # type: () -> Sequence[Requirement]
         raise NotImplementedError("Override in subclass")
 
     def get_install_requirement(self):

src/pip/_internal/resolution/resolvelib/candidates.py

@@ -12,12 +12,13 @@ if MYPY_CHECK_RUNNING:
     from typing import Any, Optional, Sequence, Set
 
     from pip._internal.models.link import Link
-    from pip._internal.operations.prepare import RequirementPreparer
-    from pip._internal.resolution.base import InstallRequirementProvider
 
     from pip._vendor.packaging.version import _BaseVersion
     from pip._vendor.pkg_resources import Distribution
 
+    from .base import Requirement
+    from .factory import Factory
+
 
 logger = logging.getLogger(__name__)
 
@@ -40,22 +41,11 @@ def make_install_req_from_link(link, parent):
 
 
 class LinkCandidate(Candidate):
-    def __init__(
-        self,
-        link,      # type: Link
-        preparer,  # type: RequirementPreparer
-        parent,    # type: InstallRequirement
-        make_install_req,  # type: InstallRequirementProvider
-    ):
-        # type: (...) -> None
+    def __init__(self, link, parent, factory):
+        # type: (Link, InstallRequirement, Factory) -> None
         self.link = link
-        self._preparer = preparer
+        self._factory = factory
         self._ireq = make_install_req_from_link(link, parent)
-        self._make_install_req = lambda spec: make_install_req(
-            spec,
-            self._ireq
-        )
-
         self._name = None  # type: Optional[str]
         self._version = None  # type: Optional[_BaseVersion]
         self._dist = None  # type: Optional[Distribution]
@@ -90,7 +80,7 @@ class LinkCandidate(Candidate):
     def dist(self):
         # type: () -> Distribution
         if self._dist is None:
-            abstract_dist = self._preparer.prepare_linked_requirement(
+            abstract_dist = self._factory.preparer.prepare_linked_requirement(
                 self._ireq
             )
             self._dist = abstract_dist.get_pkg_resources_distribution()
@@ -106,8 +96,11 @@ class LinkCandidate(Candidate):
         return self._dist
 
     def get_dependencies(self):
-        # type: () -> Sequence[InstallRequirement]
-        return [self._make_install_req(str(r)) for r in self.dist.requires()]
+        # type: () -> Sequence[Requirement]
+        return [
+            self._factory.make_requirement_from_spec(str(r), self._ireq)
+            for r in self.dist.requires()
+        ]
 
     def get_install_requirement(self):
         # type: () -> Optional[InstallRequirement]
@@ -138,12 +131,8 @@ class ExtrasCandidate(LinkCandidate):
     version 2.0. Having those candidates depend on foo=1.0 and foo=2.0
     respectively forces the resolver to recognise that this is a conflict.
     """
-    def __init__(
-        self,
-        base,      # type: LinkCandidate
-        extras,    # type: Set[str]
-    ):
-        # type: (...) -> None
+    def __init__(self, base, extras):
+        # type: (LinkCandidate, Set[str]) -> None
         self.base = base
         self.extras = extras
         self.link = base.link
@@ -160,7 +149,8 @@ class ExtrasCandidate(LinkCandidate):
         return self.base.version
 
     def get_dependencies(self):
-        # type: () -> Sequence[InstallRequirement]
+        # type: () -> Sequence[Requirement]
+        factory = self.base._factory
 
         # The user may have specified extras that the candidate doesn't
         # support. We ignore any unsupported extras here.
@@ -174,13 +164,13 @@ class ExtrasCandidate(LinkCandidate):
             )
 
         deps = [
-            self.base._make_install_req(str(r))
+            factory.make_requirement_from_spec(str(r), self.base._ireq)
             for r in self.base.dist.requires(valid_extras)
         ]
         # Add a dependency on the exact base.
         # (See note 2b in the class docstring)
         spec = "{}=={}".format(self.base.name, self.base.version)
-        deps.append(self.base._make_install_req(spec))
+        deps.append(factory.make_requirement_from_spec(spec, self.base._ireq))
         return deps
 
     def get_install_requirement(self):

src/pip/_internal/resolution/resolvelib/factory.py

@@ -23,9 +23,9 @@ class Factory(object):
         make_install_req,  # type: InstallRequirementProvider
     ):
         # type: (...) -> None
-        self._finder = finder
-        self._preparer = preparer
-        self._make_install_req = make_install_req
+        self.finder = finder
+        self.preparer = preparer
+        self._make_install_req_from_spec = make_install_req
         self._candidate_cache = {}  # type: Dict[Link, LinkCandidate]
 
     def make_candidate(
@@ -37,25 +37,22 @@ class Factory(object):
         # type: (...) -> Candidate
         if link not in self._candidate_cache:
             self._candidate_cache[link] = LinkCandidate(
-                link,
-                self._preparer,
-                parent=parent,
-                make_install_req=self._make_install_req
+                link, parent, factory=self,
             )
         base = self._candidate_cache[link]
         if extras:
             return ExtrasCandidate(base, extras)
         return base
 
-    def make_requirement(self, ireq):
+    def make_requirement_from_install_req(self, ireq):
         # type: (InstallRequirement) -> Requirement
         if ireq.link:
             cand = self.make_candidate(ireq.link, extras=set(), parent=ireq)
             return ExplicitRequirement(cand)
         else:
-            return SpecifierRequirement(
-                ireq,
-                finder=self._finder,
-                factory=self,
-                make_install_req=self._make_install_req,
-            )
+            return SpecifierRequirement(ireq, factory=self)
+
+    def make_requirement_from_spec(self, specifier, comes_from):
+        # type: (str, InstallRequirement) -> Requirement
+        ireq = self._make_install_req_from_spec(specifier, comes_from)
+        return self.make_requirement_from_install_req(ireq)

src/pip/_internal/resolution/resolvelib/provider.py

@@ -51,7 +51,4 @@ class PipProvider(AbstractProvider):
         # type: (Candidate) -> Sequence[Requirement]
         if self._ignore_dependencies:
             return []
-        return [
-            self._factory.make_requirement(r)
-            for r in candidate.get_dependencies()
-        ]
+        return candidate.get_dependencies()

src/pip/_internal/resolution/resolvelib/requirements.py

@@ -7,9 +7,7 @@ from .base import Requirement, format_name
 if MYPY_CHECK_RUNNING:
     from typing import Sequence
 
-    from pip._internal.index.package_finder import PackageFinder
     from pip._internal.req.req_install import InstallRequirement
-    from pip._internal.resolution.base import InstallRequirementProvider
 
     from .base import Candidate
     from .factory import Factory
@@ -36,19 +34,11 @@ class ExplicitRequirement(Requirement):
 
 
 class SpecifierRequirement(Requirement):
-    def __init__(
-        self,
-        ireq,      # type: InstallRequirement
-        finder,    # type: PackageFinder
-        factory,   # type: Factory
-        make_install_req  # type: InstallRequirementProvider
-    ):
-        # type: (...) -> None
+    def __init__(self, ireq, factory):
+        # type: (InstallRequirement, Factory) -> None
         assert ireq.link is None, "This is a link, not a specifier"
         self._ireq = ireq
         self._factory = factory
-        self._finder = finder
-        self._make_install_req = make_install_req
         self.extras = ireq.req.extras
 
     @property
@@ -59,7 +49,7 @@ class SpecifierRequirement(Requirement):
 
     def find_matches(self):
         # type: () -> Sequence[Candidate]
-        found = self._finder.find_best_candidate(
+        found = self._factory.finder.find_best_candidate(
             project_name=self._ireq.req.name,
             specifier=self._ireq.req.specifier,
             hashes=self._ireq.hashes(trust_internet=False),

src/pip/_internal/resolution/resolvelib/resolver.py

@@ -56,7 +56,10 @@ class Resolver(BaseResolver):
         reporter = BaseReporter()
         resolver = RLResolver(provider, reporter)
 
-        requirements = [self.factory.make_requirement(r) for r in root_reqs]
+        requirements = [
+            self.factory.make_requirement_from_install_req(r)
+            for r in root_reqs
+        ]
         self._result = resolver.resolve(requirements)
 
         req_set = RequirementSet(check_supported_wheels=check_supported_wheels)

tests/unit/resolution_resolvelib/conftest.py

-from functools import partial
-
 import pytest
 
 from pip._internal.cli.req_command import RequirementCommand
@@ -10,7 +8,7 @@ from pip._internal.index.package_finder import PackageFinder
 from pip._internal.models.search_scope import SearchScope
 from pip._internal.models.selection_prefs import SelectionPreferences
 from pip._internal.network.session import PipSession
-from pip._internal.req.constructors import install_req_from_req_string
+from pip._internal.req.constructors import install_req_from_line
 from pip._internal.req.req_tracker import get_requirement_tracker
 from pip._internal.resolution.resolvelib.factory import Factory
 from pip._internal.resolution.resolvelib.provider import PipProvider
@@ -50,16 +48,10 @@ def preparer(finder):
 
 @pytest.fixture
 def factory(finder, preparer):
-    make_install_req = partial(
-        install_req_from_req_string,
-        isolated=False,
-        wheel_cache=None,
-        use_pep517=None,
-    )
     yield Factory(
         finder=finder,
         preparer=preparer,
-        make_install_req=make_install_req,
+        make_install_req=install_req_from_line,
     )
 
 

tests/unit/resolution_resolvelib/test_requirement.py

 import pytest
 from pip._vendor.resolvelib import BaseReporter, Resolver
 
-from pip._internal.req.constructors import install_req_from_line
 from pip._internal.resolution.resolvelib.base import Candidate
 from pip._internal.utils.urls import path_to_url
 
@@ -50,25 +49,22 @@ def test_cases(data):
 
 def test_new_resolver_requirement_has_name(test_cases, factory):
     """All requirements should have a name"""
-    for requirement, name, matches in test_cases:
-        ireq = install_req_from_line(requirement)
-        req = factory.make_requirement(ireq)
+    for spec, name, matches in test_cases:
+        req = factory.make_requirement_from_spec(spec, comes_from=None)
         assert req.name == name
 
 
 def test_new_resolver_correct_number_of_matches(test_cases, factory):
     """Requirements should return the correct number of candidates"""
-    for requirement, name, matches in test_cases:
-        ireq = install_req_from_line(requirement)
-        req = factory.make_requirement(ireq)
+    for spec, name, matches in test_cases:
+        req = factory.make_requirement_from_spec(spec, comes_from=None)
         assert len(req.find_matches()) == matches
 
 
 def test_new_resolver_candidates_match_requirement(test_cases, factory):
     """Candidates returned from find_matches should satisfy the requirement"""
-    for requirement, name, matches in test_cases:
-        ireq = install_req_from_line(requirement)
-        req = factory.make_requirement(ireq)
+    for spec, name, matches in test_cases:
+        req = factory.make_requirement_from_spec(spec, comes_from=None)
         for c in req.find_matches():
             assert isinstance(c, Candidate)
             assert req.is_satisfied_by(c)
@@ -76,8 +72,7 @@ def test_new_resolver_candidates_match_requirement(test_cases, factory):
 
 def test_new_resolver_full_resolve(factory, provider):
     """A very basic full resolve"""
-    ireq = install_req_from_line("simplewheel")
-    req = factory.make_requirement(ireq)
+    req = factory.make_requirement_from_spec("simplewheel", comes_from=None)
     r = Resolver(provider, BaseReporter())
     result = r.resolve([req])
     assert set(result.mapping.keys()) == {'simplewheel'}