Fix -pthread handling in Debian buster (ros#1021)

* Fix -pthread handling in Debian buster

FindBoost.cmake blindly adds `${CMAKE_THREAD_LIBS_INIT}` to
`${Boost_LIBRARIES}` when the component `thread` is found.
On Debian buster the `FindThreads.cmake` sets that to `-pthread`.
This breaks a bunch of stuff becakse `-pthread` is a linker flag, not a
library.

There were earlier fixes for `-lpthread`.
This PR expands upon them.
First this PR modifies the fix from ros#998 to not add `-l` to any linker flag.
Second it adds to the fix in ros#975 to make sure `-pthread` is passed to
downstream users.
There's no standard cmake variable for linker flags, so this PR opts to
create an interface target with just the flag, and add that to
`${PROJECT_NAME}_LIBRARIES` instead.

Both this PR and ros-visualization/python_qt_binding#68 are required to strip or `qt_gui_cpp` will fail at link time.

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* math() output actually used

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* dummyN -> wrapped-linker-optionsN

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Fix pre-3.13.0 target property setting

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Add test for propagation of linker options

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Force add CMakeLists.txt

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Indent using 2 spaces

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

* Increment until target is unique

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

cmake/catkin_package.cmake

@@ -270,7 +270,8 @@ function(_catkin_package)
   catkin_filter_libraries_for_build_configuration(libraries ${PKG_CONFIG_LIBRARIES})
   foreach(library ${libraries})
     if(NOT IS_ABSOLUTE ${library})
-      if(NOT ${library} MATCHES "^-l")
+      # Append -l to anything that's not a linker flag
+      if(NOT ${library} MATCHES "^-")
         set(library "-l${library}")
       endif()
     endif()

cmake/templates/pkgConfig.cmake.in

@@ -123,6 +123,29 @@ foreach(library ${libraries})
     list(APPEND @PROJECT_NAME@_LIBRARIES ${library})
   elseif(${library} MATCHES "^-l")
     list(APPEND @PROJECT_NAME@_LIBRARIES ${library})
+  elseif(${library} MATCHES "^-")
+    # This is a linker flag/option (like -pthread)
+    # There's no standard variable for these, so create an interface library to hold it
+    if(NOT @PROJECT_NAME@_NUM_DUMMY_TARGETS)
+      set(@PROJECT_NAME@_NUM_DUMMY_TARGETS 0)
+    endif()
+    # Make sure the target name is unique
+    set(interface_target_name "catkin::@PROJECT_NAME@::wrapped-linker-option${@PROJECT_NAME@_NUM_DUMMY_TARGETS}")
+    while(TARGET "${interface_target_name}")
+      math(EXPR @PROJECT_NAME@_NUM_DUMMY_TARGETS "${@PROJECT_NAME@_NUM_DUMMY_TARGETS}+1")
+      set(interface_target_name "catkin::@PROJECT_NAME@::wrapped-linker-option${@PROJECT_NAME@_NUM_DUMMY_TARGETS}")
+    endwhile()
+    add_library("${interface_target_name}" INTERFACE IMPORTED)
+    if("${CMAKE_VERSION}" VERSION_LESS "3.13.0")
+      set_property(
+        TARGET
+        "${interface_target_name}"
+        APPEND PROPERTY
+        INTERFACE_LINK_LIBRARIES "${library}")
+    else()
+      target_link_options("${interface_target_name}" INTERFACE "${library}")
+    endif()
+    list(APPEND @PROJECT_NAME@_LIBRARIES "${interface_target_name}")
   elseif(TARGET ${library})
     list(APPEND @PROJECT_NAME@_LIBRARIES ${library})
   elseif(IS_ABSOLUTE ${library})

test/local_tests/test_with_mock_workspace.py

@@ -39,6 +39,18 @@ def test_catkin_only(self):
                       "setup.sh",
                       "setup.zsh")
 
