Repository navigation
Conversation
Signed-off-by: Maurice <mauricepurnawan@gmail.com>
Only executables and SHARED libraries (e.g. rclcpp components) need libbackward forced in. Linking it into MODULE targets such as pybind11 extensions made importing them replace Python's faulthandler output. Also drop the CI cache for backward-ros so the patch is actually rebuilt (temporary, remove before merge). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Maurice <mauricepurnawan@gmail.com>
vinca skips recipes already published on robostack-rolling with the same build number, so the patch change was never built in CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Maurice <mauricepurnawan@gmail.com>
|
@traversaro Oops, sorry — I actually meant to ask you to take a look at this one as well, but I accidentally left the comment on the wrong PR :( |
| +# libbackward (e.g. component_container) then pulls it in and installs the handler. | ||
| +# MODULE targets (Python extensions) are left alone so importing them does not | ||
| +# replace the interpreter's own crash handling. | ||
| +SET(CMAKE_SHARED_LINKER_FLAGS "${backward_ros_link_flags} ${CMAKE_SHARED_LINKER_FLAGS}") |
There was a problem hiding this comment.
Thinking more about this today, and based on what we learned from #51, this approach seems potentially quite dangerous. Maybe we should propose a target-based backward_ros instead?
There was a problem hiding this comment.
Indeed, I would say that is not a great idea in general. Unfortunately, not linking any target seems to be exactly how the library is meant to be integrated, see the README https://github.com/pal-robotics/backward_ros#integration-of-backward_ros and https://github.com/pal-robotics/backward_ros/blob/3c71c9f184223f885459ad67a223eb36a5ca347f/cmake/BackwardConfigAment.cmake#L27 . That combined with the fact that I imagine this library is used also as a transitive dependency (so its global effect may appear even if the project does not directly call find_package(backward_ros)) make its use really confusing, but I guess there is not a lot we can do as downstream packagers on this.
There was a problem hiding this comment.
What if I limit the changes here to just fixing macOS? Would you agree with that part? I can remove the CMAKE_SHARED_LINKER_FLAGS change here and propose the target-based design upstream instead.
|
Oh, I think macOS should already work in non-component mode after #51 is merged. I never actually tested the two together though. Sorry for the noise here! |
2 participants
crash_node.cpp:8and a source snippetcrash_node.cpp:8and a source snippetcrash_component.cpp:12and a source snippetThis PR fixes two things: