[CDRIVER-2175] libbson cmake: pthreads not correctly compiled in Created: 05/Jun/17  Updated: 28/Oct/23  Resolved: 12/Jun/17

Status: Closed
Project: C Driver
Component/s: None
Affects Version/s: 1.7.0
Fix Version/s: 1.7.0

Type: Bug Priority: Major - P3
Reporter: Hannes Magnusson Assignee: Hannes Magnusson
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to CDRIVER-2166 Improve autotools/pkg-config detection Closed
is related to CDRIVER-2083 Distribute CMake config packages for ... Closed
Epic Link: convenient-static

 Description   

Our CMake check for pthreads is incorrect.
The way we add the build flags is also incorrect, we only link against it with -lpthread, but we are missing the more important part, compiling libbson using -pthread.

cmake 3.1 adds a easy way to do this: https://cmake.org/cmake/help/latest/module/FindThreads.html

set(THREADS_PREFER_PTHREAD_FLAG ON)
find_package(Threads REQUIRED)
target_link_libraries(bson_shared Threads::Threads)
target_link_libraries(bson_static Threads::Threads)

The c++ driver requires cmake 3.2, so I think it would be fine if we bump our dependency to 3.1.



 Comments   
Comment by Githook User [ 12/Jun/17 ]

Author:

{u'username': u'bjori', u'name': u'Hannes Magnusson', u'email': u'bjori@php.net'}

Message: CDRIVER-2175 pthreads not correctly compiled in
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/6c2c918551608657461969ccc3154bfc979b9ca2

Comment by Githook User [ 12/Jun/17 ]

Author:

{u'username': u'bjori', u'name': u'Hannes Magnusson', u'email': u'bjori@php.net'}

Message: CDRIVER-2175 pthreads not compiled correctly in
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/1f0cb5a1644cd662718418001c9bef16469241af

Comment by Githook User [ 12/Jun/17 ]

Author:

{'username': 'bjori', 'name': 'Hannes Magnusson', 'email': 'bjori@php.net'}

Message: CDRIVER-2175 pthreads not correctly compiled in (when using cmake)
Branch: master
https://github.com/mongodb/libbson/commit/a3523bb6937d75f1f3da8ff1fbcde28539a5df66

Comment by Hannes Magnusson [ 05/Jun/17 ]

It looks the find_package(FOOBAR) usually exposes FOOBAR_LIBRARIES FOOBAR_FLAGS variables we should be catching to build up our secondary list of compiler and linker flags. Then we don't need to do the aggressive platform assumptions when building that CONFIG_LIBS string.

Comment by Hannes Magnusson [ 05/Jun/17 ]

commit c825dfb82b66b9f76feaeabacc476e01ee452e3d
Author: Hannes Magnusson <bjori@php.net>
Date:   Mon Jun 5 12:09:22 2017 -0700
 
    CDRIVER-2175 pthreads not correctly compiled in (when using cmake)
 
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2824e63..d03f959 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -210,11 +210,13 @@ if (RT_LIBRARY)
     target_link_libraries (bson_static ${RT_LIBRARY})
 endif()
 
-if (UNIX)
-    find_package (Threads)
-    target_link_libraries (bson_shared ${CMAKE_THREAD_LIBS_INIT})
-    target_link_libraries (bson_static ${CMAKE_THREAD_LIBS_INIT})
-else()
+set(THREADS_PREFER_PTHREAD_FLAG ON)
+find_package (Threads REQUIRED)
+target_link_libraries(bson_shared Threads::Threads)
+target_link_libraries(bson_static Threads::Threads)
+
+if (NOT UNIX)
+    # gethostbyname
     target_link_libraries (bson_shared ws2_32)
     target_link_libraries (bson_static ws2_32)
 endif()
@@ -309,8 +311,6 @@ if (WIN32)
     set (CONFIG_LIBS ws2_32)
     set (CONFIG_STATIC_LIBS ${CONFIG_LIBS})
 else ()
-    find_package (Threads REQUIRED)
-    set (CONFIG_LIBS ${CMAKE_THREAD_LIBS_INIT})
     if (NOT APPLE)
         set (CONFIG_LIBS ${CONFIG_LIBS} rt)
     endif ()
@@ -318,21 +318,12 @@ else ()
 endif ()
 
 # pkg-config needs libs joined by spaces not semicolons and must
-# start with "-l" if it's not already a flag
 foreach(FLAG ${CONFIG_LIBS})
-    if ( ${FLAG} MATCHES "^-" )
-        set(pkg_config_libs "${pkg_config_libs} ${FLAG}")
-    else ()
-        set(pkg_config_libs "${pkg_config_libs} -l${FLAG}")
-    endif ()
+    set(pkg_config_libs "${pkg_config_libs} -l${FLAG}")
 endforeach()
 
 foreach(FLAG ${CONFIG_STATIC_LIBS})
-    if ( ${FLAG} MATCHES "^-" )
-        set(pkg_config_static_libs "${pkg_config_static_libs} ${FLAG}")
-    else ()
-        set(pkg_config_static_libs "${pkg_config_static_libs} -l${FLAG}")
-    endif ()
+    set(pkg_config_static_libs "${pkg_config_static_libs} -l${FLAG}")
 endforeach()
 
 set(VERSION "${BSON_VERSION}")

this patch now fails the static linking evergreen task as our alternative compiler and linker flag strings no longer hardcode pthreads for the linker stage.

I think that is a separate bug though, we shouldn't need to be maintaining secondary lists of all the compiler and linker flags?

Comment by Hannes Magnusson [ 05/Jun/17 ]

I think the way -lpthread was added to the linker flags also caused the confusion when the .cmake and .pc files were being created.

set (CONFIG_LIBS ${CMAKE_THREAD_LIBS_INIT})

that flag contained -lpthread, while it was the only flag in the manually created CONFIG_LIBS string that contained -l, which resulted in those prefix checks later added

Generated at Wed Feb 07 21:14:24 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.