From 744b8cf96eb3c3d4504048621c9406dd648fb71f Mon Sep 17 00:00:00 2001 From: Benoit Pierre Date: Thu, 25 Oct 2018 17:35:25 +0200 Subject: [PATCH 01/29] improve build environment - check build requirements for conflicts - better isolation (ignore system site packages) - support 2 prefixes: a "normal" one, and an "overlay" one (with higher priority over "normal") --- .appveyor.yml | 2 +- src/pip/_internal/build_env.py | 169 +++++++++------ src/pip/_internal/operations/prepare.py | 23 ++- tests/conftest.py | 8 - .../pep518_conflicting_requires/MANIFEST.in | 1 + .../src/pep518_conflicting_requires/pep518.py | 1 + .../pyproject.toml | 2 + .../src/pep518_conflicting_requires/setup.py | 8 + tests/functional/test_install.py | 20 +- tests/functional/test_pep517.py | 5 +- tests/lib/__init__.py | 8 +- tests/unit/test_build_env.py | 194 ++++++++++++++++++ tools/travis/run.sh | 2 +- 13 files changed, 359 insertions(+), 84 deletions(-) create mode 100644 tests/data/src/pep518_conflicting_requires/MANIFEST.in create mode 100644 tests/data/src/pep518_conflicting_requires/pep518.py create mode 100644 tests/data/src/pep518_conflicting_requires/pyproject.toml create mode 100644 tests/data/src/pep518_conflicting_requires/setup.py create mode 100644 tests/unit/test_build_env.py diff --git a/.appveyor.yml b/.appveyor.yml index e4324b470de..6dc80479fa0 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -65,6 +65,6 @@ test_script: tox -e py -- --use-venv -m integration -n 3 --duration=5 } else { - tox -e py -- -m unit -n 3 + tox -e py -- --use-venv -m unit -n 3 } } diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py index f702992dc14..f99fdf522f7 100644 --- a/src/pip/_internal/build_env.py +++ b/src/pip/_internal/build_env.py @@ -5,6 +5,7 @@ import os import sys import textwrap +from collections import OrderedDict from distutils.sysconfig import get_python_lib from sysconfig import get_paths @@ -18,6 +19,25 @@ logger = logging.getLogger(__name__) +class _Prefix: + + def __init__(self, path): + self.path = path + self.setup = False + self.bin_dir = get_paths( + 'nt' if os.name == 'nt' else 'posix_prefix', + vars={'base': path, 'platbase': path} + )['scripts'] + # Note: prefer distutils' sysconfig to get the + # library paths so PyPy is correctly supported. + purelib = get_python_lib(plat_specific=0, prefix=path) + platlib = get_python_lib(plat_specific=1, prefix=path) + if purelib == platlib: + self.lib_dirs = [purelib] + else: + self.lib_dirs = [purelib, platlib] + + class BuildEnvironment(object): """Creates and manages an isolated environment to install build deps """ @@ -26,86 +46,113 @@ def __init__(self): self._temp_dir = TempDirectory(kind="build-env") self._temp_dir.create() - @property - def path(self): - return self._temp_dir.path - - def __enter__(self): - self.save_path = os.environ.get('PATH', None) - self.save_pythonpath = os.environ.get('PYTHONPATH', None) - self.save_nousersite = os.environ.get('PYTHONNOUSERSITE', None) - - install_scheme = 'nt' if (os.name == 'nt') else 'posix_prefix' - install_dirs = get_paths(install_scheme, vars={ - 'base': self.path, - 'platbase': self.path, - }) - - scripts = install_dirs['scripts'] - if self.save_path: - os.environ['PATH'] = scripts + os.pathsep + self.save_path - else: - os.environ['PATH'] = scripts + os.pathsep + os.defpath - - # Note: prefer distutils' sysconfig to get the - # library paths so PyPy is correctly supported. - purelib = get_python_lib(plat_specific=0, prefix=self.path) - platlib = get_python_lib(plat_specific=1, prefix=self.path) - if purelib == platlib: - lib_dirs = purelib - else: - lib_dirs = purelib + os.pathsep + platlib - if self.save_pythonpath: - os.environ['PYTHONPATH'] = lib_dirs + os.pathsep + \ - self.save_pythonpath - else: - os.environ['PYTHONPATH'] = lib_dirs - - os.environ['PYTHONNOUSERSITE'] = '1' - - # Ensure .pth files are honored. - with open(os.path.join(purelib, 'sitecustomize.py'), 'w') as fp: + self._prefixes = OrderedDict(( + (name, _Prefix(os.path.join(self._temp_dir.path, name))) + for name in ('normal', 'overlay') + )) + + self._bin_dirs = [] + self._lib_dirs = [] + for prefix in reversed(list(self._prefixes.values())): + self._bin_dirs.append(prefix.bin_dir) + self._lib_dirs.extend(prefix.lib_dirs) + + # Customize site to: + # - ensure .pth files are honored + # - prevent access to system site packages + system_sites = { + os.path.normcase(site) for site in ( + get_python_lib(plat_specific=0), + get_python_lib(plat_specific=1), + ) + } + self._site_dir = os.path.join(self._temp_dir.path, 'site') + if not os.path.exists(self._site_dir): + os.mkdir(self._site_dir) + with open(os.path.join(self._site_dir, 'sitecustomize.py'), 'w') as fp: fp.write(textwrap.dedent( ''' - import site - site.addsitedir({!r}) + import os, site, sys + + # First, drop system-sites related paths. + original_sys_path = sys.path[:] + known_paths = set() + for path in {system_sites!r}: + site.addsitedir(path, known_paths=known_paths) + system_paths = set( + os.path.normcase(path) + for path in sys.path[len(original_sys_path):] + ) + original_sys_path = [ + path for path in original_sys_path + if os.path.normcase(path) not in system_paths + ] + sys.path = original_sys_path + + # Second, add lib directories. + # ensuring .pth file are processed. + for path in {lib_dirs!r}: + assert not path in sys.path + site.addsitedir(path) ''' - ).format(purelib)) + ).format(system_sites=system_sites, lib_dirs=self._lib_dirs)) - return self.path + def __enter__(self): + self._save_env = { + name: os.environ.get(name, None) + for name in ('PATH', 'PYTHONNOUSERSITE', 'PYTHONPATH') + } + + path = self._bin_dirs[:] + old_path = self._save_env['PATH'] + if old_path: + path.extend(old_path.split(os.pathsep)) + + pythonpath = [self._site_dir] + + os.environ.update({ + 'PATH': os.pathsep.join(path), + 'PYTHONNOUSERSITE': '1', + 'PYTHONPATH': os.pathsep.join(pythonpath), + }) def __exit__(self, exc_type, exc_val, exc_tb): - def restore_var(varname, old_value): + for varname, old_value in self._save_env.items(): if old_value is None: os.environ.pop(varname, None) else: os.environ[varname] = old_value - restore_var('PATH', self.save_path) - restore_var('PYTHONPATH', self.save_pythonpath) - restore_var('PYTHONNOUSERSITE', self.save_nousersite) - def cleanup(self): self._temp_dir.cleanup() - def missing_requirements(self, reqs): - """Return a list of the requirements from reqs that are not present + def check_requirements(self, reqs): + """Return 2 sets: + - conflicting requirements: set of (installed, wanted) reqs tuples + - missing requirements: set of reqs """ - missing = [] - with self: - ws = WorkingSet(os.environ["PYTHONPATH"].split(os.pathsep)) + missing = set() + conflicting = set() + if reqs: + ws = WorkingSet(self._lib_dirs) for req in reqs: try: if ws.find(Requirement.parse(req)) is None: - missing.append(req) - except VersionConflict: - missing.append(req) - return missing - - def install_requirements(self, finder, requirements, message): + missing.add(req) + except VersionConflict as e: + conflicting.add((str(e.args[0].as_requirement()), + str(e.args[1]))) + return conflicting, missing + + def install_requirements(self, finder, requirements, prefix, message): + prefix = self._prefixes[prefix] + assert not prefix.setup + prefix.setup = True + if not requirements: + return args = [ sys.executable, os.path.dirname(pip_location), 'install', - '--ignore-installed', '--no-user', '--prefix', self.path, + '--ignore-installed', '--no-user', '--prefix', prefix.path, '--no-warn-script-location', ] if logger.getEffectiveLevel() <= logging.DEBUG: @@ -150,5 +197,5 @@ def __exit__(self, exc_type, exc_val, exc_tb): def cleanup(self): pass - def install_requirements(self, finder, requirements, message): + def install_requirements(self, finder, requirements, prefix, message): raise NotImplementedError() diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 104bea33b40..8812eba3fe8 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -100,26 +100,35 @@ def prep_for_dist(self, finder, build_isolation): self.req.load_pyproject_toml() should_isolate = self.req.use_pep517 and build_isolation + def _raise_conflicts(conflicting_with, conflicting_reqs): + raise InstallationError( + "Some build dependencies for %s conflict with %s: %s." % ( + self.req, conflicting_with, ', '.join( + '%s is incompatible with %s' % (installed, wanted) + for installed, wanted in sorted(conflicting)))) + if should_isolate: # Isolate in a BuildEnvironment and install the build-time # requirements. self.req.build_env = BuildEnvironment() self.req.build_env.install_requirements( - finder, self.req.pyproject_requires, + finder, self.req.pyproject_requires, 'overlay', "Installing build dependencies" ) - missing = [] - if self.req.requirements_to_check: - check = self.req.requirements_to_check - missing = self.req.build_env.missing_requirements(check) + conflicting, missing = self.req.build_env.check_requirements( + self.req.requirements_to_check + ) + if conflicting: + _raise_conflicts("PEP 517/518 supported requirements", + conflicting) if missing: logger.warning( "Missing build requirements in pyproject.toml for %s.", self.req, ) logger.warning( - "The project does not specify a build backend, and pip " - "cannot fall back to setuptools without %s.", + "The project does not specify a build backend, and " + "pip cannot fall back to setuptools without %s.", " and ".join(map(repr, sorted(missing))) ) diff --git a/tests/conftest.py b/tests/conftest.py index c86a91e3a08..8f30afef1f5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -56,14 +56,6 @@ def pytest_collection_modifyitems(config, items): item.add_marker(pytest.mark.integration) elif module_root_dir.startswith("unit"): item.add_marker(pytest.mark.unit) - - # We don't want to allow using the script resource if this is a - # unit test, as unit tests should not need all that heavy lifting - if set(getattr(item, "funcargnames", [])) & {"script"}: - raise RuntimeError( - "Cannot use the ``script`` funcarg in a unit test: " - "(filename = {}, item = {})".format(module_path, item) - ) else: raise RuntimeError( "Unknown test type (filename = {})".format(module_path) diff --git a/tests/data/src/pep518_conflicting_requires/MANIFEST.in b/tests/data/src/pep518_conflicting_requires/MANIFEST.in new file mode 100644 index 00000000000..bec201fc83b --- /dev/null +++ b/tests/data/src/pep518_conflicting_requires/MANIFEST.in @@ -0,0 +1 @@ +include pyproject.toml diff --git a/tests/data/src/pep518_conflicting_requires/pep518.py b/tests/data/src/pep518_conflicting_requires/pep518.py new file mode 100644 index 00000000000..7986d11379a --- /dev/null +++ b/tests/data/src/pep518_conflicting_requires/pep518.py @@ -0,0 +1 @@ +#dummy diff --git a/tests/data/src/pep518_conflicting_requires/pyproject.toml b/tests/data/src/pep518_conflicting_requires/pyproject.toml new file mode 100644 index 00000000000..e58132a6920 --- /dev/null +++ b/tests/data/src/pep518_conflicting_requires/pyproject.toml @@ -0,0 +1,2 @@ +[build-system] +requires = ["setuptools==1.0", "wheel"] diff --git a/tests/data/src/pep518_conflicting_requires/setup.py b/tests/data/src/pep518_conflicting_requires/setup.py new file mode 100644 index 00000000000..34bdc16b5aa --- /dev/null +++ b/tests/data/src/pep518_conflicting_requires/setup.py @@ -0,0 +1,8 @@ +#!/usr/bin/env python +from setuptools import setup + +setup( + name='pep518_conflicting_requires', + version='1.0.0', + py_modules=['pep518'], +) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 40504a183e9..930ca3873c9 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -12,9 +12,9 @@ from pip._internal.models.index import PyPI, TestPyPI from pip._internal.utils.misc import rmtree from tests.lib import ( - _create_svn_repo, _create_test_package, create_test_package_with_setup, - need_bzr, need_mercurial, path_to_url, pyversion, pyversion_tuple, - requirements_file, + _create_svn_repo, _create_test_package, create_basic_wheel_for_package, + create_test_package_with_setup, need_bzr, need_mercurial, path_to_url, + pyversion, pyversion_tuple, requirements_file, ) from tests.lib.local_repos import local_checkout from tests.lib.path import Path @@ -50,6 +50,20 @@ def test_pep518_build_env_uses_same_pip(script, data, pip_src, common_wheels): ) +def test_pep518_refuses_conflicting_requires(script, data): + create_basic_wheel_for_package(script, 'setuptools', '1.0') + create_basic_wheel_for_package(script, 'wheel', '1.0') + project_dir = data.src.join("pep518_conflicting_requires") + result = script.pip_install_local('-f', script.scratch_path, + project_dir, expect_error=True) + assert ( + result.returncode != 0 and + ('Some build dependencies for %s conflict with PEP 517/518 supported ' + 'requirements: setuptools==1.0 is incompatible with ' + 'setuptools>=38.2.5.' % path_to_url(project_dir)) in result.stderr + ), str(result) + + def test_pep518_refuses_invalid_requires(script, data, common_wheels): result = script.pip( 'install', '-f', common_wheels, diff --git a/tests/functional/test_pep517.py b/tests/functional/test_pep517.py index 3a689244226..77555daa155 100644 --- a/tests/functional/test_pep517.py +++ b/tests/functional/test_pep517.py @@ -22,8 +22,9 @@ def test_backend(tmpdir, data): req.load_pyproject_toml() env = BuildEnvironment() finder = PackageFinder([data.backends], [], session=PipSession()) - env.install_requirements(finder, ["dummy_backend"], "Installing") - assert not env.missing_requirements(["dummy_backend"]) + env.install_requirements(finder, ["dummy_backend"], 'normal', "Installing") + conflicting, missing = env.check_requirements(["dummy_backend"]) + assert not conflicting and not missing assert hasattr(req.pep517_backend, 'build_wheel') with env: assert req.pep517_backend.build_wheel("dir") == "Backend called" diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py index f62ecc5d6ef..472157aab3a 100644 --- a/tests/lib/__init__.py +++ b/tests/lib/__init__.py @@ -681,9 +681,15 @@ def create_test_package_with_setup(script, **setup_kwargs): return pkg_path -def create_basic_wheel_for_package(script, name, version, depends, extras): +def create_basic_wheel_for_package(script, name, version, + depends=None, extras=None): + if depends is None: + depends = [] + if extras is None: + extras = {} files = { "{name}/__init__.py": """ + __version__ = {version} def hello(): return "Hello From {name}" """, diff --git a/tests/unit/test_build_env.py b/tests/unit/test_build_env.py new file mode 100644 index 00000000000..ef975b3573f --- /dev/null +++ b/tests/unit/test_build_env.py @@ -0,0 +1,194 @@ +from textwrap import dedent + +import pytest + +from pip._internal.build_env import BuildEnvironment +from pip._internal.download import PipSession +from pip._internal.index import PackageFinder +from tests.lib import create_basic_wheel_for_package + + +def indent(text, prefix): + return '\n'.join((prefix if line else '') + line + for line in text.split('\n')) + + +def run_with_build_env(script, setup_script_contents, + test_script_contents=None): + build_env_script = script.scratch_path / 'build_env.py' + build_env_script.write( + dedent( + ''' + from __future__ import print_function + import subprocess + import sys + + from pip._internal.build_env import BuildEnvironment + from pip._internal.download import PipSession + from pip._internal.index import PackageFinder + + finder = PackageFinder([%r], [], session=PipSession()) + build_env = BuildEnvironment() + + try: + ''' % str(script.scratch_path)) + + indent(dedent(setup_script_contents), ' ') + + dedent( + ''' + if len(sys.argv) > 1: + with build_env: + subprocess.check_call((sys.executable, sys.argv[1])) + finally: + build_env.cleanup() + ''') + ) + args = ['python', build_env_script] + if test_script_contents is not None: + test_script = script.scratch_path / 'test.py' + test_script.write(dedent(test_script_contents)) + args.append(test_script) + return script.run(*args) + + +def test_build_env_allow_empty_requirements_install(): + build_env = BuildEnvironment() + for prefix in ('normal', 'overlay'): + build_env.install_requirements(None, [], prefix, None) + + +def test_build_env_allow_only_one_install(script): + create_basic_wheel_for_package(script, 'foo', '1.0') + create_basic_wheel_for_package(script, 'bar', '1.0') + finder = PackageFinder([script.scratch_path], [], session=PipSession()) + build_env = BuildEnvironment() + for prefix in ('normal', 'overlay'): + build_env.install_requirements(finder, ['foo'], prefix, + 'installing foo in %s' % prefix) + with pytest.raises(AssertionError): + build_env.install_requirements(finder, ['bar'], prefix, + 'installing bar in %s' % prefix) + with pytest.raises(AssertionError): + build_env.install_requirements(finder, [], prefix, + 'installing in %s' % prefix) + + +def test_build_env_requirements_check(script): + + create_basic_wheel_for_package(script, 'foo', '2.0') + create_basic_wheel_for_package(script, 'bar', '1.0') + create_basic_wheel_for_package(script, 'bar', '3.0') + create_basic_wheel_for_package(script, 'other', '0.5') + + script.pip_install_local('-f', script.scratch_path, 'foo', 'bar', 'other') + + run_with_build_env( + script, + ''' + r = build_env.check_requirements(['foo', 'bar', 'other']) + assert r == (set(), {'foo', 'bar', 'other'}), repr(r) + + r = build_env.check_requirements(['foo>1.0', 'bar==3.0']) + assert r == (set(), {'foo>1.0', 'bar==3.0'}), repr(r) + + r = build_env.check_requirements(['foo>3.0', 'bar>=2.5']) + assert r == (set(), {'foo>3.0', 'bar>=2.5'}), repr(r) + ''') + + run_with_build_env( + script, + ''' + build_env.install_requirements(finder, ['foo', 'bar==3.0'], 'normal', + 'installing foo in normal') + + r = build_env.check_requirements(['foo', 'bar', 'other']) + assert r == (set(), {'other'}), repr(r) + + r = build_env.check_requirements(['foo>1.0', 'bar==3.0']) + assert r == (set(), set()), repr(r) + + r = build_env.check_requirements(['foo>3.0', 'bar>=2.5']) + assert r == ({('foo==2.0', 'foo>3.0')}, set()), repr(r) + ''') + + run_with_build_env( + script, + ''' + build_env.install_requirements(finder, ['foo', 'bar==3.0'], 'normal', + 'installing foo in normal') + build_env.install_requirements(finder, ['bar==1.0'], 'overlay', + 'installing foo in overlay') + + r = build_env.check_requirements(['foo', 'bar', 'other']) + assert r == (set(), {'other'}), repr(r) + + r = build_env.check_requirements(['foo>1.0', 'bar==3.0']) + assert r == ({('bar==1.0', 'bar==3.0')}, set()), repr(r) + + r = build_env.check_requirements(['foo>3.0', 'bar>=2.5']) + assert r == ({('bar==1.0', 'bar>=2.5'), ('foo==2.0', 'foo>3.0')}, \ + set()), repr(r) + ''') + + +def test_build_env_overlay_prefix_has_priority(script): + create_basic_wheel_for_package(script, 'pkg', '2.0') + create_basic_wheel_for_package(script, 'pkg', '4.3') + result = run_with_build_env( + script, + ''' + build_env.install_requirements(finder, ['pkg==2.0'], 'overlay', + 'installing pkg==2.0 in overlay') + build_env.install_requirements(finder, ['pkg==4.3'], 'normal', + 'installing pkg==4.3 in normal') + ''', + ''' + from __future__ import print_function + + print(__import__('pkg').__version__) + ''') + assert result.stdout.strip() == '2.0', str(result) + + +def test_build_env_isolation(script): + + # Create dummy `pkg` wheel. + pkg_whl = create_basic_wheel_for_package(script, 'pkg', '1.0') + + # Install it to site packages. + script.pip_install_local(pkg_whl) + + # And a copy in the user site. + script.pip_install_local('--ignore-installed', '--user', pkg_whl) + + # And to another directory available through a .pth file. + target = script.scratch_path / 'pth_install' + script.pip_install_local('-t', target, pkg_whl) + (script.site_packages_path / 'build_requires.pth').write( + str(target) + '\n' + ) + + # And finally to yet another directory available through PYTHONPATH. + target = script.scratch_path / 'pypath_install' + script.pip_install_local('-t', target, pkg_whl) + script.environ["PYTHONPATH"] = target + + run_with_build_env( + script, '', + r''' + from __future__ import print_function + from distutils.sysconfig import get_python_lib + import sys + + try: + import pkg + except ImportError: + pass + else: + print('imported `pkg` from `%s`' % pkg.__file__, file=sys.stderr) + print('system sites:\n ' + '\n '.join(sorted({ + get_python_lib(plat_specific=0), + get_python_lib(plat_specific=1), + })), file=sys.stderr) + print('sys.path:\n ' + '\n '.join(sys.path), file=sys.stderr) + sys.exit(1) + ''') diff --git a/tools/travis/run.sh b/tools/travis/run.sh index 6f4c424e8ca..b76240db29c 100755 --- a/tools/travis/run.sh +++ b/tools/travis/run.sh @@ -41,7 +41,7 @@ echo "TOXENV=${TOXENV}" set -x if [[ "$GROUP" == "1" ]]; then # Unit tests - tox -- -m unit + tox -- --use-venv -m unit # Integration tests (not the ones for 'pip install') tox -- --use-venv -m integration -n 4 --duration=5 -k "not test_install" elif [[ "$GROUP" == "2" ]]; then From de4d5038f8c86b2dbcb3fe95d5f7c49424c6c8f5 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 16 Aug 2018 15:07:50 +0100 Subject: [PATCH 02/29] Phase 1 - build wheels using PEP 517 hook --- src/pip/_internal/wheel.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index b57ac7d20d2..7804dabd990 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -668,7 +668,11 @@ def _build_one(self, req, output_dir, python_tag=None): def _build_one_inside_env(self, req, output_dir, python_tag=None): with TempDirectory(kind="wheel") as temp_dir: - if self.__build_one(req, temp_dir.path, python_tag=python_tag): + if req.use_pep517: + builder = self._build_one_pep517 + else: + builder = self._build_one_legacy + if builder(req, temp_dir.path, python_tag=python_tag): try: wheel_name = os.listdir(temp_dir.path)[0] wheel_path = os.path.join(output_dir, wheel_name) @@ -693,7 +697,25 @@ def _base_setup_args(self, req): SETUPTOOLS_SHIM % req.setup_py ] + list(self.global_options) - def __build_one(self, req, tempd, python_tag=None): + def _build_one_pep517(self, req, tempd, python_tag=None): + # TODO: Cannot support python_tag + spin_message = 'Running PEP 517 build_wheel for %s' % (req.name,) + with open_spinner(spin_message) as spinner: + logger.debug('Destination directory: %s', tempd) + # assert req.metadata_directory is not None + try: + req.pep517_backend.build_wheel( + tempd, + # metadata_directory=req.metadata_directory + metadata_directory=None + ) + return True + except Exception: + spinner.finish("error") + logger.error('Failed building wheel for %s', req.name) + return False + + def _build_one_legacy(self, req, tempd, python_tag=None): base_args = self._base_setup_args(req) spin_message = 'Running setup.py bdist_wheel for %s' % (req.name,) From 14f35f91f5335c0e092bf943f4727e70c8c47df8 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 17 Aug 2018 11:38:04 +0100 Subject: [PATCH 03/29] Experimental fix to pep517 to use pip's subprocess caller --- src/pip/_internal/wheel.py | 19 ++++++++++++++----- src/pip/_vendor/pep517/wrappers.py | 28 +++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 7804dabd990..8361c086093 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -704,11 +704,20 @@ def _build_one_pep517(self, req, tempd, python_tag=None): logger.debug('Destination directory: %s', tempd) # assert req.metadata_directory is not None try: - req.pep517_backend.build_wheel( - tempd, - # metadata_directory=req.metadata_directory - metadata_directory=None - ) + def runner(cmd, cwd=None, extra_environ=None): + call_subprocess( + cmd, + cwd=cwd, + extra_environ=extra_environ, + show_stdout=False, + spinner=spinner + ) + with req.pep517_backend.subprocess_runner(runner): + req.pep517_backend.build_wheel( + tempd, + # metadata_directory=req.metadata_directory + metadata_directory=None + ) return True except Exception: spinner.finish("error") diff --git a/src/pip/_vendor/pep517/wrappers.py b/src/pip/_vendor/pep517/wrappers.py index 28260f320dd..91f2ac2c29a 100644 --- a/src/pip/_vendor/pep517/wrappers.py +++ b/src/pip/_vendor/pep517/wrappers.py @@ -21,6 +21,14 @@ def tempdir(): class UnsupportedOperation(Exception): """May be raised by build_sdist if the backend indicates that it can't.""" +def default_subprocess_runner(cmd, cwd=None, extra_environ=None): + """The default method of calling the wrapper subprocess.""" + env = os.environ.copy() + if extra_environ: + env.update(extra_environ) + + check_call(cmd, cwd=cwd, env=env) + class Pep517HookCaller(object): """A wrapper around a source directory to be built with a PEP 517 backend. @@ -30,6 +38,16 @@ class Pep517HookCaller(object): def __init__(self, source_dir, build_backend): self.source_dir = abspath(source_dir) self.build_backend = build_backend + self._subprocess_runner = default_subprocess_runner + + # TODO: Is this over-engineered? Maybe frontends only need to + # set this when creating the wrapper, not on every call. + @contextmanager + def subprocess_runner(self, runner): + prev = self._subprocess_runner + self._subprocess_runner = runner + yield + self._subprocess_runner = prev def get_requires_for_build_wheel(self, config_settings=None): """Identify packages required for building a wheel @@ -105,8 +123,6 @@ def build_sdist(self, sdist_directory, config_settings=None): def _call_hook(self, hook_name, kwargs): - env = os.environ.copy() - # On Python 2, pytoml returns Unicode values (which is correct) but the # environment passed to check_call needs to contain string values. We # convert here by encoding using ASCII (the backend can only contain @@ -118,14 +134,16 @@ def _call_hook(self, hook_name, kwargs): else: build_backend = self.build_backend - env['PEP517_BUILD_BACKEND'] = build_backend with tempdir() as td: compat.write_json({'kwargs': kwargs}, pjoin(td, 'input.json'), indent=2) # Run the hook in a subprocess - check_call([sys.executable, _in_proc_script, hook_name, td], - cwd=self.source_dir, env=env) + self._subprocess_runner( + [sys.executable, _in_proc_script, hook_name, td], + cwd=self.source_dir, + extra_environ={'PEP517_BUILD_BACKEND': build_backend} + ) data = compat.read_json(pjoin(td, 'output.json')) if data.get('unsupported'): From 8fbf78d407543753da938874dcd0b836eb8f47fb Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 20 Aug 2018 13:37:36 +0100 Subject: [PATCH 04/29] Phase 2 - generate metadata using PEP 517 hook --- src/pip/_internal/commands/install.py | 18 ++++ src/pip/_internal/operations/prepare.py | 2 +- src/pip/_internal/req/req_install.py | 115 ++++++++++++++++++------ src/pip/_internal/wheel.py | 5 +- tests/unit/test_req.py | 2 +- 5 files changed, 108 insertions(+), 34 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 49a488730b7..afe0ded7056 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -336,6 +336,24 @@ def run(self, options, args): session=session, autobuilding=True ) + # If we're using PEP 517, we cannot do a direct install + # so we fail here. + # TODO: Technically, if it's a setuptools-based project + # we could fall back to setup.py install even if we've + # been assuming PEP 517 to this point, but that would be + # complicated to achieve, as none of the legacy setup has + # been done. Better to get the user to specify + # --no-use-pep517. + failed_builds = [ + r for r in requirement_set.requirements.values() + if r.use_pep517 and not r.is_wheel + ] + + if failed_builds: + raise InstallationError( + "Could not build wheels for {}".format( + failed_builds)) + to_install = resolver.get_installation_order( requirement_set ) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 8812eba3fe8..6aa30d998fc 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -132,7 +132,7 @@ def _raise_conflicts(conflicting_with, conflicting_reqs): " and ".join(map(repr, sorted(missing))) ) - self.req.run_egg_info() + self.req.prepare_metadata() self.req.assert_source_matches_version() diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index aa4bb559bd9..7033e96eba5 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -107,6 +107,12 @@ def __init__(self, req, comes_from, source_dir=None, editable=False, self.isolated = isolated self.build_env = NoOpBuildEnvironment() + # For PEP 517, the directory where we request the project metadata + # gets stored. We need this to pass to build_wheel, so the backend + # can ensure that the wheel matches the metadata (see the PEP for + # details). + self.metadata_directory = None + # The static build requirements (from pyproject.toml) self.pyproject_requires = None @@ -311,6 +317,14 @@ def _correct_build_location(self): self.source_dir = os.path.normpath(os.path.abspath(new_location)) self._egg_info_path = None + # Correct the metadata directory, if it exists + if self.metadata_directory: + old_meta = self.metadata_directory + rel = os.path.relpath(old_meta, start=old_location) + new_meta = os.path.join(new_location, rel) + new_meta = os.path.normpath(os.path.abspath(new_meta)) + self.metadata_directory = new_meta + def remove_temporary_source(self): """Remove the source files from this requirement, if they are marked for deletion""" @@ -437,7 +451,12 @@ def load_pyproject_toml(self): self.pyproject_requires = requires self.pep517_backend = Pep517HookCaller(self.setup_py_dir, backend) - def run_egg_info(self): + def prepare_metadata(self): + """Ensure that project metadata is available. + + Under PEP 517, call the backend hook to prepare the metadata. + Under legacy processing, call setup.py egg-info. + """ assert self.source_dir if self.name: logger.debug( @@ -451,26 +470,10 @@ def run_egg_info(self): ) with indent_log(): - script = SETUPTOOLS_SHIM % self.setup_py - base_cmd = [sys.executable, '-c', script] - if self.isolated: - base_cmd += ["--no-user-cfg"] - egg_info_cmd = base_cmd + ['egg_info'] - # We can't put the .egg-info files at the root, because then the - # source code will be mistaken for an installed egg, causing - # problems - if self.editable: - egg_base_option = [] + if self.use_pep517: + self.prepare_pep517_metadata() else: - egg_info_dir = os.path.join(self.setup_py_dir, 'pip-egg-info') - ensure_dir(egg_info_dir) - egg_base_option = ['--egg-base', 'pip-egg-info'] - with self.build_env: - call_subprocess( - egg_info_cmd + egg_base_option, - cwd=self.setup_py_dir, - show_stdout=False, - command_desc='python setup.py egg_info') + self.run_egg_info() if not self.req: if isinstance(parse_version(self.metadata["Version"]), Version): @@ -489,13 +492,56 @@ def run_egg_info(self): metadata_name = canonicalize_name(self.metadata["Name"]) if canonicalize_name(self.req.name) != metadata_name: logger.warning( - 'Running setup.py (path:%s) egg_info for package %s ' + 'Generating metadata for package %s ' 'produced metadata for project name %s. Fix your ' '#egg=%s fragments.', - self.setup_py, self.name, metadata_name, self.name + self.name, metadata_name, self.name ) self.req = Requirement(metadata_name) + def prepare_pep517_metadata(self): + assert self.pep517_backend is not None + + metadata_dir = os.path.join( + self.setup_py_dir, + 'pip-wheel-metadata' + ) + ensure_dir(metadata_dir) + + with self.build_env: + # Note that Pep517HookCaller implements a fallback for + # prepare_metadata_for_build_wheel, so we don't have to + # consider the possibility that this hook doesn't exist. + backend = self.pep517_backend + distinfo_dir = backend.prepare_metadata_for_build_wheel( + metadata_dir + ) + + self.metadata_directory = os.path.join(metadata_dir, distinfo_dir) + + def run_egg_info(self): + script = SETUPTOOLS_SHIM % self.setup_py + base_cmd = [sys.executable, '-c', script] + if self.isolated: + base_cmd += ["--no-user-cfg"] + egg_info_cmd = base_cmd + ['egg_info'] + # We can't put the .egg-info files at the root, because then the + # source code will be mistaken for an installed egg, causing + # problems + if self.editable: + egg_base_option = [] + else: + egg_info_dir = os.path.join(self.setup_py_dir, 'pip-egg-info') + ensure_dir(egg_info_dir) + egg_base_option = ['--egg-base', 'pip-egg-info'] + with self.build_env: + call_subprocess( + egg_info_cmd + egg_base_option, + cwd=self.setup_py_dir, + show_stdout=False, + command_desc='python setup.py egg_info') + + @property def egg_info_path(self): if self._egg_info_path is None: @@ -556,13 +602,23 @@ def metadata(self): return self._metadata def get_dist(self): - """Return a pkg_resources.Distribution built from self.egg_info_path""" - egg_info = self.egg_info_path.rstrip(os.path.sep) - base_dir = os.path.dirname(egg_info) - metadata = pkg_resources.PathMetadata(base_dir, egg_info) - dist_name = os.path.splitext(os.path.basename(egg_info))[0] - return pkg_resources.Distribution( - os.path.dirname(egg_info), + """Return a pkg_resources.Distribution for this requirement""" + if self.metadata_directory: + base_dir, distinfo = os.path.split(self.metadata_directory) + metadata = pkg_resources.PathMetadata( + base_dir, self.metadata_directory + ) + dist_name = os.path.splitext(distinfo)[0] + typ = pkg_resources.DistInfoDistribution + else: + egg_info = self.egg_info_path.rstrip(os.path.sep) + base_dir = os.path.dirname(egg_info) + metadata = pkg_resources.PathMetadata(base_dir, egg_info) + dist_name = os.path.splitext(os.path.basename(egg_info))[0] + typ = pkg_resources.Distribution + + return typ( + base_dir, project_name=dist_name, metadata=metadata, ) @@ -693,6 +749,7 @@ def _clean_zip_name(self, name, prefix): # only used by archive. # TODO: Investigate if this should be kept in InstallRequirement # Seems to be used only when VCS + downloads + # TODO: Consider PEP 517 implications def archive(self, build_dir): assert self.source_dir create_archive = True diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 8361c086093..3997e06d872 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -702,7 +702,7 @@ def _build_one_pep517(self, req, tempd, python_tag=None): spin_message = 'Running PEP 517 build_wheel for %s' % (req.name,) with open_spinner(spin_message) as spinner: logger.debug('Destination directory: %s', tempd) - # assert req.metadata_directory is not None + assert req.metadata_directory is not None try: def runner(cmd, cwd=None, extra_environ=None): call_subprocess( @@ -715,8 +715,7 @@ def runner(cmd, cwd=None, extra_environ=None): with req.pep517_backend.subprocess_runner(runner): req.pep517_backend.build_wheel( tempd, - # metadata_directory=req.metadata_directory - metadata_directory=None + metadata_directory=req.metadata_directory ) return True except Exception: diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 10a20b7ac50..570170cc523 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -630,7 +630,7 @@ def test_mismatched_versions(caplog, tmpdir): shutil.copytree(original_source, source_dir) req = InstallRequirement(req=Requirement('simplewheel==2.0'), comes_from=None, source_dir=source_dir) - req.run_egg_info() + req.prepare_metadata() req.assert_source_matches_version() assert caplog.records[-1].message == ( 'Requested simplewheel==2.0, ' From 4de291511315cca95c7cc5fef790d90294eb2fbd Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 24 Aug 2018 09:42:12 +0100 Subject: [PATCH 05/29] Update required setuptools version for PEP 517 --- src/pip/_internal/pyproject.py | 4 ++-- tests/functional/test_install.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index f938a763da3..d4027d2980e 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -88,7 +88,7 @@ def load_pyproject_toml(use_pep517, pyproject_toml, setup_py, req_name): # assume the setuptools backend, and require wheel and a version # of setuptools that supports that backend. build_system = { - "requires": ["setuptools>=38.2.5", "wheel"], + "requires": ["setuptools>=40.2.0", "wheel"], "build-backend": "setuptools.build_meta", } @@ -139,6 +139,6 @@ def load_pyproject_toml(use_pep517, pyproject_toml, setup_py, req_name): # 518 code had a similar check (but implemented in a different # way). backend = "setuptools.build_meta" - check = ["setuptools>=38.2.5", "wheel"] + check = ["setuptools>=40.2.0", "wheel"] return (requires, backend, check) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 930ca3873c9..e836a9988ff 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -60,7 +60,7 @@ def test_pep518_refuses_conflicting_requires(script, data): result.returncode != 0 and ('Some build dependencies for %s conflict with PEP 517/518 supported ' 'requirements: setuptools==1.0 is incompatible with ' - 'setuptools>=38.2.5.' % path_to_url(project_dir)) in result.stderr + 'setuptools>=40.2.0.' % path_to_url(project_dir)) in result.stderr ), str(result) From b9e92a70fd3256d06af155468796b1602108e961 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 27 Aug 2018 15:07:06 +0100 Subject: [PATCH 06/29] With build isolation, we shouldn't check if wheel is installed to decide if we build wheels --- src/pip/_internal/commands/install.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index afe0ded7056..f79c96487c6 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -30,11 +30,6 @@ from pip._internal.utils.temp_dir import TempDirectory from pip._internal.wheel import WheelBuilder -try: - import wheel -except ImportError: - wheel = None - logger = logging.getLogger(__name__) @@ -321,9 +316,8 @@ def run(self, options, args): modifying_pip=requirement_set.has_requirement("pip") ) - # If caching is disabled or wheel is not installed don't - # try to build wheels. - if wheel and options.cache_dir: + # If caching is disabled don't try to build wheels. + if options.cache_dir: # build wheels before install. wb = WheelBuilder( finder, preparer, wheel_cache, From b62284a81ba5baf73d423ec08c1ad45d1a9d761d Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Mon, 27 Aug 2018 16:38:56 +0100 Subject: [PATCH 07/29] Build PEP 517 and legacy wheels separately --- src/pip/_internal/commands/install.py | 61 ++++++++++++++++----------- src/pip/_internal/commands/wheel.py | 4 +- src/pip/_internal/req/req_install.py | 1 - src/pip/_internal/wheel.py | 6 ++- 4 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index f79c96487c6..bfa74f011fb 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -316,37 +316,50 @@ def run(self, options, args): modifying_pip=requirement_set.has_requirement("pip") ) - # If caching is disabled don't try to build wheels. - if options.cache_dir: - # build wheels before install. - wb = WheelBuilder( - finder, preparer, wheel_cache, - build_options=[], global_options=[], - ) - # Ignore the result: a failed wheel will be - # installed from the sdist/vcs whatever. + # Consider legacy and PEP517-using requirements separately + legacy_requirements = [] + pep517_requirements = [] + for req in requirement_set.requirements.values(): + if req.use_pep517: + pep517_requirements.append(req) + else: + legacy_requirements.append(req) + + # We don't build wheels for legacy requirements if we + # don't have wheel installed or we don't have a cache dir + try: + import wheel # noqa: F401 + build_legacy = bool(options.cache_dir) + except ImportError: + build_legacy = False + + wb = WheelBuilder( + finder, preparer, wheel_cache, + build_options=[], global_options=[], + ) + + # Always build PEP 517 requirements + build_failures = wb.build( + pep517_requirements, + session=session, autobuilding=True + ) + + if build_legacy: + # We don't care about failures building legacy + # requirements, as we'll fall through to a direct + # install for those. wb.build( - requirement_set.requirements.values(), + pep517_requirements, session=session, autobuilding=True ) # If we're using PEP 517, we cannot do a direct install # so we fail here. - # TODO: Technically, if it's a setuptools-based project - # we could fall back to setup.py install even if we've - # been assuming PEP 517 to this point, but that would be - # complicated to achieve, as none of the legacy setup has - # been done. Better to get the user to specify - # --no-use-pep517. - failed_builds = [ - r for r in requirement_set.requirements.values() - if r.use_pep517 and not r.is_wheel - ] - - if failed_builds: + if build_failures: raise InstallationError( - "Could not build wheels for {}".format( - failed_builds)) + "Could not build wheels for {} which use" + + " PEP 517 and cannot be installed directly".format( + ", ".join(r.name for r in build_failures))) to_install = resolver.get_installation_order( requirement_set diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index 9c1f1497fa4..6dd07a78575 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -167,10 +167,10 @@ def run(self, options, args): global_options=options.global_options or [], no_clean=options.no_clean, ) - wheels_built_successfully = wb.build( + build_failures = wb.build( requirement_set.requirements.values(), session=session, ) - if not wheels_built_successfully: + if len(build_failures) != 0: raise CommandError( "Failed to build one or more wheels" ) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 7033e96eba5..9b6c86b77ec 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -541,7 +541,6 @@ def run_egg_info(self): show_stdout=False, command_desc='python setup.py egg_info') - @property def egg_info_path(self): if self._egg_info_path is None: diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 3997e06d872..12f05cf7b4b 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -766,6 +766,8 @@ def build(self, requirements, session, autobuilding=False): from pip._internal import index from pip._internal.models.link import Link + # TODO: This check fails if --no-cache-dir is set. And yet we + # might be able to build into the ephemeral cache, surely? building_is_possible = self._wheel_dir or ( autobuilding and self.wheel_cache.cache_dir ) @@ -806,7 +808,7 @@ def build(self, requirements, session, autobuilding=False): buildset.append((req, ephem_cache)) if not buildset: - return True + return [] # Build the wheels. logger.info( @@ -879,4 +881,4 @@ def build(self, requirements, session, autobuilding=False): ' '.join([req.name for req in build_failure]), ) # Return True if all builds were successful - return len(build_failure) == 0 + return build_failure From 48e9cb693ffa3b1009d8355ebdac8c31daa352b0 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 28 Aug 2018 09:50:19 +0100 Subject: [PATCH 08/29] Address test failures --- src/pip/_internal/commands/install.py | 1 - tests/conftest.py | 2 +- tests/functional/test_install.py | 5 ++++- tests/functional/test_install_reqs.py | 1 + tests/functional/test_install_user.py | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index bfa74f011fb..86f36a12360 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -30,7 +30,6 @@ from pip._internal.utils.temp_dir import TempDirectory from pip._internal.wheel import WheelBuilder - logger = logging.getLogger(__name__) diff --git a/tests/conftest.py b/tests/conftest.py index 8f30afef1f5..4ee4ccea867 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -172,7 +172,7 @@ def pip_src(tmpdir_factory): SRC_DIR, pip_src.abspath, ignore=shutil.ignore_patterns( - "*.pyc", "__pycache__", "contrib", "docs", "tasks", "*.txt", + "*.pyc", "__pycache__", "contrib", "docs", "tasks", "tests", "pip.egg-info", "build", "dist", ".tox", ".git", ), ) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index e836a9988ff..e6d70664e73 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -1117,6 +1117,7 @@ def test_install_wheel_broken(script, with_wheel): assert "Successfully installed wheelbroken-0.1" in str(res), str(res) +@pytest.mark.xfail(reason="Test depends on exact output from setuptools") def test_cleanup_after_failed_wheel(script, with_wheel): res = script.pip_install_local('wheelbrokenafter', expect_stderr=True) # One of the effects of not cleaning up is broken scripts: @@ -1128,6 +1129,7 @@ def test_cleanup_after_failed_wheel(script, with_wheel): assert "Running setup.py clean for wheelbrokenafter" in str(res), str(res) +@pytest.mark.xfail(reason="Test depends on exact output from setuptools") def test_install_builds_wheels(script, data, with_wheel): # We need to use a subprocess to get the right value on Windows. res = script.run('python', '-c', ( @@ -1169,6 +1171,7 @@ def test_install_builds_wheels(script, data, with_wheel): ] +@pytest.mark.xfail(reason="Test depends on exact output from setuptools") def test_install_no_binary_disables_building_wheels(script, data, with_wheel): to_install = data.packages.join('requires_wheelbroken_upper') res = script.pip( @@ -1218,7 +1221,7 @@ def test_install_editable_with_wrong_egg_name(script): result = script.pip( 'install', '--editable', 'file://%s#egg=pkgb' % pkga_path, expect_error=True) - assert ("egg_info for package pkgb produced metadata " + assert ("Generating metadata for package pkgb produced metadata " "for project name pkga. Fix your #egg=pkgb " "fragments.") in result.stderr assert "Successfully installed pkga" in str(result), str(result) diff --git a/tests/functional/test_install_reqs.py b/tests/functional/test_install_reqs.py index ff000fcd0c9..cb4efd900a9 100644 --- a/tests/functional/test_install_reqs.py +++ b/tests/functional/test_install_reqs.py @@ -223,6 +223,7 @@ def test_install_local_with_subdirectory(script): result.assert_installed('version_subpkg.py', editable=False) +@pytest.mark.xfail(reason="Test depends on exact output from setuptools") def test_wheel_user_with_prefix_in_pydistutils_cfg( script, data, with_wheel): if os.name == 'posix': diff --git a/tests/functional/test_install_user.py b/tests/functional/test_install_user.py index 17d09768ba7..f52b0172796 100644 --- a/tests/functional/test_install_user.py +++ b/tests/functional/test_install_user.py @@ -75,7 +75,7 @@ def test_install_from_current_directory_into_usersite( dist_info_folder = ( script.user_site / 'FSPkg-0.1.dev0.dist-info' ) - assert dist_info_folder in result.files_created + assert dist_info_folder in result.files_created, result.files_created @pytest.mark.incompatible_with_test_venv def test_install_user_venv_nositepkgs_fails(self, virtualenv, From ab3e21635a54ce974e19a19f0dcabd19f5923840 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 28 Aug 2018 10:19:46 +0100 Subject: [PATCH 09/29] Add a news file --- news/5743.feature | 1 + src/pip/_internal/wheel.py | 4 ++-- tests/functional/test_install.py | 23 ++++++++++------------- tests/functional/test_install_reqs.py | 1 - tests/functional/test_install_user.py | 2 +- 5 files changed, 14 insertions(+), 17 deletions(-) create mode 100644 news/5743.feature diff --git a/news/5743.feature b/news/5743.feature new file mode 100644 index 00000000000..1181b347955 --- /dev/null +++ b/news/5743.feature @@ -0,0 +1 @@ +Implement PEP 517 (allow projects to specify a build backend via pyproject.toml). diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 12f05cf7b4b..097fcaf7b81 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -699,7 +699,7 @@ def _base_setup_args(self, req): def _build_one_pep517(self, req, tempd, python_tag=None): # TODO: Cannot support python_tag - spin_message = 'Running PEP 517 build_wheel for %s' % (req.name,) + spin_message = 'Building wheel for %s (PEP 517)' % (req.name,) with open_spinner(spin_message) as spinner: logger.debug('Destination directory: %s', tempd) assert req.metadata_directory is not None @@ -726,7 +726,7 @@ def runner(cmd, cwd=None, extra_environ=None): def _build_one_legacy(self, req, tempd, python_tag=None): base_args = self._base_setup_args(req) - spin_message = 'Running setup.py bdist_wheel for %s' % (req.name,) + spin_message = 'Building wheel for %s (setup.py)' % (req.name,) with open_spinner(spin_message) as spinner: logger.debug('Destination directory: %s', tempd) wheel_args = base_args + ['bdist_wheel', '-d', tempd] \ diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index e6d70664e73..b743791a76b 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -1117,7 +1117,6 @@ def test_install_wheel_broken(script, with_wheel): assert "Successfully installed wheelbroken-0.1" in str(res), str(res) -@pytest.mark.xfail(reason="Test depends on exact output from setuptools") def test_cleanup_after_failed_wheel(script, with_wheel): res = script.pip_install_local('wheelbrokenafter', expect_stderr=True) # One of the effects of not cleaning up is broken scripts: @@ -1129,7 +1128,6 @@ def test_cleanup_after_failed_wheel(script, with_wheel): assert "Running setup.py clean for wheelbrokenafter" in str(res), str(res) -@pytest.mark.xfail(reason="Test depends on exact output from setuptools") def test_install_builds_wheels(script, data, with_wheel): # We need to use a subprocess to get the right value on Windows. res = script.run('python', '-c', ( @@ -1152,10 +1150,10 @@ def test_install_builds_wheels(script, data, with_wheel): for top, dirs, files in os.walk(wheels_cache): wheels.extend(files) # and built wheels for upper and wheelbroken - assert "Running setup.py bdist_wheel for upper" in str(res), str(res) - assert "Running setup.py bdist_wheel for wheelb" in str(res), str(res) + assert "Building wheel for upper" in str(res), str(res) + assert "Building wheel for wheelb" in str(res), str(res) # Wheels are built for local directories, but not cached. - assert "Running setup.py bdist_wheel for requir" in str(res), str(res) + assert "Building wheel for requir" in str(res), str(res) # wheelbroken has to run install # into the cache assert wheels != [], str(res) @@ -1171,7 +1169,6 @@ def test_install_builds_wheels(script, data, with_wheel): ] -@pytest.mark.xfail(reason="Test depends on exact output from setuptools") def test_install_no_binary_disables_building_wheels(script, data, with_wheel): to_install = data.packages.join('requires_wheelbroken_upper') res = script.pip( @@ -1182,16 +1179,16 @@ def test_install_no_binary_disables_building_wheels(script, data, with_wheel): # Must have installed it all assert expected in str(res), str(res) # and built wheels for wheelbroken only - assert "Running setup.py bdist_wheel for wheelb" in str(res), str(res) + assert "Building wheel for wheelb" in str(res), str(res) # Wheels are built for local directories, but not cached across runs - assert "Running setup.py bdist_wheel for requir" in str(res), str(res) + assert "Building wheel for requir" in str(res), str(res) # Don't build wheel for upper which was blacklisted - assert "Running setup.py bdist_wheel for upper" not in str(res), str(res) + assert "Building wheel for upper" not in str(res), str(res) # Wheels are built for local directories, but not cached across runs - assert "Running setup.py install for requir" not in str(res), str(res) + assert "Building wheel for requir" not in str(res), str(res) # And these two fell back to sdist based installed. - assert "Running setup.py install for wheelb" in str(res), str(res) - assert "Running setup.py install for upper" in str(res), str(res) + assert "Building wheel for wheelb" in str(res), str(res) + assert "Building wheel for upper" in str(res), str(res) def test_install_no_binary_disables_cached_wheels(script, data, with_wheel): @@ -1205,7 +1202,7 @@ def test_install_no_binary_disables_cached_wheels(script, data, with_wheel): 'upper', expect_stderr=True) assert "Successfully installed upper-2.0" in str(res), str(res) # No wheel building for upper, which was blacklisted - assert "Running setup.py bdist_wheel for upper" not in str(res), str(res) + assert "Building wheel for upper" not in str(res), str(res) # Must have used source, not a cached wheel to install upper. assert "Running setup.py install for upper" in str(res), str(res) diff --git a/tests/functional/test_install_reqs.py b/tests/functional/test_install_reqs.py index cb4efd900a9..ff000fcd0c9 100644 --- a/tests/functional/test_install_reqs.py +++ b/tests/functional/test_install_reqs.py @@ -223,7 +223,6 @@ def test_install_local_with_subdirectory(script): result.assert_installed('version_subpkg.py', editable=False) -@pytest.mark.xfail(reason="Test depends on exact output from setuptools") def test_wheel_user_with_prefix_in_pydistutils_cfg( script, data, with_wheel): if os.name == 'posix': diff --git a/tests/functional/test_install_user.py b/tests/functional/test_install_user.py index f52b0172796..17d09768ba7 100644 --- a/tests/functional/test_install_user.py +++ b/tests/functional/test_install_user.py @@ -75,7 +75,7 @@ def test_install_from_current_directory_into_usersite( dist_info_folder = ( script.user_site / 'FSPkg-0.1.dev0.dist-info' ) - assert dist_info_folder in result.files_created, result.files_created + assert dist_info_folder in result.files_created @pytest.mark.incompatible_with_test_venv def test_install_user_venv_nositepkgs_fails(self, virtualenv, From a82b7ce5e70a97ba512c382a6365ecacfe97f8bc Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 28 Aug 2018 11:31:33 +0100 Subject: [PATCH 10/29] Fix test_pep518_with_user_pip which was getting errors due to irrelevant changes in setuptools output --- tests/functional/test_install.py | 15 ++++++++++++++- tests/functional/test_install_config.py | 2 +- tests/functional/test_wheel.py | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index b743791a76b..aae296efb47 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -101,7 +101,20 @@ def test_pep518_allows_missing_requires(script, data, common_wheels): def test_pep518_with_user_pip(script, pip_src, data, common_wheels): - script.pip("install", "--ignore-installed", "--user", pip_src) + """ + Check that build dependencies are installed into the build + environment without using build isolation for the pip invocation. + + To ensure that we're not using build isolation when installing + the build dependencies, we install a user copy of pip in the + non-isolated environment, and break pip in the system site-packages, + so that isolated uses of pip will fail. + """ + # Set expect_stderr, not so much because we expect output on stderr, + # rather we simply don't care (setuptools can write warnings to stderr + # which we'll ignore). + script.pip("install", "--ignore-installed", "--user", pip_src, + expect_stderr=True) system_pip_dir = script.site_packages_path / 'pip' system_pip_dir.rmtree() system_pip_dir.mkdir() diff --git a/tests/functional/test_install_config.py b/tests/functional/test_install_config.py index e4ad7e9f23c..cf405ca3f13 100644 --- a/tests/functional/test_install_config.py +++ b/tests/functional/test_install_config.py @@ -216,6 +216,6 @@ def test_install_no_binary_via_config_disables_cached_wheels( os.unlink(config_file.name) assert "Successfully installed upper-2.0" in str(res), str(res) # No wheel building for upper, which was blacklisted - assert "Running setup.py bdist_wheel for upper" not in str(res), str(res) + assert "Building wheel for upper" not in str(res), str(res) # Must have used source, not a cached wheel to install upper. assert "Running setup.py install for upper" in str(res), str(res) diff --git a/tests/functional/test_wheel.py b/tests/functional/test_wheel.py index 0e1e2e54ad6..7a64345051d 100644 --- a/tests/functional/test_wheel.py +++ b/tests/functional/test_wheel.py @@ -65,7 +65,7 @@ def test_pip_wheel_builds_when_no_binary_set(script, data): 'wheel', '--no-index', '--no-binary', ':all:', '-f', data.find_links, 'simple==3.0') - assert "Running setup.py bdist_wheel for simple" in str(res), str(res) + assert "Building wheel for simple" in str(res), str(res) def test_pip_wheel_builds_editable_deps(script, data): From 4281bf8e6162dedec46956be7d1cb6612a586429 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 28 Aug 2018 12:22:44 +0100 Subject: [PATCH 11/29] Correct an out of date comment --- src/pip/_internal/wheel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 097fcaf7b81..138d782c79f 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -880,5 +880,5 @@ def build(self, requirements, session, autobuilding=False): 'Failed to build %s', ' '.join([req.name for req in build_failure]), ) - # Return True if all builds were successful + # Return a list of requirements that failed to build return build_failure From c8d8e37ea4ea17303c0c170092ad034f5701a6dd Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 7 Sep 2018 22:02:42 +0100 Subject: [PATCH 12/29] Fix copy and paste error --- src/pip/_internal/commands/install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 86f36a12360..9ac2e15c811 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -348,7 +348,7 @@ def run(self, options, args): # requirements, as we'll fall through to a direct # install for those. wb.build( - pep517_requirements, + legacy_requirements, session=session, autobuilding=True ) From c0ed4381a65cccb8af1d41c0774b0d370d605752 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 7 Sep 2018 22:27:21 +0100 Subject: [PATCH 13/29] Fix for test_install_no_binary_disables_building_wheels --- tests/functional/test_install.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index aae296efb47..7b6f529c02f 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -1198,10 +1198,10 @@ def test_install_no_binary_disables_building_wheels(script, data, with_wheel): # Don't build wheel for upper which was blacklisted assert "Building wheel for upper" not in str(res), str(res) # Wheels are built for local directories, but not cached across runs - assert "Building wheel for requir" not in str(res), str(res) + assert "Running setup.py install for requir" not in str(res), str(res) # And these two fell back to sdist based installed. - assert "Building wheel for wheelb" in str(res), str(res) - assert "Building wheel for upper" in str(res), str(res) + assert "Running setup.py install for wheelb" in str(res), str(res) + assert "Running setup.py install for upper" in str(res), str(res) def test_install_no_binary_disables_cached_wheels(script, data, with_wheel): From 41b07c9fb5b4416ea2560c6c31f09abd434e3ca4 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 9 Oct 2018 14:00:39 +0100 Subject: [PATCH 14/29] Include backend-provided requirements in build environment --- src/pip/_internal/operations/prepare.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 6aa30d998fc..1ea58b87632 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -131,6 +131,19 @@ def _raise_conflicts(conflicting_with, conflicting_reqs): "pip cannot fall back to setuptools without %s.", " and ".join(map(repr, sorted(missing))) ) + # Install any extra build dependencies that the backend requests. + # This must be done in a second pass, as the pyproject.toml + # dependencies must be installed before we can call the backend. + with self.req.build_env: + # We need to have the env active when calling the hook. + reqs = self.req.pep517_backend.get_requires_for_build_wheel() + conflicting, missing = self.req.build_env.check_requirements(reqs) + if conflicting: + _raise_conflicts("the backend dependencies", conflicting) + self.req.build_env.install_requirements( + finder, missing, 'normal', + "Installing backend dependencies" + ) self.req.prepare_metadata() self.req.assert_source_matches_version() From 9d2b17854db108e7295ab41512506ba7ec5df3c3 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 9 Oct 2018 15:18:00 +0100 Subject: [PATCH 15/29] Add --[no-]use-pep517 command line flag --- src/pip/_internal/cli/cmdoptions.py | 19 +++++++++++++++++++ src/pip/_internal/commands/download.py | 2 ++ src/pip/_internal/commands/install.py | 2 ++ src/pip/_internal/commands/wheel.py | 2 ++ 4 files changed, 25 insertions(+) diff --git a/src/pip/_internal/cli/cmdoptions.py b/src/pip/_internal/cli/cmdoptions.py index 3033cd4b5e6..6c0c2425162 100644 --- a/src/pip/_internal/cli/cmdoptions.py +++ b/src/pip/_internal/cli/cmdoptions.py @@ -568,6 +568,25 @@ def prefer_binary(): 'if this option is used.' ) # type: Any +use_pep517 = partial( + Option, + '--use-pep517', + dest='use_pep517', + action='store_true', + default=None, + help='Use PEP 517 for building source distributions ' + '(use --no-use-pep517 to force legacy behaviour).' +) # type: Any + +no_use_pep517 = partial( + Option, + '--no-use-pep517', + dest='use_pep517', + action='store_false', + default=None, + help=SUPPRESS_HELP +) # type: Any + install_options = partial( Option, '--install-option', diff --git a/src/pip/_internal/commands/download.py b/src/pip/_internal/commands/download.py index b3f3c6ec762..a57e4bc4cb8 100644 --- a/src/pip/_internal/commands/download.py +++ b/src/pip/_internal/commands/download.py @@ -58,6 +58,8 @@ def __init__(self, *args, **kw): cmd_opts.add_option(cmdoptions.require_hashes()) cmd_opts.add_option(cmdoptions.progress_bar()) cmd_opts.add_option(cmdoptions.no_build_isolation()) + cmd_opts.add_option(cmdoptions.use_pep517()) + cmd_opts.add_option(cmdoptions.no_use_pep517()) cmd_opts.add_option( '-d', '--dest', '--destination-dir', '--destination-directory', diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 9ac2e15c811..2ba6042a6ef 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -152,6 +152,8 @@ def __init__(self, *args, **kw): cmd_opts.add_option(cmdoptions.ignore_requires_python()) cmd_opts.add_option(cmdoptions.no_build_isolation()) + cmd_opts.add_option(cmdoptions.use_pep517()) + cmd_opts.add_option(cmdoptions.no_use_pep517()) cmd_opts.add_option(cmdoptions.install_options()) cmd_opts.add_option(cmdoptions.global_options()) diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index 6dd07a78575..aae2c9251c3 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -67,6 +67,8 @@ def __init__(self, *args, **kw): help="Extra arguments to be supplied to 'setup.py bdist_wheel'.", ) cmd_opts.add_option(cmdoptions.no_build_isolation()) + cmd_opts.add_option(cmdoptions.use_pep517()) + cmd_opts.add_option(cmdoptions.no_use_pep517()) cmd_opts.add_option(cmdoptions.constraints()) cmd_opts.add_option(cmdoptions.editable()) cmd_opts.add_option(cmdoptions.requirements()) From f40491b8119082660a8a7ce77d5e5a3d114b56b5 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 9 Oct 2018 17:40:18 +0100 Subject: [PATCH 16/29] Vendor the new version of pep517 --- src/pip/_vendor/pep517/__init__.py | 2 +- src/pip/_vendor/pep517/_in_process.py | 10 +++++++++- src/pip/_vendor/pep517/wrappers.py | 5 +++++ src/pip/_vendor/vendor.txt | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/pip/_vendor/pep517/__init__.py b/src/pip/_vendor/pep517/__init__.py index 8beedea4794..3d46629c2a5 100644 --- a/src/pip/_vendor/pep517/__init__.py +++ b/src/pip/_vendor/pep517/__init__.py @@ -1,4 +1,4 @@ """Wrappers to build Python packages using PEP 517 hooks """ -__version__ = '0.2' +__version__ = '0.3' diff --git a/src/pip/_vendor/pep517/_in_process.py b/src/pip/_vendor/pep517/_in_process.py index baa14d381a8..d1ad7b7e588 100644 --- a/src/pip/_vendor/pep517/_in_process.py +++ b/src/pip/_vendor/pep517/_in_process.py @@ -21,11 +21,17 @@ # This is run as a script, not a module, so it can't do a relative import import compat +class BackendUnavailable(Exception): + """Raised if we cannot import the backend""" + def _build_backend(): """Find and load the build backend""" ep = os.environ['PEP517_BUILD_BACKEND'] mod_path, _, obj_path = ep.partition(':') - obj = import_module(mod_path) + try: + obj = import_module(mod_path) + except ImportError: + raise BackendUnavailable if obj_path: for path_part in obj_path.split('.'): obj = getattr(obj, path_part) @@ -173,6 +179,8 @@ def main(): json_out = {'unsupported': False, 'return_val': None} try: json_out['return_val'] = hook(**hook_input['kwargs']) + except BackendUnavailable: + json_out['no_backend'] = True except GotUnsupportedOperation: json_out['unsupported'] = True diff --git a/src/pip/_vendor/pep517/wrappers.py b/src/pip/_vendor/pep517/wrappers.py index 91f2ac2c29a..d14338ba34a 100644 --- a/src/pip/_vendor/pep517/wrappers.py +++ b/src/pip/_vendor/pep517/wrappers.py @@ -18,6 +18,9 @@ def tempdir(): finally: shutil.rmtree(td) +class BackendUnavailable(Exception): + """Will be raised if the backend cannot be imported in the hook process.""" + class UnsupportedOperation(Exception): """May be raised by build_sdist if the backend indicates that it can't.""" @@ -148,5 +151,7 @@ def _call_hook(self, hook_name, kwargs): data = compat.read_json(pjoin(td, 'output.json')) if data.get('unsupported'): raise UnsupportedOperation + if data.get('no_backend'): + raise BackendUnavailable return data['return_val'] diff --git a/src/pip/_vendor/vendor.txt b/src/pip/_vendor/vendor.txt index 9389dd947d7..0273578a998 100644 --- a/src/pip/_vendor/vendor.txt +++ b/src/pip/_vendor/vendor.txt @@ -10,7 +10,7 @@ lockfile==0.12.2 progress==1.4 ipaddress==1.0.22 # Only needed on 2.6 and 2.7 packaging==18.0 -pep517==0.2 +pep517==0.3 pyparsing==2.2.1 pytoml==0.1.19 retrying==1.3.3 From 83979fedaafeae19ec53c1330b9b08f489bfcc6a Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 9 Oct 2018 18:16:08 +0100 Subject: [PATCH 17/29] Actually use the new --[no-]use-pep517 option --- src/pip/_internal/cli/base_command.py | 5 ++++- src/pip/_internal/req/constructors.py | 12 +++++++----- src/pip/_internal/req/req_file.py | 10 +++++++--- src/pip/_internal/req/req_install.py | 4 ++-- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index dac4b053d09..f89df117bc9 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -214,6 +214,7 @@ def populate_requirement_set(requirement_set, args, options, finder, for req in args: req_to_add = install_req_from_line( req, None, isolated=options.isolated_mode, + use_pep517=options.use_pep517, wheel_cache=wheel_cache ) req_to_add.is_direct = True @@ -223,6 +224,7 @@ def populate_requirement_set(requirement_set, args, options, finder, req_to_add = install_req_from_editable( req, isolated=options.isolated_mode, + use_pep517=options.use_pep517, wheel_cache=wheel_cache ) req_to_add.is_direct = True @@ -232,7 +234,8 @@ def populate_requirement_set(requirement_set, args, options, finder, for req_to_add in parse_requirements( filename, finder=finder, options=options, session=session, - wheel_cache=wheel_cache): + wheel_cache=wheel_cache, + use_pep517=options.use_pep517): req_to_add.is_direct = True requirement_set.add_requirement(req_to_add) # If --require-hashes was a line in a requirements file, tell diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index 4c4641dc883..b36c95d9269 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -146,8 +146,8 @@ def deduce_helpful_msg(req): def install_req_from_editable( - editable_req, comes_from=None, isolated=False, options=None, - wheel_cache=None, constraint=False + editable_req, comes_from=None, use_pep517=None, isolated=False, + options=None, wheel_cache=None, constraint=False ): name, url, extras_override = parse_editable(editable_req) if url.startswith('file:'): @@ -167,6 +167,7 @@ def install_req_from_editable( editable=True, link=Link(url), constraint=constraint, + use_pep517=use_pep517, isolated=isolated, options=options if options else {}, wheel_cache=wheel_cache, @@ -175,8 +176,8 @@ def install_req_from_editable( def install_req_from_line( - name, comes_from=None, isolated=False, options=None, wheel_cache=None, - constraint=False + name, comes_from=None, use_pep517=None, isolated=False, options=None, + wheel_cache=None, constraint=False ): """Creates an InstallRequirement from a name, which might be a requirement, directory containing 'setup.py', filename, or URL. @@ -265,7 +266,7 @@ def install_req_from_line( return InstallRequirement( req, comes_from, link=link, markers=markers, - isolated=isolated, + use_pep517=use_pep517, isolated=isolated, options=options if options else {}, wheel_cache=wheel_cache, constraint=constraint, @@ -293,6 +294,7 @@ def install_req_from_req( "%s depends on %s " % (comes_from.name, req) ) + # TODO: use_pep517? return InstallRequirement( req, comes_from, isolated=isolated, wheel_cache=wheel_cache ) diff --git a/src/pip/_internal/req/req_file.py b/src/pip/_internal/req/req_file.py index e7acf7cb8e3..b332f6853a1 100644 --- a/src/pip/_internal/req/req_file.py +++ b/src/pip/_internal/req/req_file.py @@ -60,7 +60,8 @@ def parse_requirements(filename, finder=None, comes_from=None, options=None, - session=None, constraint=False, wheel_cache=None): + session=None, constraint=False, wheel_cache=None, + use_pep517=None): """Parse a requirements file and yield InstallRequirement instances. :param filename: Path or url of requirements file. @@ -71,6 +72,7 @@ def parse_requirements(filename, finder=None, comes_from=None, options=None, :param constraint: If true, parsing a constraint file rather than requirements file. :param wheel_cache: Instance of pip.wheel.WheelCache + :param use_pep517: Value of the --use-pep517 option. """ if session is None: raise TypeError( @@ -87,7 +89,7 @@ def parse_requirements(filename, finder=None, comes_from=None, options=None, for line_number, line in lines_enum: req_iter = process_line(line, filename, line_number, finder, comes_from, options, session, wheel_cache, - constraint=constraint) + use_pep517=use_pep517, constraint=constraint) for req in req_iter: yield req @@ -108,7 +110,7 @@ def preprocess(content, options): def process_line(line, filename, line_number, finder=None, comes_from=None, options=None, session=None, wheel_cache=None, - constraint=False): + use_pep517=None, constraint=False): """Process a single requirements line; This can result in creating/yielding requirements, or updating the finder. @@ -155,6 +157,7 @@ def process_line(line, filename, line_number, finder=None, comes_from=None, req_options[dest] = opts.__dict__[dest] yield install_req_from_line( args_str, line_comes_from, constraint=constraint, + use_pep517=use_pep517, isolated=isolated, options=req_options, wheel_cache=wheel_cache ) @@ -163,6 +166,7 @@ def process_line(line, filename, line_number, finder=None, comes_from=None, isolated = options.isolated_mode if options else False yield install_req_from_editable( opts.editables[0], comes_from=line_comes_from, + use_pep517=use_pep517, constraint=constraint, isolated=isolated, wheel_cache=wheel_cache ) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 9b6c86b77ec..fdca7c340d6 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -50,7 +50,7 @@ class InstallRequirement(object): """ def __init__(self, req, comes_from, source_dir=None, editable=False, - link=None, update=True, markers=None, + link=None, update=True, markers=None, use_pep517=None, isolated=False, options=None, wheel_cache=None, constraint=False, extras=()): assert req is None or isinstance(req, Requirement), req @@ -128,7 +128,7 @@ def __init__(self, req, comes_from, source_dir=None, editable=False, # and False. Before loading, None is valid (meaning "use the default"). # Setting an explicit value before loading pyproject.toml is supported, # but after loading this flag should be treated as read only. - self.use_pep517 = None + self.use_pep517 = use_pep517 def __str__(self): if self.req: From f10be259ce866db45a2057cc7c17ea11d2afd695 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 10 Oct 2018 11:25:26 +0100 Subject: [PATCH 18/29] PEP 517 tests --- .../test_backend-0.1-py2.py3-none-any.whl | Bin 0 -> 2530 bytes tests/data/backends/test_backend-0.1.tar.gz | Bin 0 -> 2466 bytes tests/functional/test_pep517.py | 54 ++++++++++++++++-- 3 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 tests/data/backends/test_backend-0.1-py2.py3-none-any.whl create mode 100644 tests/data/backends/test_backend-0.1.tar.gz diff --git a/tests/data/backends/test_backend-0.1-py2.py3-none-any.whl b/tests/data/backends/test_backend-0.1-py2.py3-none-any.whl new file mode 100644 index 0000000000000000000000000000000000000000..60b69cea16a194a5867d2373b44532f6488df397 GIT binary patch literal 2530 zcmaKu2T)V#7RN7b=>igp!iqFSBosj*0#ZVhG?Jx*6a@mJ5u`R4hrmr4{-BScj+t@G=;Ou1pikmqp`oT?vCP9d71-NF{Y&S(IV_wnoC&`@p?1PZKcm-w zMbrK2xZtgM$DpBshun!~sDrMB|%!@Nj?)! z=lALGFQ-^9ils^s{c;lU_=oNEc0qYL#5#6RDa27kQv0!ISlskVx~iOD{fRY#2`w*9 zqF{ZFsQeLHREioZZFBb4I%V;D=OK?6NK(DuyfkQ7F8MuE_^BO@z*mfYB@uS6E$wHL z@d)4Z@p8{tl$`SUmSZ}V$d=&t4@5Htt8X3RbOaw|;l` zL7=00{X@2R8wx;W%i7A6t#F-0e7yDxASaDTPtB^)= zOo7r6Dw7l^h_SQP5T4vZ$ZO3yB=>`0_&RO7*!I%ax&gBD+snhF<(&o1VRhJd*Y{&^ ztL?{JLpnacp$2ry-#mq_3}#iNHf%lpm|t>(s>yk`xFx@K6e?oYQ2j zKWJTejdl`M)Fe$1kvLFKHg;5U4pZ=ScN>m$9S%Mud7!+z#OW4^l=$poZlwxYvOY~* zrm$v6f4*NNbwwZIcE76-y0*m#)z+I*T51b<-0rm%Ahp3r%nYRKZ8X`)^6q?x zOVE?X&JhiMfkrrhw&Sx1h2d99yY8%XgxWch_SBx$jv)+Q=7;U|)9lnWH6a*|>&{d6 zZuh2n^(Zz#$!MP^5gT2&Ns z^Etw$TlYa^ixO4+M?&v9LS5`8U~ceL-*8=Fm!M7(B!Yru>H1Tq`Qy`%9WT~c=CW_Y z=r(D%g+5E-bROy5taRw8Gf#Gomqz(7C2AQ)RY9$CO#N!hNLkrDpkA~Wg)&#fQw(*F zJ|ricbO^kSSs7oCD{qp4;vn%)D6Bh;WXSW}zMGrbEUk#_lZcUn#$o}&SIgelC7dQJ zFZ-4H*;c3H&K-)(&Cplfn~JZO?Plcq+@yt4p8C-od`*+~O-0!?tPza#MKea%3^ofk z;mzMZI_tav$##RXEF41`78;xUm>`M{4Baj>Z}iLT2m6QdTuy3SH>m?(d10F7SKfS) zzmOx|Rm~-;KRVAM=}BSi_$OOt3PYk?EUVtTycq6Dq6WR%(hN9mBU`muud&_KoDQPP z#TZqRmr7DHAoCL?b?_V4BZM@KBBpc$r@v8I)LGa(x#Cj+Ahe|5-rB9)lv$V|5%Pw% zzCF6}>2Rj7%_+X@r1xh`$_wO9j`ndK#E&=me)av?h>MWsF>veXkcHdOO5?!1ZbDoj61O1g)M z^w^2ACSpa$sTZ8z@b3{3fTxWYTP!(kKJK^WryO9P7Bsl*HZWJ4hRH$`*`|l zpkdY~W+v7qAJJeq3a$q47e<0ryBR%ImYRLqvmcNQ^b<=JI9RMm887yGi>47c>frkAsbhW@$|pwqLp3~ zl|@glX5~A@@kwPh@&JEV3`P&sdVE*Pj)S`^wljypP=YhIjWR-z&mt2{xFD%~-{xAXvO< z3HKTot1OQnT>)-^6yYni<=_L5ZKoIZllH0KmZgq9O0&rkYTpQ9?N>FV=Pzd6Zq_Rs zvr%e%ZE+>sk0VJs_~5q#iFxwbx{7(kGmvZ91IE^LV}|~R!8t zMSI6IhuSFdYrXaETRg)pTJZc!X<6x$=gTm(JZg1v7DOt0pm;ba=9R*5in&28#f|W) zWwtk#3B<7`kT<+WBv$eZI#4x{X)){dp(;v`br#-ad%Z3qPtevbWy+G%m8*p1Pu`;` z0KsE#4Q&VpJW9n=f?d2no|@?IuN|YsUz7je{rEE#0BWKyvgeb(CjY~2 z*$Lcf-ycBPpLPZQVdkB*omTrnqwP-noAGv1cY5u=)M^fK-d#2R={ziwhkZBz@UT}j L+pb8y?_d80{2VY| literal 0 HcmV?d00001 diff --git a/tests/data/backends/test_backend-0.1.tar.gz b/tests/data/backends/test_backend-0.1.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..e735710abd8ba4ff38836f112703ef1c5dbe462f GIT binary patch literal 2466 zcmV;T30?LdiwFp@RlZvS|72-%bX;_0b97%~VPk7$Ze%SmE-@~2VR8WNTUm4BxEAhb zSNT7&8@3FXUw>r-VM#jao~F}%XZ$_bNIH@% zo%0xmVva z`u70zos>GBe8l@7yVpM*>OaHM@%!J0;EST^$O4vA0F19V=3D4h(Dh`aK*f>Oe>(o# z@c*AX{U6{zO;XSNXQ_z)3`NKM{|Jm-+aw}gL)$hyLN^`Tg`1qu#8|g9$wAhE-s(IY z!?7qoiDLwUh29Yh7h8L|_`JznY+e!z7s!@&dAkM*N6jTKaqKd7GJh)nqZJ|iGVt?0 z;6485sD1p;(lp|KmZIYI|Ks@Y$Z%@t%8vXGD2ZcHi6VO-%kB!4EDJ2v)&P#}{KV9U zy0K6C3bG#+c-}rM*f@^ol9VbN#@2;=L8U3M0;&&X#Q>j)#BSM1px+0fN#g6)mv~UK zeONeXu3YI0BF{(8B$6cxgauYG84e?v2-`?prY451Bolw9mQy2fi6leIMuN{HN=8*v zE|4`z31H}+qV{wH5TWet)|I)s1(RXMm)jfiYf1xA<%I01x*jzgN^KjD#BRvjui*rY zZI{q=m(VRDnNWP)(83``PlxVX)}_E?a;;lqJ8`+~a5KRv*Mu$rNz}k+LZ4W+3nbvW zw(Avot`B~~jlSUj8RP!%cRp2Ly!PgCQ^zW-;@J^udx2kHN(P6hn<{(p|i#QXmrf_L}-ujBpy z*#D=yEwNLobW?Bz&(Htxo6iydX_Css`+pz9|NU`K?D^Fn+r0!}N7jJSFLnp9)BOVM zwGW8c8@tNb+ba(Cu0ei7LIVy;14|7gL$_w$8=}y5Bm*pf@z3o0KfwRjCi86kC-?h5 zTmI7w!^Zso2*k^0y#9X&|9{FuzRQ0qbHaa$q+m{{@3#bTR7sB`yo zr8^_J2or9HUU*S!_Y2wVKWU}>ct{(*JnW2zL1qcsP42J_9OIfLwL7D&Hf5ESFt0HS zui&^9a>+kc{L!7U)=HG4n4<+xQq_K?T50z?#T$}@U2%2KS1b|7%AyAwUaq;$3Eiep zUuPcf`E`Y+M15ihUcS-m=BAUGH{u$DQkN0d71o=(olYsOtn=ySL*6g11C^Bg%qw|QH673#HR#Fxx-xTnH?|Q}N=2Y9+v3qEH)AAW zPa7~Jlz~QFi%OJtM@eb*j@WOOZu0e>%5;v$)?Tx%e$cz2cpT$J+Y7o^fjFBsg6`7S zyEnB)pKmt>ZY{w2nriiolIK3~`5MIwk3yqFHP))p^o+XQ=sH3Mh>floFru#8g1`$n z7Tn}{N@SXIw$0yloA)=vdXcLZc`h6tLWwJLR>ct>YEDCI+nbSwmnFK?ih)8q#v{{) z%@Vuf&5na(DoV+NjHv{9Qelc?<*rYU21{S7P02>^)L4^_!26ZbYrS|izjd|nP)>2q zp~V8-j!U}Xa3_v zFf0AOeTAG`X*kNvNfy~-3qE-FPpdRJm4j;einqL}F<`)YsI?Tq7$_CWnq|1-O zS|38jWb0SWr`ck?bZWHFOE(9OR-Ft>jqa+aHgU}6GdJF-0onAlr;Dvxd!`FltJ1P2 zK;UMnc{4i|r^Qz1L3d^#Ew+K{$_-QH>~U7F7HLc9z!CrG*UZ+O9qPV?W5UAB6z4h1 zWpg>F-Qla<;lNRHwO;#?r}|v&ju|_%Y>yiktF`jP*6-Nzs74_zwd*4Hia$W}c;XTE zDAALsLUE~@`T1xdkUW(iBaNX!=EuBM%pmR*xoMlLZe7dre$~LSo~)Gzst}q0HxFAV zvd0bca4d>%7|AA$O2ww(L3T zkE@l#{I`A4eYH&@P9b&{+mBH_G;!6Cg_R=5mD#P%3pf@n7C5GXG2w!_m)sq0L5XpA zzy>WR+lWI7%fok!i;aXnCM-K#fKuTZr4(4{9`usidb?&eO+nwaHtD4fr-bbQYbW@b zxNt8n3E3mY2UWroIfib5@JP;KeuyZ)5ZzK9baf|rc`Se)ERvBeEqcPOr0ha3>^_$I#DT(?VjK-qBI5SjF1f@oO2(EnZSY`A-m!KeSwD(8_94^qBBZIdI}*)n_xha zlI~^r@Vg!1P Date: Tue, 16 Oct 2018 13:53:34 +0100 Subject: [PATCH 19/29] Support --python-tag for PEP 517 builds --- src/pip/_internal/wheel.py | 19 ++++++++++++++++++- tests/unit/test_wheel.py | 15 +++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 138d782c79f..d824c6c1f32 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -74,6 +74,14 @@ def open_for_csv(name, mode): return open(name, mode + bin, **nl) +def replace_python_tag(wheelname, new_tag): + """Replace the Python tag in a wheel file name with a new value. + """ + parts = wheelname.split('-') + parts[-3] = new_tag + return '-'.join(parts) + + def fix_script(path): """Replace #!python with #!/path/to/python Return True if file was changed.""" @@ -713,10 +721,19 @@ def runner(cmd, cwd=None, extra_environ=None): spinner=spinner ) with req.pep517_backend.subprocess_runner(runner): - req.pep517_backend.build_wheel( + wheelname = req.pep517_backend.build_wheel( tempd, metadata_directory=req.metadata_directory ) + if python_tag: + # General PEP 517 backends don't necessarily support + # a "--python-tag" option, so we rename the wheel + # file directly. + newname = replace_python_tag(wheelname, python_tag) + os.rename( + os.path.join(tempd, wheelname), + os.path.join(tempd, newname) + ) return True except Exception: spinner.finish("error") diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index a132fb614df..ac89b095956 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -70,6 +70,21 @@ def test_wheel_version(tmpdir, data): assert not wheel.wheel_version(tmpdir + 'broken') +def test_python_tag(): + wheelnames = [ + 'simplewheel-1.0-py2.py3-none-any.whl', + 'simplewheel-1.0-py27-none-any.whl', + 'simplewheel-2.0-1-py2.py3-none-any.whl', + ] + newnames = [ + 'simplewheel-1.0-py37-none-any.whl', + 'simplewheel-1.0-py37-none-any.whl', + 'simplewheel-2.0-1-py37-none-any.whl', + ] + for name, new in zip(wheelnames, newnames): + assert wheel.replace_python_tag(name, 'py37') == new + + def test_check_compatibility(): name = 'test' vc = wheel.VERSION_COMPATIBLE From a4c7d7d9969a6816d73620cd088b97f6f1fee254 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 19 Oct 2018 11:49:15 +0100 Subject: [PATCH 20/29] Add documentation --- docs/html/reference/pip.rst | 101 +++++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 35 deletions(-) diff --git a/docs/html/reference/pip.rst b/docs/html/reference/pip.rst index fff2e652bbc..676f6f10f03 100644 --- a/docs/html/reference/pip.rst +++ b/docs/html/reference/pip.rst @@ -71,13 +71,13 @@ when decision is needed. Build System Interface ====================== -Pip builds packages by invoking the build system. Presently, the only supported -build system is ``setuptools``, but in the future, pip will support :pep:`517` -which allows projects to specify an alternative build system in a -``pyproject.toml`` file. As well as package building, the build system is also -invoked to install packages direct from source. This is handled by invoking -the build system to build a wheel, and then installing from that wheel. The -built wheel is cached locally by pip to avoid repeated identical builds. +Pip builds packages by invoking the build system. By default, builds will use +``setuptools``, but if a project specifies a different build system using a +``pyproject.toml`` file, as per :pep:`517`, pip will use that instead. As well +as package building, the build system is also invoked to install packages +direct from source. This is handled by invoking the build system to build a +wheel, and then installing from that wheel. The built wheel is cached locally +by pip to avoid repeated identical builds. The current interface to the build system is via the ``setup.py`` command line script - all build actions are defined in terms of the specific ``setup.py`` @@ -86,13 +86,16 @@ command line that will be run to invoke the required action. Setuptools Injection ~~~~~~~~~~~~~~~~~~~~ -As noted above, the supported build system is ``setuptools``. However, not all -packages use ``setuptools`` in their build scripts. To support projects that -use "pure ``distutils``", pip injects ``setuptools`` into ``sys.modules`` -before invoking ``setup.py``. The injection should be transparent to -``distutils``-based projects, but 3rd party build tools wishing to provide a -``setup.py`` emulating the commands pip requires may need to be aware that it -takes place. +When :pep:`517` is not used, the supported build system is ``setuptools``. +However, not all packages use ``setuptools`` in their build scripts. To support +projects that use "pure ``distutils``", pip injects ``setuptools`` into +``sys.modules`` before invoking ``setup.py``. The injection should be +transparent to ``distutils``-based projects, but 3rd party build tools wishing +to provide a ``setup.py`` emulating the commands pip requires may need to be +aware that it takes place. + +Projects using :pep:`517` *must* explicitly use setuptools - pip does not do +the above injection process in this case. Build System Output ~~~~~~~~~~~~~~~~~~~ @@ -113,13 +116,20 @@ unexpected byte sequences to Python-style hexadecimal escape sequences (``"\x80\xff"``, etc). However, it is still possible for output to be displayed using an incorrect encoding (mojibake). -PEP 518 Support -~~~~~~~~~~~~~~~ +Under :pep:`517`, handling of build tool output is the backend's responsibility, +and pip simply displays the output produced by the backend. (Backends, however, +will likely still have to address the issues described above). + +PEP 517 and 518 Support +~~~~~~~~~~~~~~~~~~~~~~~ -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 -:pep:`518`. When building a project, pip will install the required dependencies -locally, and make them available to the build process. +As of version 10.0, pip supports projects declaring dependencies that are +required at install time using a ``pyproject.toml`` file, in the form described +in :pep:`518`. When building a project, pip will install the required +dependencies locally, and make them available to the build process. +Furthermore, from version 19.0 onwards, pip supports projects specifying the +build backend they use in ``pyproject.toml``, in the form described in +:pep:`517`. When making build requirements available, pip does so in an *isolated environment*. That is, pip does not install those requirements into the user's @@ -137,24 +147,45 @@ can be problematic. If this is the case, pip provides a flag are responsible for ensuring the build environment is managed appropriately. -.. _pep-518-limitations: - -**Limitations**: +By default, pip will continue to use the legacy (``setuptools`` based) build +processing for projects that do not have a ``pyproject.toml`` file. Projects +with a ``pyproject.toml`` file will use a :pep:`517` backend. Projects with +a ``pyproject.toml`` file, but which don't have a ``build-system`` section, +will be assumed to have the following backend settings:: + + [build-system] + requires = ["setuptools>=40.2.0", "wheel"] + build-backend = "setuptools.build_meta" + +(``setuptools`` 40.2.0 is the first version with full :pep:`517` support). If +a project has ``[build-system]``, but no ``build-backend``, pip will use +``setuptools.build_meta``, but will assume the project requirements include +``setuptools>=40.2.0`` and ``wheel`` (and will report an error if not). + +If a user wants to explicitly request :pep:`517` handling even though a project +doesn't have a ``pyproject.toml`` file, this can be done using the +``--use-pep517`` command line option. Similarly, to request legacy processing +even though ``pyproject.toml`` is present, the ``--no-use-pep517`` option is +available (although obviously it is an error to choose ``--no-use-pep517`` if +the project has no ``setup.py``, or explicitly requests a build backend). As +with other command line flags, pip recognises the ``PIP_USE_PEP517`` +environment veriable and a ``use-pep517`` config file option (set to true or +false) to set this option globally. Note that overriding pip's choice of +whether to use :pep:`517` processing in this way does *not* affect whether pip +will use an isolated build environment (which is controlled via +``--no-build-isolation`` as noted above). + +Except in the case noted above (projects with no :pep:`518` ``[build-system]`` +section in ``pyproject.toml``), pip will never implicitly install a build +system. Projects **must** ensure that the correct build system is listed in +their ``requires`` list (this applies even if pip assumes that the +``setuptools`` backend is being used, as noted above). -* until :pep:`517` 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``. +.. _pep-518-limitations: -* 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 :pep:`518` 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. +**Historical Limitations**: -* ``pip<18.0``: only support installing build requirements from wheels, and +* ``pip<18.0``: only supports installing build requirements from wheels, and does not support the use of environment markers and extras (only version specifiers are respected). From f805ac15a71ec0b70eff5092de3cee5de668cd04 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 19 Oct 2018 20:57:47 +0100 Subject: [PATCH 21/29] Properly wrap all hook calls in our subprocess runner --- src/pip/_internal/operations/prepare.py | 1 + src/pip/_internal/req/req_install.py | 17 ++++++++ src/pip/_internal/wheel.py | 52 ++++++++++--------------- 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 1ea58b87632..e0585db872b 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -136,6 +136,7 @@ def _raise_conflicts(conflicting_with, conflicting_reqs): # dependencies must be installed before we can call the backend. with self.req.build_env: # We need to have the env active when calling the hook. + self.req.spin_message = "Getting requirements to build wheel" reqs = self.req.pep517_backend.get_requires_for_build_wheel() conflicting, missing = self.req.build_env.check_requirements(reqs) if conflicting: diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index fdca7c340d6..c76d69be37d 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -451,6 +451,22 @@ def load_pyproject_toml(self): self.pyproject_requires = requires self.pep517_backend = Pep517HookCaller(self.setup_py_dir, backend) + # Use a custom function to call subprocesses + self.spin_message = "" + + def runner(cmd, cwd=None, extra_environ=None): + with open_spinner(self.spin_message) as spinner: + call_subprocess( + cmd, + cwd=cwd, + extra_environ=extra_environ, + show_stdout=False, + spinner=spinner + ) + self.spin_message = "" + + self.pep517_backend._subprocess_runner = runner + def prepare_metadata(self): """Ensure that project metadata is available. @@ -513,6 +529,7 @@ def prepare_pep517_metadata(self): # prepare_metadata_for_build_wheel, so we don't have to # consider the possibility that this hook doesn't exist. backend = self.pep517_backend + self.spin_message = "Preparing wheel metadata" distinfo_dir = backend.prepare_metadata_for_build_wheel( metadata_dir ) diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index d824c6c1f32..863ed1ed11e 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -706,39 +706,27 @@ def _base_setup_args(self, req): ] + list(self.global_options) def _build_one_pep517(self, req, tempd, python_tag=None): - # TODO: Cannot support python_tag - spin_message = 'Building wheel for %s (PEP 517)' % (req.name,) - with open_spinner(spin_message) as spinner: + assert req.metadata_directory is not None + try: + req.spin_message = 'Building wheel for %s (PEP 517)' % (req.name,) logger.debug('Destination directory: %s', tempd) - assert req.metadata_directory is not None - try: - def runner(cmd, cwd=None, extra_environ=None): - call_subprocess( - cmd, - cwd=cwd, - extra_environ=extra_environ, - show_stdout=False, - spinner=spinner - ) - with req.pep517_backend.subprocess_runner(runner): - wheelname = req.pep517_backend.build_wheel( - tempd, - metadata_directory=req.metadata_directory - ) - if python_tag: - # General PEP 517 backends don't necessarily support - # a "--python-tag" option, so we rename the wheel - # file directly. - newname = replace_python_tag(wheelname, python_tag) - os.rename( - os.path.join(tempd, wheelname), - os.path.join(tempd, newname) - ) - return True - except Exception: - spinner.finish("error") - logger.error('Failed building wheel for %s', req.name) - return False + wheelname = req.pep517_backend.build_wheel( + tempd, + metadata_directory=req.metadata_directory + ) + if python_tag: + # General PEP 517 backends don't necessarily support + # a "--python-tag" option, so we rename the wheel + # file directly. + newname = replace_python_tag(wheelname, python_tag) + os.rename( + os.path.join(tempd, wheelname), + os.path.join(tempd, newname) + ) + return True + except Exception: + logger.error('Failed building wheel for %s', req.name) + return False def _build_one_legacy(self, req, tempd, python_tag=None): base_args = self._base_setup_args(req) From 689f97c4edab3e3e6a17e70ce1927870fbd2c87e Mon Sep 17 00:00:00 2001 From: Benoit Pierre Date: Fri, 26 Oct 2018 23:04:08 +0200 Subject: [PATCH 22/29] fix failing tests --- tests/functional/test_completion.py | 5 +++-- tests/functional/test_install.py | 12 +++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/functional/test_completion.py b/tests/functional/test_completion.py index bee64d319f3..53380bc37f8 100644 --- a/tests/functional/test_completion.py +++ b/tests/functional/test_completion.py @@ -40,12 +40,13 @@ COMPLETION_FOR_SUPPORTED_SHELLS_TESTS, ids=[t[0] for t in COMPLETION_FOR_SUPPORTED_SHELLS_TESTS], ) -def test_completion_for_supported_shells(script, pip_src, shell, completion): +def test_completion_for_supported_shells(script, pip_src, common_wheels, + shell, completion): """ Test getting completion for bash shell """ # Re-install pip so we get the launchers. - script.pip_install_local('--no-build-isolation', pip_src) + script.pip_install_local('-f', common_wheels, pip_src) result = script.pip('completion', '--' + shell, use_module=False) assert completion in result.stdout, str(result.stdout) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 7b6f529c02f..4ee64dd8e07 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -110,11 +110,8 @@ def test_pep518_with_user_pip(script, pip_src, data, common_wheels): non-isolated environment, and break pip in the system site-packages, so that isolated uses of pip will fail. """ - # Set expect_stderr, not so much because we expect output on stderr, - # rather we simply don't care (setuptools can write warnings to stderr - # which we'll ignore). - script.pip("install", "--ignore-installed", "--user", pip_src, - expect_stderr=True) + script.pip("install", "--ignore-installed", + "-f", common_wheels, "--user", pip_src) system_pip_dir = script.site_packages_path / 'pip' system_pip_dir.rmtree() system_pip_dir.mkdir() @@ -165,12 +162,13 @@ def test_pep518_forkbombs(script, data, common_wheels, command, package): @pytest.mark.network -def test_pip_second_command_line_interface_works(script, data, pip_src): +def test_pip_second_command_line_interface_works(script, pip_src, data, + common_wheels): """ Check if ``pip`` commands behaves equally """ # Re-install pip so we get the launchers. - script.pip_install_local('--no-build-isolation', pip_src) + script.pip_install_local('-f', common_wheels, pip_src) # On old versions of Python, urllib3/requests will raise a warning about # the lack of an SSLContext. kwargs = {} From 817ef1a4cb7faed6539b2b2f57ba7dc8a0f93200 Mon Sep 17 00:00:00 2001 From: Benoit Pierre Date: Fri, 26 Oct 2018 23:27:25 +0200 Subject: [PATCH 23/29] add a couple of additional PEP 517 tests --- tests/functional/test_pep517.py | 37 +++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/functional/test_pep517.py b/tests/functional/test_pep517.py index 55272427289..52e3acded90 100644 --- a/tests/functional/test_pep517.py +++ b/tests/functional/test_pep517.py @@ -4,6 +4,7 @@ from pip._internal.download import PipSession from pip._internal.index import PackageFinder from pip._internal.req import InstallRequirement +from tests.lib import path_to_url def make_project(tmpdir, requires=[], backend=None): @@ -72,3 +73,39 @@ def test_no_use_pep517_without_setup_py(script, tmpdir, data): expect_error=True ) assert 'project does not have a setup.py' in result.stderr + + +def test_conflicting_pep517_backend_requirements(script, tmpdir, data): + project_dir = make_project( + tmpdir, requires=['test_backend', 'simplewheel==1.0'], + backend="test_backend" + ) + project_dir.join("backend_reqs.txt").write("simplewheel==2.0") + result = script.pip( + 'install', '--no-index', + '-f', data.backends, + '-f', data.packages, + project_dir, + expect_error=True + ) + assert ( + result.returncode != 0 and + ('Some build dependencies for %s conflict with the backend ' + 'dependencies: simplewheel==1.0 is incompatible with ' + 'simplewheel==2.0.' % path_to_url(project_dir)) in result.stderr + ), str(result) + + +def test_pep517_backend_requirements_already_satisfied(script, tmpdir, data): + project_dir = make_project( + tmpdir, requires=['test_backend', 'simplewheel==1.0'], + backend="test_backend" + ) + project_dir.join("backend_reqs.txt").write("simplewheel") + result = script.pip( + 'install', '--no-index', + '-f', data.backends, + '-f', data.packages, + project_dir, + ) + assert 'Installing backend dependencies:' not in result.stdout From cf4d84e58fe50ce7f2886ebd8602853d4550df40 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 14 Nov 2018 13:15:55 +0000 Subject: [PATCH 24/29] Address doc review comments --- docs/html/reference/pip.rst | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/html/reference/pip.rst b/docs/html/reference/pip.rst index 676f6f10f03..e3c0f44a441 100644 --- a/docs/html/reference/pip.rst +++ b/docs/html/reference/pip.rst @@ -157,8 +157,12 @@ will be assumed to have the following backend settings:: requires = ["setuptools>=40.2.0", "wheel"] build-backend = "setuptools.build_meta" -(``setuptools`` 40.2.0 is the first version with full :pep:`517` support). If -a project has ``[build-system]``, but no ``build-backend``, pip will use +.. note:: + + ``setuptools`` 40.2.0 is the first version of setuptools with full + :pep:`517` support. + +If a project has ``[build-system]``, but no ``build-backend``, pip will use ``setuptools.build_meta``, but will assume the project requirements include ``setuptools>=40.2.0`` and ``wheel`` (and will report an error if not). From e8f7aa1446d7aa9093912991b9d4ef649df185d3 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 14 Nov 2018 14:06:35 +0000 Subject: [PATCH 25/29] Pass use_pep517 option to resolver --- src/pip/_internal/commands/install.py | 1 + src/pip/_internal/req/constructors.py | 7 ++++--- src/pip/_internal/resolve.py | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 2ba6042a6ef..1058a56d9d6 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -310,6 +310,7 @@ def run(self, options, args): ignore_requires_python=options.ignore_requires_python, ignore_installed=options.ignore_installed, isolated=options.isolated_mode, + use_pep517=options.use_pep517 ) resolver.resolve(requirement_set) diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index 68ad56063ca..640efd453f8 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -274,7 +274,8 @@ def install_req_from_line( def install_req_from_req( - req, comes_from=None, isolated=False, wheel_cache=None + req, comes_from=None, isolated=False, wheel_cache=None, + use_pep517=None ): try: req = Requirement(req) @@ -293,7 +294,7 @@ def install_req_from_req( "%s depends on %s " % (comes_from.name, req) ) - # TODO: use_pep517? return InstallRequirement( - req, comes_from, isolated=isolated, wheel_cache=wheel_cache + req, comes_from, isolated=isolated, wheel_cache=wheel_cache, + use_pep517=use_pep517 ) diff --git a/src/pip/_internal/resolve.py b/src/pip/_internal/resolve.py index 2d9f1c56c81..a911a348b5a 100644 --- a/src/pip/_internal/resolve.py +++ b/src/pip/_internal/resolve.py @@ -35,7 +35,7 @@ class Resolver(object): def __init__(self, preparer, session, finder, wheel_cache, use_user_site, ignore_dependencies, ignore_installed, ignore_requires_python, - force_reinstall, isolated, upgrade_strategy): + force_reinstall, isolated, upgrade_strategy, use_pep517=None): super(Resolver, self).__init__() assert upgrade_strategy in self._allowed_strategies @@ -56,6 +56,7 @@ def __init__(self, preparer, session, finder, wheel_cache, use_user_site, self.ignore_installed = ignore_installed self.ignore_requires_python = ignore_requires_python self.use_user_site = use_user_site + self.use_pep517 = use_pep517 self._discovered_dependencies = defaultdict(list) @@ -273,6 +274,7 @@ def add_req(subreq, extras_requested): req_to_install, isolated=self.isolated, wheel_cache=self.wheel_cache, + use_pep517=self.use_pep517 ) parent_req_name = req_to_install.name to_scan_again, add_to_parent = requirement_set.add_requirement( From 3a0f9b1c71e05942cfcb0b0618fe035b42eae04b Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 14 Nov 2018 14:31:24 +0000 Subject: [PATCH 26/29] Remove unneeded TODO --- src/pip/_internal/req/req_install.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index c76d69be37d..f4feed5165b 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -765,7 +765,6 @@ def _clean_zip_name(self, name, prefix): # only used by archive. # TODO: Investigate if this should be kept in InstallRequirement # Seems to be used only when VCS + downloads - # TODO: Consider PEP 517 implications def archive(self, build_dir): assert self.source_dir create_archive = True From 6b7473d66451147034c8b71fbc74bdf75ece33fb Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 14 Nov 2018 18:18:48 +0000 Subject: [PATCH 27/29] Pass --use-pep517 option to the resolver in the pip wheel command --- src/pip/_internal/commands/wheel.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pip/_internal/commands/wheel.py b/src/pip/_internal/commands/wheel.py index aae2c9251c3..cd72a3df1a7 100644 --- a/src/pip/_internal/commands/wheel.py +++ b/src/pip/_internal/commands/wheel.py @@ -159,6 +159,7 @@ def run(self, options, args): ignore_requires_python=options.ignore_requires_python, ignore_installed=True, isolated=options.isolated_mode, + use_pep517=options.use_pep517 ) resolver.resolve(requirement_set) From 85e4f8ec4166da06469a984bc9c7c326aedcf986 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 14 Nov 2018 18:24:55 +0000 Subject: [PATCH 28/29] Fix some remaining TODO comments --- src/pip/_internal/pyproject.py | 13 ++++++------- src/pip/_internal/req/req_install.py | 1 - 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/pip/_internal/pyproject.py b/src/pip/_internal/pyproject.py index d4027d2980e..c5cda41ee90 100644 --- a/src/pip/_internal/pyproject.py +++ b/src/pip/_internal/pyproject.py @@ -131,13 +131,12 @@ def load_pyproject_toml(use_pep517, pyproject_toml, setup_py, req_name): # (which is neede by the backend) in their requirements. So we # make a note to check that those requirements are present once # we have set up the environment. - # TODO: Review this - it's quite a lot of work to check for a very - # specific case. The problem is, that case is potentially quite - # common - projects that adopted PEP 518 early for the ability to - # specify requirements to execute setup.py, but never considered - # needing to mention the build tools themselves. The original PEP - # 518 code had a similar check (but implemented in a different - # way). + # This is quite a lot of work to check for a very specific case. But + # the problem is, that case is potentially quite common - projects that + # adopted PEP 518 early for the ability to specify requirements to + # execute setup.py, but never considered needing to mention the build + # tools themselves. The original PEP 518 code had a similar check (but + # implemented in a different way). backend = "setuptools.build_meta" check = ["setuptools>=40.2.0", "wheel"] diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index f4feed5165b..ec3b500a1c1 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -117,7 +117,6 @@ def __init__(self, req, comes_from, source_dir=None, editable=False, self.pyproject_requires = None # Build requirements that we will check are available - # TODO: We don't do this for --no-build-isolation. Should we? self.requirements_to_check = [] # The PEP 517 backend we should use to build the project From f06a0cb5609dca79950d0dd73f8042242a800869 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 14 Nov 2018 19:24:27 +0000 Subject: [PATCH 29/29] Move setup.py egg_info logging into run_egg_info --- src/pip/_internal/req/req_install.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index ec3b500a1c1..075f86e9b2c 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -473,16 +473,6 @@ def prepare_metadata(self): Under legacy processing, call setup.py egg-info. """ assert self.source_dir - if self.name: - logger.debug( - 'Running setup.py (path:%s) egg_info for package %s', - self.setup_py, self.name, - ) - else: - logger.debug( - 'Running setup.py (path:%s) egg_info for package from %s', - self.setup_py, self.link, - ) with indent_log(): if self.use_pep517: @@ -536,6 +526,16 @@ def prepare_pep517_metadata(self): self.metadata_directory = os.path.join(metadata_dir, distinfo_dir) def run_egg_info(self): + if self.name: + logger.debug( + 'Running setup.py (path:%s) egg_info for package %s', + self.setup_py, self.name, + ) + else: + logger.debug( + 'Running setup.py (path:%s) egg_info for package from %s', + self.setup_py, self.link, + ) script = SETUPTOOLS_SHIM % self.setup_py base_cmd = [sys.executable, '-c', script] if self.isolated: