From 771a2ce6108f5e2da3293a9ed9f0fc64e05af4e0 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Fri, 11 Jul 2025 16:30:01 +0100 Subject: [PATCH 1/3] Remove unused type ignore mark --- distutils/compilers/C/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distutils/compilers/C/base.py b/distutils/compilers/C/base.py index 93385e13..6412dc34 100644 --- a/distutils/compilers/C/base.py +++ b/distutils/compilers/C/base.py @@ -70,7 +70,7 @@ class Compiler: # dictionary (see below -- used by the 'new_compiler()' factory # function) -- authors of new compiler interface classes are # responsible for updating 'compiler_class'! - compiler_type: ClassVar[str] = None # type: ignore[assignment] + compiler_type: ClassVar[str] = None # XXX things not handled by this compiler abstraction model: # * client can't provide additional options for a compiler, From 4bed279a1800033b34878744f76e7e84a0793814 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Fri, 11 Jul 2025 13:43:10 +0100 Subject: [PATCH 2/3] Use dataclass for typing Extension --- distutils/core.py | 20 +-- distutils/extension.py | 274 ++++++++++++++++-------------- distutils/tests/test_extension.py | 2 +- 3 files changed, 151 insertions(+), 145 deletions(-) diff --git a/distutils/core.py b/distutils/core.py index bd62546b..ef39600b 100644 --- a/distutils/core.py +++ b/distutils/core.py @@ -25,6 +25,7 @@ DistutilsSetupError, ) from .extension import Extension +from .extension import _safe as extension_keywords # noqa # backwards compatibility __all__ = ['Distribution', 'Command', 'Extension', 'setup'] @@ -74,25 +75,6 @@ def gen_usage(script_name): 'obsoletes', ) -# Legal keyword arguments for the Extension constructor -extension_keywords = ( - 'name', - 'sources', - 'include_dirs', - 'define_macros', - 'undef_macros', - 'library_dirs', - 'libraries', - 'runtime_library_dirs', - 'extra_objects', - 'extra_compile_args', - 'extra_link_args', - 'swig_opts', - 'export_symbols', - 'depends', - 'language', -) - def setup(**attrs): # noqa: C901 """The gateway to the Distutils: do everything your setup script needs diff --git a/distutils/extension.py b/distutils/extension.py index f5141126..b12a94a1 100644 --- a/distutils/extension.py +++ b/distutils/extension.py @@ -8,6 +8,8 @@ import os import warnings from collections.abc import Iterable +from dataclasses import dataclass, field, fields +from typing import TYPE_CHECKING # This class is really only used by the "build_ext" command, so it might # make sense to put it in distutils.command.build_ext. However, that @@ -20,137 +22,159 @@ # order to do anything. -class Extension: +@dataclass +class _Extension: """Just a collection of attributes that describes an extension module and everything needed to build it (hopefully in a portable way, but there are hooks that let you be as unportable as you need). + """ + + # The use of a parent class as a "trick": + # - We need to modify __init__ so to achieve backwards compatibility + # - But we don't want to throw away the dataclass-generated __init__ + # - We also want to fool the typechecker to consider the same type + # signature as the dataclass-generated __init__ + + name: str + """ + the full name of the extension, including any packages -- ie. + *not* a filename or pathname, but Python dotted name + """ + + sources: Iterable[str | os.PathLike[str]] + """ + iterable of source filenames (except strings, which could be misinterpreted + as a single filename), relative to the distribution root (where the setup + script lives), in Unix form (slash-separated) for portability. Can be any + non-string iterable (list, tuple, set, etc.) containing strings or + PathLike objects. Source files may be C, C++, SWIG (.i), platform-specific + resource files, or whatever else is recognized by the "build_ext" command + as source for a Python extension. + """ + + include_dirs: list[str] = field(default_factory=list) + """ + list of directories to search for C/C++ header files (in Unix + form for portability) + """ + + define_macros: list[tuple[str, str | None]] = field(default_factory=list) + """ + list of macros to define; each macro is defined using a 2-tuple, + where 'value' is either the string to define it to or None to + define it without a particular value (equivalent of "#define + FOO" in source or -DFOO on Unix C compiler command line) + """ + + undef_macros: list[str] = field(default_factory=list) + """list of macros to undefine explicitly""" + + library_dirs: list[str] = field(default_factory=list) + """list of directories to search for C/C++ libraries at link time""" + + libraries: list[str] = field(default_factory=list) + """list of library names (not filenames or paths) to link against""" + + runtime_library_dirs: list[str] = field(default_factory=list) + """ + list of directories to search for C/C++ libraries at run time + (for shared extensions, this is when the extension is loaded) + """ + + extra_objects: list[str] = field(default_factory=list) + """ + list of extra files to link with (eg. object files not implied + by 'sources', static library that must be explicitly specified, + binary resource files, etc.) + """ + + extra_compile_args: list[str] = field(default_factory=list) + """ + any extra platform- and compiler-specific information to use + when compiling the source files in 'sources'. For platforms and + compilers where "command line" makes sense, this is typically a + list of command-line arguments, but for other platforms it could + be anything. + """ + + extra_link_args: list[str] = field(default_factory=list) + """ + any extra platform- and compiler-specific information to use + when linking object files together to create the extension (or + to create a new static Python interpreter). Similar + interpretation as for 'extra_compile_args'. + """ + + export_symbols: list[str] = field(default_factory=list) + """ + list of symbols to be exported from a shared extension. Not + used on all platforms, and not generally necessary for Python + extensions, which typically export exactly one symbol: "init" + + extension_name. + """ - Instance attributes: - name : string - the full name of the extension, including any packages -- ie. - *not* a filename or pathname, but Python dotted name - sources : Iterable[string | os.PathLike] - iterable of source filenames (except strings, which could be misinterpreted - as a single filename), relative to the distribution root (where the setup - script lives), in Unix form (slash-separated) for portability. Can be any - non-string iterable (list, tuple, set, etc.) containing strings or - PathLike objects. Source files may be C, C++, SWIG (.i), platform-specific - resource files, or whatever else is recognized by the "build_ext" command - as source for a Python extension. - include_dirs : [string] - list of directories to search for C/C++ header files (in Unix - form for portability) - define_macros : [(name : string, value : string|None)] - list of macros to define; each macro is defined using a 2-tuple, - where 'value' is either the string to define it to or None to - define it without a particular value (equivalent of "#define - FOO" in source or -DFOO on Unix C compiler command line) - undef_macros : [string] - list of macros to undefine explicitly - library_dirs : [string] - list of directories to search for C/C++ libraries at link time - libraries : [string] - list of library names (not filenames or paths) to link against - runtime_library_dirs : [string] - list of directories to search for C/C++ libraries at run time - (for shared extensions, this is when the extension is loaded) - extra_objects : [string] - list of extra files to link with (eg. object files not implied - by 'sources', static library that must be explicitly specified, - binary resource files, etc.) - extra_compile_args : [string] - any extra platform- and compiler-specific information to use - when compiling the source files in 'sources'. For platforms and - compilers where "command line" makes sense, this is typically a - list of command-line arguments, but for other platforms it could - be anything. - extra_link_args : [string] - any extra platform- and compiler-specific information to use - when linking object files together to create the extension (or - to create a new static Python interpreter). Similar - interpretation as for 'extra_compile_args'. - export_symbols : [string] - list of symbols to be exported from a shared extension. Not - used on all platforms, and not generally necessary for Python - extensions, which typically export exactly one symbol: "init" + - extension_name. - swig_opts : [string] - any extra options to pass to SWIG if a source file has the .i - extension. - depends : [string] - list of files that the extension depends on - language : string - extension language (i.e. "c", "c++", "objc"). Will be detected - from the source extensions if not provided. - optional : boolean - specifies that a build failure in the extension should not abort the - build process, but simply not install the failing extension. + swig_opts: list[str] = field(default_factory=list) """ + any extra options to pass to SWIG if a source file has the .i + extension. + """ + + depends: list[str] = field(default_factory=list) + """list of files that the extension depends on""" + + language: str | None = None + """ + extension language (i.e. "c", "c++", "objc"). Will be detected + from the source extensions if not provided. + """ + + optional: bool = False + """ + specifies that a build failure in the extension should not abort the + build process, but simply not install the failing extension. + """ + + +# Legal keyword arguments for the Extension constructor +_safe = tuple(f.name for f in fields(_Extension)) + + +if TYPE_CHECKING: + + @dataclass + class Extension(_Extension): + pass + +else: + + @dataclass(init=False) + class Extension(_Extension): + def __init__(self, name, sources, *args, **kwargs): + if not isinstance(name, str): + raise TypeError("'name' must be a string") + + # handle the string case first; since strings are iterable, disallow them + if isinstance(sources, str): + raise TypeError( + "'sources' must be an iterable of strings or PathLike objects, not a string" + ) + + # now we check if it's iterable and contains valid types + try: + sources = list(map(os.fspath, sources)) + except TypeError: + raise TypeError( + "'sources' must be an iterable of strings or PathLike objects" + ) + + extra = {repr(k): kwargs.pop(k) for k in tuple(kwargs) if k not in _safe} + if extra: + warnings.warn(f"Unknown Extension options: {','.join(extra)}") - # When adding arguments to this constructor, be sure to update - # setup_keywords in core.py. - def __init__( - self, - name: str, - sources: Iterable[str | os.PathLike[str]], - include_dirs: list[str] | None = None, - define_macros: list[tuple[str, str | None]] | None = None, - undef_macros: list[str] | None = None, - library_dirs: list[str] | None = None, - libraries: list[str] | None = None, - runtime_library_dirs: list[str] | None = None, - extra_objects: list[str] | None = None, - extra_compile_args: list[str] | None = None, - extra_link_args: list[str] | None = None, - export_symbols: list[str] | None = None, - swig_opts: list[str] | None = None, - depends: list[str] | None = None, - language: str | None = None, - optional: bool | None = None, - **kw, # To catch unknown keywords - ): - if not isinstance(name, str): - raise TypeError("'name' must be a string") - - # handle the string case first; since strings are iterable, disallow them - if isinstance(sources, str): - raise TypeError( - "'sources' must be an iterable of strings or PathLike objects, not a string" - ) - - # now we check if it's iterable and contains valid types - try: - self.sources = list(map(os.fspath, sources)) - except TypeError: - raise TypeError( - "'sources' must be an iterable of strings or PathLike objects" - ) - - self.name = name - self.include_dirs = include_dirs or [] - self.define_macros = define_macros or [] - self.undef_macros = undef_macros or [] - self.library_dirs = library_dirs or [] - self.libraries = libraries or [] - self.runtime_library_dirs = runtime_library_dirs or [] - self.extra_objects = extra_objects or [] - self.extra_compile_args = extra_compile_args or [] - self.extra_link_args = extra_link_args or [] - self.export_symbols = export_symbols or [] - self.swig_opts = swig_opts or [] - self.depends = depends or [] - self.language = language - self.optional = optional - - # If there are unknown keyword options, warn about them - if len(kw) > 0: - options = [repr(option) for option in kw] - options = ', '.join(sorted(options)) - msg = f"Unknown Extension options: {options}" - warnings.warn(msg) - - def __repr__(self): - return f'<{self.__class__.__module__}.{self.__class__.__qualname__}({self.name!r}) at {id(self):#x}>' + # Ensure default values (e.g. []) are used instead of None: + positional = {k: v for k, v in zip(_safe[2:], args) if v is not None} + keywords = {k: v for k, v in kwargs.items() if v is not None} + super().__init__(name, sources, **positional, **keywords) def read_setup_file(filename): # noqa: C901 diff --git a/distutils/tests/test_extension.py b/distutils/tests/test_extension.py index 5e8e7682..17d52579 100644 --- a/distutils/tests/test_extension.py +++ b/distutils/tests/test_extension.py @@ -106,7 +106,7 @@ def test_extension_init(self): assert getattr(ext, attr) == [] assert ext.language is None - assert ext.optional is None + assert ext.optional is False # if there are unknown keyword options, warn about them with check_warnings() as w: From 37447a402c09c9acc6332057e60dd57c1389fa93 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Fri, 11 Jul 2025 16:13:53 +0100 Subject: [PATCH 3/3] Separate __post_init__ --- distutils/extension.py | 65 ++++++++++++++++--------------- distutils/tests/test_extension.py | 47 ++++++++++++++++++++-- 2 files changed, 76 insertions(+), 36 deletions(-) diff --git a/distutils/extension.py b/distutils/extension.py index b12a94a1..8a3b156b 100644 --- a/distutils/extension.py +++ b/distutils/extension.py @@ -31,9 +31,10 @@ class _Extension: # The use of a parent class as a "trick": # - We need to modify __init__ so to achieve backwards compatibility + # and keep allowing arbitrary keywords to be ignored # - But we don't want to throw away the dataclass-generated __init__ - # - We also want to fool the typechecker to consider the same type - # signature as the dataclass-generated __init__ + # specially because we don't want to have to redefine all the typing + # for the function arguments name: str """ @@ -139,42 +140,42 @@ class _Extension: _safe = tuple(f.name for f in fields(_Extension)) -if TYPE_CHECKING: - - @dataclass - class Extension(_Extension): - pass - -else: - - @dataclass(init=False) - class Extension(_Extension): - def __init__(self, name, sources, *args, **kwargs): - if not isinstance(name, str): - raise TypeError("'name' must be a string") - - # handle the string case first; since strings are iterable, disallow them - if isinstance(sources, str): - raise TypeError( - "'sources' must be an iterable of strings or PathLike objects, not a string" - ) - - # now we check if it's iterable and contains valid types - try: - sources = list(map(os.fspath, sources)) - except TypeError: - raise TypeError( - "'sources' must be an iterable of strings or PathLike objects" - ) +@dataclass(init=True if TYPE_CHECKING else False) # type: ignore[literal-required] +class Extension(_Extension): + if not TYPE_CHECKING: + def __init__(self, *args, **kwargs): extra = {repr(k): kwargs.pop(k) for k in tuple(kwargs) if k not in _safe} if extra: - warnings.warn(f"Unknown Extension options: {','.join(extra)}") + msg = f""" + Please remove unknown `Extension` options: {','.join(extra)} + this kind of usage is deprecated and may cause errors in the future. + """ + warnings.warn(msg) # Ensure default values (e.g. []) are used instead of None: - positional = {k: v for k, v in zip(_safe[2:], args) if v is not None} + positional = {k: v for k, v in zip(_safe, args) if v is not None} keywords = {k: v for k, v in kwargs.items() if v is not None} - super().__init__(name, sources, **positional, **keywords) + super().__init__(**positional, **keywords) + self.__post_init__() # does not seem to be called when customizing __init__ + + def __post_init__(self): + if not isinstance(self.name, str): + raise TypeError("'name' must be a string") + + # handle the string case first; since strings are iterable, disallow them + if isinstance(self.sources, str): + raise TypeError( + "'sources' must be an iterable of strings or PathLike objects, not a string" + ) + + # now we check if it's iterable and contains valid types + try: + self.sources = list(map(os.fspath, self.sources)) + except TypeError: + raise TypeError( + "'sources' must be an iterable of strings or PathLike objects" + ) def read_setup_file(filename): # noqa: C901 diff --git a/distutils/tests/test_extension.py b/distutils/tests/test_extension.py index 17d52579..4042d547 100644 --- a/distutils/tests/test_extension.py +++ b/distutils/tests/test_extension.py @@ -2,11 +2,13 @@ import os import pathlib +import re import warnings +from dataclasses import dataclass, field from distutils.extension import Extension, read_setup_file +from typing import TYPE_CHECKING import pytest -from test.support.warnings_helper import check_warnings class TestExtension: @@ -109,9 +111,46 @@ def test_extension_init(self): assert ext.optional is False # if there are unknown keyword options, warn about them - with check_warnings() as w: + msg = re.escape("unknown `Extension` options: 'chic'") + with pytest.warns(UserWarning, match=msg) as w: warnings.simplefilter('always') ext = Extension('name', ['file1', 'file2'], chic=True) - assert len(w.warnings) == 1 - assert str(w.warnings[0].message) == "Unknown Extension options: 'chic'" + assert len(w) == 1 + + +def test_can_be_extended_by_setuptools() -> None: + # Emulate how it could be extended in setuptools + + @dataclass(init=True if TYPE_CHECKING else False) # type: ignore[literal-required] + class setuptools_Extension(Extension): + py_limited_api: bool = False + _full_name: str = field(init=False, repr=False) + + if not TYPE_CHECKING: + # Custom __init__ is only needed for backwards compatibility + # (to ignore arbitrary keywords) + + def __init__(self, *args, py_limited_api=False, **kwargs): + self.py_limited_api = py_limited_api + super().__init__(*args, **kwargs) + + ext1 = setuptools_Extension("name", ["hello.c"], py_limited_api=True) + assert ext1.py_limited_api is True + assert ext1.define_macros == [] + + msg = re.escape("unknown `Extension` options: 'world'") + with pytest.warns(UserWarning, match=msg): + ext2 = setuptools_Extension("name", ["hello.c"], world=True) # type: ignore[call-arg] + + assert "world" not in ext2.__dict__ + assert ext2.py_limited_api is False + + # Without __init__ customization the following warning would be an error: + msg = re.escape("unknown `Extension` options: '_full_name'") + with pytest.warns(UserWarning, match=msg): + ext3 = setuptools_Extension("name", ["hello.c"], _full_name="hello") # type: ignore[call-arg] + + assert "_full_name" not in ext3.__dict__ + ext3._full_name = "hello world" # can still be set in build_ext + assert ext3._full_name == "hello world"