Skip to content

Commit

Permalink
cmake: make package config relocatable
Browse files Browse the repository at this point in the history
  • Loading branch information
@sergiud
sergiud committed Oct 19, 2021
1 parent 2f30903 commit eb9100b
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 15 deletions.
3 changes: 3 additions & 0 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ string(SUBSTRING ${VERSION} 0 1 GENERIC_LIB_SOVERSION)
include(CheckCXXCompilerFlag)
include(AddCXXCompilerFlag)
include(CXXFeatureCheck)
include(CheckLibraryExists)

check_library_exists(rt shm_open "" HAVE_LIB_RT)

This comment has been minimized.

Sign in to view

Copy link
@mtrofin

mtrofin Apr 2, 2022

Contributor

@sergiud , can this succeed but not be sufficient for target_link_libraries to know how to pass the right flag to the linker, i.e. should find_library still be used? See https://reviews.llvm.org/D121343#inline-1176497

This comment has been minimized.

Sign in to view

Copy link
@sergiud

sergiud Apr 2, 2022

Author Contributor

No, find_library should not be used because it will unconditionally embed the path into CMake package config and make the library unusable when cross-compiling. This is the whole point of the PR.

Besides, the error does not make much sense: initially, the linker does correctly find librt.so but later fails to do so. If I had to guess, you are possibly modifying linker arguments afterwards which eventually causes the error.

This comment has been minimized.

Sign in to view

Copy link
@sergiud

sergiud Apr 2, 2022

Author Contributor

Also note that librt.so is generally located in sysroot. Hence, specifying an absolute path to the library is pointless. Consequently, your setup/toolchain might be broken.

This comment has been minimized.

Sign in to view

Copy link
@mtrofin

mtrofin Apr 2, 2022

Contributor

It's possible; just checked, and on my linux setup this works fine (i.e. HAVE_LIB_RT=1, and MicroBenchmarks builds fine)

Thanks for the background on this, will dig more on our side then!

This comment has been minimized.

Sign in to view

Copy link
@mtrofin

mtrofin Apr 2, 2022

Contributor

BTW - did you happen to try it on OSX (it'd help narrow down possible causes)

This comment has been minimized.

Sign in to view

Copy link
@sergiud

sergiud Apr 2, 2022

Author Contributor

Unfortunately, not on macOS. However, I regularly cross-compile benchmark for ARM targets using Android NDK, clang+musl and GCC.

This comment has been minimized.

Sign in to view

Copy link
@sergiud

sergiud Apr 2, 2022

Author Contributor

I see that benchmark already uses a Github workflow with a macOS runner.

if (BENCHMARK_BUILD_32_BITS)
add_required_cxx_compiler_flag(-m32)
Expand Down
4 changes: 4 additions & 0 deletions cmake/Config.cmake.in
View file
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
include (CMakeFindDependencyMacro)

find_dependency (Threads)

include("${CMAKE_CURRENT_LIST_DIR}/@targets_export_name@.cmake")
25 changes: 10 additions & 15 deletions src/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,33 +29,28 @@ target_include_directories(benchmark PUBLIC

# libpfm, if available
if (HAVE_LIBPFM)
target_link_libraries(benchmark libpfm.a)
target_link_libraries(benchmark PRIVATE pfm)
add_definitions(-DHAVE_LIBPFM)
endif()

# Link threads.
target_link_libraries(benchmark ${BENCHMARK_CXX_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
find_library(LIBRT rt)
if(LIBRT)
target_link_libraries(benchmark ${LIBRT})
endif()
target_link_libraries(benchmark PRIVATE Threads::Threads)

target_link_libraries(benchmark PRIVATE ${BENCHMARK_CXX_LIBRARIES})

if(HAVE_LIB_RT)
target_link_libraries(benchmark PRIVATE rt)
endif(HAVE_LIB_RT)

if(CMAKE_BUILD_TYPE)
string(TOUPPER ${CMAKE_BUILD_TYPE} CMAKE_BUILD_TYPE_UPPER)
endif()
if(NOT CMAKE_THREAD_LIBS_INIT AND "${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${CMAKE_BUILD_TYPE_UPPER}}" MATCHES ".*-fsanitize=[^ ]*address.*")
message(WARNING "CMake's FindThreads.cmake did not fail, but CMAKE_THREAD_LIBS_INIT ended up being empty. This was fixed in https://github.com/Kitware/CMake/commit/d53317130e84898c5328c237186dbd995aaf1c12 Let's guess that -pthread is sufficient.")
target_link_libraries(benchmark -pthread)
endif()

# We need extra libraries on Windows
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
target_link_libraries(benchmark shlwapi)
target_link_libraries(benchmark PRIVATE shlwapi)
endif()

# We need extra libraries on Solaris
if(${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
target_link_libraries(benchmark kstat)
target_link_libraries(benchmark PRIVATE kstat)
endif()

# Benchmark main library
Expand Down

0 comments on commit eb9100b

Please sign in to comment.