Search RUNPATH to find library on Linux #44

wieset · 2020-02-12T15:13:39Z

Proposal to fix #40

Based on a code snippet from Stack Overflow:
https://stackoverflow.com/questions/2836330/is-there-a-way-to-inspect-the-current-rpath-on-linux

Signed-off-by: Tim Wiese <[email protected]>

BUILD_RPATH is set to find test library INSTALL_RPATH_USE_LINK_PATH adds linked library paths to RUNPATH Signed-off-by: Tim Wiese <[email protected]>

Signed-off-by: Tim Wiese <[email protected]>

src/find_library.cpp

test/test_find_library.cpp

thomas-moulard · 2020-02-12T17:11:41Z

Thanks for your contribution! The approach LGTM, a few nits about the implementation and test.

Signed-off-by: Tim Wiese <[email protected]>

src/find_library.cpp

test/test_find_library.cpp

Signed-off-by: Tim Wiese <[email protected]>

…_library test for RUNPATH test case This should avoid unintended effects of global environment changes, as cmake's set_tests_properties restores the environment to its previous state after the test is done. It also emulates better the effect of a setcap / setuid executable for the RUNPATH test case (LD_LIBRARY_PATH being dropped at execution). Signed-off-by: Tim Wiese <[email protected]>

test/test_find_library.cpp

src/find_library.cpp

thomas-moulard · 2020-03-16T18:40:51Z

