Skip to content

Remove SHARED from pybind11_add_module#1305

Merged
ahcorde merged 1 commit intoros2:rollingfrom
wolfv:patch-1
Feb 28, 2025
Merged

Remove SHARED from pybind11_add_module#1305
ahcorde merged 1 commit intoros2:rollingfrom
wolfv:patch-1

Conversation

@wolfv
Copy link
Copy Markdown
Contributor

@wolfv wolfv commented Jun 20, 2024

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

sloretz commented Jun 20, 2024

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)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@wolfv
Copy link
Copy Markdown
Contributor Author

wolfv commented Jun 20, 2024

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

@wolfv
Copy link
Copy Markdown
Contributor Author

wolfv commented Jun 21, 2024

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

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

wolfv commented Jun 21, 2024

Damnation.

@wolfv
Copy link
Copy Markdown
Contributor Author

wolfv commented Jun 21, 2024

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.

@fujitatomoya
Copy link
Copy Markdown
Collaborator

@Mergifyio rebase

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"?

Signed-off-by: Wolf Vollprecht <w.vollprecht@gmail.com>
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Feb 27, 2025

rebase

✅ Branch has been successfully rebased

@fujitatomoya
Copy link
Copy Markdown
Collaborator

Pulls: #1305
Gist: https://gist.githubusercontent.com/fujitatomoya/f3edff6e70e07b92b751acd2dc92783e/raw/1f035dde991304b202c0027dce36b57e2315302c/ros2.repos
BUILD args: --packages-above-and-dependencies rclpy
TEST args: --packages-above rclpy
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15253

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@traversaro
Copy link
Copy Markdown

Great, now Windows CI seems to be happy, together with all other builds.

@ahcorde ahcorde merged commit 7fb093b into ros2:rolling Feb 28, 2025
@traversaro
Copy link
Copy Markdown

Thanks!

@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Feb 28, 2025

do we need to backport this to jazzy ?

@traversaro
Copy link
Copy Markdown

do we need to backport this to jazzy ?

That would permit to fix the macos crashes on conda-forge on vanilla build from sources. On robostack we have a patch for this on both jazzy and humble so we do not strictly need to backport this, but it is beneficial for someone build stock ROS 2 without patches.

@traversaro
Copy link
Copy Markdown

@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Mar 3, 2025

https://github.com/Mergifyio backport jazzy humble

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 3, 2025

backport jazzy humble

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Mar 3, 2025
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.

Signed-off-by: Wolf Vollprecht <w.vollprecht@gmail.com>
(cherry picked from commit 7fb093b)
mergify bot pushed a commit that referenced this pull request Mar 3, 2025
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.

Signed-off-by: Wolf Vollprecht <w.vollprecht@gmail.com>
(cherry picked from commit 7fb093b)
ahcorde pushed a commit that referenced this pull request Mar 3, 2025
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.

Signed-off-by: Wolf Vollprecht <w.vollprecht@gmail.com>
(cherry picked from commit 7fb093b)

Co-authored-by: Wolf Vollprecht <w.vollprecht@gmail.com>
ahcorde pushed a commit that referenced this pull request Mar 4, 2025
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.

Signed-off-by: Wolf Vollprecht <w.vollprecht@gmail.com>
(cherry picked from commit 7fb093b)

Co-authored-by: Wolf Vollprecht <w.vollprecht@gmail.com>
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

Successfully merging this pull request may close these issues.

6 participants