search

ros2 / rclpy

rclpy (ROS Client Library for Python)
Apache License 2.0
268 stars 221 forks source link

Remove `SHARED` from `pybind11_add_module` #1305

Open wolfv opened 1 week ago

wolfv commented 1 week ago

When the module is compiled with MODULE (the default), the proper linker flags are added on macOS (specifically -undefined dynamic_lookup). Otherwise, rclpy segfaults when linked on conda.

Is the SHARED really necessary? The pybind11 documentation says:

Specifying SHARED will create a more traditional dynamic library which can also be linked from elsewhere.

Is anyone linking against this library as a "traditional dynamic library"?

sloretz commented 1 week ago

Seems like a reasonable change to me.

Is anyone linking against this library as a "traditional dynamic library"?

I'm not sure. Let's find out!

CI (repos file build: --packages-above-and-dependencies rclpy test: --packages-above rclpy)

wolfv commented 1 week ago

I guess that means .. it works?! :)

wolfv commented 1 week ago

If I understand the CI correctly, then something went wrong in a package on Windows? Do you think that's related to this change?

clalancette commented 1 week ago

If I understand the CI correctly, then something went wrong in a package on Windows? Do you think that's related to this change?

Unclear. It may be, because we don't see that same failure in the nightly builds: https://ci.ros2.org/view/nightly/job/nightly_win_rel/3083/

wolfv commented 1 week ago

Damnation.

wolfv commented 1 week ago

I would be happy to make this conditional (keep the current one on Windows). I don't really think that it should change anything though.