Merge pull request #7882 from sbidoul/build-in-place-7555-sbi

Build local directories in place

Author: pradyunsg

docs/html/reference/pip_install.rst

@@ -728,8 +728,13 @@ You can install local projects by specifying the project path to pip::
 
 $ pip install path/to/SomeProject
 
-During regular installation, pip will copy the entire project directory to a temporary location and install from there.
-The exception is that pip will exclude .tox and .nox directories present in the top level of the project from being copied.
+Until version 20.0, pip copied the entire project directory to a temporary
+location and installed from there. This approach was the cause of several
+performance and correctness issues. As of version 20.1 pip installs from the
+local project directory. Depending on the build backend used by the project,
+this may generate secondary build artifacts in the project directory, such as
+the ``.egg-info`` and ``build`` directories in the case of the setuptools
+backend.
 
 
 .. _`editable-installs`:

news/7555.removal

@@ -0,0 +1,9 @@
+Building of local directories is now done in place. Previously pip did copy the
+local directory tree to a temporary location before building. That approach had
+a number of drawbacks, among which performance issues, as well as various
+issues arising when the python project directory depends on its parent
+directory (such as the presence of a VCS directory). The user visible effect of
+this change is that secondary build artifacts, if any, may therefore be created
+in the local directory, whereas before they were created in a temporary copy of
+the directory and then deleted. This notably includes the ``build`` and
+``.egg-info`` directories in the case of the setuptools build backend.

src/pip/_internal/operations/prepare.py

@@ -24,15 +24,9 @@
     PreviousBuildDirError,
     VcsHashUnsupported,
 )
-from pip._internal.utils.filesystem import copy2_fixed
 from pip._internal.utils.hashes import MissingHashes
 from pip._internal.utils.logging import indent_log
-from pip._internal.utils.misc import (
-    display_path,
-    hide_url,
-    path_to_display,
-    rmtree,
-)
+from pip._internal.utils.misc import display_path, hide_url
 from pip._internal.utils.temp_dir import TempDirectory
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
 from pip._internal.utils.unpacking import unpack_file
