OlivierLDff / asio.cmake

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

feat: interface, static or shared library type #35

Open carlocorradini opened 1 month ago

carlocorradini commented 1 month ago

Part of PR #17

It's now possible to build different library types. SHARED, STATIC (default), or INTERFACE.

CMake options:

Flow pseudocode:

if (NOT ASIO_SEPARATE_COMPILATION)
  // INTERFACE
elseif (NOT ASIO_DYN_LINK)
  // STATIC
else()
  // SHARED
endif()
carlocorradini commented 1 month ago

@OlivierLDff I've removed INCLUDE_HEADER_PATTERN "*.hpp" from packageProject because INTERFACE library requires *.ipp files. Currently packageProject does not support multiple INCLUDE_HEADER_PATTERN and therefore there is no way to specify both .cpp and .ipp files. Waiting issue https://github.com/TheLartians/PackageProject.cmake/issues/44 to be fixed

carlocorradini commented 1 month ago

@OlivierLDff CI fails with CMake 3.14 on ubuntu-latest. However, specifying: cmake -DCMAKE_BUILD_TYPE="" -DCMAKE_INSTALL_PREFIX=build/output -DASIO_USE_CPM="" -DASIO_SEPARATE_COMPILATION="" -DASIO_DYN_LINK="" -DASIO_NO_DEPRECATED="" -DASIO_ENABLE_EXAMPLES=ON -DASIO_ENABLE_INSTALL=ON -B build -S . on my systems (Windows and Ubuntu) works flawlessly. It looks like ASIO_LIBRARY_TYPE is set to INTERFACE but this is strange since both in my tests and logically should be STATIC. Any thoughts?

OlivierLDff commented 1 month ago

I guess if(NOT ASIO_SEPARATE_COMPILATION) returns true when ASIO_SEPARATE_COMPILATION is set to "". You could add logs? or specify values for ASIO_SEPARATE_COMPILATION in the include of the action?

This look like annoying CMake rules to debug

carlocorradini commented 1 month ago

@OlivierLDff Do we really need a job that sets all options to ""? Does adding more code for checking "" is worth it?

NOT ASIO_SEPARATE_COMPILATION is TRUE because ASIO_SEPARATE_COMPILATION is "" and is evaluated as a FALSE value

OlivierLDff commented 1 month ago

@OlivierLDff Do we really need a job that sets all options to ""? Does adding more code for checking "" is worth it?

NOT ASIO_SEPARATE_COMPILATION is TRUE because ASIO_SEPARATE_COMPILATION is "" and is evaluated as a FALSE value

No no additional code should be added, to me the problem is in the workflow file, that is buggy. Value should either be ON or OFF for all options, also don't wrap in "" the matrix options. So add a default value in the include of the action

carlocorradini commented 1 month ago

@OlivierLDff The problem is https://github.com/OlivierLDff/asio.cmake/blob/194137dd7300616ff1a3d65f43e57255bf751126/.github/workflows/main.yml#L27 This does not set any option (all are ""), what values (as default) to you prefer? STATIC library?

OlivierLDff commented 1 month ago

Yes include all parameters to the default values, this is the easiest I guess

carlocorradini commented 1 month ago

@OlivierLDff Fixed! 🥳