CMake: Fix link to atomic library in armhf and mips arch #790
Conversation
|
@bulwahn friendly ping |
For arm & mips architecture, the linker must explicitly be asked to link the atomic library (with `-latomic`). Otherwise, the linking fails with: ``` | devel/lib/libmavros.so: undefined reference to `__atomic_load_8' | devel/lib/libmavros.so: undefined reference to `__atomic_store_8' | collect2: error: ld returned 1 exit status ``` Linking `atomic` unconditionally as library is strictly needed only for arm & mips, but it seems not to imply any further differences with other architectures. Hence, this commit simply adds `atomic` unconditionally for a uniform handling of all machine architectures. This is an alternative solution to the proposed solution in mavlink#790. The issue was discovered cross-compiling mavros in meta-ros, the OpenEmbedded layer for ROS. Some further pointers are available at: ros/meta-ros#525 Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
|
@TSC21 I proposed an alternative solution in #797, which I came up with in parallel to your solution on last Friday. Maybe you can have a look, consider which you would find better and test #797 as well, esp. if it breaks your default build setups. I will test today if this pull request also resolves the issue in meta-ros. |
|
@TSC21 I tested your commit applied in meta-ros, and it remains to fail with: for the arm architecture. |
|
I see your point. But I would like to this being added to the CMakeList and only be added the flag when required for the target architecture. Maybe changing from the HOST to the TARGET arch would work? I will add that change, and if it works we can merge this. If not, we can accept your PR. |
|
@bulwahn I think I found a solution to check for the TARGET arch. Will replace the code of this PR soon. |
|
@TSC21 Once a new commit is available, just ping me and run tests in the cross-compile tool chain to check if it works. By the way, the issue occurs with arm, mips and ppc, not just arm and mips. |
|
https://github.com/axr/solar-cmake/blob/master/TargetArch.cmake seems to aid on the solution. The trouble is: there are so many archs that can be linked to the |
For arm & mips architecture, the linker must explicitly be asked to link the atomic library (with `-latomic`). Otherwise, the linking fails with: ``` | devel/lib/libmavros.so: undefined reference to `__atomic_load_8' | devel/lib/libmavros.so: undefined reference to `__atomic_store_8' | collect2: error: ld returned 1 exit status ``` Linking `atomic` unconditionally as library is strictly needed only for arm & mips, but it seems not to imply any further differences with other architectures. Hence, this commit simply adds `atomic` unconditionally for a uniform handling of all machine architectures. This is an alternative solution to the proposed solution in #790. The issue was discovered cross-compiling mavros in meta-ros, the OpenEmbedded layer for ROS. Some further pointers are available at: ros/meta-ros#525 Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Fixes ros/meta-ros#525.
@bulwahn can you test it and check if this addresses and fixes the problem? Thanks