Conversation
First commit with CMakeLists.txt and tests CMakeLists.txt enabled
added tests to tests/CMakeLists.txt libtbb, benchmark, unittests and fuzztests
Adapt makefiles, cpp to match new include/concurrentqueue include path
typo in CMake file
…a/concurrentqueue into feature/add-cmake-support
|
This is the cleanest cmake support I've seen yet :-) |
| unittests/unittests.cpp | ||
| unittests/mallocmacro.cpp | ||
| common/simplethread.cpp | ||
| common/systemtime.cpp |
There was a problem hiding this comment.
As I understand, it doesn't build c_api files as a separated library, so the current approach will have the error like:
/usr/bin/ld: unittests.cpp:(.text._ZN10moodycamel20ConcurrentQueueTests13c_api_enqueueEv[_ZN10moodycamel20ConcurrentQueueTests13c_api_enqueueEv]+0x56): undefined reference to `moodycamel_cq_destroy'Because of that, it's important to add c_api files to UNITTESTS_SRC, isn't it? (or add a target to build c_api library)
set(UNITTESTS_SRC
unittests/unittests.cpp
unittests/mallocmacro.cpp
common/simplethread.cpp
common/systemtime.cpp
../c_api/concurrentqueue.cpp
../c_api/blockingconcurrentqueue.cpp
)There was a problem hiding this comment.
I don't think the c_api contribution existed back then.
There was a problem hiding this comment.
Oh, okay, in this case it is a necessary to add c_api support to CMake build system.
| ) | ||
| add_executable(unittests ${UNITTESTS_SRC}) | ||
| target_link_libraries(unittests PRIVATE concurrentqueue) | ||
| add_test(NAME unittests COMMAND unittests) |
There was a problem hiding this comment.
unittests command has --disable-prompt flag, I think it will be good to add such flag here to remove Press enter to continue at the end of tests.
There have been a few issues/pr's here related to adding CMake/Package support and it's definitely a great way to make this package more acceptable.
I understand you're not willing to add Conan in this repo (#74), but looks like Cmake is doable given:
fits into ais relatively concise Feature/add cmake support #161builddirectoryI looked at both #93 and #161 and #161 seems like cleaner implementation. Both PR's are stale and have been for a long time, so I figured I'd refresh them here.