mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.64k stars 1.64k forks source link

python: compiling libraries against Limited API in addition to extensions? #13824

Open lgarrison opened 4 weeks ago

lgarrison commented 4 weeks ago

This is a continuation of mesonbuild/wrapdb#1758, but @eli-schwartz suggested the correct fix is a modification of Meson itself rather than the nanobind wrapper, so I thought I'd open an issue here for visibility/discussion.

The question is how a library like nanobind that has a Python dependency, but is not a Python extension, can target the Limited API. This requires a define like -DPy_LIMITED_API=0x030c0000 that encodes the API version. Currently, limited_api is only exposed as a kwarg for py.extension_module().

Based on my limited (pun intended) Meson experience, I can see two options:

  1. Expose the hex define via a method of the python_installation. For example:
py = import('python').find_installation()
py_dep = py.dependency()
limited_api_define = py.get_limited_api_define('3.12')  # NEW

nanobind_lib = static_library(
    ...
    dependencies: [py_dep, ...],
    cpp_args: [limited_api_define],  # NEW
)
  1. Add limited_api as a kwarg of py.dependency(). The dependency object can add the hex define internally to its compile_args. For example:
py = import('python').find_installation()
py_dep = py.dependency(limited_api : '3.12')  # NEW

nanobind_lib = static_library(
    ...
    dependencies: [py_dep, ...],
)

Option 1 seems simpler to implement, it would just be exposing a string. It's also more flexible, in case someone needs the define without expressing a proper dependency on Python (seems niche though).

Option 2 seems more elegant, and seems less error prone in the sense that a user doesn't need to remember to add the define to every library that targets Python.

In terms of possibly implementing Option 2, it looks like py.extension_module already generates a python_dependency internally (here). I think this means that the logic that currently resides in the extension_module for targeting the Limited API (here) could mostly be moved upstream to the dependency. Then anything that targets a python_dependency—an extension_module implicitly or a library explicitly—could inherit the right define.

What do others think? Is either of these solutions on the right track?

rgommers commented 3 weeks ago

Option 2 does seem nice and useful. Perhaps even nicer (or do both): add the limited_api to the pymod.find_installation. That way it functions analogously to the pure keyword and can propagate automatically to all py.extension_module() calls as well as to py.dependency(). Just like pure, limited_api is a setting that should normally be the same for all targets in a Python package after all.

Option 1 I don't like as much. Something like that can already be done with declare_dependency() (using the hex version, so not as nice, but it works today):

py = import('python').find_installation()
py_dep = py.dependency()
py_dep = declare_dependency(
    dependencies: py_dep,
    compile_args: '-DPy_LIMITED_API='0x030100',
)

The py.get_limited_api_define('3.12') in Option 1 is basically only the hex number conversion.

amcn commented 3 weeks ago

@rgommers The suggestion of adding limited_api to find_installation and have it function like pure is probably what the API should have been all along.

I regret not implementing it that way from the start as it's definitely a better API than having it be on extension_module. It probably isn't too difficult to implement this, IIRC much of the complexity of implementing it originally was dealing with Windows.

It's an API change, so users will have to be informed, but it's relatively recent functionality so there's probably not yet many who would be affected (a priori).

rgommers commented 3 weeks ago

It's an API change, so users will have to be informed, but it's relatively recent functionality so there's probably not yet many who would be affected (a priori).

I don't think it needs to be an API change, right? It's only a new keyword for find_installation, with the default being false so it's backwards-compatible to add. The keyword doesn't need to be removed from extension_module. It's okay to keep it there, again similar to pure, since there are probably corner cases where one may want a per-extension module setting (e.g., two separate Python packages or executables in a single repo, where one can use the limited API and the other can't, built with meson not `meson-python).

eli-schwartz commented 3 weeks ago

I think that having the kwarg be on extension_module was the right choice and moreover I don't see how it could be used via either the dependency or find_installation to begin with. A limited API module does more than just pass a define: it changes the name of the extension module filename itself.

It's fundamentally not about what kind of dependency you retrieve from your own upstreams, but rather about what kind of artifacts you yourself create.

eli-schwartz commented 3 weeks ago

find_installation I could hear inasmuch as all it would do is change the default value of the limited API when invoking extension_module. Just like how pure works -- you can always override it per installed object. Ultimately it is data tied to the installed object, not the py_installation.

rgommers commented 3 weeks ago

find_installation I could hear inasmuch as all it would do is change the default value of the limited API when invoking extension_module. Just like how pure works

Exactly, that - it's a close analogy. Just like for pure it'll be less verbose and less error-prone to set the default value once in find_installation.

Ultimately it is data tied to the installed object, not the py_installation.

Sure, agreed.

lgarrison commented 3 weeks ago

find_installation I could hear inasmuch as all it would do is change the default value of the limited API when invoking extension_module. Just like how pure works

In terms of implementation, would it be okay for the logic that generates the -DPy_LIMITED_API define to be upstreamed from the extension_module to the Python dependency or installation? That way non-extensions (i.e. nanobind) could inherit it. From an extension_module API perspective, I agree that it would just look like the find_installation option changes the default.

amcn commented 3 weeks ago

find_installation I could hear inasmuch as all it would do is change the default value of the limited API when invoking extension_module. Just like how pure works -- you can always override it per installed object. Ultimately it is data tied to the installed object, not the py_installation.

This is what I meant by a better API: having it be a default but overridable. And also having its behaviour be like pure.

I don't think it needs to be an API change, right? It's only a new keyword for find_installation, with the default being false so it's backwards-compatible to add. The keyword doesn't need to be removed from extension_module.

I was envisaging situations where we might need to detect if a user set a limited_api value for find_installation but then sets a different, potentially incompatible limited_api value when declaring their extension_module. Is this something that is likely to occur?

pymod = import('python')
py3_inst = pymod.find_installation(
  limited_api: '3.10'
)

# 'limited_api' unspecified, defaults to 3.10 from the installation.
my_mod = py3_inst.extension_module(
  'my_mod.c'
)

# manually overriden to 3.8
my_other_mod = py3_inst.extension_module(
  'my_other_mod.c',
  limited_api: '3.8'
)

Huh, maybe this is perfectly fine in fact, now that I've written it out and read it back to myself.