Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lgarrison opened this issue Oct 23, 2024 · 4 comments
Open

Comments

@lgarrison
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

WillAyd commented Oct 23, 2024

Glad you have been enjoying the wrapper!

As far as the issue is concerned, I think this goes back to @wjakob callout before #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 mesonbuild/meson#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
Copy link
Contributor Author

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
Copy link
Contributor

WillAyd commented Oct 24, 2024

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

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

#1556 (comment)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants