From eeb3d8fdff94080b7978c4d2d7126020cddf9c93 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri, 28 Jul 2023 01:00:06 -0400 Subject: [PATCH 1/5] synthesize a traceback to get a normal exc_info from an exception - as per https://docs.python.org/3.12/whatsnew/3.12.html#shutil, we must expect only an exception and *not* the full exc_info from the new onexc function (the documentation of this is very misleading and still uses the label "excinfo": https://docs.python.org/3.12/library/shutil.html#shutil.rmtree) --- src/pip/_internal/utils/misc.py | 36 +++++++++++++++++++++++++---- src/pip/_internal/utils/temp_dir.py | 3 ++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index b7b32f0f8cf..1e79dbe988b 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -1,5 +1,6 @@ import contextlib import errno +import functools import getpass import hashlib import io @@ -14,7 +15,8 @@ from functools import partial from io import StringIO from itertools import filterfalse, tee, zip_longest -from types import TracebackType +from pathlib import Path +from types import FunctionType, TracebackType from typing import ( Any, BinaryIO, @@ -67,6 +69,8 @@ ExcInfo = Tuple[Type[BaseException], BaseException, TracebackType] VersionInfo = Tuple[int, int, int] NetlocTuple = Tuple[str, Tuple[Optional[str], Optional[str]]] +OnExc = Callable[[FunctionType, Path, BaseException], Any] +OnErr = Callable[[FunctionType, Path, ExcInfo], Any] def get_pip_version() -> str: @@ -121,22 +125,44 @@ def get_prog() -> str: return "pip" +def bare_exc_to_onexc(exc_val: BaseException) -> ExcInfo: + exc_ty = type(exc_val) + tb = exc_val.__traceback__ + if tb is None: + import inspect + + frame = inspect.currentframe() + assert frame is not None + tb = TracebackType(None, frame, frame.f_lasti, frame.f_lineno) + return (exc_ty, exc_val, tb) + + +def extract_exc_info_arg(f: OnErr) -> OnExc: + def g(fn: FunctionType, p: Path, e: BaseException) -> Any: + info = bare_exc_to_onexc(e) + return f(fn, p, info) + + return functools.update_wrapper(g, f) + + # Retry every half second for up to 3 seconds # Tenacity raises RetryError by default, explicitly raise the original exception @retry(reraise=True, stop=stop_after_delay(3), wait=wait_fixed(0.5)) def rmtree( dir: str, ignore_errors: bool = False, - onexc: Optional[Callable[[Any, Any, Any], Any]] = None, + onexc: Optional[OnErr] = None, ) -> None: if ignore_errors: onexc = _onerror_ignore - elif onexc is None: + if onexc is None: onexc = _onerror_reraise + handler: OnErr = partial(rmtree_errorhandler, onexc=onexc) if sys.version_info >= (3, 12): - shutil.rmtree(dir, onexc=partial(rmtree_errorhandler, onexc=onexc)) + exc_handler = extract_exc_info_arg(handler) + shutil.rmtree(dir, onexc=exc_handler) else: - shutil.rmtree(dir, onerror=partial(rmtree_errorhandler, onexc=onexc)) + shutil.rmtree(dir, onerror=handler) def _onerror_ignore(*_args: Any) -> None: diff --git a/src/pip/_internal/utils/temp_dir.py b/src/pip/_internal/utils/temp_dir.py index 99d1ba834ef..130be333932 100644 --- a/src/pip/_internal/utils/temp_dir.py +++ b/src/pip/_internal/utils/temp_dir.py @@ -5,6 +5,7 @@ import tempfile import traceback from contextlib import ExitStack, contextmanager +from pathlib import Path from typing import ( Any, Callable, @@ -189,7 +190,7 @@ def cleanup(self) -> None: def onerror( func: Callable[[str], Any], - path: str, + path: Path, exc_info: Tuple[Type[BaseException], BaseException, Any], ) -> None: """Log a warning for a `rmtree` error and continue""" From 4f036be496a40b90fa665432c001e9f9cce3dbee Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri, 28 Jul 2023 01:19:46 -0400 Subject: [PATCH 2/5] add news entry --- news/12187.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/12187.bugfix.rst diff --git a/news/12187.bugfix.rst b/news/12187.bugfix.rst new file mode 100644 index 00000000000..63760ad91e3 --- /dev/null +++ b/news/12187.bugfix.rst @@ -0,0 +1 @@ +Fix improper handling of the new onexc argument of shutil.rmtree() in python 3.12. From eabefd40214f96e30eb2b0e607f9a4c7ffbf5e43 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri, 28 Jul 2023 02:18:36 -0400 Subject: [PATCH 3/5] revert the traceback wrapping --- src/pip/_internal/utils/misc.py | 34 +++++++++------------------------ 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index 1e79dbe988b..69540978d1d 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -1,6 +1,5 @@ import contextlib import errno -import functools import getpass import hashlib import io @@ -125,26 +124,6 @@ def get_prog() -> str: return "pip" -def bare_exc_to_onexc(exc_val: BaseException) -> ExcInfo: - exc_ty = type(exc_val) - tb = exc_val.__traceback__ - if tb is None: - import inspect - - frame = inspect.currentframe() - assert frame is not None - tb = TracebackType(None, frame, frame.f_lasti, frame.f_lineno) - return (exc_ty, exc_val, tb) - - -def extract_exc_info_arg(f: OnErr) -> OnExc: - def g(fn: FunctionType, p: Path, e: BaseException) -> Any: - info = bare_exc_to_onexc(e) - return f(fn, p, info) - - return functools.update_wrapper(g, f) - - # Retry every half second for up to 3 seconds # Tenacity raises RetryError by default, explicitly raise the original exception @retry(reraise=True, stop=stop_after_delay(3), wait=wait_fixed(0.5)) @@ -157,10 +136,15 @@ def rmtree( onexc = _onerror_ignore if onexc is None: onexc = _onerror_reraise - handler: OnErr = partial(rmtree_errorhandler, onexc=onexc) + handler: OnErr = partial( + # `[func, path, Union[ExcInfo, BaseException]] -> Any` is equivalent to + # `Union[([func, path, ExcInfo] -> Any), ([func, path, BaseException] -> Any)]`. + cast(Union[OnExc, OnErr], rmtree_errorhandler), + onexc=onexc, + ) if sys.version_info >= (3, 12): - exc_handler = extract_exc_info_arg(handler) - shutil.rmtree(dir, onexc=exc_handler) + # See https://docs.python.org/3.12/whatsnew/3.12.html#shutil. + shutil.rmtree(dir, onexc=handler) else: shutil.rmtree(dir, onerror=handler) @@ -178,7 +162,7 @@ def rmtree_errorhandler( path: str, exc_info: Union[ExcInfo, BaseException], *, - onexc: Callable[..., Any] = _onerror_reraise, + onexc: OnExc = _onerror_reraise, ) -> None: """ `rmtree` error handler to 'force' a file remove (i.e. like `rm -f`). From 454e9768fbc4250db28aa17f3365fea0d131253d Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri, 28 Jul 2023 02:43:49 -0400 Subject: [PATCH 4/5] incorporate review comments --- src/pip/_internal/utils/misc.py | 8 +++++--- src/pip/_internal/utils/temp_dir.py | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index 69540978d1d..9a6353fc8d3 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -130,7 +130,7 @@ def get_prog() -> str: def rmtree( dir: str, ignore_errors: bool = False, - onexc: Optional[OnErr] = None, + onexc: Optional[OnExc] = None, ) -> None: if ignore_errors: onexc = _onerror_ignore @@ -158,8 +158,8 @@ def _onerror_reraise(*_args: Any) -> None: def rmtree_errorhandler( - func: Callable[..., Any], - path: str, + func: FunctionType, + path: Path, exc_info: Union[ExcInfo, BaseException], *, onexc: OnExc = _onerror_reraise, @@ -193,6 +193,8 @@ def rmtree_errorhandler( except OSError: pass + if not isinstance(exc_info, BaseException): + _, exc_info, _ = exc_info onexc(func, path, exc_info) diff --git a/src/pip/_internal/utils/temp_dir.py b/src/pip/_internal/utils/temp_dir.py index 130be333932..38c5f7c7c94 100644 --- a/src/pip/_internal/utils/temp_dir.py +++ b/src/pip/_internal/utils/temp_dir.py @@ -6,15 +6,13 @@ import traceback from contextlib import ExitStack, contextmanager from pathlib import Path +from types import FunctionType from typing import ( Any, - Callable, Dict, Generator, List, Optional, - Tuple, - Type, TypeVar, Union, ) @@ -189,22 +187,24 @@ def cleanup(self) -> None: errors: List[BaseException] = [] def onerror( - func: Callable[[str], Any], + func: FunctionType, path: Path, - exc_info: Tuple[Type[BaseException], BaseException, Any], + exc_val: BaseException, ) -> 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 + formatted_exc = "\n".join( + traceback.format_exception_only(type(exc_val), exc_val) + ) + formatted_exc = formatted_exc.rstrip() # remove trailing new line if func in (os.unlink, os.remove, os.rmdir): logger.debug( "Failed to remove a temporary file '%s' due to %s.\n", path, - exc_val, + formatted_exc, ) else: - logger.debug("%s failed with %s.", func.__qualname__, exc_val) - errors.append(exc_info[1]) + logger.debug("%s failed with %s.", func.__qualname__, formatted_exc) + errors.append(exc_val) if self.ignore_cleanup_errors: try: From 6cc961ee799745a8ccf0256f670364c59d29b8c7 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Fri, 28 Jul 2023 14:46:23 +0800 Subject: [PATCH 5/5] Uppercase Python --- news/12187.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/12187.bugfix.rst b/news/12187.bugfix.rst index 63760ad91e3..b4d106b974f 100644 --- a/news/12187.bugfix.rst +++ b/news/12187.bugfix.rst @@ -1 +1 @@ -Fix improper handling of the new onexc argument of shutil.rmtree() in python 3.12. +Fix improper handling of the new onexc argument of ``shutil.rmtree()`` in Python 3.12.