Merge pull request #4925 from pradyunsg/resolver/move-dependency-info-and-install-order

Move dependency info and install order into Resolver

Author: pradyunsg

src/pip/_internal/commands/install.py

@@ -15,7 +15,7 @@
 )
 from pip._internal.locations import distutils_scheme, virtualenv_no_global
 from pip._internal.operations.prepare import RequirementPreparer
-from pip._internal.req import RequirementSet
+from pip._internal.req import RequirementSet, install_given_reqs
 from pip._internal.resolve import Resolver
 from pip._internal.status_codes import ERROR
 from pip._internal.utils.filesystem import check_path_owner
@@ -300,7 +300,11 @@ def run(self, options, args):
                             session=session, autobuilding=True
                         )
 
-                    installed = requirement_set.install(
+                    to_install = resolver.get_installation_order(
+                        requirement_set
+                    )
+                    installed = install_given_reqs(
+                        to_install,
                         install_options,
                         global_options,
                         root=options.root_path,

src/pip/_internal/req/__init__.py

@@ -1,10 +1,69 @@
 from __future__ import absolute_import
 
+import logging
+
 from .req_install import InstallRequirement
 from .req_set import RequirementSet
 from .req_file import parse_requirements
+from pip._internal.utils.logging import indent_log
+
 
 __all__ = [
     "RequirementSet", "InstallRequirement",
-    "parse_requirements",
+    "parse_requirements", "install_given_reqs",
 ]
+
+logger = logging.getLogger(__name__)
+
+
+def install_given_reqs(to_install, install_options, global_options=(),
+                       *args, **kwargs):
+    """
+    Install everything in the given list.
+
+    (to be called after having downloaded and unpacked the packages)
+    """
+
+    if to_install:
+        logger.info(
+            'Installing collected packages: %s',
+            ', '.join([req.name for req in to_install]),
+        )
+
+    with indent_log():
+        for requirement in to_install:
+            if requirement.conflicts_with:
+                logger.info(
+                    'Found existing installation: %s',
+                    requirement.conflicts_with,
+                )
+                with indent_log():
+                    uninstalled_pathset = requirement.uninstall(
+                        auto_confirm=True
+                    )
+            try:
+                requirement.install(
+                    install_options,
+                    global_options,
+                    *args,
+                    **kwargs
+                )
+            except:
+                should_rollback = (
+                    requirement.conflicts_with and
+                    not requirement.install_succeeded
+                )
+                # if install did not succeed, rollback previous uninstall
+                if should_rollback:
+                    uninstalled_pathset.rollback()
+                raise
+            else:
+                should_commit = (
+                    requirement.conflicts_with and
+                    requirement.install_succeeded
+                )
+                if should_commit:
+                    uninstalled_pathset.commit()
+            requirement.remove_temporary_source()
+
+    return to_install

src/pip/_internal/req/req_set.py

@@ -1,7 +1,7 @@
 from __future__ import absolute_import
 
 import logging
-from collections import OrderedDict, defaultdict
+from collections import OrderedDict
 
 from pip._internal.exceptions import InstallationError
 from pip._internal.utils.logging import indent_log
@@ -32,8 +32,6 @@ def __init__(self,
         self.use_user_site = use_user_site
         self.target_dir = target_dir  # set from --target option
         self.pycompile = pycompile
-        # Maps from install_req -> dependencies_of_install_req
-        self._dependencies = defaultdict(list)
 
     def __str__(self):
         reqs = [req for req in self.requirements.values()
@@ -69,7 +67,7 @@ def add_requirement(self, install_req, parent_req_name=None,
             logger.info("Ignoring %s: markers '%s' don't match your "
                         "environment", install_req.name,
                         install_req.markers)
-            return []
+            return [], None
 
         # This check has to come after we filter requirements with the
         # environment markers.
@@ -89,7 +87,7 @@ def add_requirement(self, install_req, parent_req_name=None,
         if not name:
             # url or path requirement w/o an egg fragment
             self.unnamed_requirements.append(install_req)
-            return [install_req]
+            return [install_req], None
         else:
             try:
                 existing_req = self.get_requirement(name)
@@ -135,10 +133,10 @@ def add_requirement(self, install_req, parent_req_name=None,
                 # Canonicalise to the already-added object for the backref
                 # check below.
                 install_req = existing_req
-            if parent_req_name:
-                parent_req = self.get_requirement(parent_req_name)
-                self._dependencies[parent_req].append(install_req)
-            return result
+
+            # We return install_req here to allow for the caller to add it to
+            # the dependency information for the parent package.
+            return result, install_req
 
     def has_requirement(self, project_name):
         name = project_name.lower()
@@ -168,81 +166,3 @@ def cleanup_files(self):
         with indent_log():
             for req in self.reqs_to_cleanup:
                 req.remove_temporary_source()
-
-    def _to_install(self):
-        """Create the installation order.
-
-        The installation order is topological - requirements are installed
-        before the requiring thing. We break cycles at an arbitrary point,
-        and make no other guarantees.
-        """
-        # The current implementation, which we may change at any point
-        # installs the user specified things in the order given, except when
-        # dependencies must come earlier to achieve topological order.
-        order = []
-        ordered_reqs = set()
-
-        def schedule(req):
-            if req.satisfied_by or req in ordered_reqs:
-                return
-            if req.constraint:
-                return
-            ordered_reqs.add(req)
-            for dep in self._dependencies[req]:
-                schedule(dep)
-            order.append(req)
-
-        for install_req in self.requirements.values():
-            schedule(install_req)
-        return order
-
-    def install(self, install_options, global_options=(), *args, **kwargs):
-        """
-        Install everything in this set (after having downloaded and unpacked
-        the packages)
-        """
-        to_install = self._to_install()
-
-        if to_install:
-            logger.info(
-                'Installing collected packages: %s',
-                ', '.join([req.name for req in to_install]),
-            )
-
-        with indent_log():
-            for requirement in to_install:
-                if requirement.conflicts_with:
-                    logger.info(
-                        'Found existing installation: %s',
-                        requirement.conflicts_with,
-                    )
-                    with indent_log():
-                        uninstalled_pathset = requirement.uninstall(
-                            auto_confirm=True
-                        )
-                try:
-                    requirement.install(
-                        install_options,
-                        global_options,
-                        *args,
-                        **kwargs
-                    )
-                except:
-                    should_rollback = (
-                        requirement.conflicts_with and
-                        not requirement.install_succeeded
-                    )
-                    # if install did not succeed, rollback previous uninstall
-                    if should_rollback:
-                        uninstalled_pathset.rollback()
-                    raise
-                else:
-                    should_commit = (
-                        requirement.conflicts_with and
-                        requirement.install_succeeded
-                    )
-                    if should_commit:
-                        uninstalled_pathset.commit()
-                requirement.remove_temporary_source()
-
-        return to_install

src/pip/_internal/resolve.py

@@ -11,12 +11,14 @@
 """
 
 import logging
+from collections import defaultdict
 from itertools import chain
 
 from pip._internal.exceptions import (
     BestVersionAlreadyInstalled, DistributionNotFound, HashError, HashErrors,
     UnsupportedPythonVersion,
 )
+
 from pip._internal.req.req_install import InstallRequirement
 from pip._internal.utils.logging import indent_log
 from pip._internal.utils.misc import dist_in_usersite, ensure_dir
@@ -56,6 +58,8 @@ def __init__(self, preparer, session, finder, wheel_cache, use_user_site,
         self.ignore_requires_python = ignore_requires_python
         self.use_user_site = use_user_site
 
+        self._discovered_dependencies = defaultdict(list)
+
     def resolve(self, requirement_set):
         """Resolve what operations need to be done
 
@@ -273,19 +277,26 @@ def add_req(subreq, extras_requested):
                 isolated=self.isolated,
                 wheel_cache=self.wheel_cache,
             )
-            more_reqs.extend(
-                requirement_set.add_requirement(
-                    sub_install_req, req_to_install.name,
-                    extras_requested=extras_requested
-                )
+            parent_req_name = req_to_install.name
+            to_scan_again, add_to_parent = requirement_set.add_requirement(
+                sub_install_req,
+                parent_req_name=parent_req_name,
+                extras_requested=extras_requested,
             )
+            if parent_req_name and add_to_parent:
+                self._discovered_dependencies[parent_req_name].append(
+                    add_to_parent
+                )
+            more_reqs.extend(to_scan_again)
 
         with indent_log():
             # We add req_to_install before its dependencies, so that we
             # can refer to it when adding dependencies.
             if not requirement_set.has_requirement(req_to_install.name):
                 # 'unnamed' requirements will get added here
-                requirement_set.add_requirement(req_to_install, None)
+                requirement_set.add_requirement(
+                    req_to_install, parent_req_name=None,
+                )
 
             if not self.ignore_dependencies:
                 if req_to_install.extras:
@@ -315,3 +326,30 @@ def add_req(subreq, extras_requested):
                 requirement_set.successfully_downloaded.append(req_to_install)
 
         return more_reqs
+
+    def get_installation_order(self, req_set):
+        """Create the installation order.
+
+        The installation order is topological - requirements are installed
+        before the requiring thing. We break cycles at an arbitrary point,
+        and make no other guarantees.
+        """
+        # The current implementation, which we may change at any point
+        # installs the user specified things in the order given, except when
+        # dependencies must come earlier to achieve topological order.
+        order = []
+        ordered_reqs = set()
+
+        def schedule(req):
+            if req.satisfied_by or req in ordered_reqs:
+                return
+            if req.constraint:
+                return
+            ordered_reqs.add(req)
+            for dep in self._discovered_dependencies[req.name]:
+                schedule(dep)
+            order.append(req)
+
+        for install_req in req_set.requirements.values():
+            schedule(install_req)
+        return order