readerwriterqueue icon indicating copy to clipboard operation
readerwriterqueue copied to clipboard

Made Project Properly installable via CMake

Open Cazadorro opened this issue 1 year ago • 2 comments

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.

Cazadorro avatar Jan 14 '24 17:01 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?

cameron314 avatar Jan 21 '24 17:01 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?

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.

Cazadorro avatar Jan 22 '24 04:01 Cazadorro