@@ -133,59 +127,6 @@ def get_http_url(
     return File(from_path, content_type)
 
 
-def _copy2_ignoring_special_files(src, dest):
-    # type: (str, str) -> None
-    """Copying special files is not supported, but as a convenience to users
-    we skip errors copying them. This supports tools that may create e.g.
-    socket files in the project source directory.
-    """
-    try:
-        copy2_fixed(src, dest)
-    except shutil.SpecialFileError as e:
-        # SpecialFileError may be raised due to either the source or
-        # destination. If the destination was the cause then we would actually
-        # care, but since the destination directory is deleted prior to
-        # copy we ignore all of them assuming it is caused by the source.
-        logger.warning(
-            "Ignoring special file error '%s' encountered copying %s to %s.",
-            str(e),
-            path_to_display(src),
-            path_to_display(dest),
-        )
-
-
-def _copy_source_tree(source, target):
-    # type: (str, str) -> None
-    target_abspath = os.path.abspath(target)
-    target_basename = os.path.basename(target_abspath)
-    target_dirname = os.path.dirname(target_abspath)
-
-    def ignore(d, names):
-        # type: (str, List[str]) -> List[str]
-        skipped = []  # type: List[str]
-        if d == source:
-            # Pulling in those directories can potentially be very slow,
-            # exclude the following directories if they appear in the top
-            # level dir (and only it).
-            # See discussion at https://github.com/pypa/pip/pull/6770
-            skipped += ['.tox', '.nox']
-        if os.path.abspath(d) == target_dirname:
-            # Prevent an infinite recursion if the target is in source.
-            # This can happen when TMPDIR is set to ${PWD}/...
-            # and we copy PWD to TMPDIR.
-            skipped += [target_basename]
-        return skipped
-
-    kwargs = dict(ignore=ignore, symlinks=True)  # type: CopytreeKwargs
-
-    if not PY2:
-        # Python 2 does not support copy_function, so we only ignore
-        # errors on special file copy in Python 3.
-        kwargs['copy_function'] = _copy2_ignoring_special_files
-
-    shutil.copytree(source, target, **kwargs)
-
-
 def get_file_url(
     link,  # type: Link
     download_dir=None,  # type: Optional[str]
@@ -239,11 +180,9 @@ def unpack_url(
         unpack_vcs_link(link, location)
         return None
 
-    # If it's a url to a local directory
+    # If it's a url to a local directory, we build in-place.
+    # There is nothing to be done here.
     if link.is_existing_dir():
-        if os.path.isdir(location):
-            rmtree(location)
-        _copy_source_tree(link.file_path, location)
         return None
 
     # file urls
@@ -415,21 +354,25 @@ def prepare_linked_requirement(
         with indent_log():
             # Since source_dir is only set for editable requirements.
             assert req.source_dir is None
-            req.ensure_has_source_dir(self.build_dir, autodelete_unpacked)
-            # If a checkout exists, it's unwise to keep going.  version
-            # inconsistencies are logged later, but do not fail the
-            # installation.
-            # FIXME: this won't upgrade when there's an existing
-            # package unpacked in `req.source_dir`
-            if os.path.exists(os.path.join(req.source_dir, 'setup.py')):
-                raise PreviousBuildDirError(
-                    "pip can't proceed with requirements '{}' due to a"
-                    " pre-existing build directory ({}). This is "
-                    "likely due to a previous installation that failed"
-                    ". pip is being responsible and not assuming it "
-                    "can delete this. Please delete it and try again."
-                    .format(req, req.source_dir)
-                )
+            if link.is_existing_dir():
+                # Build local directories in place.
+                req.source_dir = link.file_path
+            else:
+                req.ensure_has_source_dir(self.build_dir, autodelete_unpacked)
+                # If a checkout exists, it's unwise to keep going.  version
+                # inconsistencies are logged later, but do not fail the
+                # installation.
+                # FIXME: this won't upgrade when there's an existing
+                # package unpacked in `req.source_dir`
+                if os.path.exists(os.path.join(req.source_dir, 'setup.py')):
+                    raise PreviousBuildDirError(
+                        "pip can't proceed with requirements '{}' due to a"
+                        " pre-existing build directory ({}). This is "
+                        "likely due to a previous installation that failed"
+                        ". pip is being responsible and not assuming it "
+                        "can delete this. Please delete it and try again."
+                        .format(req, req.source_dir)
+                    )
 
             # Now that we have the real link, we can tell what kind of
             # requirements we have and raise some more informative errors

src/pip/_internal/utils/filesystem.py

@@ -3,8 +3,6 @@
 import os
 import os.path
 import random
-import shutil
-import stat
 import sys
 from contextlib import contextmanager
 from tempfile import NamedTemporaryFile
@@ -56,36 +54,6 @@ def check_path_owner(path):
     return False  # assume we don't own the path
 
 
-def copy2_fixed(src, dest):
-    # type: (str, str) -> None
-    """Wrap shutil.copy2() but map errors copying socket files to
-    SpecialFileError as expected.
-
-    See also https://bugs.python.org/issue37700.
-    """
-    try:
-        shutil.copy2(src, dest)
-    except (OSError, IOError):
-        for f in [src, dest]:
-            try:
-                is_socket_file = is_socket(f)
-            except OSError:
-                # An error has already occurred. Another error here is not
-                # a problem and we can ignore it.
-                pass
-            else:
-                if is_socket_file:
-                    raise shutil.SpecialFileError(
-                        "`{f}` is a socket".format(**locals()))
-
-        raise
-
-
-def is_socket(path):
-    # type: (str) -> bool
-    return stat.S_ISSOCK(os.lstat(path).st_mode)
-
-
 @contextmanager
 def adjacent_tmp_file(path, **kwargs):
     # type: (str, **Any) -> Iterator[NamedTemporaryFileResult]

tests/functional/test_cli.py

@@ -27,7 +27,9 @@ def test_entrypoints_work(entrypoint, script):
     )
     """.format(entrypoint)))
 
-    script.pip("install", "-vvv", str(fake_pkg))
+    # expect_temp=True, because pip install calls setup.py which
+    # in turn creates fake_pkg.egg-info.
+    script.pip("install", "-vvv", str(fake_pkg), expect_temp=True)
     result = script.pip("-V")
     result2 = script.run("fake_pip", "-V", allow_stderr_warning=True)
     assert result.stdout == result2.stdout

tests/functional/test_install.py

@@ -2,7 +2,6 @@
 import glob
 import os
 import re
-import shutil
 import ssl
 import sys
 import textwrap
@@ -29,7 +28,6 @@
     skip_if_python2,
     windows_workaround_7667,
 )
-from tests.lib.filesystem import make_socket_file
 from tests.lib.local_repos import local_checkout
 from tests.lib.path import Path
 from tests.lib.server import (
@@ -576,30 +574,6 @@ def test_install_from_local_directory_with_symlinks_to_directories(
     assert egg_info_folder in result.files_created, str(result)
 
 
-@pytest.mark.skipif("sys.platform == 'win32' or sys.version_info < (3,)")
-def test_install_from_local_directory_with_socket_file(script, data, tmpdir):
-    """
-    Test installing from a local directory containing a socket file.
-    """
-    egg_info_file = (
-        script.site_packages /
-        "FSPkg-0.1.dev0-py{pyversion}.egg-info".format(**globals())
-    )
-    package_folder = script.site_packages / "fspkg"
-    to_copy = data.packages.joinpath("FSPkg")
-    to_install = tmpdir.joinpath("src")
-
-    shutil.copytree(to_copy, to_install)
-    # Socket file, should be ignored.
-    socket_file_path = os.path.join(to_install, "example")
-    make_socket_file(socket_file_path)
-
-    result = script.pip("install", "--verbose", to_install)
-    assert package_folder in result.files_created, str(result.stdout)
-    assert egg_info_file in result.files_created, str(result)
-    assert str(socket_file_path) in result.stderr
-
-
 def test_install_from_local_directory_with_no_setup_py(script, data):
     """
     Test installing from a local directory with no 'setup.py'.

tests/functional/test_uninstall.py

@@ -271,7 +271,15 @@ def test_uninstall_console_scripts(script):
         sorted(result.files_created.keys())
     )
     result2 = script.pip('uninstall', 'discover', '-y')
-    assert_all_changes(result, result2, [script.venv / 'build', 'cache'])
+    assert_all_changes(
+        result,
+        result2,
+        [
+            script.venv / 'build',
+            'cache',
+            script.scratch / 'discover' / 'discover.egg-info',
+        ]
+    )
 
 
 def test_uninstall_console_scripts_uppercase_name(script):