2020-03-16T15:29:09.7674334Z Scanning dependencies of target rcpputils
2020-03-16T15:29:09.7674432Z [ 12%] Building CXX object CMakeFiles/rcpputils.dir/src/asserts.cpp.o
2020-03-16T15:29:09.7674538Z [ 16%] Building CXX object CMakeFiles/rcpputils.dir/src/find_library.cpp.o
2020-03-16T15:29:09.7675520Z /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp: In function â€˜std::__cxx11::string rcpputils::find_library_path(const string&)â€™:
2020-03-16T15:29:09.7676045Z /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp:113:64: error: passing â€˜const std::__cxx11::list<std::__cxx11::basic_string<char> >â€™ as â€˜thisâ€™ argument discards qualifiers [-fpermissive]
2020-03-16T15:29:09.7676169Z    search_paths.splice(search_paths.cend(), search_paths_runpath);
2020-03-16T15:29:09.7676276Z                                                                 ^
2020-03-16T15:29:09.7676377Z In file included from /usr/include/c++/7/list:63:0,
2020-03-16T15:29:09.7676485Z                  from /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp:24:
2020-03-16T15:29:09.7677142Z /usr/include/c++/7/bits/stl_list.h:1443:7: note:   in call to â€˜void std::__cxx11::list<_Tp, _Alloc>::splice(std::__cxx11::list<_Tp, _Alloc>::const_iterator, std::__cxx11::list<_Tp, _Alloc>&) [with _Tp = std::__cxx11::basic_string<char>; _Alloc = std::allocator<std::__cxx11::basic_string<char> >; std::__cxx11::list<_Tp, _Alloc>::const_iterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >]â€™
2020-03-16T15:29:09.7677401Z        splice(const_iterator __position, list& __x) noexcept
2020-03-16T15:29:09.7677499Z        ^~~~~~
2020-03-16T15:29:09.7678051Z /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp:113:64: error: binding reference of type â€˜std::__cxx11::list<std::__cxx11::basic_string<char> >&â€™ to â€˜const std::__cxx11::list<std::__cxx11::basic_string<char> >â€™ discards qualifiers
2020-03-16T15:29:09.7678184Z    search_paths.splice(search_paths.cend(), search_paths_runpath);
2020-03-16T15:29:09.7678285Z                                                                 ^
2020-03-16T15:29:09.7678440Z In file included from /usr/include/c++/7/list:63:0,
2020-03-16T15:29:09.7678549Z                  from /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp:24:
2020-03-16T15:29:09.7679199Z /usr/include/c++/7/bits/stl_list.h:1443:7: note:   initializing argument 2 of â€˜void std::__cxx11::list<_Tp, _Alloc>::splice(std::__cxx11::list<_Tp, _Alloc>::const_iterator, std::__cxx11::list<_Tp, _Alloc>&) [with _Tp = std::__cxx11::basic_string<char>; _Alloc = std::allocator<std::__cxx11::basic_string<char> >; std::__cxx11::list<_Tp, _Alloc>::const_iterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >]â€™
2020-03-16T15:29:09.7679345Z        splice(const_iterator __position, list& __x) noexcept
2020-03-16T15:29:09.7679442Z        ^~~~~~
2020-03-16T15:29:09.7679698Z CMakeFiles/rcpputils.dir/build.make:75: recipe for target 'CMakeFiles/rcpputils.dir/src/find_library.cpp.o' failed
2020-03-16T15:29:09.7679815Z make[2]: *** [CMakeFiles/rcpputils.dir/src/find_library.cpp.o] Error 1
2020-03-16T15:29:09.7680059Z CMakeFiles/Makefile2:405: recipe for target 'CMakeFiles/rcpputils.dir/all' failed
2020-03-16T15:29:09.7680167Z make[1]: *** [CMakeFiles/rcpputils.dir/all] Error 2
2020-03-16T15:29:09.7680268Z make[1]: *** Waiting for unfinished jobs....
2020-03-16T15:29:09.7680348Z [ 19%] Linking CXX static library libgtest_main.a
2020-03-16T15:29:09.7680440Z [ 19%] Built target gtest_main
2020-03-16T15:29:09.7680639Z Makefile:140: recipe for target 'all' failed
2020-03-16T15:29:09.7680733Z make: *** [all] Error 2
2020-03-16T15:29:09.7680903Z ---
2020-03-16T15:29:09.7681076Z --- stderr: rcpputils
2020-03-16T15:29:09.7681499Z /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp: In function â€˜std::__cxx11::string rcpputils::find_library_path(const string&)â€™:
2020-03-16T15:29:09.7681995Z /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp:113:64: error: passing â€˜const std::__cxx11::list<std::__cxx11::basic_string<char> >â€™ as â€˜thisâ€™ argument discards qualifiers [-fpermissive]
2020-03-16T15:29:09.7682121Z    search_paths.splice(search_paths.cend(), search_paths_runpath);
2020-03-16T15:29:09.7682223Z                                                                 ^
2020-03-16T15:29:09.7682328Z In file included from /usr/include/c++/7/list:63:0,
2020-03-16T15:29:09.7682433Z                  from /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp:24:
2020-03-16T15:29:09.7683220Z /usr/include/c++/7/bits/stl_list.h:1443:7: note:   in call to â€˜void std::__cxx11::list<_Tp, _Alloc>::splice(std::__cxx11::list<_Tp, _Alloc>::const_iterator, std::__cxx11::list<_Tp, _Alloc>&) [with _Tp = std::__cxx11::basic_string<char>; _Alloc = std::allocator<std::__cxx11::basic_string<char> >; std::__cxx11::list<_Tp, _Alloc>::const_iterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >]â€™
2020-03-16T15:29:09.7683380Z        splice(const_iterator __position, list& __x) noexcept
2020-03-16T15:29:09.7683488Z        ^~~~~~
2020-03-16T15:29:09.7684088Z /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp:113:64: error: binding reference of type â€˜std::__cxx11::list<std::__cxx11::basic_string<char> >&â€™ to â€˜const std::__cxx11::list<std::__cxx11::basic_string<char> >â€™ discards qualifiers
2020-03-16T15:29:09.7684284Z    search_paths.splice(search_paths.cend(), search_paths_runpath);
2020-03-16T15:29:09.7684397Z                                                                 ^
2020-03-16T15:29:09.7684509Z In file included from /usr/include/c++/7/list:63:0,
2020-03-16T15:29:09.7684627Z                  from /home/runner/work/rcpputils/rcpputils/ros_ws/src/rcpputils/src/find_library.cpp:24:
2020-03-16T15:29:09.7685398Z /usr/include/c++/7/bits/stl_list.h:1443:7: note:   initializing argument 2 of â€˜void std::__cxx11::list<_Tp, _Alloc>::splice(std::__cxx11::list<_Tp, _Alloc>::const_iterator, std::__cxx11::list<_Tp, _Alloc>&) [with _Tp = std::__cxx11::basic_string<char>; _Alloc = std::allocator<std::__cxx11::basic_string<char> >; std::__cxx11::list<_Tp, _Alloc>::const_iterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >]â€™
2020-03-16T15:29:09.7685565Z        splice(const_iterator __position, list& __x) noexcept
2020-03-16T15:29:09.7685679Z        ^~~~~~
2020-03-16T15:29:09.7685786Z make[2]: *** [CMakeFiles/rcpputils.dir/src/find_library.cpp.o] Error 1
2020-03-16T15:29:09.7685902Z make[1]: *** [CMakeFiles/rcpputils.dir/all] Error 2
2020-03-16T15:29:09.7686012Z make[1]: *** Waiting for unfinished jobs....
2020-03-16T15:29:09.7686119Z make: *** [all] Error 2
2020-03-16T15:29:09.7686319Z ---
2020-03-16T15:29:09.7686407Z Failed   <<< rcpputils	[ Exited with code 2 ]
2020-03-16T15:29:09.8171310Z 
2020-03-16T15:29:09.8171894Z Summary: 50 packages finished [59.0s]
2020-03-16T15:29:09.8172064Z   1 package failed: rcpputils
2020-03-16T15:29:09.8172169Z   1 package had stderr output: rcpputils
2020-03-16T15:29:09.8631007Z ##[endgroup]
2020-03-16T15:29:09.8635441Z ##[error]The process 'bash' failed with exit code 2
2020-03-16T15:29:09.8753653Z Cleaning up orphan processes

