Improve handling of PEP 518 build requirements

Merge pull request #5286 from benoit-pierre/improve_pep518_build_requirements_handling

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.
+**Limitations**:
 
-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.
+* 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 @@ class BuildEnvironment(object):
         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 @@ class BuildEnvironment(object):
     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 @@ class NoOpBuildEnvironment(BuildEnvironment):
 
     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 @@ from pip._internal.exceptions import (
     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 @@ class IsSDist(DistAbstraction):
             )
 
         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.packaging.version import Version
 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 @@ class InstallRequirement(object):
         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.packaging.utils import canonicalize_name
 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/src/pep518-3.0/setup.cfg

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

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

@@ -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/src/pep518_with_extra_and_markers-1.0/MANIFEST.in

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

tests/data/src/pep518_with_extra_and_markers-1.0/pep518_with_extra_and_markers.py

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

tests/data/src/pep518_with_extra_and_markers-1.0/pyproject.toml

@@ -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/src/pep518_with_extra_and_markers-1.0/setup.py

@@ -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/simplewheel-1.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='1.0',
-      packages=find_packages()
+      version=simplewheel.__version__,
+      packages=['simplewheel'],
       )

tests/data/src/simplewheel-1.0/simple/__init__.py

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

tests/data/src/simplewheel-1.0/simplewheel/__init__.py

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

tests/data/src/simplewheel-2.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/simplewheel-2.0/simple/__init__.py

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

tests/data/src/simplewheel-2.0/simplewheel/__init__.py

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

tests/functional/test_install.py

@@ -19,43 +19,52 @@ from tests.lib.local_repos import local_checkout
 from tests.lib.path import Path
 
 
-@pytest.mark.parametrize('original_setuptools', ('missing', 'bad'))
-def test_pep518_uses_build_env(script, data, original_setuptools):
-    if original_setuptools == 'missing':
+@pytest.mark.parametrize('command', ('install', 'wheel'))
+@pytest.mark.parametrize('variant', ('missing_setuptools', 'bad_setuptools'))
+def test_pep518_uses_build_env(script, data, common_wheels, command, variant):
+    if variant == 'missing_setuptools':
         script.pip("uninstall", "-y", "setuptools")
-    elif original_setuptools == 'bad':
+    elif variant == 'bad_setuptools':
         setuptools_init_path = script.site_packages_path.join(
             "setuptools", "__init__.py")
         with open(setuptools_init_path, 'a') as f:
             f.write('\nraise ImportError("toto")')
     else:
-        raise ValueError(original_setuptools)
-    to_install = data.src.join("pep518-3.0")
-    for command in ('install', 'wheel'):
-        script.run(
-            "python", "-c",
-            "import pip._internal; pip._internal.main(["
-            "%r, " "'-f', %r, " "%r, "
-            "])" % (command, str(data.packages), str(to_install)),
-        )
+        raise ValueError(variant)
+    script.pip(
+        command, '--no-index', '-f', common_wheels, '-f', data.packages,
+        data.src.join("pep518-3.0"), use_module=True
+    )
 
 
-def test_pep518_with_user_pip(script, virtualenv, pip_src, data):
+def test_pep518_with_user_pip(script, virtualenv, pip_src,
+                              data, common_wheels):
     virtualenv.system_site_packages = True
-    script.pip("install", "--ignore-installed", "--user", pip_src)
+    script.pip_install_local("--ignore-installed",
+                             "-f", common_wheels,
+                             "--user", pip_src)
     system_pip_dir = script.site_packages_path / 'pip'
     system_pip_dir.rmtree()
     system_pip_dir.mkdir()
     with open(system_pip_dir / '__init__.py', 'w') as fp:
         fp.write('raise ImportError\n')
