From 2220d40199352a5999a7ed0bd899f284d9ed27c9 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 25a9eee937..706d3bea04 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, TypeVar +from typing import TYPE_CHECKING 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 @@ -1788,16 +1787,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 @@ -2020,24 +2009,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 CommandSpec(list): """ A command spec for a #! header, specified as a list of arguments akin to @@ -2340,10 +2311,6 @@ def load_launcher_manifest(name): return manifest.decode('utf-8') % vars() -def _rmtree(path, ignore_errors=False, onexc=auto_chmod): - return py311.shutil_rmtree(path, ignore_errors, onexc) - - def current_umask(): tmp = os.umask(0o022) os.umask(tmp) From 47c215b2f628d04da7077bbbc4e82847af22be11 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 14:29:29 +0100 Subject: [PATCH 2/7] Avoid creating an empty .egg-info directory directly in sys.path --- setuptools/command/egg_info.py | 53 +++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py index 280eb5e807..0e91bdf308 100644 --- a/setuptools/command/egg_info.py +++ b/setuptools/command/egg_info.py @@ -6,8 +6,10 @@ import functools import os import re +import shutil import sys import time +from tempfile import TemporaryDirectory import packaging import packaging.requirements @@ -20,7 +22,7 @@ from setuptools.command.setopt import edit_config from setuptools.glob import glob -from .. import _entry_points, _normalization +from .. import _entry_points, _normalization, _shutil from .._importlib import metadata from ..warnings import SetuptoolsDeprecationWarning from . import _requirestxt @@ -293,20 +295,36 @@ def delete_file(self, filename): os.unlink(filename) def run(self): - self.mkpath(self.egg_info) - try: - os.utime(self.egg_info, None) - except OSError as e: - msg = f"Cannot update time stamp of directory '{self.egg_info}'" - raise distutils.errors.DistutilsFileError(msg) from e - for ep in metadata.entry_points(group='egg_info.writers'): - writer = ep.load() - writer(self, ep.name, os.path.join(self.egg_info, ep.name)) - - # Get rid of native_libs.txt if it was put there by older bdist_egg - nl = os.path.join(self.egg_info, "native_libs.txt") - if os.path.exists(nl): - self.delete_file(nl) + # Avoid adding an empty .egg-info to sys.path while using importlib.metadata + # See pypa/pyproject-hooks#206 + self.mkpath(self.egg_base) # avoid file system errors with tmpdir / os.replace + with TemporaryDirectory(prefix=".tmp-", dir=self.egg_base) as tmp: + staging = os.path.join(tmp, "egg_info") + if os.path.isdir(self.egg_info): + shutil.copytree(self.egg_info, staging) # account pre-existing files + else: + os.mkdir(staging) + + for ep in metadata.entry_points(group='egg_info.writers'): + writer = ep.load() + writer(self, ep.name, os.path.join(staging, ep.name)) + + # Get rid of native_libs.txt if it was put there by older bdist_egg + nl = os.path.join(staging, "native_libs.txt") + if os.path.exists(nl): + self.delete_file(nl) + + # Remove old directory and create the new one + try: + # Unfortunately os.replace does not work for existing destination dirs, + # so we cannot have a single atomic operation + if os.path.isdir(self.egg_info): + _shutil.rmtree(self.egg_info) + os.replace(staging, self.egg_info) + log.info(f"renaming {staging!r} to {self.egg_info!r}") + except OSError as e: + msg = f"Cannot create directory '{self.egg_info}'" + raise distutils.errors.DistutilsFileError(msg) from e self.find_sources() @@ -654,13 +672,14 @@ def write_pkg_info(cmd, basename, filename): try: # write unescaped data to PKG-INFO, so older pkg_resources # can still parse it - metadata.write_pkg_info(cmd.egg_info) + parent_dir = os.path.dirname(filename) + metadata.write_pkg_info(parent_dir) finally: metadata.name, metadata.version = oldname, oldver safe = getattr(cmd.distribution, 'zip_safe', None) - bdist_egg.write_safety_flag(cmd.egg_info, safe) + bdist_egg.write_safety_flag(parent_dir, safe) def warn_depends_obsolete(cmd, basename, filename): From aa7e04f12a4fc7e193770d0f7130cfa105363356 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 14:35:19 +0100 Subject: [PATCH 3/7] Extract common pattern to remove dir if exists to setuptools._shutil --- setuptools/_shutil.py | 5 +++++ setuptools/command/dist_info.py | 6 +----- setuptools/command/egg_info.py | 3 +-- 3 files changed, 7 insertions(+), 7 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 1db3fbf6bd..d47153c038 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): # 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) diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py index 0e91bdf308..7ab426b9cf 100644 --- a/setuptools/command/egg_info.py +++ b/setuptools/command/egg_info.py @@ -318,8 +318,7 @@ def run(self): try: # Unfortunately os.replace does not work for existing destination dirs, # so we cannot have a single atomic operation - if os.path.isdir(self.egg_info): - _shutil.rmtree(self.egg_info) + _shutil.rmdir(self.egg_info) os.replace(staging, self.egg_info) log.info(f"renaming {staging!r} to {self.egg_info!r}") except OSError as e: From d4864319000777bb6dbb7367bc3cf2c8153b582b Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 15:21:17 +0100 Subject: [PATCH 4/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 e1cd904683683ac3555da044e8c6d0ce35312337 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 15:21:50 +0100 Subject: [PATCH 5/7] Modify expectation in test --- setuptools/command/egg_info.py | 2 +- setuptools/tests/test_egg_info.py | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py index 7ab426b9cf..f217cba097 100644 --- a/setuptools/command/egg_info.py +++ b/setuptools/command/egg_info.py @@ -322,7 +322,7 @@ def run(self): os.replace(staging, self.egg_info) log.info(f"renaming {staging!r} to {self.egg_info!r}") except OSError as e: - msg = f"Cannot create directory '{self.egg_info}'" + msg = f"Cannot create directory '{self.egg_info}' ({e})" raise distutils.errors.DistutilsFileError(msg) from e self.find_sources() diff --git a/setuptools/tests/test_egg_info.py b/setuptools/tests/test_egg_info.py index 6e8d0c68c3..9ea7340a92 100644 --- a/setuptools/tests/test_egg_info.py +++ b/setuptools/tests/test_egg_info.py @@ -165,19 +165,17 @@ def test_expected_files_produced(self, tmpdir_cwd, env): ] assert sorted(actual) == expected - def test_handling_utime_error(self, tmpdir_cwd, env): - dist = Distribution() + def test_handling_file_system_error(self, tmpdir_cwd, env): + dist = Distribution({"script_name": "hello.py"}) ei = egg_info(dist) - utime_patch = mock.patch('os.utime', side_effect=OSError("TEST")) - mkpath_patch = mock.patch( - 'setuptools.command.egg_info.egg_info.mkpath', return_val=None - ) + rm_patch = mock.patch('setuptools._shutil.rmdir', side_effect=OSError("TEST")) - with utime_patch, mkpath_patch: + with rm_patch: import distutils.errors - msg = r"Cannot update time stamp of directory 'None'" + msg = r"Cannot create directory .*egg-info" with pytest.raises(distutils.errors.DistutilsFileError, match=msg): + ei.ensure_finalized() ei.run() def test_license_is_a_string(self, tmpdir_cwd, env): From 8421e5d036d26751bc59ec183a5dfac7533bdaec Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 15:40:23 +0100 Subject: [PATCH 6/7] Post-pone deprecation for now It seems that there are many projects that haven't adapted yet, so we should probably avoid disruption. Evidence: https://github.com/search?q=%2Fauthor-email%7Cmaintainer-email%7Clong-description%7Clicense-file%2F+path%3Asetup.cfg&type=code --- setuptools/dist.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setuptools/dist.py b/setuptools/dist.py index 68f877decd..7c516fefb8 100644 --- a/setuptools/dist.py +++ b/setuptools/dist.py @@ -504,7 +504,7 @@ def warn_dash_deprecation(self, opt, section): versions. Please use the underscore name {underscore_opt!r} instead. """, see_docs="userguide/declarative_config.html", - due_date=(2024, 9, 26), + due_date=(2025, 3, 3), # Warning initially introduced in 3 Mar 2021 ) return underscore_opt @@ -529,7 +529,7 @@ def make_option_lowercase(self, opt, section): future versions. Please use lowercase {lowercase_opt!r} instead. """, see_docs="userguide/declarative_config.html", - due_date=(2024, 9, 26), + due_date=(2025, 3, 3), # Warning initially introduced in 6 Mar 2021 ) return lowercase_opt From e588f89ffbb3f278dc4e4b816362096c46f294c1 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 7 Oct 2024 16:05:02 +0100 Subject: [PATCH 7/7] Ignore some lines for coverage --- setuptools/_shutil.py | 8 +++++--- setuptools/command/egg_info.py | 2 +- 2 files changed, 6 insertions(+), 4 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': diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py index f217cba097..b94cfc75a6 100644 --- a/setuptools/command/egg_info.py +++ b/setuptools/command/egg_info.py @@ -311,7 +311,7 @@ def run(self): # Get rid of native_libs.txt if it was put there by older bdist_egg nl = os.path.join(staging, "native_libs.txt") - if os.path.exists(nl): + if os.path.exists(nl): # pragma: no cover self.delete_file(nl) # Remove old directory and create the new one