Address review comments.

Author: chrahunt

src/pip/_internal/download.py

@@ -32,7 +32,7 @@
 # Import ssl from compat so the initial import occurs in only one place.
 from pip._internal.utils.compat import HAS_TLS, ssl
 from pip._internal.utils.encoding import auto_decode
-from pip._internal.utils.filesystem import check_path_owner, is_socket
+from pip._internal.utils.filesystem import check_path_owner, copytree
 from pip._internal.utils.glibc import libc_ver
 from pip._internal.utils.marker_files import write_delete_marker_file
 from pip._internal.utils.misc import (
@@ -46,6 +46,7 @@
     display_path,
     format_size,
     get_installed_version,
+    path_to_display,
     path_to_url,
     remove_auth_from_url,
     rmtree,
@@ -60,7 +61,7 @@
 
 if MYPY_CHECK_RUNNING:
     from typing import (
-        Dict, IO, List, Optional, Text, Tuple, Union
+        Dict, IO, Optional, Text, Tuple, Union
     )
     from optparse import Values
     from pip._internal.models.link import Link
@@ -963,25 +964,36 @@ def ignore(d, names):
         # See discussion at https://github.com/pypa/pip/pull/6770
         return ['.tox', '.nox'] if d == source else []
 
+    def ignore_special_file_errors(error_details):
+        # Copying special files is not supported, so we skip errors related to
+        # them. This is a convenience to support users that may have tools
+        # creating e.g. socket files in their source directory.
+        src, dest, error = error_details
+
+        if not isinstance(error, shutil.SpecialFileError):
+            # Then it is some other kind of error that we do want to report.
+            return True
+
+        # 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(error),
+            path_to_display(src),
+            path_to_display(dest),
+        )
+
     try:
-        shutil.copytree(source, target, ignore=ignore, symlinks=True)
+        copytree(source, target, ignore=ignore, symlinks=True)
     except shutil.Error as e:
-        errors = e.args[0]  # type: List[Tuple[str, str, shutil.Error]]
-        # Users may have locally-created socket files in the source
-        # directory. Copying socket files is not supported, so we skip errors
-        # related to them, for convenience.
-
-        # Copy list to avoid mutation while iterating.
-        errors_copy = errors[:]
-        for i, err in reversed(list(enumerate(errors_copy))):
-            src, _dest, _error = err
-            if is_socket(src):
-                # Remove errors related to sockets to prevent distractions if
-                # we end up re-raising.
-                errors.pop(i)
-
-        if errors:
-            raise
+        errors = e.args[0]
+        normal_file_errors = list(
+            filter(ignore_special_file_errors, errors)
+        )
+        if normal_file_errors:
+            raise shutil.Error(normal_file_errors)
 
 
 def unpack_file_url(

src/pip/_internal/utils/filesystem.py

@@ -1,5 +1,6 @@
 import os
 import os.path
+import shutil
 import stat
 
 from pip._internal.utils.compat import get_path_uid
@@ -31,6 +32,35 @@ def check_path_owner(path):
     return False  # assume we don't own the path
 
 
+def copytree(*args, **kwargs):
+    """Wrap shutil.copytree() to map errors copying socket file to
+    SpecialFileError.
+
+    See also https://bugs.python.org/issue37700.
+    """
+    def to_correct_error(src, dest, error):
+        for f in [src, dest]:
+            try:
+                if is_socket(f):
+                    new_error = shutil.SpecialFileError("`%s` is a socket" % f)
+                    return (src, dest, new_error)
+            except OSError:
+                # An error has already occurred. Another error here is not
+                # a problem and we can ignore it.
+                pass
+
+        return (src, dest, error)
+
+    try:
+        shutil.copytree(*args, **kwargs)
+    except shutil.Error as e:
+        errors = e.args[0]
+        new_errors = [
+            to_correct_error(src, dest, error) for src, dest, error in errors
+        ]
+        raise shutil.Error(new_errors)
+
+
 def is_socket(path):
     # type: (str) -> bool
     return stat.S_ISSOCK(os.lstat(path).st_mode)

tests/functional/test_install.py

@@ -504,11 +504,13 @@ def test_install_from_local_directory_with_socket_file(script, data, tmpdir):
 
     shutil.copytree(to_copy, to_install)
     # Socket file, should be ignored.
-    make_socket_file(os.path.join(to_install, "example"))
+    socket_file_path = os.path.join(to_install, "example")
+    make_socket_file(socket_file_path)
 
-    result = script.pip("install", to_install, expect_error=False)
+    result = script.pip("install", "--verbose", to_install, expect_error=False)
     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):