-    to_install = data.src.join("pep518-3.0")
-    for command in ('install', 'wheel'):
-        script.run(
-            "python", "-c",
-            "import pip._internal; pip._internal.main(["
-            "%r, " "'-f', %r, " "%r, "
-            "])" % (command, str(data.packages), str(to_install)),
-        )
+    script.pip(
+        'wheel', '--no-index', '-f', common_wheels, '-f', data.packages,
+        data.src.join("pep518-3.0"), use_module=True,
+    )
+
+
+def test_pep518_with_extra_and_markers(script, data, common_wheels):
+    script.pip(
+        'wheel', '--no-index',
+        '-f', common_wheels,
+        '-f', data.find_links,
+        # Add tests/data/packages4, which contains a wheel for
+        # simple==1.0 (needed by requires_simple_extra[extra]).
+        '-f', data.find_links4,
+        data.src.join("pep518_with_extra_and_markers-1.0"),
+        use_module=True,
+    )
 
 
 @pytest.mark.network
@@ -197,8 +206,8 @@ def test_install_editable_uninstalls_existing_from_path(script, data):
     to_install = data.src.join('simplewheel-1.0')
     result = script.pip_install_local(to_install)
     assert 'Successfully installed simplewheel' in result.stdout
-    simple_folder = script.site_packages / 'simple'
-    result.assert_installed('simple', editable=False)
+    simple_folder = script.site_packages / 'simplewheel'
+    result.assert_installed('simplewheel', editable=False)
     assert simple_folder in result.files_created, str(result.stdout)
 
     result = script.pip(

tests/functional/test_wheel.py

@@ -200,12 +200,10 @@ def test_wheel_package_with_latin1_setup(script, data, common_wheels):
 
 
 @pytest.mark.network
-def test_pip_wheel_with_pep518_build_reqs(script, data):
-    script.pip('install', 'wheel')
-    script.pip('download', 'setuptools', 'wheel', '-d', data.packages)
-    result = script.pip(
-        'wheel', '--no-index', '-f', data.find_links, 'pep518==3.0',
-    )
+def test_pip_wheel_with_pep518_build_reqs(script, data, common_wheels):
+    script.pip_install_local('-f', common_wheels, 'wheel')
+    result = script.pip('wheel', '--no-index', '-f', data.find_links,
+                        '-f', common_wheels, 'pep518==3.0',)
     wheel_file_name = 'pep518-3.0-py%s-none-any.whl' % pyversion[0]
     wheel_file_path = script.scratch / wheel_file_name
     assert wheel_file_path in result.files_created, result.stdout
@@ -214,11 +212,12 @@ def test_pip_wheel_with_pep518_build_reqs(script, data):
 
 
 @pytest.mark.network
-def test_pip_wheel_with_pep518_build_reqs_no_isolation(script, data):
-    script.pip('install', 'wheel')
+def test_pip_wheel_with_pep518_build_reqs_no_isolation(script, data,
+                                                       common_wheels):
+    script.pip_install_local('-f', common_wheels, 'wheel', 'simplewheel==2.0')
     result = script.pip(
-        'wheel', '--no-index', '-f', data.find_links, '--no-build-isolation',
-        'pep518==3.0',
+        'wheel', '--no-index', '-f', data.find_links,
+        '--no-build-isolation', 'pep518==3.0',
     )
     wheel_file_name = 'pep518-3.0-py%s-none-any.whl' % pyversion[0]
     wheel_file_path = script.scratch / wheel_file_name

tests/lib/__init__.py

@@ -100,6 +100,10 @@ class TestData(object):
     def packages3(self):
         return self.root.join("packages3")
 
+    @property
+    def packages4(self):
+        return self.root.join("packages4")
+
     @property
     def src(self):
         return self.root.join("src")
@@ -124,6 +128,10 @@ class TestData(object):
     def find_links3(self):
         return path_to_url(self.packages3)
 
+    @property
+    def find_links4(self):
+        return path_to_url(self.packages4)
+
     def index_url(self, index="simple"):
         return path_to_url(self.root.join("indexes", index))