From d9975c6a31b88bd6ae8963e9c981525cc910f82f Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 14:12:38 +0100 Subject: [PATCH 1/7] Extract convenience wrapper of rmtree to setuptools._shutil for reuse --- setuptools/_shutil.py | 41 ++++++++++++++++++++++++++++++ setuptools/command/easy_install.py | 39 +++------------------------- 2 files changed, 44 insertions(+), 36 deletions(-) create mode 100644 setuptools/_shutil.py diff --git a/setuptools/_shutil.py b/setuptools/_shutil.py new file mode 100644 index 0000000000..8abf5faa6b --- /dev/null +++ b/setuptools/_shutil.py @@ -0,0 +1,41 @@ +"""Convenience layer on top of stdlib's shutil and os""" + +import os +import stat +from typing import Callable, TypeVar + +from .compat import py311 + +from distutils import log + +try: + from os import chmod +except ImportError: + # Jython compatibility + def chmod(*args: object, **kwargs: object) -> None: # type: ignore[misc] # Mypy reuses the imported definition anyway + pass + + +_T = TypeVar("_T") + + +def attempt_chmod_verbose(path, mode): + log.debug("changing mode of %s to %o", path, mode) + try: + chmod(path, mode) + except OSError as e: + log.debug("chmod failed: %s", e) + + +# Must match shutil._OnExcCallback +def _auto_chmod(func: Callable[..., _T], arg: str, exc: BaseException) -> _T: + """shutils onexc callback to automatically call chmod for certain functions.""" + # Only retry for scenarios known to have an issue + if func in [os.unlink, os.remove] and os.name == 'nt': + attempt_chmod_verbose(arg, stat.S_IWRITE) + return func(arg) + raise exc + + +def rmtree(path, ignore_errors=False, onexc=_auto_chmod): + return py311.shutil_rmtree(path, ignore_errors, onexc) diff --git a/setuptools/command/easy_install.py b/setuptools/command/easy_install.py index 21e6f008d7..b40610f8ba 100644 --- a/setuptools/command/easy_install.py +++ b/setuptools/command/easy_install.py @@ -34,7 +34,7 @@ from collections.abc import Iterable from glob import glob from sysconfig import get_path -from typing import TYPE_CHECKING, Callable, NoReturn, TypedDict, TypeVar +from typing import TYPE_CHECKING, NoReturn, TypedDict from jaraco.text import yield_lines @@ -63,7 +63,8 @@ from setuptools.wheel import Wheel from .._path import ensure_directory -from ..compat import py39, py311, py312 +from .._shutil import attempt_chmod_verbose as chmod, rmtree as _rmtree +from ..compat import py39, py312 from distutils import dir_util, log from distutils.command import install @@ -89,8 +90,6 @@ 'get_exe_prefixes', ] -_T = TypeVar("_T") - def is_64bit(): return struct.calcsize("P") == 8 @@ -1789,16 +1788,6 @@ def _first_line_re(): return re.compile(first_line_re.pattern.decode()) -# Must match shutil._OnExcCallback -def auto_chmod(func: Callable[..., _T], arg: str, exc: BaseException) -> _T: - """shutils onexc callback to automatically call chmod for certain functions.""" - # Only retry for scenarios known to have an issue - if func in [os.unlink, os.remove] and os.name == 'nt': - chmod(arg, stat.S_IWRITE) - return func(arg) - raise exc - - def update_dist_caches(dist_path, fix_zipimporter_caches): """ Fix any globally cached `dist_path` related data @@ -2021,24 +2010,6 @@ def is_python_script(script_text, filename): return False # Not any Python I can recognize -try: - from os import ( - chmod as _chmod, # pyright: ignore[reportAssignmentType] # Losing type-safety w/ pyright, but that's ok - ) -except ImportError: - # Jython compatibility - def _chmod(*args: object, **kwargs: object) -> None: # type: ignore[misc] # Mypy reuses the imported definition anyway - pass - - -def chmod(path, mode): - log.debug("changing mode of %s to %o", path, mode) - try: - _chmod(path, mode) - except OSError as e: - log.debug("chmod failed: %s", e) - - class _SplitArgs(TypedDict, total=False): comments: bool posix: bool @@ -2350,10 +2321,6 @@ def load_launcher_manifest(name): return manifest.decode('utf-8') % vars() -def _rmtree(path, ignore_errors: bool = False, onexc=auto_chmod): - return py311.shutil_rmtree(path, ignore_errors, onexc) - - def current_umask(): tmp = os.umask(0o022) os.umask(tmp) From 1678730e70272129044cb1c47ca7f7f05cd4db46 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 14:35:19 +0100 Subject: [PATCH 2/7] Extract common pattern to remove dir if exists to setuptools._shutil --- setuptools/_shutil.py | 5 +++++ setuptools/command/dist_info.py | 6 +----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/setuptools/_shutil.py b/setuptools/_shutil.py index 8abf5faa6b..3d2c11e019 100644 --- a/setuptools/_shutil.py +++ b/setuptools/_shutil.py @@ -39,3 +39,8 @@ def _auto_chmod(func: Callable[..., _T], arg: str, exc: BaseException) -> _T: def rmtree(path, ignore_errors=False, onexc=_auto_chmod): return py311.shutil_rmtree(path, ignore_errors, onexc) + + +def rmdir(path, **opts): + if os.path.isdir(path): + rmtree(path, **opts) diff --git a/setuptools/command/dist_info.py b/setuptools/command/dist_info.py index 3ad27ed708..0192ebb260 100644 --- a/setuptools/command/dist_info.py +++ b/setuptools/command/dist_info.py @@ -10,6 +10,7 @@ from typing import cast from .. import _normalization +from .._shutil import rmdir as _rm from .egg_info import egg_info as egg_info_cls from distutils import log @@ -100,8 +101,3 @@ def run(self) -> None: # TODO: if bdist_wheel if merged into setuptools, just add "keep_egg_info" there with self._maybe_bkp_dir(egg_info_dir, self.keep_egg_info): bdist_wheel.egg2dist(egg_info_dir, self.dist_info_dir) - - -def _rm(dir_name, **opts): - if os.path.isdir(dir_name): - shutil.rmtree(dir_name, **opts) From b9be1442ba86c62d3473e476d0784e808da4af23 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 15:21:17 +0100 Subject: [PATCH 3/7] Attempt to solve typechecking problems --- setuptools/_shutil.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setuptools/_shutil.py b/setuptools/_shutil.py index 3d2c11e019..2db65abb63 100644 --- a/setuptools/_shutil.py +++ b/setuptools/_shutil.py @@ -9,7 +9,8 @@ from distutils import log try: - from os import chmod + from os import chmod # pyright: ignore[reportAssignmentType] + # Losing type-safety w/ pyright, but that's ok except ImportError: # Jython compatibility def chmod(*args: object, **kwargs: object) -> None: # type: ignore[misc] # Mypy reuses the imported definition anyway From 6ddac39a5ee7a0bc25466fc44a24416fd902527f Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 16:05:02 +0100 Subject: [PATCH 4/7] Ignore some lines for coverage --- setuptools/_shutil.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/setuptools/_shutil.py b/setuptools/_shutil.py index 2db65abb63..ca6397343c 100644 --- a/setuptools/_shutil.py +++ b/setuptools/_shutil.py @@ -11,7 +11,7 @@ try: from os import chmod # pyright: ignore[reportAssignmentType] # Losing type-safety w/ pyright, but that's ok -except ImportError: +except ImportError: # pragma: no cover # Jython compatibility def chmod(*args: object, **kwargs: object) -> None: # type: ignore[misc] # Mypy reuses the imported definition anyway pass @@ -24,12 +24,14 @@ def attempt_chmod_verbose(path, mode): log.debug("changing mode of %s to %o", path, mode) try: chmod(path, mode) - except OSError as e: + except OSError as e: # pragma: no cover log.debug("chmod failed: %s", e) # Must match shutil._OnExcCallback -def _auto_chmod(func: Callable[..., _T], arg: str, exc: BaseException) -> _T: +def _auto_chmod( + func: Callable[..., _T], arg: str, exc: BaseException +) -> _T: # pragma: no cover """shutils onexc callback to automatically call chmod for certain functions.""" # Only retry for scenarios known to have an issue if func in [os.unlink, os.remove] and os.name == 'nt': From 8272bc3186fb5991e290c2afd31d1f5fb2d74fb5 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Tue, 15 Oct 2024 17:20:45 +0100 Subject: [PATCH 5/7] Refactor usage of shutil.rmtree in other parts of setuptools --- setuptools/command/bdist_wheel.py | 33 +++++----------------------- setuptools/command/editable_wheel.py | 4 ++-- setuptools/command/rotate.py | 5 ++--- 3 files changed, 9 insertions(+), 33 deletions(-) diff --git a/setuptools/command/bdist_wheel.py b/setuptools/command/bdist_wheel.py index 8cf91538f9..5855a8a832 100644 --- a/setuptools/command/bdist_wheel.py +++ b/setuptools/command/bdist_wheel.py @@ -9,7 +9,6 @@ import os import re import shutil -import stat import struct import sys import sysconfig @@ -18,23 +17,19 @@ from email.generator import BytesGenerator, Generator from email.policy import EmailPolicy from glob import iglob -from shutil import rmtree -from typing import TYPE_CHECKING, Callable, Literal, cast +from typing import Literal, cast from zipfile import ZIP_DEFLATED, ZIP_STORED from packaging import tags, version as _packaging_version from wheel.metadata import pkginfo_to_metadata from wheel.wheelfile import WheelFile -from .. import Command, __version__ +from .. import Command, __version__, _shutil from ..warnings import SetuptoolsDeprecationWarning from .egg_info import egg_info as egg_info_cls from distutils import log -if TYPE_CHECKING: - from _typeshed import ExcInfo - def safe_name(name: str) -> str: """Convert an arbitrary string to a standard distribution name @@ -148,21 +143,6 @@ def safer_version(version: str) -> str: return safe_version(version).replace("-", "_") -def remove_readonly( - func: Callable[..., object], - path: str, - excinfo: ExcInfo, -) -> None: - remove_readonly_exc(func, path, excinfo[1]) - - -def remove_readonly_exc( - func: Callable[..., object], path: str, exc: BaseException -) -> None: - os.chmod(path, stat.S_IWRITE) - func(path) - - class bdist_wheel(Command): description = "create a wheel distribution" @@ -458,7 +438,7 @@ def run(self): shutil.copytree(self.dist_info_dir, distinfo_dir) # Egg info is still generated, so remove it now to avoid it getting # copied into the wheel. - shutil.rmtree(self.egginfo_dir) + _shutil.rmtree(self.egginfo_dir) else: # Convert the generated egg-info into dist-info. self.egg2dist(self.egginfo_dir, distinfo_dir) @@ -483,10 +463,7 @@ def run(self): if not self.keep_temp: log.info(f"removing {self.bdist_dir}") if not self.dry_run: - if sys.version_info < (3, 12): - rmtree(self.bdist_dir, onerror=remove_readonly) - else: - rmtree(self.bdist_dir, onexc=remove_readonly_exc) + _shutil.rmtree(self.bdist_dir) def write_wheelfile( self, wheelfile_base: str, generator: str = f"setuptools ({__version__})" @@ -570,7 +547,7 @@ def egg2dist(self, egginfo_path: str, distinfo_path: str) -> None: def adios(p: str) -> None: """Appropriately delete directory, file or link.""" if os.path.exists(p) and not os.path.islink(p) and os.path.isdir(p): - shutil.rmtree(p) + _shutil.rmtree(p) elif os.path.exists(p): os.unlink(p) diff --git a/setuptools/command/editable_wheel.py b/setuptools/command/editable_wheel.py index 30570e092a..db9a50c3af 100644 --- a/setuptools/command/editable_wheel.py +++ b/setuptools/command/editable_wheel.py @@ -27,7 +27,7 @@ from types import TracebackType from typing import TYPE_CHECKING, Protocol, TypeVar, cast -from .. import Command, _normalization, _path, errors, namespaces +from .. import Command, _normalization, _path, _shutil, errors, namespaces from .._path import StrPath from ..compat import py312 from ..discovery import find_package_path @@ -773,7 +773,7 @@ def _is_nested(pkg: str, pkg_path: str, parent: str, parent_path: str) -> bool: def _empty_dir(dir_: _P) -> _P: """Create a directory ensured to be empty. Existing files may be removed.""" - shutil.rmtree(dir_, ignore_errors=True) + _shutil.rmtree(dir_, ignore_errors=True) os.makedirs(dir_) return dir_ diff --git a/setuptools/command/rotate.py b/setuptools/command/rotate.py index c10e8d5024..acdce07baa 100644 --- a/setuptools/command/rotate.py +++ b/setuptools/command/rotate.py @@ -1,10 +1,9 @@ from __future__ import annotations import os -import shutil from typing import ClassVar -from setuptools import Command +from .. import Command, _shutil from distutils import log from distutils.errors import DistutilsOptionError @@ -61,6 +60,6 @@ def run(self) -> None: log.info("Deleting %s", f) if not self.dry_run: if os.path.isdir(f): - shutil.rmtree(f) + _shutil.rmtree(f) else: os.unlink(f) From bb93502e7f7fd7ef68cec7039034bfeaacb50fdb Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Tue, 15 Oct 2024 17:22:28 +0100 Subject: [PATCH 6/7] Add docstring --- setuptools/_shutil.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/setuptools/_shutil.py b/setuptools/_shutil.py index ca6397343c..6acbb4281f 100644 --- a/setuptools/_shutil.py +++ b/setuptools/_shutil.py @@ -41,6 +41,10 @@ def _auto_chmod( def rmtree(path, ignore_errors=False, onexc=_auto_chmod): + """ + Similar to ``shutil.rmtree`` but automatically executes ``chmod`` + for well know Windows failure scenarios. + """ return py311.shutil_rmtree(path, ignore_errors, onexc) From db2b2065bfc20c9edf1c5d8ea9ad0ae68e64acdd Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Tue, 15 Oct 2024 21:37:44 +0100 Subject: [PATCH 7/7] Extract test for shutil.rmtree callback to its own file --- setuptools/tests/test_bdist_wheel.py | 31 +------------------------ setuptools/tests/test_shutil_wrapper.py | 23 ++++++++++++++++++ 2 files changed, 24 insertions(+), 30 deletions(-) create mode 100644 setuptools/tests/test_shutil_wrapper.py diff --git a/setuptools/tests/test_bdist_wheel.py b/setuptools/tests/test_bdist_wheel.py index 3dfa9c850c..d51dfbeb6d 100644 --- a/setuptools/tests/test_bdist_wheel.py +++ b/setuptools/tests/test_bdist_wheel.py @@ -11,7 +11,6 @@ import sysconfig from contextlib import suppress from inspect import cleandoc -from unittest.mock import Mock from zipfile import ZipFile import jaraco.path @@ -19,12 +18,7 @@ from packaging import tags import setuptools -from setuptools.command.bdist_wheel import ( - bdist_wheel, - get_abi_tag, - remove_readonly, - remove_readonly_exc, -) +from setuptools.command.bdist_wheel import bdist_wheel, get_abi_tag from setuptools.dist import Distribution from setuptools.warnings import SetuptoolsDeprecationWarning @@ -510,29 +504,6 @@ def test_platform_with_space(dummy_dist, monkeypatch): bdist_wheel_cmd(plat_name="isilon onefs").run() -def test_rmtree_readonly(monkeypatch, tmp_path): - """Verify onerr works as expected""" - - bdist_dir = tmp_path / "with_readonly" - bdist_dir.mkdir() - some_file = bdist_dir.joinpath("file.txt") - some_file.touch() - some_file.chmod(stat.S_IREAD) - - expected_count = 1 if sys.platform.startswith("win") else 0 - - if sys.version_info < (3, 12): - count_remove_readonly = Mock(side_effect=remove_readonly) - shutil.rmtree(bdist_dir, onerror=count_remove_readonly) - assert count_remove_readonly.call_count == expected_count - else: - count_remove_readonly_exc = Mock(side_effect=remove_readonly_exc) - shutil.rmtree(bdist_dir, onexc=count_remove_readonly_exc) - assert count_remove_readonly_exc.call_count == expected_count - - assert not bdist_dir.is_dir() - - def test_data_dir_with_tag_build(monkeypatch, tmp_path): """ Setuptools allow authors to set PEP 440's local version segments diff --git a/setuptools/tests/test_shutil_wrapper.py b/setuptools/tests/test_shutil_wrapper.py new file mode 100644 index 0000000000..74ff7e9a89 --- /dev/null +++ b/setuptools/tests/test_shutil_wrapper.py @@ -0,0 +1,23 @@ +import stat +import sys +from unittest.mock import Mock + +from setuptools import _shutil + + +def test_rmtree_readonly(monkeypatch, tmp_path): + """Verify onerr works as expected""" + + tmp_dir = tmp_path / "with_readonly" + tmp_dir.mkdir() + some_file = tmp_dir.joinpath("file.txt") + some_file.touch() + some_file.chmod(stat.S_IREAD) + + expected_count = 1 if sys.platform.startswith("win") else 0 + chmod_fn = Mock(wraps=_shutil.attempt_chmod_verbose) + monkeypatch.setattr(_shutil, "attempt_chmod_verbose", chmod_fn) + + _shutil.rmtree(tmp_dir) + assert chmod_fn.call_count == expected_count + assert not tmp_dir.is_dir()