OlivierLDff / asio.cmake

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

feat: asio as interface library, option to enable/disable deprecated … #28

Open carlocorradini opened 2 months ago

carlocorradini commented 2 months ago

Part of PR #17 Fix #16

asio as interface library, option to enable/disable deprecated asio functions, default asio version as asio-1-31-0 and avoid re-including cpm if already initiliazed

carlocorradini commented 2 months ago

@OlivierLDff asio is now an interface library and therefore the target asio does not exists anymore. Build only examples in CI?

OlivierLDff commented 2 months ago

Again can we split this PR,

Then we can discuss the interface vs static library. In general I think we don't want to change how asio is created as a static library, with source pre-compiled. Then we can add a CMake option to use ASIO as a pure interface. Maybe I'm missing a point, but tell me why using an interface would always be better?

carlocorradini commented 1 month ago

1) PR #29 2) Waiting other PR to be merged 3) PR #30 4) PR #31 5) What is missing other than finishing CMakeLists.txt? We have the GitHub workflow that automatically create a PR to update the license year on 1st January

Asio is an header only library and therefore should be an INTERFACE library similarly to nlohmann/json. Moreover almost every snippet (e.g. from CPM's asio-standalone) or example on how to use Asio (standalone) with CMake defines it as an INTERFACE library.

OlivierLDff commented 1 month ago

I understand the fact you need/want INTERFACE library, but at the moment this project is building asio with ASIO_SEPARATE_COMPILATION.

I'm ok to add support for interface library, but the default way this project behave shouldn't change.

I suggest adding an option ASIO_SEPARATE_COMPILATION that is default to ON and then deal with the logic to support both. Like that user can choose which variants he wants.

Most example show the header only inclusion because it's easier to configure. The main goal of this repo was to take care of the "complex" so it's easy to add a compiled asio version to your project.

So if you go with the option way, please also update CI to test both cases

carlocorradini commented 1 month ago

@OlivierLDff What do you think if we add a boolean option (e.g. ASIO_SEPARATE_COMPILATION):

Le me know what you think. Nevertheless, I think the default behavior should be INTERFACE library since asio is known to be a header only library. If someone want/know that it can be built as a STATIC/SHARED must opt-in.

OlivierLDff commented 1 month ago

Create a first PR to introduce ASIO_SEPARATE_COMPILATION. I don't want to change the default behavior of the project. Default values are part of the "public API" of the repo. Changing a default would be breaking change and we definitely don't want that over the argument that example of asio usage are using the library as a header only library. Another PR can follow with ASIO_DYN_LINK, same static library as default should be maintained and a new ASIO_DYN_LINK can be introduced with default value BUILD_SHARED_LIBS