From 5e97de47730c6c2e308def0115227cc3138c5405 Mon Sep 17 00:00:00 2001 From: Christopher Hunt Date: Wed, 21 Aug 2019 05:19:02 -0400 Subject: [PATCH] Ignore errors copying socket files for source installs (in Python 3). (#6844) --- news/5306.bugfix | 1 + src/pip/_internal/download.py | 84 +++++++++++++++++++++----- src/pip/_internal/utils/filesystem.py | 31 ++++++++++ tests/functional/test_install.py | 25 ++++++++ tests/lib/filesystem.py | 48 +++++++++++++++ tests/unit/test_download.py | 87 +++++++++++++++++++++++++++ tests/unit/test_utils_filesystem.py | 61 +++++++++++++++++++ tox.ini | 3 +- 8 files changed, 324 insertions(+), 16 deletions(-) create mode 100644 news/5306.bugfix create mode 100644 tests/lib/filesystem.py create mode 100644 tests/unit/test_utils_filesystem.py diff --git a/news/5306.bugfix b/news/5306.bugfix new file mode 100644 index 000000000..bf040a95f --- /dev/null +++ b/news/5306.bugfix @@ -0,0 +1 @@ +Ignore errors copying socket files for local source installs (in Python 3). diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 159a4433e..bb9bd8685 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -21,6 +21,7 @@ from pip._vendor.requests.auth import AuthBase, HTTPBasicAuth from pip._vendor.requests.models import CONTENT_CHUNK_SIZE, Response from pip._vendor.requests.structures import CaseInsensitiveDict from pip._vendor.requests.utils import get_netrc_auth +from pip._vendor.six import PY2 # NOTE: XMLRPC Client is not annotated in typeshed as on 2017-07-17, which is # why we ignore the type on this import from pip._vendor.six.moves import xmlrpc_client # type: ignore @@ -33,7 +34,7 @@ from pip._internal.models.index import PyPI # 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 +from pip._internal.utils.filesystem import check_path_owner, copy2_fixed 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 ( @@ -49,6 +50,7 @@ from pip._internal.utils.misc import ( format_size, get_installed_version, netloc_has_port, + path_to_display, path_to_url, remove_auth_from_url, rmtree, @@ -63,15 +65,39 @@ from pip._internal.vcs import vcs if MYPY_CHECK_RUNNING: from typing import ( - Optional, Tuple, Dict, IO, Text, Union + Callable, Dict, List, IO, Optional, Text, Tuple, Union ) from optparse import Values + + from mypy_extensions import TypedDict + from pip._internal.models.link import Link from pip._internal.utils.hashes import Hashes from pip._internal.vcs.versioncontrol import AuthInfo, VersionControl Credentials = Tuple[str, str, str] + if PY2: + CopytreeKwargs = TypedDict( + 'CopytreeKwargs', + { + 'ignore': Callable[[str, List[str]], List[str]], + 'symlinks': bool, + }, + total=False, + ) + else: + CopytreeKwargs = TypedDict( + 'CopytreeKwargs', + { + 'copy_function': Callable[[str, str], None], + 'ignore': Callable[[str, List[str]], List[str]], + 'ignore_dangling_symlinks': bool, + 'symlinks': bool, + }, + total=False, + ) + __all__ = ['get_file_content', 'is_url', 'url_to_path', 'path_to_url', @@ -939,6 +965,46 @@ def unpack_http_url( os.unlink(from_path) +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 + def ignore(d, names): + # 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 + return ['.tox', '.nox'] if d == source else [] + + 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 unpack_file_url( link, # type: Link location, # type: str @@ -954,21 +1020,9 @@ def unpack_file_url( link_path = url_to_path(link.url_without_fragment) # If it's a url to a local directory if is_dir_url(link): - - def ignore(d, names): - # 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 - return ['.tox', '.nox'] if d == link_path else [] - if os.path.isdir(location): rmtree(location) - shutil.copytree(link_path, - location, - symlinks=True, - ignore=ignore) - + _copy_source_tree(link_path, location) if download_dir: logger.info('Link is a directory, ignoring download_dir') return diff --git a/src/pip/_internal/utils/filesystem.py b/src/pip/_internal/utils/filesystem.py index 1e6b03385..c5233ebbc 100644 --- a/src/pip/_internal/utils/filesystem.py +++ b/src/pip/_internal/utils/filesystem.py @@ -1,5 +1,7 @@ import os import os.path +import shutil +import stat from pip._internal.utils.compat import get_path_uid @@ -28,3 +30,32 @@ def check_path_owner(path): else: previous, path = path, os.path.dirname(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("`%s` is a socket" % f) + + raise + + +def is_socket(path): + # type: (str) -> bool + return stat.S_ISSOCK(os.lstat(path).st_mode) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 6e1e3dd40..3151b77bb 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -1,6 +1,7 @@ import distutils import glob import os +import shutil import sys import textwrap from os.path import curdir, join, pardir @@ -23,6 +24,7 @@ from tests.lib import ( pyversion_tuple, requirements_file, ) +from tests.lib.filesystem import make_socket_file from tests.lib.local_repos import local_checkout from tests.lib.path import Path @@ -488,6 +490,29 @@ 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%s.egg-info" % pyversion + ) + 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, 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): """ Test installing from a local directory with no 'setup.py'. diff --git a/tests/lib/filesystem.py b/tests/lib/filesystem.py new file mode 100644 index 000000000..dc14b323e --- /dev/null +++ b/tests/lib/filesystem.py @@ -0,0 +1,48 @@ +"""Helpers for filesystem-dependent tests. +""" +import os +import socket +import subprocess +import sys +from functools import partial +from itertools import chain + +from .path import Path + + +def make_socket_file(path): + # Socket paths are limited to 108 characters (sometimes less) so we + # chdir before creating it and use a relative path name. + cwd = os.getcwd() + os.chdir(os.path.dirname(path)) + try: + sock = socket.socket(socket.AF_UNIX) + sock.bind(os.path.basename(path)) + finally: + os.chdir(cwd) + + +def make_unreadable_file(path): + Path(path).touch() + os.chmod(path, 0o000) + if sys.platform == "win32": + # Once we drop PY2 we can use `os.getlogin()` instead. + username = os.environ["USERNAME"] + # Remove "Read Data/List Directory" permission for current user, but + # leave everything else. + args = ["icacls", path, "/deny", username + ":(RD)"] + subprocess.check_call(args) + + +def get_filelist(base): + def join(dirpath, dirnames, filenames): + relative_dirpath = os.path.relpath(dirpath, base) + join_dirpath = partial(os.path.join, relative_dirpath) + return chain( + (join_dirpath(p) for p in dirnames), + (join_dirpath(p) for p in filenames), + ) + + return set(chain.from_iterable( + join(*dirinfo) for dirinfo in os.walk(base) + )) diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py index f7595f52b..22eb9576d 100644 --- a/tests/unit/test_download.py +++ b/tests/unit/test_download.py @@ -1,6 +1,7 @@ import functools import hashlib import os +import shutil import sys from io import BytesIO from shutil import copy, rmtree @@ -15,6 +16,7 @@ from pip._internal.download import ( MultiDomainBasicAuth, PipSession, SafeFileCache, + _copy_source_tree, _download_http_url, _get_url_scheme, parse_content_disposition, @@ -28,6 +30,12 @@ from pip._internal.models.link import Link from pip._internal.utils.hashes import Hashes from pip._internal.utils.misc import path_to_url from tests.lib import create_file +from tests.lib.filesystem import ( + get_filelist, + make_socket_file, + make_unreadable_file, +) +from tests.lib.path import Path @pytest.fixture(scope="function") @@ -334,6 +342,85 @@ def test_url_to_path_path_to_url_symmetry_win(): assert url_to_path(path_to_url(unc_path)) == unc_path +@pytest.fixture +def clean_project(tmpdir_factory, data): + tmpdir = Path(str(tmpdir_factory.mktemp("clean_project"))) + new_project_dir = tmpdir.joinpath("FSPkg") + path = data.packages.joinpath("FSPkg") + shutil.copytree(path, new_project_dir) + return new_project_dir + + +def test_copy_source_tree(clean_project, tmpdir): + target = tmpdir.joinpath("target") + expected_files = get_filelist(clean_project) + assert len(expected_files) == 3 + + _copy_source_tree(clean_project, target) + + copied_files = get_filelist(target) + assert expected_files == copied_files + + +@pytest.mark.skipif("sys.platform == 'win32' or sys.version_info < (3,)") +def test_copy_source_tree_with_socket(clean_project, tmpdir, caplog): + target = tmpdir.joinpath("target") + expected_files = get_filelist(clean_project) + 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' or sys.version_info < (3,)") +def test_copy_source_tree_with_socket_fails_with_no_socket_error( + clean_project, tmpdir +): + target = tmpdir.joinpath("target") + expected_files = get_filelist(clean_project) + make_socket_file(clean_project.joinpath("aaa")) + unreadable_file = clean_project.joinpath("bbb") + make_unreadable_file(unreadable_file) + + with pytest.raises(shutil.Error) as e: + _copy_source_tree(clean_project, target) + + errored_files = [err[0] for err in e.value.args[0]] + assert len(errored_files) == 1 + assert unreadable_file in errored_files + + copied_files = get_filelist(target) + # All files without errors should have been copied. + assert expected_files == copied_files + + +def test_copy_source_tree_with_unreadable_dir_fails(clean_project, tmpdir): + target = tmpdir.joinpath("target") + expected_files = get_filelist(clean_project) + unreadable_file = clean_project.joinpath("bbb") + make_unreadable_file(unreadable_file) + + with pytest.raises(shutil.Error) as e: + _copy_source_tree(clean_project, target) + + errored_files = [err[0] for err in e.value.args[0]] + assert len(errored_files) == 1 + assert unreadable_file in errored_files + + copied_files = get_filelist(target) + # All files without errors should have been copied. + assert expected_files == copied_files + + class Test_unpack_file_url(object): def prep(self, tmpdir, data): diff --git a/tests/unit/test_utils_filesystem.py b/tests/unit/test_utils_filesystem.py new file mode 100644 index 000000000..3ef814dce --- /dev/null +++ b/tests/unit/test_utils_filesystem.py @@ -0,0 +1,61 @@ +import os +import shutil + +import pytest + +from pip._internal.utils.filesystem import copy2_fixed, is_socket +from tests.lib.filesystem import make_socket_file, make_unreadable_file +from tests.lib.path import Path + + +def make_file(path): + Path(path).touch() + + +def make_valid_symlink(path): + target = path + "1" + make_file(target) + os.symlink(target, path) + + +def make_broken_symlink(path): + os.symlink("foo", path) + + +def make_dir(path): + os.mkdir(path) + + +skip_on_windows = pytest.mark.skipif("sys.platform == 'win32'") + + +@skip_on_windows +@pytest.mark.parametrize("create,result", [ + (make_socket_file, True), + (make_file, False), + (make_valid_symlink, False), + (make_broken_symlink, False), + (make_dir, False), +]) +def test_is_socket(create, result, tmpdir): + target = tmpdir.joinpath("target") + create(target) + assert os.path.lexists(target) + assert is_socket(target) == result + + +@pytest.mark.parametrize("create,error_type", [ + pytest.param( + make_socket_file, shutil.SpecialFileError, marks=skip_on_windows + ), + (make_unreadable_file, OSError), +]) +def test_copy2_fixed_raises_appropriate_errors(create, error_type, tmpdir): + src = tmpdir.joinpath("src") + create(src) + dest = tmpdir.joinpath("dest") + + with pytest.raises(error_type): + copy2_fixed(src, dest) + + assert not dest.exists() diff --git a/tox.ini b/tox.ini index fc1f3bd98..608d491e3 100644 --- a/tox.ini +++ b/tox.ini @@ -10,7 +10,8 @@ envlist = pip = python {toxinidir}/tools/tox_pip.py [testenv] -passenv = CI GIT_SSL_CAINFO +# Remove USERNAME once we drop PY2. +passenv = CI GIT_SSL_CAINFO USERNAME setenv = # This is required in order to get UTF-8 output inside of the subprocesses # that our tests use. -- GitLab