1
1
Fork 0
mirror of https://github.com/pypa/pip synced 2023-12-13 21:30:23 +01:00

Ignore errors in temporary directory cleanup

pip should not exit with an error when it fails to cleanup temporary
files after it has already successfully installed packages.
This commit is contained in:
Ales Erjavec 2019-12-09 16:59:36 +01:00
parent 593b85f4ab
commit 660dafb37f
4 changed files with 94 additions and 13 deletions

View file

@ -11,6 +11,7 @@ import stat
import sys import sys
import sysconfig import sysconfig
import urllib.parse import urllib.parse
from functools import partial
from io import StringIO from io import StringIO
from itertools import filterfalse, tee, zip_longest from itertools import filterfalse, tee, zip_longest
from types import TracebackType from types import TracebackType
@ -123,15 +124,35 @@ def get_prog() -> str:
# Retry every half second for up to 3 seconds # Retry every half second for up to 3 seconds
# Tenacity raises RetryError by default, explicitly raise the original exception # Tenacity raises RetryError by default, explicitly raise the original exception
@retry(reraise=True, stop=stop_after_delay(3), wait=wait_fixed(0.5)) @retry(reraise=True, stop=stop_after_delay(3), wait=wait_fixed(0.5))
def rmtree(dir: str, ignore_errors: bool = False) -> None: def rmtree(
dir: str,
ignore_errors: bool = False,
onexc: Optional[Callable[[Any, Any, Any], Any]] = None,
) -> None:
if ignore_errors:
onexc = _onerror_ignore
elif onexc is None:
onexc = _onerror_reraise
if sys.version_info >= (3, 12): if sys.version_info >= (3, 12):
shutil.rmtree(dir, ignore_errors=ignore_errors, onexc=rmtree_errorhandler) shutil.rmtree(dir, onexc=partial(rmtree_errorhandler, onexc=onexc))
else: else:
shutil.rmtree(dir, ignore_errors=ignore_errors, onerror=rmtree_errorhandler) shutil.rmtree(dir, onerror=partial(rmtree_errorhandler, onexc=onexc))
def _onerror_ignore(*_args: Any) -> None:
pass
def _onerror_reraise(*_args: Any) -> None:
raise
def rmtree_errorhandler( def rmtree_errorhandler(
func: Callable[..., Any], path: str, exc_info: Union[ExcInfo, BaseException] func: Callable[..., Any],
path: str,
exc_info: Union[ExcInfo, BaseException],
*,
onexc: Callable[..., Any] = _onerror_reraise,
) -> None: ) -> None:
"""On Windows, the files in .svn are read-only, so when rmtree() tries to """On Windows, the files in .svn are read-only, so when rmtree() tries to
remove them, an exception is thrown. We catch that here, remove the remove them, an exception is thrown. We catch that here, remove the
@ -146,10 +167,13 @@ def rmtree_errorhandler(
# convert to read/write # convert to read/write
os.chmod(path, stat.S_IWRITE) os.chmod(path, stat.S_IWRITE)
# use the original function to repeat the operation # use the original function to repeat the operation
func(path) try:
return func(path)
else: return
raise except OSError:
pass
onexc(func, path, exc_info)
def display_path(path: str) -> str: def display_path(path: str) -> str:

View file

@ -3,8 +3,9 @@ import itertools
import logging import logging
import os.path import os.path
import tempfile import tempfile
import traceback
from contextlib import ExitStack, contextmanager from contextlib import ExitStack, contextmanager
from typing import Any, Dict, Generator, Optional, TypeVar, Union from typing import Any, Callable, Dict, Generator, Optional, Tuple, Type, TypeVar, Union
from pip._internal.utils.misc import enum, rmtree from pip._internal.utils.misc import enum, rmtree
@ -106,6 +107,7 @@ class TempDirectory:
delete: Union[bool, None, _Default] = _default, delete: Union[bool, None, _Default] = _default,
kind: str = "temp", kind: str = "temp",
globally_managed: bool = False, globally_managed: bool = False,
ignore_cleanup_errors: bool = True,
): ):
super().__init__() super().__init__()
@ -128,6 +130,7 @@ class TempDirectory:
self._deleted = False self._deleted = False
self.delete = delete self.delete = delete
self.kind = kind self.kind = kind
self.ignore_cleanup_errors = ignore_cleanup_errors
if globally_managed: if globally_managed:
assert _tempdir_manager is not None assert _tempdir_manager is not None
@ -170,7 +173,34 @@ class TempDirectory:
self._deleted = True self._deleted = True
if not os.path.exists(self._path): if not os.path.exists(self._path):
return return
rmtree(self._path)
def onerror(
func: Callable[[str], Any],
path: str,
exc_info: Tuple[Type[BaseException], BaseException, Any],
) -> None:
"""Log a warning for a `rmtree` error and continue"""
exc_val = "\n".join(traceback.format_exception_only(*exc_info[:2]))
exc_val = exc_val.rstrip() # remove trailing new line
if func in (os.unlink, os.remove, os.rmdir):
logging.warning(
"Failed to remove a temporary file '%s' due to %s.\n"
"You can safely remove it manually.",
path,
exc_val,
)
else:
logging.warning("%s failed with %s.", func.__qualname__, exc_val)
if self.ignore_cleanup_errors:
try:
# first try with tenacity; retrying to handle ephemeral errors
rmtree(self._path, ignore_errors=False)
except OSError:
# last pass ignore/log all errors
rmtree(self._path, onexc=onerror)
else:
rmtree(self._path)
class AdjacentTempDirectory(TempDirectory): class AdjacentTempDirectory(TempDirectory):

