Improve handling of PEP 518 build requirements

Merge pull request #5286 from benoit-pierre/improve_pep518_build_requirements_handling

Author: pradyunsg

docs/reference/pip.rst

@@ -116,23 +116,10 @@ using an incorrect encoding (mojibake).
 PEP 518 Support
 ~~~~~~~~~~~~~~~
 
-Pip supports projects declaring dependencies that are required at install time
-using a ``pyproject.toml`` file, in the form described in `PEP518`_. When
-building a project, pip will install the required dependencies locally, and
-make them available to the build process.
-
-As noted in the PEP, the minimum requirements for pip to be able to build a
-project are::
-
-    [build-system]
-    # Minimum requirements for the build system to execute.
-    requires = ["setuptools", "wheel"]
-
-``setuptools`` and ``wheel`` **must** be included in any ``pyproject.toml``
-provided by a project - pip will assume these as a default, but will not add
-them to an explicitly supplied list in a project supplied ``pyproject.toml``
-file. Once `PEP517`_ support is added, this restriction will be lifted and
-alternative build tools will be allowed.
+As of 10.0, pip supports projects declaring dependencies that are required at
+install time using a ``pyproject.toml`` file, in the form described in
+`PEP518`_. When building a project, pip will install the required dependencies
+locally, and make them available to the build process.
 
 When making build requirements available, pip does so in an *isolated
 environment*. That is, pip does not install those requirements into the user's
@@ -152,17 +139,23 @@ appropriately.
 
 .. _pep-518-limitations:
 
-The current implementation of `PEP518`_ in pip requires that any dependencies
-specified in ``pyproject.toml`` are available as wheels. This is a technical
-limitation of the implementation - dependencies only available as source would
-require a build step of their own, which would recursively invoke the `PEP518`_
-dependency installation process. The potentially unbounded recursion involved
-was not considered acceptable, and so installation of build dependencies from
-source has been disabled until a safe resolution of this issue has been found.
-
-Further, it also doesn't support the use of environment markers and extras,
-only version specifiers are respected. Support for markers and extras will be
-added in a future release.
+**Limitations**:
+
+* until `PEP517`_ support is added, ``setuptools`` and ``wheel`` **must** be
+  included in the list of build requirements: pip will assume these as default,
+  but will not automatically add them to the list of build requirements if
+  explicitly defined in ``pyproject.toml``.
+
+* the current implementation only support installing build requirements from
+  wheels: this is a technical limitation of the implementation - source
+  installs would require a build step of their own, potentially recursively
+  triggering another `PEP518`_ dependency installation process. The possible
+  unbounded recursion involved was not considered acceptable, and so
+  installation of build dependencies from source has been disabled until a safe
+  resolution of this issue is found.
+
+* ``pip<18.0`` does not support the use of environment markers and extras, only
+  version specifiers are respected.
 
 .. _PEP517: http://www.python.org/dev/peps/pep-0517/
 .. _PEP518: http://www.python.org/dev/peps/pep-0518/

news/5230.bugfix

@@ -0,0 +1 @@
+Improve handling of PEP 518 build requirements: support environment markers and extras.

news/5265.bugfix

@@ -0,0 +1 @@
+Improve handling of PEP 518 build requirements: support environment markers and extras.

src/pip/_internal/build_env.py

@@ -1,28 +1,33 @@
 """Build Environment used for isolation during sdist building
 """
 
+import logging
 import os
+import sys
 from distutils.sysconfig import get_python_lib
 from sysconfig import get_paths
 
+from pip._internal.utils.misc import call_subprocess
 from pip._internal.utils.temp_dir import TempDirectory
+from pip._internal.utils.ui import open_spinner
+
+
+logger = logging.getLogger(__name__)
 
 
 class BuildEnvironment(object):
     """Creates and manages an isolated environment to install build deps
     """
 
-    def __init__(self, no_clean):
+    def __init__(self):
         self._temp_dir = TempDirectory(kind="build-env")
-        self._no_clean = no_clean
+        self._temp_dir.create()
 
     @property
     def path(self):
         return self._temp_dir.path
 
     def __enter__(self):
-        self._temp_dir.create()
-
         self.save_path = os.environ.get('PATH', None)
         self.save_pythonpath = os.environ.get('PYTHONPATH', None)
         self.save_nousersite = os.environ.get('PYTHONNOUSERSITE', None)
