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

@@ -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
             }
         }

src/pip/_internal/build_env.py

@@ -5,6 +5,7 @@ import logging
 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 @@ from pip._internal.utils.ui import open_spinner
 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 @@ class BuildEnvironment(object):
         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 @@ class NoOpBuildEnvironment(BuildEnvironment):
     def cleanup(self):
         pass
 
-    def install_requirements(self, finder, requirements, message):
+    def install_requirements(self, finder, requirements, prefix, message):
         raise NotImplementedError()

src/pip/_internal/operations/prepare.py

@@ -100,26 +100,35 @@ class IsSDist(DistAbstraction):
         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)))
                 )
 

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)

tests/data/src/pep518_conflicting_requires/MANIFEST.in

+include pyproject.toml

tests/data/src/pep518_conflicting_requires/pep518.py

+#dummy

tests/data/src/pep518_conflicting_requires/pyproject.toml

+[build-system]
+requires = ["setuptools==1.0", "wheel"]

tests/data/src/pep518_conflicting_requires/setup.py

+#!/usr/bin/env python
+from setuptools import setup
+
+setup(
+    name='pep518_conflicting_requires',
+    version='1.0.0',
+    py_modules=['pep518'],
+)

tests/functional/test_install.py

@@ -12,9 +12,9 @@ from pip._internal.cli.status_codes import ERROR, SUCCESS
 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,

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"

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}"
         """,

tests/unit/test_build_env.py

+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)
+        ''')

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