Skip to content

Fix backward ros when using component node and macos #53

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.

Sign up for GitHub

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

Closed
mini-1235 wants to merge 3 commits into from
Closed

Fix backward ros when using component node and macos #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb028cc
Fix backward ros when using component node and macos
mini-1235 Sep 26, 2026
ac54a44
backward_ros: don't force libbackward into MODULE targets
mini-1235 Sep 26, 2026
2bacb6a
Bump backward_ros build number so the patched package is rebuilt
mini-1235 Sep 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/testpr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ jobs:
# You can uncomment/modify the line below in case the cache for some packages becomes corrupted and needs to be regenerated.
# Make sure you don't merge these changes, though!
# rm -rf ${{ matrix.folder_cache }}/ros-rolling-moveit-core* ${{ matrix.folder_cache }}/ros2-moveit-core* 2>/dev/null || true
rm -rf ${{ matrix.folder_cache }}/ros-rolling-backward-ros* ${{ matrix.folder_cache }}/ros2-backward-ros* 2>/dev/null || true
mkdir -p ${{ matrix.folder_cache }}
pixi run rattler-index fs ${{ matrix.folder_cache }}/.. --force

Expand Down
47 changes: 24 additions & 23 deletions patch/ros-rolling-backward-ros.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 317f65f..1f38e81 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -27,6 +27,7 @@ include(cmake/BackwardConfig.cmake)

set(CMAKE_CXX_STANDARD_REQUIRED True)
set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)

###############################################################################
# COMPILER FLAGS
diff --git a/cmake/BackwardConfig.cmake b/cmake/BackwardConfig.cmake
index 77d22d2..8e95287 100644
--- a/cmake/BackwardConfig.cmake
Expand All @@ -33,25 +21,38 @@ index 77d22d2..8e95287 100644
list(APPEND _BACKWARD_INCLUDE_DIRS ${BACKWARD_INCLUDE_DIR})

diff --git a/cmake/BackwardConfigAment.cmake b/cmake/BackwardConfigAment.cmake
index 273a51a..3a1a726 100644
index 273a51a..d52dd05 100644
--- a/cmake/BackwardConfigAment.cmake
+++ b/cmake/BackwardConfigAment.cmake
@@ -24,5 +24,12 @@ foreach(lib ${backward_ros_forced_LIBRARIES})
@@ -24,5 +24,25 @@ foreach(lib ${backward_ros_forced_LIBRARIES})
set(backward_ros_full_path_LIBRARIES "${backward_ros_full_path_LIBRARIES} ${lib}")
endif()
endforeach()
-SET(CMAKE_EXE_LINKER_FLAGS "-Wl,--no-as-needed ${backward_ros_full_path_LIBRARIES} -Wl,--as-needed ${CMAKE_EXE_LINKER_FLAGS}")
+set(no_as_needed)
+set(as_needed)

+if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+ set(no_as_needed "-Wl,--no-as-needed")
+ set(as_needed "-Wl,--as-needed")
+# Nothing references libbackward's symbols; it works through a static
+# initializer. Force the linker to keep it anyway.
+if(APPLE)
+ # ld64 has no --no-as-needed; -needed_library survives -dead_strip_dylibs.
+ set(backward_ros_link_flags "")
+ separate_arguments(backward_ros_full_path_LIBRARIES)
+ foreach(lib ${backward_ros_full_path_LIBRARIES})
+ set(backward_ros_link_flags "${backward_ros_link_flags} -Wl,-needed_library,${lib}")
+ endforeach()
+elseif(MSVC)
+ set(backward_ros_link_flags "${backward_ros_full_path_LIBRARIES}")
+else()
+ # GNU ld, gold, lld and mold all accept --no-as-needed, whichever compiler drives them.
+ set(backward_ros_link_flags "-Wl,--no-as-needed ${backward_ros_full_path_LIBRARIES} -Wl,--as-needed")
+endif()
+
+SET(CMAKE_EXE_LINKER_FLAGS "${no_as_needed} ${backward_ros_full_path_LIBRARIES} ${as_needed} ${CMAKE_EXE_LINKER_FLAGS}")
+SET(CMAKE_EXE_LINKER_FLAGS "${backward_ros_link_flags} ${CMAKE_EXE_LINKER_FLAGS}")
+# Shared libraries too: a component dlopen'ed by a host that does not link
+# 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}")

@mini-1235 mini-1235 Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@traversaro traversaro Sep 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mini-1235 mini-1235 Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


diff --git a/package.xml b/package.xml
index 6de8f0e..3ac0db1 100644
index 88c1304..f7832d8 100644
--- a/package.xml
+++ b/package.xml
@@ -11,6 +11,7 @@
Expand Down
2 changes: 2 additions & 0 deletions pkg_additional_info.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ visp:
override_version: '3.6.0'
zenoh_cpp_vendor:
additional_cmake_args: "-DAMENT_VENDOR_POLICY=NEVER_VENDOR_IGNORE_SATISFIED_CHECK -DUSE_SYSTEM_ZENOH=ON"
backward_ros:
build_number: 26
diagnostic_updater:
additional_cmake_args: "-DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=ON"
build_number: 26
Expand Down
Loading