@@ -58,9 +63,6 @@ def __enter__(self):
         return self.path
 
     def __exit__(self, exc_type, exc_val, exc_tb):
-        if not self._no_clean:
-            self._temp_dir.cleanup()
-
         def restore_var(varname, old_value):
             if old_value is None:
                 os.environ.pop(varname, None)
@@ -74,12 +76,39 @@ def restore_var(varname, old_value):
     def cleanup(self):
         self._temp_dir.cleanup()
 
+    def install_requirements(self, finder, requirements, message):
+        args = [
+            sys.executable, '-m', 'pip', 'install', '--ignore-installed',
+            '--no-user', '--prefix', self.path, '--no-warn-script-location',
+            '--only-binary', ':all:',
+        ]
+        if logger.getEffectiveLevel() <= logging.DEBUG:
+            args.append('-v')
+        if finder.index_urls:
+            args.extend(['-i', finder.index_urls[0]])
+            for extra_index in finder.index_urls[1:]:
+                args.extend(['--extra-index-url', extra_index])
+        else:
+            args.append('--no-index')
+        for link in finder.find_links:
+            args.extend(['--find-links', link])
+        for _, host, _ in finder.secure_origins:
+            args.extend(['--trusted-host', host])
+        if finder.allow_all_prereleases:
+            args.append('--pre')
+        if finder.process_dependency_links:
+            args.append('--process-dependency-links')
+        args.append('--')
+        args.extend(requirements)
+        with open_spinner(message) as spinner:
+            call_subprocess(args, show_stdout=False, spinner=spinner)
+
 
 class NoOpBuildEnvironment(BuildEnvironment):
     """A no-op drop-in replacement for BuildEnvironment
     """
 
-    def __init__(self, no_clean):
+    def __init__(self):
         pass
 
     def __enter__(self):
@@ -90,3 +119,6 @@ def __exit__(self, exc_type, exc_val, exc_tb):
 
     def cleanup(self):
         pass
+
+    def install_requirements(self, finder, requirements, message):
+        raise NotImplementedError()

src/pip/_internal/operations/prepare.py

@@ -1,15 +1,12 @@
 """Prepares a distribution for installation
 """
 
-import itertools
 import logging
 import os
-import sys
-from copy import copy
 
 from pip._vendor import pkg_resources, requests
 
-from pip._internal.build_env import NoOpBuildEnvironment
+from pip._internal.build_env import BuildEnvironment
 from pip._internal.compat import expanduser
 from pip._internal.download import (
     is_dir_url, is_file_url, is_vcs_url, unpack_url, url_to_path,
@@ -18,14 +15,9 @@
     DirectoryUrlHashUnsupported, HashUnpinned, InstallationError,
     PreviousBuildDirError, VcsHashUnsupported,
 )
-from pip._internal.index import FormatControl
-from pip._internal.req.req_install import InstallRequirement
 from pip._internal.utils.hashes import MissingHashes
 from pip._internal.utils.logging import indent_log
-from pip._internal.utils.misc import (
-    call_subprocess, display_path, normalize_path,
-)
-from pip._internal.utils.ui import open_spinner
+from pip._internal.utils.misc import display_path, normalize_path
 from pip._internal.vcs import vcs
 
 logger = logging.getLogger(__name__)
@@ -47,26 +39,6 @@ def make_abstract_dist(req):
         return IsSDist(req)
 
 