View file

@ -257,9 +257,13 @@ def test_rmtree_errorhandler_reraises_error(tmpdir: Path) -> None:
except RuntimeError: except RuntimeError:
# Make sure the handler reraises an exception # Make sure the handler reraises an exception
with pytest.raises(RuntimeError, match="test message"): with pytest.raises(RuntimeError, match="test message"):
# Argument 3 to "rmtree_errorhandler" has incompatible type "None"; expected # Argument 3 to "rmtree_errorhandler" has incompatible type
# "Tuple[Type[BaseException], BaseException, TracebackType]" # "Union[Tuple[Type[BaseException], BaseException, TracebackType],
rmtree_errorhandler(mock_func, path, None) # type: ignore[arg-type] # Tuple[None, None, None]]"; expected "Tuple[Type[BaseException],
# BaseException, TracebackType]"
rmtree_errorhandler(
mock_func, path, sys.exc_info() # type: ignore[arg-type]
)
mock_func.assert_not_called() mock_func.assert_not_called()

View file

@ -4,6 +4,7 @@ import stat
import tempfile import tempfile
from pathlib import Path from pathlib import Path
from typing import Any, Iterator, Optional, Union from typing import Any, Iterator, Optional, Union
from unittest import mock
import pytest import pytest
@ -274,3 +275,25 @@ def test_tempdir_registry_lazy(should_delete: bool) -> None:
registry.set_delete("test-for-lazy", should_delete) registry.set_delete("test-for-lazy", should_delete)
assert os.path.exists(path) assert os.path.exists(path)
assert os.path.exists(path) == (not should_delete) assert os.path.exists(path) == (not should_delete)
def test_tempdir_cleanup_ignore_errors() -> None:
os_unlink = os.unlink
# mock os.unlink to fail with EACCES for a specific filename to simulate
# how removing a loaded exe/dll behaves.
def unlink(name: str, *args: Any, **kwargs: Any) -> None:
if "bomb" in name:
raise PermissionError(name)
else:
os_unlink(name)
with mock.patch("os.unlink", unlink):
with TempDirectory(ignore_cleanup_errors=True) as tmp_dir:
path = tmp_dir.path
with open(os.path.join(path, "bomb"), "a"):
pass
filename = os.path.join(path, "bomb")
assert os.path.isfile(filename)
os.unlink(filename)