Merge pull request #6859 from cjerdonek/wheel-strict-optional

Remove WheelBuilder's dependence on PipSession

Author: cjerdonek

src/pip/_internal/commands/install.py

@@ -1,3 +1,10 @@
+
+# The following comment should be removed at some point in the future.
+# It's included for now because without it InstallCommand.run() has a
+# couple errors where we have to know req.name is str rather than
+# Optional[str] for the InstallRequirement req.
+# mypy: strict-optional=False
+
 from __future__ import absolute_import
 
 import errno
@@ -13,7 +20,7 @@
 from pip._internal.cli import cmdoptions
 from pip._internal.cli.cmdoptions import make_target_python
 from pip._internal.cli.req_command import RequirementCommand
-from pip._internal.cli.status_codes import ERROR
+from pip._internal.cli.status_codes import ERROR, SUCCESS
 from pip._internal.exceptions import (
     CommandError,
     InstallationError,
@@ -30,9 +37,17 @@
     protect_pip_from_modification_on_windows,
 )
 from pip._internal.utils.temp_dir import TempDirectory
+from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 from pip._internal.utils.virtualenv import virtualenv_no_global
 from pip._internal.wheel import WheelBuilder
 
+if MYPY_CHECK_RUNNING:
+    from optparse import Values
+    from typing import Any, List
+
+    from pip._internal.req.req_install import InstallRequirement
+
+
 logger = logging.getLogger(__name__)
 
 
@@ -48,7 +63,12 @@ def is_wheel_installed():
     return True
 
 
-def build_wheels(builder, pep517_requirements, legacy_requirements, session):
+def build_wheels(
+    builder,              # type: WheelBuilder
+    pep517_requirements,  # type: List[InstallRequirement]
+    legacy_requirements,  # type: List[InstallRequirement]
+):
+    # type: (...) -> List[InstallRequirement]
     """
     Build wheels for requirements, depending on whether wheel is installed.
     """
@@ -58,7 +78,7 @@ def build_wheels(builder, pep517_requirements, legacy_requirements, session):
     # Always build PEP 517 requirements
     build_failures = builder.build(
         pep517_requirements,
-        session=session, autobuilding=True
+        autobuilding=True,
     )
 
     if should_build_legacy:
@@ -67,7 +87,7 @@ def build_wheels(builder, pep517_requirements, legacy_requirements, session):
         # install for those.
         builder.build(
             legacy_requirements,
-            session=session, autobuilding=True
+            autobuilding=True,
         )
 
     return build_failures
@@ -242,6 +262,7 @@ def __init__(self, *args, **kw):
         self.parser.insert_option_group(0, cmd_opts)
 
     def run(self, options, args):
+        # type: (Values, List[Any]) -> int
         cmdoptions.check_install_build_global(options)
         upgrade_strategy = "to-satisfy-only"
         if options.upgrade:
@@ -362,7 +383,6 @@ def run(self, options, args):
                         builder=wheel_builder,
                         pep517_requirements=pep517_requirements,
                         legacy_requirements=legacy_requirements,
-                        session=session,
                     )
 
                     # If we're using PEP 517, we cannot do a direct install
@@ -425,9 +445,11 @@ def run(self, options, args):
                         except Exception:
                             pass
                         items.append(item)
-                    installed = ' '.join(items)
-                    if installed:
-                        logger.info('Successfully installed %s', installed)
+                    installed_desc = ' '.join(items)
+                    if installed_desc:
+                        logger.info(
+                            'Successfully installed %s', installed_desc,
+                        )
                 except EnvironmentError as error:
                     show_traceback = (self.verbosity >= 1)
 
@@ -450,7 +472,8 @@ def run(self, options, args):
             self._handle_target_dir(
                 options.target_dir, target_temp_dir, options.upgrade
             )
-        return requirement_set
+
+        return SUCCESS
 
     def _handle_target_dir(self, target_dir, target_temp_dir, upgrade):
         ensure_dir(target_dir)

src/pip/_internal/commands/wheel.py

@@ -11,8 +11,14 @@
 from pip._internal.req import RequirementSet
 from pip._internal.req.req_tracker import RequirementTracker
 from pip._internal.utils.temp_dir import TempDirectory
+from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 from pip._internal.wheel import WheelBuilder
 
