Allow users to target Py_LIMITED_API via python's extension_module #11745
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| ## Support targeting Python's limited C API | ||
|
|
||
| The Python module's `extension_module` function has gained the ability | ||
| to build extensions which target Python's limited C API via a new keyword | ||
| argument: `limited_api`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ | |
| # limitations under the License. | ||
| from __future__ import annotations | ||
|
|
||
| import copy, json, os, shutil, re | ||
| import typing as T | ||
|
|
||
| from . import ExtensionModule, ModuleInfo | ||
|
|
@@ -32,7 +32,7 @@ | |
| InvalidArguments, typed_pos_args, typed_kwargs, KwargInfo, | ||
| FeatureNew, FeatureNewKwargs, disablerIfNotFound | ||
| ) | ||
| from ..mesonlib import MachineChoice, OptionKey | ||
| from ..programs import ExternalProgram, NonExistingExternalProgram | ||
|
|
||
| if T.TYPE_CHECKING: | ||
|
|
@@ -63,7 +63,7 @@ class ExtensionModuleKw(SharedModuleKw): | |
| subdir: NotRequired[T.Optional[str]] | ||
|
|
||
|
|
||
| mod_kwargs = {'subdir', 'limited_api'} | ||
|
There was a problem hiding this comment. Can you leave the empty line in there? |
||
| mod_kwargs.update(known_shmod_kwargs) | ||
| mod_kwargs -= {'name_prefix', 'name_suffix'} | ||
|
|
||
|
|
@@ -112,6 +112,7 @@ def _get_path(self, state: T.Optional['ModuleState'], key: str) -> None: | |
|
|
||
| _PURE_KW = KwargInfo('pure', (bool, NoneType)) | ||
| _SUBDIR_KW = KwargInfo('subdir', str, default='') | ||
| _LIMITED_API_KW = KwargInfo('limited_api', str, default='', since='1.3.0') | ||
| _DEFAULTABLE_SUBDIR_KW = KwargInfo('subdir', (str, NoneType)) | ||
|
|
||
| class PythonInstallation(ExternalProgramHolder): | ||
|
|
@@ -122,6 +123,7 @@ def __init__(self, python: 'PythonExternalProgram', interpreter: 'Interpreter'): | |
| assert isinstance(prefix, str), 'for mypy' | ||
| self.variables = info['variables'] | ||
| self.suffix = info['suffix'] | ||
| self.limited_api_suffix = info['limited_api_suffix'] | ||
| self.paths = info['paths'] | ||
| self.pure = python.pure | ||
| self.platlib_install_path = os.path.join(prefix, python.platlib) | ||
|
|
@@ -146,7 +148,7 @@ def __init__(self, python: 'PythonExternalProgram', interpreter: 'Interpreter'): | |
|
|
||
| @permittedKwargs(mod_kwargs) | ||
| @typed_pos_args('python.extension_module', str, varargs=(str, mesonlib.File, CustomTarget, CustomTargetIndex, GeneratedList, StructuredSources, ExtractedObjects, BuildTarget)) | ||
| @typed_kwargs('python.extension_module', *_MOD_KWARGS, _DEFAULTABLE_SUBDIR_KW, _LIMITED_API_KW, allow_unknown=True) | ||
| def extension_module_method(self, args: T.Tuple[str, T.List[BuildTargetSource]], kwargs: ExtensionModuleKw) -> 'SharedModule': | ||
| if 'install_dir' in kwargs: | ||
| if kwargs['subdir'] is not None: | ||
|
|
@@ -159,32 +161,97 @@ def extension_module_method(self, args: T.Tuple[str, T.List[BuildTargetSource]], | |
|
|
||
| kwargs['install_dir'] = self._get_install_dir_impl(False, subdir) | ||
|
|
||
| target_suffix = self.suffix | ||
|
|
||
| new_deps = mesonlib.extract_as_list(kwargs, 'dependencies') | ||
| pydep = next((dep for dep in new_deps if isinstance(dep, _PythonDependencyBase)), None) | ||
| if pydep is None: | ||
| pydep = self._dependency_method_impl({}) | ||
| if not pydep.found(): | ||
| raise mesonlib.MesonException('Python dependency not found') | ||
| new_deps.append(pydep) | ||
| FeatureNew.single_use('python_installation.extension_module with implicit dependency on python', | ||
| '0.63.0', self.subproject, 'use python_installation.dependency()', | ||
| self.current_node) | ||
|
|
||
| limited_api_version = kwargs.pop('limited_api') | ||
| allow_limited_api = self.interpreter.environment.coredata.get_option(OptionKey('allow_limited_api', module='python')) | ||
| if limited_api_version != '' and allow_limited_api: | ||
|
|
||
| target_suffix = self.limited_api_suffix | ||
|
|
||
| limited_api_version_hex = self._convert_api_version_to_py_version_hex(limited_api_version, pydep.version) | ||
| limited_api_definition = f'-DPy_LIMITED_API={limited_api_version_hex}' | ||
|
|
||
| new_c_args = mesonlib.extract_as_list(kwargs, 'c_args') | ||
| new_c_args.append(limited_api_definition) | ||
| kwargs['c_args'] = new_c_args | ||
|
|
||
| new_cpp_args = mesonlib.extract_as_list(kwargs, 'cpp_args') | ||
| new_cpp_args.append(limited_api_definition) | ||
| kwargs['cpp_args'] = new_cpp_args | ||
|
|
||
| # When compiled under MSVC, Python's PC/pyconfig.h forcibly inserts pythonMAJOR.MINOR.lib | ||
| # into the linker path when not running in debug mode via a series #pragma comment(lib, "") | ||
| # directives. We manually override these here as this interferes with the intended | ||
| # use of the 'limited_api' kwarg | ||
|
There was a problem hiding this comment. It is a shame you need this. I think the intention of the header was deal with linking the correct import lib, and meson should ideally not specify any link arguments to deal with the import lib at all. But it seems the header does not properly link to the "limited api + debug" import lib. I opened python/cpython#107585 to clarify that There was a problem hiding this comment. Ideally meson would read a sysconfig setting specifying which import lib to link to, and use that. Doing this via MSVC specific pragmas is painful, weird, and bad, for a variety of reasons but perhaps the simplest and most persuasive one is that it cripples meson's ability to notice a bug in CPython and add a quirk like this to work around the problem. You should see the crimes against computing that pybind11 does to allow building a python extension with debug symbols for user code without forcing to link against the debug version of python311.dll -- meson simply spawns a scary warning if you try to mix them. Hot take: the computing industry would be better off if that MSVC pragma had not been invented. There was a problem hiding this comment. I also regret that this was necessary but it does seem that CPython's Back when I was starting this work I recall finding a PR in CPython which aimed to fix the logic in That being said, even if a fix makes it into CPython, I think Meson should probably still aim to support users with older versions of CPython that are already in the wild, right? |
||
| for_machine = self.interpreter.machine_from_native_kwarg(kwargs) | ||
| compilers = self.interpreter.environment.coredata.compilers[for_machine] | ||
| if any(compiler.get_id() == 'msvc' for compiler in compilers.values()): | ||
| pydep_copy = copy.copy(pydep) | ||
| pydep_copy.find_libpy_windows(self.env, limited_api=True) | ||
| if not pydep_copy.found(): | ||
| raise mesonlib.MesonException('Python dependency supporting limited API not found') | ||
|
|
||
| new_deps.remove(pydep) | ||
| new_deps.append(pydep_copy) | ||
|
Comment on lines
+200
to
+207
There was a problem hiding this comment. I was trying to think whether there was value to making this part of I think it's fairly unlikely that anyone will ever want to touch the limited API for reasons other than building an extension_module, so we do not need to make this directly exposed. |
||
|
|
||
| pyver = pydep.version.replace('.', '') | ||
| python_windows_debug_link_exception = f'/NODEFAULTLIB:python{pyver}_d.lib' | ||
| python_windows_release_link_exception = f'/NODEFAULTLIB:python{pyver}.lib' | ||
|
|
||
| new_link_args = mesonlib.extract_as_list(kwargs, 'link_args') | ||
|
|
||
| is_debug = self.interpreter.environment.coredata.options[OptionKey('debug')].value | ||
| if is_debug: | ||
| new_link_args.append(python_windows_debug_link_exception) | ||
| else: | ||
| new_link_args.append(python_windows_release_link_exception) | ||
|
|
||
| kwargs['link_args'] = new_link_args | ||
|
|
||
| kwargs['dependencies'] = new_deps | ||
|
|
||
| # msys2's python3 has "-cpython-36m.dll", we have to be clever | ||
| # FIXME: explain what the specific cleverness is here | ||
| split, target_suffix = target_suffix.rsplit('.', 1) | ||
| args = (args[0] + split, args[1]) | ||
|
|
||
| kwargs['name_prefix'] = '' | ||
| kwargs['name_suffix'] = target_suffix | ||
|
|
||
| if 'gnu_symbol_visibility' not in kwargs and \ | ||
| (self.is_pypy or mesonlib.version_compare(self.version, '>=3.9')): | ||
| kwargs['gnu_symbol_visibility'] = 'inlineshidden' | ||
|
|
||
| return self.interpreter.build_target(self.current_node, args, kwargs, SharedModule) | ||
|
|
||
| def _convert_api_version_to_py_version_hex(self, api_version: str, detected_version: str) -> str: | ||
| python_api_version_format = re.compile(r'[0-9]\.[0-9]{1,2}') | ||
| decimal_match = python_api_version_format.fullmatch(api_version) | ||
| if not decimal_match: | ||
| raise InvalidArguments(f'Python API version invalid: "{api_version}".') | ||
| if mesonlib.version_compare(api_version, '<3.2'): | ||
| raise InvalidArguments(f'Python Limited API version invalid: {api_version} (must be greater than 3.2)') | ||
| if mesonlib.version_compare(api_version, '>' + detected_version): | ||
| raise InvalidArguments(f'Python Limited API version too high: {api_version} (detected {detected_version})') | ||
|
|
||
| version_components = api_version.split('.') | ||
| major = int(version_components[0]) | ||
| minor = int(version_components[1]) | ||
|
|
||
| return '0x{:02x}{:02x}0000'.format(major, minor) | ||
|
|
||
| def _dependency_method_impl(self, kwargs: TYPE_kwargs) -> Dependency: | ||
| for_machine = self.interpreter.machine_from_native_kwarg(kwargs) | ||
| identifier = get_dep_identifier(self._full_path(), kwargs) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,20 @@ def links_against_libpython(): | |
| else: | ||
| suffix = variables.get('EXT_SUFFIX') | ||
|
|
||
| limited_api_suffix = None | ||
| if sys.version_info >= (3, 2): | ||
| try: | ||
| from importlib.machinery import EXTENSION_SUFFIXES | ||
| limited_api_suffix = EXTENSION_SUFFIXES[1] | ||
|
There was a problem hiding this comment. This does not seem to be very robust. Is the order in which extension suffixes appear in There was a problem hiding this comment. I don't believe the order in which extensions appear in this list is guaranteed by any implementation of Python. In all the versions of CPython that I could test locally, the limited API suffix always appeared second. pypy supports the limited API but does not care about the suffix and so there is only one element in this list. There isn't, to my knowledge, a documented way to reliably get this information from different Python implementations. My first implementation used I would welcome any information from Python contributors about this. The current implementation seems to work for everything I have tested so far but I agree with you that this is not robust. I personally don't like this code and am eager to change it to something more robust. There was a problem hiding this comment. Python will happily try to load any file that happens to have an extension contained in The only thing I find in the documentation is:
I think the only way to know what is the correct thing to do is to check what @FFY00 Do you have any insight? There was a problem hiding this comment. There was a problem hiding this comment. Is mirroring what There was a problem hiding this comment. @amcnulty-fermat You should bring this up with the CPython developers. AFAIK there isn't any better way. There was a problem hiding this comment. There is a new API design for sysconfig, I commented on the compile-specific issue There was a problem hiding this comment.
Yes. This is an implementation detail. On current CPython versions, and most other implementations, the suffix list is sorted by extension module lookup order. My recommendation here would be to look for the suffix matching the following regex That said, I don't think using In the future, this information should be provided by sysconfig. There was a problem hiding this comment.
Can you please clarify this? On Windows, for Python versions 3.7 to 3.11, There was a problem hiding this comment. You're right, I thought that had changed at some point, but it's not the case. What changed was support in wheels, tooling should be able to handle |
||
| except Exception: | ||
| pass | ||
|
|
||
| # pypy supports modules targetting the limited api but | ||
| # does not use a special suffix to distinguish them: | ||
| # https://doc.pypy.org/en/latest/cpython_differences.html#permitted-abi-tags-in-extensions | ||
| if '__pypy__' in sys.builtin_module_names: | ||
| limited_api_suffix = suffix | ||
|
|
||
| print(json.dumps({ | ||
| 'variables': variables, | ||
| 'paths': paths, | ||
|
|
@@ -76,4 +90,5 @@ def links_against_libpython(): | |
| 'is_venv': sys.prefix != variables['base_prefix'], | ||
| 'link_libpython': links_against_libpython(), | ||
| 'suffix': suffix, | ||
| 'limited_api_suffix': limited_api_suffix, | ||
| })) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| project('Python limited api disabled', 'c', | ||
| default_options : ['buildtype=release', 'werror=true', 'python.allow_limited_api=false']) | ||
|
|
||
| py_mod = import('python') | ||
| py = py_mod.find_installation() | ||
|
|
||
| module = py.extension_module('my_module', | ||
| 'module.c', | ||
| limited_api: '3.7', | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #include <Python.h> | ||
|
|
||
| #if defined(Py_LIMITED_API) | ||
| #error "Py_LIMITED_API's definition by Meson should have been disabled." | ||
| #endif | ||
|
|
||
| static struct PyModuleDef my_module = { | ||
| PyModuleDef_HEAD_INIT, | ||
| "my_module", | ||
| NULL, | ||
| -1, | ||
| NULL | ||
| }; | ||
|
|
||
| PyMODINIT_FUNC PyInit_my_module(void) { | ||
| return PyModule_Create(&my_module); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| #include <Python.h> | ||
|
|
||
| #ifndef Py_LIMITED_API | ||
| #error Py_LIMITED_API must be defined. | ||
| #elif Py_LIMITED_API != 0x03070000 | ||
| #error Wrong value for Py_LIMITED_API | ||
| #endif | ||
|
|
||
| static struct PyModuleDef limited_module = { | ||
| PyModuleDef_HEAD_INIT, | ||
| "limited_api_test", | ||
| NULL, | ||
| -1, | ||
| NULL | ||
| }; | ||
|
|
||
| PyMODINIT_FUNC PyInit_limited(void) { | ||
| return PyModule_Create(&limited_module); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation calls this
limited_api, here it islimited_api_suffix. They should be aligned.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is part of the introspection data gathered from the interpreter the extension is being built for, it does not indicate whether the extension is built for the limited API or not. I think it is fine as it is.