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 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()

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

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

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

tests/data/src/pep518_conflicting_requires/pep518.py

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

tests/data/src/pep518_conflicting_requires/pyproject.toml

@@ -0,0 +1,2 @@
+[build-system]
+requires = ["setuptools==1.0", "wheel"]

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

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,

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