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