-def _install_build_reqs(finder, prefix, build_requirements):
-    # NOTE: What follows is not a very good thing.
-    #       Eventually, this should move into the BuildEnvironment class and
-    #       that should handle all the isolation and sub-process invocation.
-    finder = copy(finder)
-    finder.format_control = FormatControl(set(), set([":all:"]))
-    urls = [
-        finder.find_requirement(
-            InstallRequirement.from_line(r), upgrade=False).url
-        for r in build_requirements
-    ]
-    args = [
-        sys.executable, '-m', 'pip', 'install', '--ignore-installed',
-        '--no-user', '--prefix', prefix,
-    ] + list(urls)
-
-    with open_spinner("Installing build dependencies") as spinner:
-        call_subprocess(args, show_stdout=False, spinner=spinner)
-
-
 class DistAbstraction(object):
     """Abstracts out the wheel vs non-wheel Resolver.resolve() logic.
 
@@ -144,12 +116,10 @@ def format_reqs(rs):
             )
 
         if should_isolate:
-            with self.req.build_env:
-                pass
-            _install_build_reqs(finder, self.req.build_env.path,
-                                build_requirements)
-        else:
-            self.req.build_env = NoOpBuildEnvironment(no_clean=False)
+            self.req.build_env = BuildEnvironment()
+            self.req.build_env.install_requirements(
+                finder, build_requirements,
+                "Installing build dependencies")
 
         self.req.run_egg_info()
         self.req.assert_source_matches_version()

src/pip/_internal/req/req_install.py

@@ -22,7 +22,7 @@
 from pip._vendor.pkg_resources import RequirementParseError, parse_requirements
 
 from pip._internal import wheel
-from pip._internal.build_env import BuildEnvironment
+from pip._internal.build_env import NoOpBuildEnvironment
 from pip._internal.compat import native_str
 from pip._internal.download import (
     is_archive_file, is_url, path_to_url, url_to_path,
@@ -127,7 +127,7 @@ def __init__(self, req, comes_from, source_dir=None, editable=False,
         self.is_direct = False
 
         self.isolated = isolated
-        self.build_env = BuildEnvironment(no_clean=True)
+        self.build_env = NoOpBuildEnvironment()
 
     @classmethod
     def from_editable(cls, editable_req, comes_from=None, isolated=False,

src/pip/_internal/wheel.py

@@ -24,7 +24,6 @@
 from pip._vendor.six import StringIO
 
 from pip._internal import pep425tags
-from pip._internal.build_env import BuildEnvironment
 from pip._internal.download import path_to_url, unpack_url
 from pip._internal.exceptions import (
     InstallationError, InvalidWheelFilename, UnsupportedWheel,

tests/data/packages/pep518-3.0.tar.gz

@@ -1,5 +0,0 @@
-[egg_info]
-tag_build = 
-tag_date = 0
-tag_svn_revision = 0
-

tests/data/packages/pep518_with_extra_and_markers-1.0.tar.gz

@@ -1,9 +1,9 @@
 #!/usr/bin/env python
-from setuptools import find_packages, setup
+from setuptools import setup
 
-import simple  # ensure dependency is installed
+import simplewheel  # ensure dependency is installed
 
 setup(name='pep518',
       version='3.0',
-      packages=find_packages()
+      py_modules=['pep518'],
       )

tests/data/packages/simplewheel-1.0-py2.py3-none-any.whl

@@ -0,0 +1 @@
+include pyproject.toml

tests/data/packages/simplewheel-2.0-1-py2.py3-none-any.whl

@@ -0,0 +1 @@
+#dummy

tests/data/packages/simplewheel-2.0-py2.py3-none-any.whl

@@ -0,0 +1,7 @@
+[build-system]
+requires=[
+	"requires_simple_extra[extra]",
+	"simplewheel==1.0; python_version < '3'",
+	"simplewheel==2.0; python_version >= '3'",
+	"setuptools", "wheel",
+]

tests/data/packages4/simple-1.0-py2.py3-none-any.whl

@@ -0,0 +1,15 @@
+#!/usr/bin/env python
+import sys
+
+from setuptools import setup
+
+# ensure dependencies are installed
+import simple
+import simplewheel
+
+assert simplewheel.__version__ == '1.0' if sys.version_info < (3,) else '2.0'
+
+setup(name='pep518_with_extra_and_markers',
+      version='1.0',
+      py_modules=['pep518_with_extra_and_markers'],
+      )

tests/data/src/pep518-3.0/simple/__init__.py renamed to tests/data/src/pep518-3.0/pep518.py

@@ -1,7 +1,9 @@
 #!/usr/bin/env python
-from setuptools import find_packages, setup
+from setuptools import setup
+
+import simplewheel
 
 setup(name='simplewheel',
-      version='1.0',
-      packages=find_packages()
+      version=simplewheel.__version__,
+      packages=['simplewheel'],
       )

tests/data/src/pep518-3.0/setup.cfg

@@ -0,0 +1 @@
+__version__ = '1.0'

tests/data/src/pep518-3.0/setup.py

@@ -1,7 +1,9 @@
 #!/usr/bin/env python
-from setuptools import find_packages, setup
+from setuptools import setup
+
+import simplewheel
 
 setup(name='simplewheel',
-      version='2.0',
-      packages=find_packages()
+      version=simplewheel.__version__,
+      packages=['simplewheel'],
       )

tests/data/src/pep518_with_extra_and_markers-1.0/MANIFEST.in

@@ -0,0 +1 @@
+__version__ = '2.0'