mesonbuild / wrapdb

New wrap requests
https://mesonbuild.com/Adding-new-projects-to-wrapdb.html
MIT License
78 stars 203 forks source link

nanobind: `limited_api` not propagated to nanobind static library build #1758

Open lgarrison opened 2 weeks ago

lgarrison commented 2 weeks ago

Hi @WillAyd,

Thank you for the excellent nanobind wrapper! I'm running into an issue when compiling limited API extensions that you might have some insight into. If I have a meson.build snippet like this:

nanobind_dep = dependency('nanobind', static: true)
py.extension_module(
  'my_module_name',
  sources: ['src/example.cpp'],
  dependencies: [nanobind_dep],
  install: true,
  limited_api : '3.12'
)

The compiler output shows that src/example.cpp is built with -DPy_LIMITED_API=0x030c0000, but nanobind itself is not. For example:

[1/13] ccache c++ -Imy_module_name.abi3.so.p -I. -I.. -I../subprojects/nanobind-2.2.0/include -I../subprojects/robin-map-1.3.0/include -I/install/include/python3.12 -I/mnt/home/lgarrison/.local/share/uv/python/cpython-3.12.7-linux-x86_64-gnu/include/python3.12 -fvisibility=hidden -fvisibility-inlines-hidden -fdiagnostics-color=always -D_GLIBCXX_ASSERTIONS=1 -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++17 -O0 -g -fPIC -ffunction-sections -fdata-sections -DPy_LIMITED_API=0x030c0000 -MD -MQ my_module_name.abi3.so.p/src_example.cpp.o -MF my_module_name.abi3.so.p/src_example.cpp.o.d -o my_module_name.abi3.so.p/src_example.cpp.o -c ../src/example.cpp
[2/13] ccache c++ -Isubprojects/nanobind-2.2.0/libnanobind.a.p -Isubprojects/nanobind-2.2.0 -I../subprojects/nanobind-2.2.0 -I../subprojects/nanobind-2.2.0/include -I../subprojects/robin-map-1.3.0/include -I/install/include/python3.12 -I/mnt/home/lgarrison/.local/share/uv/python/cpython-3.12.7-linux-x86_64-gnu/include/python3.12 -fvisibility=hidden -fdiagnostics-color=always -D_GLIBCXX_ASSERTIONS=1 -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++17 -O0 -g -fPIC -MD -MQ subprojects/nanobind-2.2.0/libnanobind.a.p/src_nb_static_property.cpp.o -MF subprojects/nanobind-2.2.0/libnanobind.a.p/src_nb_static_property.cpp.o.d -o subprojects/nanobind-2.2.0/libnanobind.a.p/src_nb_static_property.cpp.o -c ../subprojects/nanobind-2.2.0/src/nb_static_property.cpp

When building wheels this way, this leads to abi3audit violations, which is how I noticed this.

So my question is: what's the right way to build the nanobind_dep static lib itself with the limited API flag?

I poked around in the Meson source a bit, and it looks like limited_api mostly just sets -DPy_LIMITED_API to the right hex value in the c_args and cpp_args of the shared extension (here). It doesn't seem like anything there knows about dependencies.

So the question seems to be how to get the right Py_LIMITED_API value to pass to nanobind_dep. Maybe it could be extracted from the extension_module args (doesn't seem super clean), or maybe it could be exposed somewhere by Meson itself?

eli-schwartz commented 2 weeks ago

So the question seems to be how to get the right Py_LIMITED_API value to pass to nanobind_dep. Maybe it could be extracted from the extension_module args (doesn't seem super clean), or maybe it could be exposed somewhere by Meson itself?

Well. It would be the latter, of course.

The original limited API handling in meson didn't foresee the use of static libraries using the cpython API and implemented an express mode for extension modules but nothing for common utility libraries such as nanobind. This could be manually handled via hardcoded flags but we should probably add an API for it.

This was discussed in the "implementation of nanobind" PR if I recall correctly but I don't know that anyone opened a dedicated tracking ticket for it.

WillAyd commented 2 weeks ago

Glad you have been enjoying the wrapper!

As far as the issue is concerned, I think this goes back to @wjakob callout before https://github.com/mesonbuild/wrapdb/pull/1726 was merged, which I never followed up on

nanobind uses its own internal macro when free-threaded builds are used, so I think we need to define NB_FREE_THREADED when applicable in the meson configuration.

Drawing inspiration from the conversation upstream in https://github.com/mesonbuild/meson/issues/13656, I think something to the effect of:

freethreaded = py.get_variable('Py_GIL_DISABLED', '') != ''
if freethreaded
  add_project_arguments('-DNB_FREE_THREADED', language : 'cpp')
endif

might do the trick in the configuration.

Would you be able to try that out and push up a PR if it works?

lgarrison commented 2 weeks ago

That would work for free threading, but wouldn't help with the limited API, right? I don't think anything useful about the limited API is exposed through py.get_variable()/sysconfig. I don't mind doing a quick PR for this; I guess -DNB_FREE_THREADED should also be added to the compile_args of nanobind's declare_dependency() so the extension gets compiled with it?

Similarly, for the limited API, what if the define was propagated through the compile_args of the python_dependency, like:

py = import('python').find_installation()
py_dep = py.dependency(limited_api : '3.12')

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

Would that make sense? In this case, the limited_api value would have to be specified in the superproject and passed to the subproject. Would the way to do that be by making this an option in the subproject?

This also wouldn't cover the case where someone wanted the -DPy_LIMITED_API value without declaring a dependency on py_dep, but that seems pretty niche.

WillAyd commented 2 weeks ago

Ah....sorry I mixed up the discussions. I should have read more closely.

Here's some discussion around this in the original PR:

https://github.com/mesonbuild/wrapdb/pull/1556#issuecomment-2183579523

I'm less knowledgeable about how this is handled in Meson / CPython so I will defer to the opinion of others here