Merge lp:~thomas-voss/mir/add-pthread-linker-and-preprocessor-flags-2 into lp:mir

[Thomas Voß]

Ensure -pthread is correctly passed to compiler and linker.

Ensure -pthread is correctly passed to compiler and linker.

[Loïc Minier (lool)]

Just wanted to +1 this change as there might be subtle breakage without it on some architecture / library combinations.

review: Approve

[Alan Griffiths (alan-griffiths)]

LGTM

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1123
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/150/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2322
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2207
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/147/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/150/rebuild

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Ignore the Jenkins failure. A fix for that should land soon.

[Daniel van Vugt (vanvugt)]

Is this for bug 1233988 still?

[Daniel van Vugt (vanvugt)]

This is incorrect and redundant:
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -pthread -Wl,--no-undefined")

Incorrect because it loses existing CMAKE_EXE_LINKER_FLAGS. And redundant because if you scroll down you'll find we already have:
  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -pthread")

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Also, AFAIK, C_FLAGS and CXX_FLAGS apply to link targets as well as compilation. So you don't need to mention any *LINKER_FLAGS.

[Daniel van Vugt (vanvugt)]

Seems to work. "-pthread" is appearing on link lines just from C(XX)_FLAGS, as desired.

I'm concerned about quietly changing the language spec though:
  -std=c++11
I think we should propose that separately in case it causes any behavioural changes.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Alrightee.

This is unnecessary:
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--no-undefined")
but if it builds, then good to have.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1124
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/152/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2328
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2213
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/149
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/149/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/152/rebuild

review: Approve (continuous-integration)

=== modified file '3rd_party/android-input/android/CMakeLists.txt'
--- 3rd_party/android-input/android/CMakeLists.txt	2013-05-30 19:24:29 +0000
+++ 3rd_party/android-input/android/CMakeLists.txt	2013-10-11 07:42:10 +0000
@@ -64,7 +64,6 @@
   android-input
 
   ${Boost_LIBRARIES}
-  -lpthread
 )
 
 set_target_properties(

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-10-09 07:04:15 +0000
+++ CMakeLists.txt	2013-10-11 07:42:10 +0000
@@ -42,9 +42,10 @@
 include (cmake/PrePush.cmake)
 include (GNUInstallDirs)
 
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -g -Werror -Wall -pedantic -Wextra -fPIC")
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -std=c++0x -Werror -Wall -fno-strict-aliasing -pedantic -Wnon-virtual-dtor -Wextra -fPIC")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pthread -g -Werror -Wall -pedantic -Wextra -fPIC")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread -g -std=c++0x -Werror -Wall -fno-strict-aliasing -pedantic -Wnon-virtual-dtor -Wextra -fPIC")
 set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--no-undefined")
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--no-undefined")
 
 if ("${CMAKE_CXX_COMPILER}" MATCHES "clang")
   set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-return-type-c-linkage -Wno-mismatched-tags")
@@ -115,7 +116,6 @@
   include_directories(SYSTEM ${PROJECT_SOURCE_DIR}/3rd_party/android-deps)
   list(APPEND CMAKE_SYSTEM_INCLUDE_PATH "${PROJECT_SOURCE_DIR}/3rd_party/android-deps")
 
-  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -pthread")
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fpermissive")
 
   add_definitions( -DANDROID )

=== modified file 'src/shared/protobuf/CMakeLists.txt'
--- src/shared/protobuf/CMakeLists.txt	2013-08-28 03:41:48 +0000
+++ src/shared/protobuf/CMakeLists.txt	2013-10-11 07:42:10 +0000
@@ -33,7 +33,6 @@
   mirprotobuf
 
   ${PROTOBUF_LIBRARIES}
-  -lpthread
 )
 
 # Export the include directories