tests/unit/test_download.py

@@ -363,16 +363,23 @@ def test_copy_source_tree(clean_project, tmpdir):
 
 
 @pytest.mark.skipif("sys.platform == 'win32'")
-def test_copy_source_tree_with_socket(clean_project, tmpdir):
+def test_copy_source_tree_with_socket(clean_project, tmpdir, caplog):
     target = tmpdir.joinpath("target")
     expected_files = get_filelist(clean_project)
-    make_socket_file(clean_project.joinpath("aaa"))
+    socket_path = str(clean_project.joinpath("aaa"))
+    make_socket_file(socket_path)
 
     _copy_source_tree(clean_project, target)
 
     copied_files = get_filelist(target)
     assert expected_files == copied_files
 
+    # Warning should have been logged.
+    assert len(caplog.records) == 1
+    record = caplog.records[0]
+    assert record.levelname == 'WARNING'
+    assert socket_path in record.message
+
 
 @pytest.mark.skipif("sys.platform == 'win32'")
 def test_copy_source_tree_with_socket_fails_with_no_socket_error(
@@ -392,7 +399,7 @@ def test_copy_source_tree_with_socket_fails_with_no_socket_error(
     assert unreadable_file in errored_files
 
     copied_files = get_filelist(target)
-    # Even with errors, all files should have been copied.
+    # All files without errors should have been copied.
     assert expected_files == copied_files
 
 
@@ -410,7 +417,7 @@ def test_copy_source_tree_with_unreadable_dir_fails(clean_project, tmpdir):
     assert unreadable_file in errored_files
 
     copied_files = get_filelist(target)
-    # Even with errors, all files should have been copied.
+    # All files without errors should have been copied.
     assert expected_files == copied_files
 
 

tests/unit/test_utils_filesystem.py

@@ -1,8 +1,9 @@
 import os
+import shutil
 
 import pytest
 
-from pip._internal.utils.filesystem import is_socket
+from pip._internal.utils.filesystem import copytree, is_socket
 
 from ..lib.filesystem import make_socket_file
 from ..lib.path import Path
@@ -39,3 +40,30 @@ def test_is_socket(create, result, tmpdir):
     create(target)
     assert os.path.lexists(target)
     assert is_socket(target) == result
+
+
+@pytest.mark.skipif("sys.platform == 'win32'")
+def test_copytree_maps_socket_errors(tmpdir):
+    src_dir = tmpdir.joinpath("src")
+    make_dir(src_dir)
+    make_file(src_dir.joinpath("a"))
+    socket_src = src_dir.joinpath("b")
+    make_socket_file(socket_src)
+    make_file(src_dir.joinpath("c"))
+
+    dest_dir = tmpdir.joinpath("dest")
+    socket_dest = dest_dir.joinpath("b")
+
+    with pytest.raises(shutil.Error) as e:
+        copytree(src_dir, dest_dir)
+
+    errors = e.value.args[0]
+    assert len(errors) == 1
+    src, dest, error = errors[0]
+    assert src == str(socket_src)
+    assert dest == str(socket_dest)
+    assert isinstance(error, shutil.SpecialFileError)
+
+    assert dest_dir.joinpath("a").exists()
+    assert not socket_dest.exists()
+    assert dest_dir.joinpath("c").exists()