-
Notifications
You must be signed in to change notification settings - Fork 676
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
Made Project Properly installable via CMake #148
base: master
Are you sure you want to change the base?
Changes from 3 commits
a3346b3
febad56
03600f4
6f613d5
7a59357
c2d3567
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,144 @@ | ||
cmake_minimum_required(VERSION 3.9) | ||
project(readerwriterqueue VERSION 1.0.0) | ||
set(CMAKE_CXX_STANDARD 11) # don't need to specify manually per target anymore. | ||
set(CMAKE_CXX_STANDARD_REQUIRED ON) | ||
|
||
include(GNUInstallDirs) | ||
|
||
add_library(${PROJECT_NAME} INTERFACE) | ||
project(readerwriterqueue) | ||
|
||
target_include_directories(readerwriterqueue INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}) | ||
add_library(readerwriterqueue INTERFACE) | ||
|
||
install(FILES atomicops.h readerwriterqueue.h readerwritercircularbuffer.h LICENSE.md | ||
# This needs to be updated everytime the library tag version updates. The version is not 1.0.0, it's 1.0.6 according to tags. | ||
set_target_properties(readerwriterqueue PROPERTIES | ||
SOVERSION 1 | ||
VERSION 1.0.6) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this physically related to the GitHub release version? Or independent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This relates to actual software compatibility, which should normally be 1:1 to the Git tag version. There's no official automated tools to grab this, but I typically manually edit it in since it's so simple to do. I assumed you were using semver, if that's not the case that's a whole new can of worms to deal with since I believe the current VCPKG port of this assumes semver. SO version is ABI incompatibility, and typically should be the same as the major version on VERSION, underneath though I believe only windows uses that. |
||
|
||
#see this talk for why these changes need to be made https://www.youtube.com/watch?v=m0DwB4OvDXk | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a comment on the PR instead of in the source code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, that explains the idea behind the commands used there, I wasn't talking about the pull request rational, and now I realize that's poorly worded, CMake documentation does not explain all the rational behind what installable functions to use where that I made here, and that's unfortunately the best place to find the kind of information I used here. i |
||
|
||
target_include_directories(readerwriterqueue INTERFACE | ||
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}> | ||
$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}> | ||
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME}> | ||
) | ||
|
||
set_target_properties(readerwriterqueue PROPERTIES EXPORT_NAME readerwriterqueue) | ||
|
||
# This is here to ensure parity with the install interface, but won't hide the previous target name. | ||
add_library(moodycamel::readerwriterqueue ALIAS readerwriterqueue) | ||
|
||
|
||
|
||
|
||
install(TARGETS readerwriterqueue | ||
EXPORT readerwriterqueue_Targets | ||
COMPONENT readerwriterqueue_Development | ||
INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} | ||
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} | ||
COMPONENT creaderwriterqueue_RunTime | ||
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
COMPONENT readerwriterqueue_RunTIme | ||
NAMELINK_COMPONENT readerwriterqueue_Development | ||
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} | ||
COMPONENT readerwriterqueue_Development | ||
) | ||
|
||
set(readerwriterqueue_INSTALL_CMAKEDIR | ||
${CMAKE_INSTALL_DATADIR}/readerwriterqueue | ||
CACHE STRING "Path to readerwriterqueue cmake files" | ||
) | ||
|
||
install(EXPORT readerwriterqueue_Targets | ||
DESTINATION ${readerwriterqueue_INSTALL_CMAKEDIR} | ||
NAMESPACE moodycamel:: | ||
FILE readerwriterqueueTargets.cmake | ||
COMPONENT readerwriterqueue_Development | ||
) | ||
|
||
include(CMakePackageConfigHelpers) | ||
configure_package_config_file(${CMAKE_CURRENT_SOURCE_DIR}/cmake/config.cmake.in | ||
"${CMAKE_CURRENT_BINARY_DIR}/readerwriterqueueConfig.cmake" | ||
INSTALL_DESTINATION ${readerwriterqueue_INSTALL_CMAKEDIR} | ||
) | ||
|
||
|
||
get_target_property(readerwriterqueue_VERSION readerwriterqueue VERSION) | ||
|
||
write_basic_package_version_file(readerwriterqueueConfigVersion.cmake VERSION ${readerwriterqueue_VERSION} COMPATIBILITY SameMajorVersion) | ||
|
||
install(FILES | ||
${CMAKE_CURRENT_BINARY_DIR}/readerwriterqueueConfig.cmake | ||
${CMAKE_CURRENT_BINARY_DIR}/readerwriterqueueConfigVersion.cmake | ||
DESTINATION ${readerwriterqueue_INSTALL_CMAKEDIR} | ||
) | ||
|
||
|
||
install(FILES | ||
atomicops.h | ||
readerwriterqueue.h | ||
readerwritercircularbuffer.h | ||
LICENSE.md | ||
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME}) | ||
|
||
|
||
option(MOODYCAMEL_READERWRITERQUEUE_ENABLE_TESTS "Enables readerwriterqueue tests" OFF) | ||
|
||
if(${MOODYCAMEL_READERWRITERQUEUE_ENABLE_TESTS}) | ||
|
||
find_package(Threads REQUIRED) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Threads is an imported target (ie a target not native to CMake which other CMake code creates an interface for) required to use std::thread in some contexts, it isn't something you have to install (unless you don't have pthreads or what ever the os specific threading library you used is), but is a CMake target CMake automatically creates for each system it supports. see https://stackoverflow.com/questions/67912839/how-do-you-specify-a-threads-dependency-in-cmake-for-distributing-a-header-only. You want to do this even on systems that don't require it, because CMake will do the right thing and do whatever the platform deems necessary for std::threads usage here. On some systems it does nothing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I find it interesting that a part of the standard library requires a special package. |
||
# add_library(simple) | ||
add_executable(unittests) | ||
target_sources(unittests PRIVATE | ||
tests/unittests/minitest.h | ||
tests/unittests/unittests.cpp | ||
tests/common/simplethread.h | ||
tests/common/simplethread.cpp | ||
) | ||
target_include_directories(unittests PRIVATE | ||
tests/unittests/) | ||
# static. | ||
set_property(TARGET unittests PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>") | ||
# handles pthread and such. | ||
target_link_libraries(unittests PRIVATE Threads::Threads) | ||
# target_link_libraries(unittests PRIVATE readerwriterqueue) | ||
target_compile_options(unittests | ||
PRIVATE | ||
$<$<CXX_COMPILER_ID:Clang>: -Wsign-conversion -Wpedantic -Wall -DNDEBUG -O3 -g> | ||
$<$<CXX_COMPILER_ID:GNU>: -Wsign-conversion -Wpedantic -Wall -DNDEBUG -O3 -g> | ||
# /W4 is similar to -Wall, next are similar to -sign-conversion, /permissive- is similar to -Wpedantic. | ||
$<$<CXX_COMPILER_ID:MSVC>: /W4 /w14287 /w14826 /permissive- /02 /DEBUG> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are the unit tests compiled in debug mode for MSVC only? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant to type /O2 which is equilvalent to O3, , which with /DEBUG would have been equivalent to -O3 -g, if I understand correctly. /Od is debug mode, not /DEBUG if I remember. /DEBUG generates debug info. I only do one configuration because I was copying what was done in your Make files (which only handled a single release with debuginfo configuration), I can make generator expressions for debug/release/relwithdebinfo instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think I was mislead by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realized that I missed it and put it in the latest changes. normally it's not necessary to add that definition as it is automatically added in appropriate CMake configs, however I added anyway, though this time under the correct usage target_compile_definitions |
||
) | ||
target_link_options(unittests | ||
PRIVATE | ||
$<$<CXX_COMPILER_ID:Clang>: -lrt -Wl,--no-as-needed> | ||
$<$<CXX_COMPILER_ID:GNU>: -lrt -Wl,--no-as-needed> | ||
) | ||
|
||
# add_library(simple) | ||
add_executable(stabtest) | ||
target_sources(stabtest PRIVATE | ||
tests/stabtest/stabtest.cpp | ||
tests/common/simplethread.h | ||
tests/common/simplethread.cpp | ||
) | ||
target_include_directories(stabtest PRIVATE | ||
tests/stabtest/) | ||
# static. | ||
set_property(TARGET stabtest PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>") | ||
# handles pthread and such. | ||
target_link_libraries(stabtest PRIVATE Threads::Threads) | ||
# target_link_libraries(unittests PRIVATE readerwriterqueue) | ||
target_compile_options(stabtest | ||
PRIVATE | ||
$<$<CXX_COMPILER_ID:Clang>: -Wsign-conversion -Wpedantic -Wall -DNDEBUG -O3 -g> | ||
$<$<CXX_COMPILER_ID:GNU>: -Wsign-conversion -Wpedantic -Wall -DNDEBUG -O3 -g> | ||
# /W4 is similar to -Wall, next are similar to -sign-conversion, /permissive- is similar to -Wpedantic. | ||
$<$<CXX_COMPILER_ID:MSVC>: /W4 /w14287 /w14826 /permissive- /02 /DEBUG> | ||
) | ||
target_link_options(stabtest | ||
PRIVATE | ||
$<$<CXX_COMPILER_ID:Clang>: -lrt -Wl,--no-as-needed> | ||
$<$<CXX_COMPILER_ID:GNU>: -lrt -Wl,--no-as-needed> | ||
) | ||
|
||
|
||
endif() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
@PACKAGE_INIT@ | ||
|
||
include("${CMAKE_CURRENT_LIST_DIR}/readerwriterqueueTargets.cmake") | ||
# normally find_dependency macro dependencies, but there are none here. | ||
check_required_components(readerwriterqueue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ReaderWriterQueue
supports C++03 on certain platforms.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you specify in exactly what scenarios this is true? If you're talking about compilers supporting individual C++11 features with out supporting C++11, Cmake supports specifying individual flags for C++ features, If I know those, I can change the target to instead depend on those c++ features. see https://cmake.org/cmake/help/latest/manual/cmake-compile-features.7.html and https://cmake.org/cmake/help/latest/command/target_compile_features.html#command:target_compile_features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly VC++2010. I guess people using that are unlikely to be using CMake anyway.