OlivierLDff / asio.cmake

Small CMake wrapper to add asio with a simple FetchContent.
MIT License
18 stars 9 forks source link

Support find_package(asio MODULE) with FindAsio.cmake #13

Open Ryanf55 opened 1 year ago

Ryanf55 commented 1 year ago

Hello,

Thanks for adding the CMake support here since it seems the core asio team does not want any CMake code in their repo.

Have you considered adding support for consuming asio in a CMake based project with a call to find_package(asio MODULE REQUIRED)? To do that, one would need to add a FindAsio.cmake script to this repo, perhaps in the existing cmake folder.

Although CMake module scripts aren't as good as config scripts, it still meets the use case that one as asio installed in their system headers. For example, on Ubuntu 22.04, I can install libasio-dev, use fetchContent to get this repo, add the cmake folder to CMAKE_MODULE_PATH. This provides the advantage of my source tree only getting to use one version of asio - the one installed in the system headers. Tools like FetchContent and CPM enable you to fetch different versions of the same library in multiple different libraries, which can be problematic.

Furthermore, the CMake module script could support version checking, through a simple re-use of the version parsing you have already implemented. For example, this can guarantee that the located version of asio is exactly 1.18.1. find_package(ASIO 1.18.1 EXACT MODULE REQUIRED)

A contrived example: https://github.com/ros-drivers/transport_drivers/blob/main/asio_cmake_module/cmake/Modules/FindASIO.cmake

Would you be willing to accept a PR that adds this capability?

Best, Ryan

OlivierLDff commented 1 year ago

I see the point of having a separate FindAsio.cmake. I would gladly accept a PR for that. You can add a test project here: https://github.com/OlivierLDff/asio.cmake/tree/main/tests and add a job in CI to test the behavior. I see the goal of doing something like:

find_package(ASIO 1.18.1 EXACT MODULE)
if(NOT ASIO_Found)
set(ASIO_REPOSITORY
    "https://github.com/chriskohlhoff/asio"
    CACHE STRING "asio git repository url"
)
set(ASIO_TAG
    "asio-1-18-0"
    CACHE STRING "asio git tag"
)
CPMAddPackage(
  NAME asiocmake
  GIT_REPOSITORY "https://github.com/OlivierLDff/asio.cmake"
  GIT_TAG "main"
)
endif()

In fact I believe it could even be entirely handled by CPM.cmake with CPM_USE_LOCAL_PACKAGES

Ryanf55 commented 1 year ago

I see the point of having a separate FindAsio.cmake. I would gladly accept a PR for that. You can add a test project here: https://github.com/OlivierLDff/asio.cmake/tree/main/tests and add a job in CI to test the behavior. I see the goal of doing something like:

find_package(ASIO 1.18.1 EXACT MODULE)
if(NOT ASIO_Found)
set(ASIO_REPOSITORY
    "https://github.com/chriskohlhoff/asio"
    CACHE STRING "asio git repository url"
)
set(ASIO_TAG
    "asio-1-18-0"
    CACHE STRING "asio git tag"
)
CPMAddPackage(
  NAME asiocmake
  GIT_REPOSITORY "https://github.com/OlivierLDff/asio.cmake"
  GIT_TAG "main"
)
endif()

In fact I believe it could even be entirely handled by CPM.cmake with CPM_USE_LOCAL_PACKAGES

Sounds good. Can you assign this issue to me?

OlivierLDff commented 1 year ago

Sure done ;)

Ryanf55 commented 1 year ago

In the find script, the cmake developer guide recommends those dependencies should also be targets, and CMake should be told that they are dependencies of this target.

For asio's dependencies, such as Threads, should those be included as a dependency in this find script? The CPM example includes it.

OlivierLDff commented 1 year ago

I would say yes, for the end user he should just link to a target Asio and everything is expected to just work.