Fix/strict_linking #1385

esteve · 2018-07-20T09:02:57Z

Status

DEVELOPMENT

Description

This makes sure that all binaries have their dependencies linked, this is particularly useful when crosscompiling since the dependencies are in a separate sysroot

Steps to Test or Reproduce

cd ros/
./catkin_make_release

Everything should be built and linked.

esteve · 2018-07-20T09:07:00Z

This is still WIP, not ready for review

esteve · 2018-07-20T09:38:06Z

There's a few more changes I need to make, I'll close this and reopen it when it's ready.

esteve · 2018-07-20T14:10:21Z

This is now ready for review. I've added a new package autoware_build_flags, which sets the linker flags for all projects so that any undefined symbols cause an error.

I triggered a job in my Travis repo that shows the packages that fail with the appropriate flags enabled (https://travis-ci.com/esteve/Autoware/builds/79572212) and in this PR I've fixed any undefined symbols.

esteve · 2018-07-20T16:20:32Z

Travis failed again for no reason:

https://travis-ci.org/CPFL/Autoware/jobs/406283890

But the other two jobs passed, even with the strict linking flags from autoware_build_flags enabled.

bdholt1 · 2018-07-21T07:44:00Z

ros/src/sensing/drivers/camera/packages/hexacam/package.xml

  <!-- Use run_depend for packages you need at runtime: -->
  <!--   <run_depend>message_runtime</run_depend> -->
  <!-- Use test_depend for packages you need only for testing: -->
  <!--   <test_depend>gtest</test_depend> -->
  <buildtool_depend>catkin</buildtool_depend>
+  <buildtool_depend>autoware_build_flags</buildtool_depend>


These seem like duplicates - should we just uncomment line 37 and 38 and delete 43 and 44?

Good catch. I've submitted #1392 to address this.

kfunaoka · 2018-07-23T03:02:31Z

@esteve @bdholt1 Thanks for Pull request and review!

We should push two works on hexacam/package.xml.

Delete redundant template comments.
Set license BSD.

But I think they can be divided into another Pull Request because they are not related to strict linking. I'll approve this PR, but if you think more works should be done here, please add them into this PR.

esteve · 2018-07-23T09:03:56Z

@kfunaoka I've submitted #1392 which does what you describe and rebased this branch on top of it. I also thought that it might make sense to move all the common compile flags to autoware_build_flags, I've seen that most CMake files have the following line (or similar):

SET(CMAKE_CXX_FLAGS "-std=c++11 -O2 -g -Wall ${CMAKE_CXX_FLAGS}")

but perhaps it'd be better to do that in a followup PR and leave this one as is.

kfunaoka · 2018-07-23T09:55:47Z

@esteve Thank you for the proposal. I think it is a good idea to move the compiler flags into the autoware_build_flags package because there are many CMAKE_CXX_FLAGS everywhere. I'll approve this PR after Travis succeeds.

kfunaoka · 2018-07-23T10:01:27Z

Already approved.
I'll merge this PR after Travis succeeds.

kfunaoka · 2018-07-23T13:38:00Z

This error is reproduced multiple times on Travis ROS_DISTRO=kinetic.
Something seems to be different from before.

Need to get 9,427 kB of archives.
After this operation, 48.7 MB of additional disk space will be used.
WARNING: The following packages cannot be authenticated!
  python-catkin-pkg-modules python-catkin-pkg python-rospkg-modules
  python-rospkg python-rosdistro-modules python-rosdistro python-rosdep
  python-vcstools python-wstool ros-kinetic-catkin
E: There were unauthenticated packages and -y was used without --allow-unauthenticated
/home/travis/.travis/job_stages: line 78: /opt/ros/kinetic/setup.bash: No such file or directory
sudo: rosdep: command not found
The program 'rosdep' is currently not installed. To run 'rosdep' please ask your administrator to install the package 'python-rosdep'

travis_time:end:0f03e6a5:start=1532352776778120743,finish=1532352780978503020,duration=4200382277
�[0K
�[31;1mThe command "if [ "${CROSS_COMPILE}" != "1" ]; then sudo sh -c "echo \"deb http://packages.ros.org/ros/ubuntu $ROS_CI_DESKTOP main\" > /etc/apt/sources.list.d/ros-latest.list"; sudo apt-key adv --keyserver hkp://ha.pool.sks-keyservers.net:80 --recv-key 421C365BD9FF1F717815A3895523BAEEB01FA116; sudo apt-get update; sudo apt-get install -y  python-catkin-pkg python-rosdep python-wstool ros-${ROS_DISTRO}-catkin; source /opt/ros/${ROS_DISTRO}/setup.bash; sudo rosdep init; rosdep update; fi" failed and exited with 127 during .�[0m

Your build has been stopped.

Error on Travis

bdholt1 · 2018-07-23T14:02:07Z

ros/src/common/cmake/autoware_build_flags/cmake/autoware_build_flags-extras.cmake

@@ -0,0 +1,5 @@
+if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+  set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--no-undefined")


These are only linker flags. Are we not interested in setting CMAKE_CXX_FLAGS and CMAKE_C_FLAGS as well?

I realise that adding something like CMAKE_CXX_FLAGS=Wall,Wextra,Werror may be too difficult to add into the code base now - there is probably quite a bit of work to do to get to the point where that is possible.

Yeah, that'd be really useful. I mentioned that in #1385 (comment), but I think it'd be best to put those flags in a separate PR so that this one can be merged sooner. Also, I noticed that the flags are not consistent across all packages (e.g. -O2 vs -O3, etc.), so for now I'd only add -Wall and the flags for enabling C++11.

I've submitted #1395 which does that, though only for C++11

esteve · 2018-07-23T14:18:36Z

@kfunaoka this seems to have caused by a network failure when trying to retrieve the key used to sign the repository:

hkp://ha.pool.sks-keyservers.net:80
--recv-key
421C365BD9FF1F717815A3895523BAEEB01FA116
gpg: requesting key B01FA116 from hkp server ha.pool.sks-keyservers.net
?: ha.pool.sks-keyservers.net: Host not found

I've had to restart the CI jobs in my repo multiple times, Travis has been very unreliable lately (e.g. jobs that don't even start, running out of memory, etc.)

esteve · 2018-07-23T15:41:12Z

@kfunaoka after countless restarts, I finally got a passing Travis job:

https://travis-ci.com/esteve/Autoware/builds/79695005

kfunaoka

@esteve Thank you for pointing out the root cause! Everything is fine.

Found missing dependency

kfunaoka · 2018-07-24T03:35:40Z

@esteve On my PC, I found the following error.

[ 72%] Linking CXX executable /home/funaoka/Autoware/ros/devel/lib/lidar_svm_detect/lidar_svm_detect
[ 72%] Built target lidar_svm_detect
[ 72%] Linking CXX shared library /home/funaoka/Autoware/ros/devel/lib/libpos_db.so
CMakeFiles/pos_db.dir/lib/pos_db/SendData.cpp.o: In function `SendData::ConnectDB()':
SendData.cpp:(.text+0x5d0): undefined reference to `libssh2_session_init_ex'
SendData.cpp:(.text+0x5ee): undefined reference to `libssh2_session_handshake'
SendData.cpp:(.text+0x607): undefined reference to `libssh2_hostkey_hash'
SendData.cpp:(.text+0x636): undefined reference to `libssh2_userauth_publickey_fromfile_ex'
SendData.cpp:(.text+0x767): undefined reference to `libssh2_session_free'
SendData.cpp:(.text+0x7e3): undefined reference to `libssh2_session_disconnect_ex'
CMakeFiles/pos_db.dir/lib/pos_db/SendData.cpp.o: In function `SendData::DisconnectDB(char const*)':
SendData.cpp:(.text+0x8b6): undefined reference to `libssh2_channel_free'
SendData.cpp:(.text+0x8e6): undefined reference to `libssh2_session_disconnect_ex'
SendData.cpp:(.text+0x8f2): undefined reference to `libssh2_session_free'
SendData.cpp:(.text+0x945): undefined reference to `libssh2_session_disconnect_ex'
CMakeFiles/pos_db.dir/lib/pos_db/SendData.cpp.o: In function `SendData::Sender(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, int)':
SendData.cpp:(.text+0x99f): undefined reference to `libssh2_session_get_timeout'
SendData.cpp:(.text+0x9b4): undefined reference to `libssh2_session_set_timeout'
SendData.cpp:(.text+0xa71): undefined reference to `libssh2_channel_direct_tcpip_ex'
SendData.cpp:(.text+0xb2e): undefined reference to `libssh2_channel_flush_ex'
SendData.cpp:(.text+0xb8f): undefined reference to `libssh2_channel_write_ex'
SendData.cpp:(.text+0xc39): undefined reference to `libssh2_channel_read_ex'
SendData.cpp:(.text+0xd10): undefined reference to `libssh2_channel_read_ex'
SendData.cpp:(.text+0xe7a): undefined reference to `libssh2_channel_free'
collect2: error: ld returned 1 exit status
data/packages/pos_db/CMakeFiles/pos_db.dir/build.make:120: recipe for target '/home/funaoka/Autoware/ros/devel/lib/libpos_db.so' failed
make[2]: *** [/home/funaoka/Autoware/ros/devel/lib/libpos_db.so] Error 1
CMakeFiles/Makefile2:29024: recipe for target 'data/packages/pos_db/CMakeFiles/pos_db.dir/all' failed
make[1]: *** [data/packages/pos_db/CMakeFiles/pos_db.dir/all] Error 2
Makefile:138: recipe for target 'all' failed
make: *** [all] Error 2
Invoking "make -j1" failed
-> [1]

The following modification fixes my problem above. Does this modification make sense? If so, would you add the following modification into this pull request?

diff --git a/ros/src/data/packages/pos_db/CMakeLists.txt b/ros/src/data/packages/pos_db/CMakeLists.txt
index 63791c1..8fae997 100644
--- a/ros/src/data/packages/pos_db/CMakeLists.txt
+++ b/ros/src/data/packages/pos_db/CMakeLists.txt
@@ -36,6 +36,9 @@ if (LIBSSH2_FOUND)
             lib/pos_db/SendData.cpp
             lib/pos_db/util.cpp
             )
+    target_link_libraries(pos_db
+            ${LIBSSH2_LIBRARIES}
+            )
 
     add_executable(pos_downloader
             nodes/pos_downloader/pos_downloader.cpp)

esteve · 2018-07-24T08:18:27Z

@kfunaoka done, thanks for the heads up.

kfunaoka

@esteve Thanks a lot! Everything works fine on my PC. I'll merge it after Travis succeeds

esteve · 2018-07-24T09:51:56Z

@kfunaoka thanks!

…autowarefoundation#1385)

esteve closed this Jul 20, 2018

esteve reopened this Jul 20, 2018

esteve closed this Jul 20, 2018

esteve reopened this Jul 20, 2018

bdholt1 reviewed Jul 21, 2018

View reviewed changes

kfunaoka self-requested a review July 23, 2018 03:02

kfunaoka added this to the v1.8 milestone Jul 23, 2018

kfunaoka added the type:new-feature New functionalities or additions, feature requests. label Jul 23, 2018

kfunaoka previously approved these changes Jul 23, 2018

View reviewed changes

esteve added 11 commits July 23, 2018 09:34

Comments removed. License set to BSD

a0790f9

Added autoware_build_flags package

ba246fb

Fix missing ND* symbols

5118822

Fix missing Qt5 library

ef2ddea

Fix missing cURL library

74eab6d

Fix missing thread library

438d79a

Fix missing ROS libraries

20e4cef

Fix missing nlopt library

f0eb372

Fix missing OpenCV library

fe6213c

Fix missing OpenGL libraries

a4c22f4

Fix missing ROS libraries

0b31fba

bdholt1 reviewed Jul 23, 2018

View reviewed changes

esteve mentioned this pull request Jul 23, 2018

Fix/common build flags #1395

Merged

kfunaoka previously approved these changes Jul 24, 2018

View reviewed changes

Fix missing libssh2 library

2e122a1

kfunaoka approved these changes Jul 24, 2018

View reviewed changes

kfunaoka merged commit 4d76f70 into autowarefoundation:develop Jul 24, 2018

esteve deleted the fix/strict_linking branch July 24, 2018 09:51

kfunaoka mentioned this pull request Aug 7, 2018

Feature/std perception msg #1418

Merged

2 tasks

anubhavashok pushed a commit to NuronLabs/autoware.ai that referenced this pull request Sep 7, 2021

[Feature] Makes sure that all binaries have their dependencies linked (…

9e9d293

…autowarefoundation#1385)

mitsudome-r added the version:autoware-ai Autoware.AI label Jun 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/strict_linking #1385

Fix/strict_linking #1385

esteve commented Jul 20, 2018 •

edited

Loading

esteve commented Jul 20, 2018

esteve commented Jul 20, 2018

esteve commented Jul 20, 2018

esteve commented Jul 20, 2018

bdholt1 Jul 21, 2018

esteve Jul 23, 2018

kfunaoka commented Jul 23, 2018

esteve commented Jul 23, 2018

kfunaoka commented Jul 23, 2018

kfunaoka commented Jul 23, 2018

kfunaoka commented Jul 23, 2018

bdholt1 Jul 23, 2018

bdholt1 Jul 23, 2018

esteve Jul 23, 2018

esteve Jul 23, 2018

esteve commented Jul 23, 2018

esteve commented Jul 23, 2018

kfunaoka left a comment

kfunaoka commented Jul 24, 2018

esteve commented Jul 24, 2018

kfunaoka left a comment

esteve commented Jul 24, 2018

		@@ -0,0 +1,5 @@
		if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
		set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--no-undefined")

Fix/strict_linking #1385

Fix/strict_linking #1385

Conversation

esteve commented Jul 20, 2018 • edited Loading

Status

Description

Steps to Test or Reproduce

esteve commented Jul 20, 2018

esteve commented Jul 20, 2018

esteve commented Jul 20, 2018

esteve commented Jul 20, 2018

bdholt1 Jul 21, 2018

Choose a reason for hiding this comment

esteve Jul 23, 2018

Choose a reason for hiding this comment

kfunaoka commented Jul 23, 2018

esteve commented Jul 23, 2018

kfunaoka commented Jul 23, 2018

kfunaoka commented Jul 23, 2018

kfunaoka commented Jul 23, 2018

bdholt1 Jul 23, 2018

Choose a reason for hiding this comment

bdholt1 Jul 23, 2018

Choose a reason for hiding this comment

esteve Jul 23, 2018

Choose a reason for hiding this comment

esteve Jul 23, 2018

Choose a reason for hiding this comment

esteve commented Jul 23, 2018

esteve commented Jul 23, 2018

kfunaoka left a comment

Choose a reason for hiding this comment

kfunaoka commented Jul 24, 2018

esteve commented Jul 24, 2018

kfunaoka left a comment

Choose a reason for hiding this comment

esteve commented Jul 24, 2018

esteve commented Jul 20, 2018 •

edited

Loading