Seems like the CI failed building the code.

Co-Authored-By: Thomas Moulard <[email protected]> Signed-off-by: Tim Wiese <[email protected]>

Signed-off-by: Tim Wiese <[email protected]>

wieset · 2020-03-17T16:35:41Z

Fixed now, thanks for reviewing!

clalancette · 2020-06-11T15:37:00Z

CMakeLists.txt

+set_target_properties(${PROJECT_NAME} PROPERTIES
+  BUILD_RPATH ${CMAKE_CURRENT_BINARY_DIR}
+  INSTALL_RPATH_USE_LINK_PATH TRUE
+)


The overall thing that concerns me about this is this bit. As far as I know, we don't currently set RPATH/RUNPATH on any of our shared objects. In order for this to truly work in general, we would have to start setting that, correct?

Sorry for taking so long to reply. I think you are right, for this to work in general, you would need to have set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) for all projects.

What we are currently doing in our project is using set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) together with set(CMAKE_EXE_LINKER_FLAGS "-Wl,--disable-new-dtags"). This makes use of RPATH instead of RUNPATH. RPATH works with indirect shared library dependencies, whereas RUNPATH does not. This, together with the changes from this pull request, allows us to load all shared libraries without having to rely on LD_LIBRARY_PATH. However, RPATH is deprecated, so I don't think this is a good general solution.

Would it be an option to set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) globally for ament_cmake? I am not sure about any downsides of this.

There has been no activity here in quite a while, so I just wanted to ask if there is any update on this. As a workaround, we are currently running our node in a root shell, but of course we would prefer not to.

Also wanted to check on the status of this. I'm working on a linuxdeploy plugin for building AppImages that bundle a ROS2 app and its dependencies (similar to the linuxdeploy qt plugin).

Certain dependencies, like RMW implementations, are searched for at runtime with RPATH/RUNPATH excluded, so even if the executable's RPATH points to the place where these libraries are located, find_library_path() fails to pick them up. Would be nice to not have to rely on setting environment variables like LD_LIBRARY_PATH or having to source the setup.bash scripts as a custom init step in the AppImage's entrypoint (they are already slow enough to start, due to the squashfs mounting step 🙂)