+if MYPY_CHECK_RUNNING:
+    from optparse import Values
+    from typing import Any, List
+
+
 logger = logging.getLogger(__name__)
 
 
@@ -101,6 +107,7 @@ def __init__(self, *args, **kw):
         self.parser.insert_option_group(0, cmd_opts)
 
     def run(self, options, args):
+        # type: (Values, List[Any]) -> None
         cmdoptions.check_install_build_global(options)
 
         if options.build_dir:
@@ -153,7 +160,7 @@ def run(self, options, args):
                         no_clean=options.no_clean,
                     )
                     build_failures = wb.build(
-                        requirement_set.requirements.values(), session=session,
+                        requirement_set.requirements.values(),
                     )
                     if len(build_failures) != 0:
                         raise CommandError(

src/pip/_internal/wheel.py

@@ -27,7 +27,7 @@
 from pip._vendor.six import StringIO
 
 from pip._internal import pep425tags
-from pip._internal.download import unpack_url
+from pip._internal.download import unpack_file_url
 from pip._internal.exceptions import (
     InstallationError,
     InvalidWheelFilename,
@@ -57,7 +57,6 @@
     )
     from pip._vendor.packaging.requirements import Requirement
     from pip._internal.req.req_install import InstallRequirement
-    from pip._internal.download import PipSession
     from pip._internal.index import FormatControl, PackageFinder
     from pip._internal.operations.prepare import (
         RequirementPreparer
@@ -1032,7 +1031,6 @@ def _clean_one(self, req):
     def build(
         self,
         requirements,  # type: Iterable[InstallRequirement]
-        session,  # type: PipSession
         autobuilding=False  # type: bool
     ):
         # type: (...) -> List[InstallRequirement]
@@ -1122,10 +1120,7 @@ def build(
                         req.link = Link(path_to_url(wheel_file))
                         assert req.link.is_wheel
                         # extract the wheel into the dir
-                        unpack_url(
-                            req.link, req.source_dir, None, False,
-                            session=session,
-                        )
+                        unpack_file_url(link=req.link, location=req.source_dir)
                 else:
                     build_failure.append(req)
 

tests/unit/test_command_install.py

@@ -9,7 +9,6 @@ def check_build_wheels(
         self,
         pep517_requirements,
         legacy_requirements,
-        session,
     ):
         """
         Return: (mock_calls, return_value).
@@ -25,8 +24,6 @@ def build(reqs, **kwargs):
             builder=builder,
             pep517_requirements=pep517_requirements,
             legacy_requirements=legacy_requirements,
-            # A session value isn't needed.
-            session='<session>',
         )
 
         return (builder.build.mock_calls, build_failures)
@@ -38,13 +35,12 @@ def test_build_wheels__wheel_installed(self, is_wheel_installed):
         mock_calls, build_failures = self.check_build_wheels(
             pep517_requirements=['a', 'b'],
             legacy_requirements=['c', 'd'],
-            session='<session>',
         )
 
         # Legacy requirements were built.
         assert mock_calls == [
-            call(['a', 'b'], autobuilding=True, session='<session>'),
-            call(['c', 'd'], autobuilding=True, session='<session>'),
+            call(['a', 'b'], autobuilding=True),
+            call(['c', 'd'], autobuilding=True),
         ]
 
         # Legacy build failures are not included in the return value.
@@ -57,12 +53,11 @@ def test_build_wheels__wheel_not_installed(self, is_wheel_installed):
         mock_calls, build_failures = self.check_build_wheels(
             pep517_requirements=['a', 'b'],
             legacy_requirements=['c', 'd'],
-            session='<session>',
         )
 
         # Legacy requirements were not built.
         assert mock_calls == [
-            call(['a', 'b'], autobuilding=True, session='<session>'),
+            call(['a', 'b'], autobuilding=True),
         ]
 
         assert build_failures == ['a']

tests/unit/test_wheel.py

@@ -700,7 +700,7 @@ def test_skip_building_wheels(self, caplog):
                 finder=Mock(), preparer=Mock(), wheel_cache=None,
             )
             with caplog.at_level(logging.INFO):
-                wb.build([wheel_req], session=Mock())
+                wb.build([wheel_req])
             assert "due to already being wheel" in caplog.text
             assert mock_build_one.mock_calls == []