Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Boost::thread fails to pull -pthread in cmake parent project config #691

Closed
emaxerrno opened this issue Oct 8, 2019 · 3 comments
Closed

Comments

@emaxerrno
Copy link
Contributor

given a cmake project definition of

ExternalProject_Add(seastar
  URL https://github.com/vectorizedio/seastar/archive/cd38452d.tar.gz
  URL_MD5 282f28305918ec65017c21719de5b887
  INSTALL_DIR    @V_DEPS_INSTALL_DIR@
  CMAKE_COMMAND ${CMAKE_COMMAND} -E env ${cmake_build_env} ${CMAKE_COMMAND}
  CMAKE_ARGS
    ${common_cmake_args}
    -DBoost_USE_STATIC_LIBS=OFF
    -DBoost_NO_BOOST_CMAKE=ON
    -DBoost_NO_SYSTEM_PATHS=TRUE
    -DProtobuf_USE_STATIC_LIBS=ON
    -DSeastar_INSTALL=ON
    -DSeastar_DPDK=@RP_ENABLE_DPDK@
    -DSeastar_APPS=ON
    -DSeastar_DEMOS=OFF
    -DSeastar_DOCS=OFF
    -DSeastar_TESTING=OFF
    -DSeastar_CXX_FLAGS=-Wno-error
    -DSeastar_LD_FLAGS=-pthread      # ................ bug/workaround
    -DSeastar_STD_OPTIONAL_VARIANT_STRINGVIEW=ON
    -DSeastar_CXX_DIALECT=c++17
  DEPENDS
    ${default_depends}
    fmt
    boost
    protobuf
    dpdk
    yaml-cpp
    c-ares
    gnutls
    lz4
    hwloc
    numactl
    xml2
    xz
    cryptopp
    lksctp-tools
    zlib)

I noticed that the main definition of seastar in cmakelist.txt under some configurations fails to import pthread for boost:

target_link_libraries (seastar
  PUBLIC
    Boost::boost
    Boost::program_options
    Boost::thread
    # .................. I can add `pthread` here and everything works
    c-ares::c-ares
    cryptopp::cryptopp
    fmt::fmt
    lz4::lz4
  PRIVATE
    ${CMAKE_DL_LIBS}
    Boost::filesystem
    GnuTLS::gnutls
    StdAtomic::atomic
    StdFilesystem::filesystem
    lksctp-tools::lksctp-tools
    protobuf::libprotobuf
    rt::rt
    yaml-cpp::yaml-cpp)


I am not sure if the issue is coming from our own boost cmake module - from the comments

# The following :prop_tgt:`IMPORTED` targets are also defined::
#
#   Boost::boost                  - Target for header-only dependencies
#                                   (Boost include directory)
#   Boost::<C>                    - Target for specific component dependency
#                                   (shared or static library); <C> is lower-
#                                   case
#   Boost::diagnostic_definitions - interface target to enable diagnostic
#                                   information about Boost's automatic linking
#                                   during compilation (adds BOOST_LIB_DIAGNOSTIC)
#   Boost::disable_autolinking    - interface target to disable automatic
#                                   linking with MSVC (adds BOOST_ALL_NO_LIB)
#   Boost::dynamic_linking        - interface target to enable dynamic linking
#                                   linking with MSVC (adds BOOST_ALL_DYN_LINK)
#
# Implicit dependencies such as Boost::filesystem requiring
# Boost::system will be automatically detected and satisfied, even
# if system is not specified when using find_package and if
# Boost::system is not added to target_link_libraries.  If using
# Boost::thread, then Threads::Threads will also be added automatically.
#

But it actually is not pulling in Threads::Threads which in normal speak means -lpthread

I can fix the problem for all platforms by adding the pthread as I mentioned in the context above, but not sure if there is a more elegant solution.

Maybe someone w/ a bit more cmake-foo experience can help.

The problem is manifested like so:

on a link.txt produced by cmake on a -GMake

/bin/c++     CMakeFiles/main.dir/main.cc.o  -o main seastar/libseastar.a /usr/lib64/libboost_program_options.so /usr/lib64/libboost_thread.so /usr/lib64/libboost_chrono.so /usr/lib64/libboost_date_time.so /usr/lib64/libboost_atomic.so -lpthread /usr/lib64/libcares.so /usr/lib64/libcryptopp.so /usr/lib64/libfmt.so.5.2.1 -Wl,--as-needed /usr/lib64
/liblz4.so -ldl /usr/lib64/libboost_filesystem.so /usr/lib64/libgnutls.so -latomic /usr/lib64/libsctp.so /usr/lib64/libprotobuf.so -lrt /usr/lib64/libyaml-cpp.so /usr/lib64/libhwloc.so /usr/lib64/libnuma.so 


Notice the -lpthread after boost_atomic

The issue is that on an application that is just pulling cmake as an ExternalProject_add the -lpthread isn't added to the link.txt files (using the -GMake generator for cmake).

I'm not sure if there is a preference.

In summary

There is a problem with externalproject_add importing seastar.

There are 2 fixes.

  1. We push the user to always add -DSeastar_LD_FLAGS=-pthread
  2. We add -pthread after boost::thread in the main cmakelist.txt file
@emaxerrno
Copy link
Contributor Author

Ok, further digging shows I was right, we'll just need to fix our boost for newer versions

CMake Warning at build/debug/v_deps_install/lib64/cmake/Seastar/FindBoost.cmake:839 (message):
  New Boost version may have incorrect or missing dependencies and imported
  targets

We use boost 1.70.0 from source

We can either close this ticket, or I can submit a patch with the -pthread fix i mentioned above

/cc @denesb

@espindola
Copy link
Contributor

espindola commented Oct 9, 2019 via email

@emaxerrno
Copy link
Contributor Author

Works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants