Respect FINEFTP_SERVER_USE_BUILTIN_ASIO

[ShaharHD]

• Fix asio cmake to take FINEFTP_SERVER_USE_BUILTIN_ASIO into account
• Removed trailing spaces

[FlorianReimold]

Hi @ShaharHD,

May I ask which issue made you implement this PR? Because the Cmake Option is handled here:

fineftp-server/CMakeLists.txt

Line 39 in bbd4e0e

if (FINEFTP_SERVER_USE_BUILTIN_ASIO)

Even if you set FINEFTP_SERVER_USE_BUILTIN_ASIO to OFF, the library still has to find asio from somewhere, therefore the find package call should still be there. The option is only supposed to enable the user to provide their own version of asio, as the built in version will not be found anymore.

[ShaharHD]

I'm integrating your code into a Yocto (custom linux build) using the attached bitbake.
(this is based on the previous version of 1.5.0 and the 0001-Fix-asio-cmake-required.patch is similar to what I have pushed in this PR)

asio is built into Yocto generated SDK a bit different (header only) so it fails on find_package(asio CONFIG REQUIRED) but if removed builds just fine (see DEPENDS = "asio" ) and then in my code I can do find_package(fineftp REQUIRED) and target_link_libraries into my project

(also worth noting I'm using asio as well for some UDP based communication in the same project)
for reference, asio in yocto is built using asio bitbake (you can see inside that it is mentioned as a ALLOW_EMPTY:${PN} = "1" meaning header-only

Moving forward, I could rename the CMake flag to be FINEFTP_SERVER_USE_ASIO_HEADER_ONLY - will this be a better solution?

SUMMARY = "Minimal FTP server library for Windows and Unix flavors"
HOMEPAGE = "https://github.com/eclipse-ecal/fineftp-server"
SECTION = "libs"
LICENSE = "MIT"
LIC_FILES_CHKSUM = "file://LICENSE;md5=255477cf76395ebf131ac789c47d0dc5"

DEPENDS = "asio"

PV .= "+git${SRCPV}"

SRC_URI = "\
    git://github.com/eclipse-ecal/fineftp-server.git;protocol=https;branch=master \
    file://0001-Fix-asio-cmake-required.patch"

SRCREV = "1f08ffaa413dd6b75d0bbb46bfbc140c7f4758ff"

S = "${WORKDIR}/git"

inherit cmake

EXTRA_OECMAKE += "\
    -DFINEFTP_SERVER_BUILD_SAMPLES=OFF \
    -DFINEFTP_SERVER_USE_BUILTIN_ASIO=OFF \
    -DFINEFTP_SERVER_USE_BUILTIN_GTEST=OFF \
    -DBUILD_SHARED_LIBS=ON"

BBCLASSEXTEND = "native nativesdk"

If you wish, you can actually submit your library as an offiical layer into Yocto at https://layers.openembedded.org/layerindex/branch/master/recipes/

[FlorianReimold]

OK, I understand you issue. For some reason, your system has asio in a standard include path, but it lacks a CMake find or config script. Thus, on that particular system you don't need a find_package call, because asio is in your standard include paths and you cannot link against asio::asio, as no find or config script created proper CMake targets.

However, simply removing the find_package(asio REQUIRED) call for non-builtin-asio is not a good solution. The FINEFTP_SERVER_USE_BUILTIN_ASIO is supposed to:

• force using asio from the vendored submodule (if ON)
• Let the user provide their own find or config script (if OFF)

If I would merge this PR in the current state, using asio from somewhere else would fail on every other system that uses a proper asio find module or config script.

I would however be OK with also providing a find_asio.cmake script that checks for other locations than the submodule. I have modified the find script to look for asio.hpp in a couple of default paths and also allow overrides from the asio_ROOT_DIR variable.

Can you ceck if having the following file ijn your CMAKE_MODULE_PATH would work for you?

find_asio.cmake:

find_path(asio_INCLUDE_DIR
  NAMES asio.hpp
  HINTS
    ${asio_ROOT_DIR}
    /usr/local
    /opt
    /usr
  PATH_SUFFIXES include
  DOC "Path to the asio include directory"
)

if(asio_INCLUDE_DIR-NOTFOUND)
  message(FATAL_ERROR "Could not find asio library")
  set(asio_FOUND FALSE)
else()
  set(asio_FOUND TRUE)
  set(ASIO_INCLUDE_DIR ${asio_INCLUDE_DIR})
endif()

if(asio_FOUND)
  include(FindPackageHandleStandardArgs)
  find_package_handle_standard_args(asio
    REQUIRED_VARS asio_INCLUDE_DIR)

  if(NOT TARGET asio::asio)
    set(asio_INCLUDE_DIRS ${asio_INCLUDE_DIR})
    add_library(asio::asio INTERFACE IMPORTED)
    set_target_properties(asio::asio PROPERTIES
      INTERFACE_INCLUDE_DIRECTORIES ${asio_INCLUDE_DIR}
      INTERFACE_COMPILE_DEFINITIONS ASIO_STANDALONE)
    mark_as_advanced(asio_INCLUDE_DIR)
  endif()
endif()

[ShaharHD]

Can please you update the PR to include the find_asio.cmake and how it is called from inside the CMakeLists.txt ?
And then I can "pull it" and try to build it.

Keep in mind, that I already have the patch of this PR integrated into my bitbake see file://0001-Fix-asio-cmake-required.patch"` in the bitbake above.
I was trying to avoid the need to inject a patch and to control everything using CMake defines)

I'm trying to confirm if you're suggesting to replace one injection patch with another?

[FlorianReimold]

No, the idea is not to use a patch here, but just place the code I posted earlier somewhere on the build system and then add the directory as CMAKE_MODULE_PATH:

EXTRA_OECMAKE += "\
    -DCMAKE_MODULE_PATH=/path/to/dir/with/find_asio/
    -DFINEFTP_SERVER_BUILD_SAMPLES=OFF \
    -DFINEFTP_SERVER_USE_BUILTIN_ASIO=OFF \
    -DFINEFTP_SERVER_USE_BUILTIN_GTEST=OFF \
    -DBUILD_SHARED_LIBS=ON"

the find_asio.cmake script does not need to be included from within the CMakeLists.txt. Doing that actually only is a workaround to make sure that the vendored asio is used.