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

Made Project Properly installable via CMake #148

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
141 changes: 137 additions & 4 deletions CMakeLists.txt
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.
Copy link
Owner

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.

Copy link
Author

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

Copy link
Owner

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.

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)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this physically related to the GitHub release version? Or independent?

Copy link
Author

Choose a reason for hiding this comment

The 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
Copy link
Owner

Choose a reason for hiding this comment

The 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?

Copy link
Author

Choose a reason for hiding this comment

The 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)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is Threads?

Copy link
Author

Choose a reason for hiding this comment

The 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.

Copy link
Owner

Choose a reason for hiding this comment

The 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>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are the unit tests compiled in debug mode for MSVC only?

Copy link
Author

@Cazadorro Cazadorro Jan 22, 2024

Choose a reason for hiding this comment

The 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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I was mislead by /DEBUG and the lack of /DNDEBUG. This makes more sense. Can you add /DNDEBUG for MSVC too?

Copy link
Author

Choose a reason for hiding this comment

The 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()
5 changes: 5 additions & 0 deletions cmake/config.cmake.in
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)