Skip to content

Commit

Permalink
[CMake] Make the pybind11_*_proto_caster libraries STATIC
Browse files Browse the repository at this point in the history
As pybind11 does not guarantee stable ABI for its types (e.g.
pybind11::handle), these types have "hidden" visibility at least on
Linux.

This visibility is propagated to any method which has any of the pybind11
types in its parameters, which is the case for many of the public methods
in proto_cast_util.{h,cc}. Trying to link to e.g. a SHARED
pybind11_native_proto_caster library will create linker errors to to
undefined symbols for any non-trivial case.

The other approach would be to move everything which uses a pybind11
type to the headers, and only leave pybind11 agnostic methods in the
shared library. While this is trivially possible for e.g.
PyBytesAsStringView, for the majority of the methods (e.g. anything
depending on the GlobalState object) a significant refactoring would
be required.

See pybind#160.
  • Loading branch information
StefanBruens committed Jun 14, 2024
1 parent e57a98c commit 885c511
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ target_include_directories(
# ============================================================================
# pybind11_native_proto_caster shared library
add_library(
pybind11_native_proto_caster SHARED
pybind11_native_proto_caster STATIC
# bazel: pybind_library: native_proto_caster
pybind11_protobuf/native_proto_caster.h
# bazel: pybind_library: enum_type_caster
Expand Down Expand Up @@ -103,7 +103,7 @@ target_include_directories(
# ============================================================================
# pybind11_wrapped_proto_caster shared library
add_library(
pybind11_wrapped_proto_caster SHARED
pybind11_wrapped_proto_caster STATIC
# bazel: pybind_library: wrapped_proto_caster
pybind11_protobuf/wrapped_proto_caster.h
# bazel: pybind_library: proto_cast_util
Expand Down

0 comments on commit 885c511

Please sign in to comment.