+    def test_linker_options_propagation(self):
+        dstdir = os.path.join(self.workspacedir, 'linker_options')
+        shutil.copytree(os.path.join(MOCK_DIR, 'src', 'linker_options'), dstdir)
+        out = self.cmake()
+        succeed(MAKE_CMD, cwd=self.builddir)
+        succeed(MAKE_CMD + ["install"], cwd=self.builddir)
+
+        assert_exists(self.installdir,
+                      "env.sh",
+                      "setup.sh",
+                      "setup.zsh")
+
     def test_nolang(self):
         dstdir = os.path.join(self.workspacedir, 'nolangs')
         shutil.copytree(os.path.join(MOCK_DIR, 'src', 'nolangs'), dstdir)

test/mock_resources/src/linker_options/src/leaf_pkg/CMakeLists.txt

@@ -0,0 +1,18 @@
+cmake_minimum_required(VERSION 3.0.2)
+project(leaf_pkg)
+
+find_package(catkin REQUIRED COMPONENTS root_pkg)
+
+add_executable(leaf leaf.cpp)
+target_link_libraries(leaf ${catkin_LIBRARIES})
+target_include_directories(leaf PUBLIC ${catkin_INCLUDE_DIRS})
+
+catkin_package()
+
+list(LENGTH catkin_LIBRARIES size)
+math(EXPR size_is_not_3 "${size} - 3")
+if (size_is_not_3)
+  message(FATAL_ERROR "Wrong size for catkin_LIBRARIES: ${catkin_LIBRARIES} : ${size}")
+else()
+  message(STATUS "All is OK: ${catkin_LIBRARIES}")
+endif()

test/mock_resources/src/linker_options/src/leaf_pkg/leaf.cpp

@@ -0,0 +1,9 @@
+#include <iostream>
+#include <root_pkg/root.hpp>
+
+int main()
+{
+  std::cout << "leaf_pkg calling ";
+  root_pkg::func();
+  return 0;
+}

test/mock_resources/src/linker_options/src/leaf_pkg/package.xml

@@ -0,0 +1,13 @@
+<package format="2">
+  <name>leaf_pkg</name>
+  <version>3.4.5</version>
+  <description>Checks library and linker options</description>
+  <author>Somebody</author>
+  <maintainer email="Somebody@somewhere.org">Somebody</maintainer>
+  <license>Unknown</license>
+  <url/>
+
+  <buildtool_depend>catkin</buildtool_depend>
+
+  <depend>root_pkg</depend>
+</package>

test/mock_resources/src/linker_options/src/root_pkg/CMakeLists.txt

@@ -0,0 +1,20 @@
+cmake_minimum_required(VERSION 3.0.2)
+project(root_pkg)
+
+find_package(catkin REQUIRED)
+
+add_library(rootlib SHARED root.cpp)
+target_include_directories(rootlib PUBLIC include)
+
+catkin_package(
+  INCLUDE_DIRS include
+  LIBRARIES -pthread -lpthread rootlib
+  )
+
+install(TARGETS rootlib
+        ARCHIVE DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
+        LIBRARY DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
+        RUNTIME DESTINATION ${CATKIN_GLOBAL_BIN_DESTINATION})
+
+install(DIRECTORY include/${PROJECT_NAME}/
+        DESTINATION ${CATKIN_PACKAGE_INCLUDE_DESTINATION})

test/mock_resources/src/linker_options/src/root_pkg/include/root_pkg/root.hpp

@@ -0,0 +1,4 @@
+namespace root_pkg
+{
+void func();
+}

test/mock_resources/src/linker_options/src/root_pkg/package.xml

@@ -0,0 +1,11 @@
+<package format="2">
+  <name>root_pkg</name>
+  <version>3.4.5</version>
+  <description>Exports library and linker options in ${root_pkg_LIBRARIES}</description>
+  <author>Somebody</author>
+  <maintainer email="Somebody@somewhere.org">Somebody</maintainer>
+  <license>Unknown</license>
+  <url/>
+
+  <buildtool_depend>catkin</buildtool_depend>
+</package>

test/mock_resources/src/linker_options/src/root_pkg/root.cpp

@@ -0,0 +1,11 @@
+#include <iostream>
+
+#include "root_pkg/root.hpp"
+
+namespace root_pkg
+{
+void func()
+{
+  std::cout << "root_pkg::func()\n";
+}
+}