Made Project Properly installable via CMake

[Cazadorro]

Motivation:

Despite this library being header only, it is actually practically not drop in to any arbitrary project that is not an executable. Today, both CMake and package managers have gotten to the point where its easier to use them than it is to maintain manually dropped header files, fetch content, or git submodules. For a typical CMake library it is as easy to use header only as not header only, for example:

    find_package(my_depedency)
    target_link_libraries(my_target PRIVATE namespace::my_depedency)

However the readerwriterqueue library requires me to find the directory and then manually use the include dirs for this project.

    find_path(READERWRITERQUEUE_INCLUDE_DIRS "readerwriterqueue/atomicops.h")
    target_include_directories(my_target PRIVATE ${READERWRITERQUEUE_INCLUDE_DIRS})

This not only breaks IDE who use CMake source files to aid in quickly resolving names, but also requires me to make my own target if I wish to properly transitively include directories for all targets using this library instead of manually repeating the above process:

    find_path(READERWRITERQUEUE_INCLUDE_DIRS "readerwriterqueue/atomicops.h")
    add_library(readerwriterqueue_proxy INTERFACE)
    target_include_directories(readerwriterqueue_proxy INTERFACE ${READERWRITERQUEUE_INCLUDE_DIRS})
   
    ...
    # then use it properly
    target_link_libraries(my_target PRIVATE readerwriterqueue_proxy )
    # or 
    target_link_libraries(my_target PUBLIC readerwriterqueue_proxy )
    # or 
    target_link_libraries(my_target INTERFACE readerwriterqueue_proxy )

This becomes even more important if cmake options/compile definitions ever get added to this library, I risk them leaking publicly or not being transitive if I don't create a proper target wrapper. And not only do I have to do this, I have to either repeat this for every project I use this library in, or I have to manage my own repo with this library, or create my own custom VCPKG registry. So it ends up being quite a lot of work just to deal with something that is supposed to be just drop in and use.

Additionally the tests don't build out of the box. Now these new cmake changes fix that, no need for msbuild, visual studio project files or make files to build the tests (though I didn't actually remove them or change the test directories in any way). I've also manually created a VCPKG port and verified this project works as an installable VCPKG project there (though that isn't public).

After this commit, after installing the package, using the library should look like this:

    find_package(readerwriterqueue)
    target_link_libraries(my_target PRIVATE moodycamel::readerwriterqueue)

Note add_subdirectory targets haven't been changed, only expanded so this works:

    add_subdirectory(external/readerwriterqueue)
    target_link_libraries(my_target PRIVATE moodycamel::readerwriterqueue)

as well as this

    add_subdirectory(external/readerwriterqueue)
    target_link_libraries(my_target PRIVATE readerwriterqueue)

Commit message changes:

Modified CMakeList.txt to be installable, now accessible through moodycamel::readerwriterqueue in both subdirectory and installed cmake project (didn't change targets that were already avaible, though projectect previously didn't work as an installable cmake project) also added test targets disabled by default configurable with variable psuedo namespaced 'MOODYCAMEL_READERWRITERQUEUE_ENABLE_TESTS', test targets are unittests and stabtest, completely ignores msvc project files and make files, so no need for those going forward, and though better practice would be to change the includes to be more target friendly (not use bespoke relative includes), I decided not to change that, so the previous way to build and run the test will also still work. Added best pairing of MSVC equivalent compile commands I could find, and I verified all tests build and run correctly on Windows 11 with MSVC 14.38.33130

Additional Discussion Topics:

Note I actually don't know the minimum required version of cmake for this, I need feedback on what this should be. Unlike other types of dependencies, upgrading cmake versions is trivial and expected to be done in the event of not having a late enough version no matter the system. Only the cmake executable itself is needed, and doesn't even need to be built for 99% of systems, and is generally compatible with any system that a previous version of cmake is on. I'm using 3.26, this library states 3.9, but I'm not using the latest features, I might even be using 3.9 features, but there's no way for me to easily tell. Though this whole question could be moot anyway, as it looks like using minimum versions in CMake is somewhat of a farce.

[cameron314]

CMake support was provided by the community. I don't use CMake for developing locally, and I'm not particularly familiar with it. Will this change break existing code that uses the current CMake support?

[cameron314]

Seems quite heavy for ultimately exposing a couple headers (granted, it also builds the tests). I don't feel like I can maintain this, but if it's backwards compatible I'm OK with merging it. The motivation in the PR description does make sense.

[cameron314]

ReaderWriterQueue supports C++03 on certain platforms.

[Cazadorro]

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

[cameron314]

Mostly VC++2010. I guess people using that are unlikely to be using CMake anyway.

[cameron314]

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

[Cazadorro]

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.

[cameron314]

Should this be a comment on the PR instead of in the source code?

[Cazadorro]

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

[cameron314]

What is Threads?

[Cazadorro]

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.

[cameron314]

Thanks. I find it interesting that a part of the standard library requires a special package.

[cameron314]

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

[Cazadorro]

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.

[cameron314]

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

[Cazadorro]

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

[Cazadorro]

CMake support was provided by the community. I don't use CMake for developing locally, and I'm not particularly familiar with it. Will this change break existing code that uses the current CMake support?

It shouldn't, the same targets are exposed regardless.

I didn't change the directory where you include things, the main difference is cmake install works properly, which means the use case outlined in the first post works. 99% of what I did only touches the install interface. There's basically the "build interface", what you use to build the library itself, and what you use when you use add_subdirectory on cmake, and "install interface", which is what you use after you run cmake install on a library, which is what happens when you need to use a library inside a package manager or just straight up install it on your system by yourself. The install functions you see peppered through out actually do not effect the build interface at all and are only run when cmake install ... is run, similarly with the generator expressions which specify BUILD_INTERFACE and INSTALL_INTERFACE.