Overall, I think there are two different things going on in this PR (please correct me if I'm wrong):

We are adding code to search the RPATH/RUNPATH for symbols (these are the changes in find_library.cpp).

We are switching the code to force RPATH instead of RUNPATH (this is the initial changes in CMakeLists.txt).

I'm fine with the first one; in our current system, that will really have no effect, but gets us closer to what @wieset and others want. I'm much less sure about the implications about the second one, which I think is what is mostly holding this up.

So my suggestion here is that we split out the second part from this PR, and just concentrate on getting the "search in RPATH" changes in. Once that is in, we can consider the "force RUNPATH -> RPATH" separately. Does that make sense?

(also, this PR needs a rebase since things have changed in the meantime)

True, there are two things going on. I think the first one is obsolete now with ros2/rcutils#320 which is great! The second one is actually a bit different. It does not force RPATH instead of RUNPATH at all. Even though CMake uses RPATH in their variable names, it refers to RUNPATH since RPATH is deprecated.

BUILD_RPATH ${CMAKE_CURRENT_BINARY_DIR} is necessary so that RUNPATH in the build binary gets populated with the path to the test library

INSTALL_RPATH_USE_LINK_PATH TRUE is necessary so that RUNPATH in the installed binary gets populated properly

However, thinking about it again, INSTALL_RPATH_USE_LINK_PATH TRUE needs to be set for ALL packages so that RUNPATH can be used as an alternative to LD_LIBRARY_PATH. The reason for this is that RUNPATH only works for direct shared object dependencies. See also the discussion here: https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/1737608

And this is what's necessary for us to be able to run a node with setcap since it drops LD_LIBRARY_PATH. A solution I see would be to have set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) in the ament_package() macro.

I hope this explanation is clear. What do you think about closing this PR and continuing the discussion in https://github.com/ament/ament_cmake/?

I hope this explanation is clear. What do you think about closing this PR and continuing the discussion in https://github.com/ament/ament_cmake/?

That sounds reasonable to me, thanks. I'll go ahead and close this; please feel free to open an issue over there.

wieset requested a review from a team as a code owner February 12, 2020 15:13

wieset mentioned this pull request Feb 12, 2020

find_library: Prefer currently loaded libraries over searching LD_LIBRARY_PATH ros2/rcutils#143

Closed

wieset added 5 commits February 12, 2020 16:35

On Linux, search RUNPATH in addition to LD_LIBRARY_PATH

3b8cf64

Signed-off-by: Tim Wiese <[email protected]>

Add test for searching RUNPATH to find library on Linux

3934468

Signed-off-by: Tim Wiese <[email protected]>

Make sure RUNPATH is set correctly

f1d15bb

BUILD_RPATH is set to find test library INSTALL_RPATH_USE_LINK_PATH adds linked library paths to RUNPATH Signed-off-by: Tim Wiese <[email protected]>

Fix header order

d4542a3

Signed-off-by: Tim Wiese <[email protected]>

Fix pointer code style

2b7b0b9

Signed-off-by: Tim Wiese <[email protected]>

thomas-moulard reviewed Feb 12, 2020

View reviewed changes

src/find_library.cpp Show resolved Hide resolved

src/find_library.cpp Outdated Show resolved Hide resolved

test/test_find_library.cpp Outdated Show resolved Hide resolved

wieset added 2 commits February 14, 2020 17:08

Put ifdef around include

71b46d1

Signed-off-by: Tim Wiese <[email protected]>

Extract RUNPATH logic into helper method

0d49a40

Signed-off-by: Tim Wiese <[email protected]>

thomas-moulard reviewed Feb 17, 2020

View reviewed changes

src/find_library.cpp Outdated Show resolved Hide resolved

test/test_find_library.cpp Outdated Show resolved Hide resolved

wieset added 2 commits February 18, 2020 11:17

Return empty list in RUNPATH search on win / macos

f4ee238

Signed-off-by: Tim Wiese <[email protected]>

claireyywang requested a review from thomas-moulard March 12, 2020 16:49

thomas-moulard approved these changes Mar 13, 2020

View reviewed changes

thomas-moulard reviewed Mar 13, 2020

View reviewed changes

test/test_find_library.cpp Outdated Show resolved Hide resolved

thomas-moulard reviewed Mar 13, 2020

View reviewed changes

src/find_library.cpp Outdated Show resolved Hide resolved

thomas-moulard reviewed Mar 13, 2020

View reviewed changes

src/find_library.cpp Show resolved Hide resolved

thomas-moulard reviewed Mar 13, 2020

View reviewed changes

src/find_library.cpp Show resolved Hide resolved

wieset and others added 2 commits March 17, 2020 17:17

Add const qualifier

f691a10

Co-Authored-By: Thomas Moulard <[email protected]> Signed-off-by: Tim Wiese <[email protected]>

Merge branch 'master' into search-runpath

eaad9e4

Signed-off-by: Tim Wiese <[email protected]>

claireyywang assigned cottsay Mar 26, 2020

clalancette reviewed Jun 11, 2020

View reviewed changes

clalancette closed this Jan 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search RUNPATH to find library on Linux #44

Search RUNPATH to find library on Linux #44

wieset commented Feb 12, 2020 •

edited

Loading

thomas-moulard commented Feb 12, 2020

thomas-moulard commented Mar 16, 2020

wieset commented Mar 17, 2020

clalancette Jun 11, 2020

wieset Jul 20, 2020

wieset Oct 28, 2020

spiderkeys Nov 15, 2020

clalancette Dec 11, 2020

wieset Jan 8, 2021

clalancette Jan 13, 2021

Search RUNPATH to find library on Linux #44

Search RUNPATH to find library on Linux #44

Conversation

wieset commented Feb 12, 2020 • edited Loading

thomas-moulard commented Feb 12, 2020

thomas-moulard commented Mar 16, 2020

wieset commented Mar 17, 2020

clalancette Jun 11, 2020

Choose a reason for hiding this comment

wieset Jul 20, 2020

Choose a reason for hiding this comment

wieset Oct 28, 2020

Choose a reason for hiding this comment

spiderkeys Nov 15, 2020

Choose a reason for hiding this comment

clalancette Dec 11, 2020

Choose a reason for hiding this comment

wieset Jan 8, 2021

Choose a reason for hiding this comment

clalancette Jan 13, 2021

Choose a reason for hiding this comment

wieset commented Feb 12, 2020 •

edited

Loading