From aa867ba09977846ff4a08854e8b6f461a8fde18a Mon Sep 17 00:00:00 2001 From: Avasam Date: Wed, 12 Jun 2024 12:45:10 -0400 Subject: [PATCH 1/7] bdist_wheel typing improvement --- mypy.ini | 2 +- setuptools/command/bdist_wheel.py | 58 ++++++++++++++++++------------- tools/vendored.py | 3 +- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/mypy.ini b/mypy.ini index c5b13942c4..25ef708842 100644 --- a/mypy.ini +++ b/mypy.ini @@ -14,7 +14,7 @@ exclude = (?x)( | ^.+?/(_vendor|extern)/ # Vendored | ^setuptools/_distutils/ # Vendored | ^setuptools/config/_validate_pyproject/ # Auto-generated - | ^setuptools/tests/bdist_wheel_testdata/ # Duplicate module name + | ^setuptools/tests/bdist_wheel_testdata/ # Duplicate module name ) # Ignoring attr-defined because setuptools wraps a lot of distutils classes, adding new attributes, diff --git a/setuptools/command/bdist_wheel.py b/setuptools/command/bdist_wheel.py index ad34539eb8..7bfb3fb024 100644 --- a/setuptools/command/bdist_wheel.py +++ b/setuptools/command/bdist_wheel.py @@ -23,13 +23,14 @@ from zipfile import ZIP_DEFLATED, ZIP_STORED from .. import Command, __version__ +from .egg_info import egg_info as egg_info_cls from ..extern.wheel.metadata import pkginfo_to_metadata from ..extern.packaging import tags from ..extern.packaging import version as _packaging_version from ..extern.wheel.wheelfile import WheelFile if TYPE_CHECKING: - import types + from _typeshed import ExcInfo def safe_name(name: str) -> str: @@ -152,12 +153,14 @@ def safer_version(version: str) -> str: def remove_readonly( func: Callable[..., object], path: str, - excinfo: tuple[type[Exception], Exception, types.TracebackType], + excinfo: ExcInfo, ) -> None: remove_readonly_exc(func, path, excinfo[1]) -def remove_readonly_exc(func: Callable[..., object], path: str, exc: Exception) -> None: +def remove_readonly_exc( + func: Callable[..., object], path: str, exc: BaseException +) -> None: os.chmod(path, stat.S_IWRITE) func(path) @@ -232,40 +235,47 @@ class bdist_wheel(Command): def initialize_options(self) -> None: self.bdist_dir: str | None = None - self.data_dir = None + self.data_dir: str | None = None self.plat_name: str | None = None - self.plat_tag = None + self.plat_tag: str | None = None self.format = "zip" self.keep_temp = False self.dist_dir: str | None = None - self.egginfo_dir = None + self.egginfo_dir: str | None = None self.root_is_pure: bool | None = None - self.skip_build = None + self.skip_build = False self.relative = False self.owner = None self.group = None self.universal: bool = False - self.compression: str | int = "deflated" + self.compression: int = ZIP_DEFLATED self.python_tag: str = python_tag() self.build_number: str | None = None self.py_limited_api: str | Literal[False] = False self.plat_name_supplied = False - def finalize_options(self): - if self.bdist_dir is None: + def finalize_options(self) -> None: + if not self.bdist_dir: bdist_base = self.get_finalized_command("bdist").bdist_base self.bdist_dir = os.path.join(bdist_base, "wheel") - egg_info = self.distribution.get_command_obj("egg_info") + egg_info = cast(egg_info_cls, self.distribution.get_command_obj("egg_info")) egg_info.ensure_finalized() # needed for correct `wheel_dist_name` self.data_dir = self.wheel_dist_name + ".data" - self.plat_name_supplied = self.plat_name is not None + self.plat_name_supplied = bool(self.plat_name) - try: - self.compression = self.supported_compressions[self.compression] - except KeyError: - raise ValueError(f"Unsupported compression: {self.compression}") from None + # Handle compression not being an int or a supported value + if not ( + isinstance(self.compression, int) + and self.compression in self.supported_compressions.values() + ): + try: + self.compression = self.supported_compressions[str(self.compression)] + except KeyError: + raise ValueError( + f"Unsupported compression: {self.compression}" + ) from None need_options = ("dist_dir", "plat_name", "skip_build") @@ -295,21 +305,21 @@ def finalize_options(self): raise ValueError("Build tag (build-number) must start with a digit.") @property - def wheel_dist_name(self): + def wheel_dist_name(self) -> str: """Return distribution full name with - replaced with _""" - components = ( + components = [ safer_name(self.distribution.get_name()), safer_version(self.distribution.get_version()), - ) + ] if self.build_number: - components += (self.build_number,) + components.append(self.build_number) return "-".join(components) def get_tag(self) -> tuple[str, str, str]: # bdist sets self.plat_name if unset, we should only use it for purepy # wheels if the user supplied it. - if self.plat_name_supplied: - plat_name = cast(str, self.plat_name) + if self.plat_name_supplied and self.plat_name: + plat_name = self.plat_name elif self.root_is_pure: plat_name = "any" else: @@ -453,7 +463,7 @@ def run(self): def write_wheelfile( self, wheelfile_base: str, generator: str = f"setuptools ({__version__})" - ): + ) -> None: from email.message import Message msg = Message() @@ -527,7 +537,7 @@ def license_paths(self) -> Iterable[str]: return files - def egg2dist(self, egginfo_path: str, distinfo_path: str): + def egg2dist(self, egginfo_path: str, distinfo_path: str) -> None: """Convert an .egg-info directory into a .dist-info directory""" def adios(p: str) -> None: diff --git a/tools/vendored.py b/tools/vendored.py index edc9195f3c..34652736be 100644 --- a/tools/vendored.py +++ b/tools/vendored.py @@ -117,6 +117,7 @@ def rewrite_wheel(pkg_files: Path): # Rewrite vendored imports to use setuptools's own vendored libraries for path in pkg_files.iterdir(): + # Upstream issue: https://github.com/jaraco/path/issues/226 if path.suffix == '.py': # type: ignore[attr-defined] code = path.read_text() if path.name == 'wheelfile.py': @@ -154,7 +155,7 @@ class WheelError(Exception): code, flags=re.MULTILINE, ) - + # Upstream issue: https://github.com/jaraco/path/issues/226 path.write_text(code) # type: ignore[attr-defined] From 2351f60f4f013f18ac80b14765c3b3c663b2355b Mon Sep 17 00:00:00 2001 From: Avasam Date: Fri, 21 Jun 2024 13:00:12 -0400 Subject: [PATCH 2/7] Extract compression conversion logic to _zip_compression --- setuptools/command/bdist_wheel.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/setuptools/command/bdist_wheel.py b/setuptools/command/bdist_wheel.py index 7bfb3fb024..c2effe0a59 100644 --- a/setuptools/command/bdist_wheel.py +++ b/setuptools/command/bdist_wheel.py @@ -248,7 +248,7 @@ def initialize_options(self) -> None: self.owner = None self.group = None self.universal: bool = False - self.compression: int = ZIP_DEFLATED + self.compression: int | str = "deflated" self.python_tag: str = python_tag() self.build_number: str | None = None self.py_limited_api: str | Literal[False] = False @@ -265,18 +265,6 @@ def finalize_options(self) -> None: self.data_dir = self.wheel_dist_name + ".data" self.plat_name_supplied = bool(self.plat_name) - # Handle compression not being an int or a supported value - if not ( - isinstance(self.compression, int) - and self.compression in self.supported_compressions.values() - ): - try: - self.compression = self.supported_compressions[str(self.compression)] - except KeyError: - raise ValueError( - f"Unsupported compression: {self.compression}" - ) from None - need_options = ("dist_dir", "plat_name", "skip_build") self.set_undefined_options("bdist", *zip(need_options, need_options)) @@ -443,7 +431,7 @@ def run(self): os.makedirs(self.dist_dir) wheel_path = os.path.join(self.dist_dir, archive_basename + ".whl") - with WheelFile(wheel_path, "w", self.compression) as wf: + with WheelFile(wheel_path, "w", self._zip_compression()) as wf: wf.write_files(archive_root) # Add to 'Distribution.dist_files' so that the "upload" command works @@ -607,3 +595,18 @@ def adios(p: str) -> None: shutil.copy(license_path, os.path.join(distinfo_path, filename)) adios(egginfo_path) + + def _zip_compression(self) -> int: + if ( + isinstance(self.compression, int) + and self.compression in self.supported_compressions.values() + ): + return self.compression + + try: + if isinstance(self.compression, str): + return self.supported_compressions[self.compression] + except KeyError: + pass + + raise ValueError(f"Unsupported compression: {self.compression!r}") From de49950decae14bb3b07afa6646793bee3acb0bb Mon Sep 17 00:00:00 2001 From: Avasam Date: Fri, 21 Jun 2024 13:15:38 -0400 Subject: [PATCH 3/7] Add newsfragment --- newsfragments/4383.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4383.bugfix.rst diff --git a/newsfragments/4383.bugfix.rst b/newsfragments/4383.bugfix.rst new file mode 100644 index 0000000000..e5fd603abb --- /dev/null +++ b/newsfragments/4383.bugfix.rst @@ -0,0 +1 @@ +Prevent an error in ``bdist_wheel`` if ``compression`` is set to a `str` (even if valid) after finalizing options but before running the command. -- by :user:`Avasam` From 246a8849450128eb0fd8338922d74f126412e862 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 8 Aug 2024 17:48:17 +0100 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Avasam --- setuptools/command/bdist_wheel.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/setuptools/command/bdist_wheel.py b/setuptools/command/bdist_wheel.py index a76d3c746b..18beb05f40 100644 --- a/setuptools/command/bdist_wheel.py +++ b/setuptools/command/bdist_wheel.py @@ -601,10 +601,8 @@ def _zip_compression(self) -> int: ): return self.compression - try: - if isinstance(self.compression, str): - return self.supported_compressions[self.compression] - except KeyError: - pass + compression = self.supported_compressions.get(self.compression) + if compression is not None: + return compression raise ValueError(f"Unsupported compression: {self.compression!r}") From 77d396938d032f9ba4c15fd88dced7efcdd63c91 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 8 Aug 2024 17:48:49 +0100 Subject: [PATCH 5/7] Update mypy.ini --- mypy.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/mypy.ini b/mypy.ini index 60ce43981f..705cbe2e08 100644 --- a/mypy.ini +++ b/mypy.ini @@ -15,7 +15,6 @@ exclude = (?x)( | ^setuptools/_vendor/ # Vendored | ^setuptools/_distutils/ # Vendored | ^setuptools/config/_validate_pyproject/ # Auto-generated - | ^setuptools/tests/bdist_wheel_testdata/ # Duplicate module name ) # Too many false-positives disable_error_code = overload-overlap From ce8ead0966ecac41f70aeb2763d8e3cd1d6ce0b8 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 8 Aug 2024 18:19:20 +0100 Subject: [PATCH 6/7] Update setuptools/command/bdist_wheel.py --- setuptools/command/bdist_wheel.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/setuptools/command/bdist_wheel.py b/setuptools/command/bdist_wheel.py index 18beb05f40..cd0b3e8f81 100644 --- a/setuptools/command/bdist_wheel.py +++ b/setuptools/command/bdist_wheel.py @@ -601,8 +601,9 @@ def _zip_compression(self) -> int: ): return self.compression - compression = self.supported_compressions.get(self.compression) - if compression is not None: + if isinstance(self.compression, str) and ( + compression := self.supported_compressions.get(self.compression) + ): return compression raise ValueError(f"Unsupported compression: {self.compression!r}") From 32285504cf1652144754cb417eaf533c20c69e21 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 8 Aug 2024 18:49:31 +0100 Subject: [PATCH 7/7] Update setuptools/command/bdist_wheel.py --- setuptools/command/bdist_wheel.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/setuptools/command/bdist_wheel.py b/setuptools/command/bdist_wheel.py index cd0b3e8f81..de6440f22f 100644 --- a/setuptools/command/bdist_wheel.py +++ b/setuptools/command/bdist_wheel.py @@ -601,9 +601,8 @@ def _zip_compression(self) -> int: ): return self.compression - if isinstance(self.compression, str) and ( - compression := self.supported_compressions.get(self.compression) - ): + compression = self.supported_compressions.get(str(self.compression)) + if compression is not None: return compression raise ValueError(f"Unsupported compression: {self.compression!r}")