search

max0x7ba / atomic_queue

C++ lockless queue.
MIT License
1.47k stars 176 forks source link

Make library target visible to downstream clients #64

Closed yaneury closed 6 months ago

yaneury commented 6 months ago

Using the library with FetchContent successfully downloaded the library onto the build/_deps directory but all the #include statements yielded a compiler error because CMake wasn't including atomic_queue directory for the target. You can test this on this repo likeso:

FetchContent_Declare(
        atomic_queue
        GIT_REPOSITORY https://github.com/max0x7ba/atomic_queue.git
        GIT_TAG v1.6.1)
FetchContent_MakeAvailable(atomic_queue)

target_link_libraries(${PROJECT_NAME} PRIVATE atomic_queue)

And link to an executable that includes the library. It should fail to compile.

If you instead link against my fork's version, it will work:

FetchContent_Declare(
        atomic_queue
        GIT_REPOSITORY https://github.com/yaneury/atomic_queue.git
        GIT_TAG stable)
FetchContent_MakeAvailable(atomic_queue)
max0x7ba commented 6 months ago

Thank you for your contribution.

CMake support is a contribution of @RedSkittleFox. I'd like him to to review the change and let us know whether it should be merged as is or if any adjustments are needed.

RedSkittleFox commented 6 months ago

Hey @yaneury The reason why your first code snippet fails is because the release v1.6.1 is outdated in regards to the master branch. It's using the commit 083f3e5 (Merge branch 'RedSkittleFox-allocator-aware-constructors') instead of the later commit which introduced CMake support (CMake Support added).

If you do:

FetchContent_Declare(
        atomic_queue
        GIT_REPOSITORY https://github.com/max0x7ba/atomic_queue.git
        GIT_TAG 10ae35d40449b07607238c3809d360765a8cff91 # Latest commit tag
)
FetchContent_MakeAvailable(atomic_queue)
target_link_libraries(${PROJECT_NAME} PRIVATE max0x7ba::atomic_queue) # use aliased target

then it should work just fine.

max0x7ba commented 6 months ago

The tag has been created now.

Should anything from this change be merged?

RedSkittleFox commented 6 months ago

The tag has been created now.

Should anything from this change be merged?

This PR can be closed without merging.

max0x7ba commented 6 months ago

@yaneury Would you like to close this PR if your issue is resolved, please?

yaneury commented 6 months ago

Just tested the updated tag and it works now. Thanks for your help!