mikke89 / RmlUi

RmlUi - The HTML/CSS User Interface library evolved
https://mikke89.github.io/RmlUiDoc/
MIT License
2.57k stars 295 forks source link

Make `find_package(Freetype)` optional #575

Closed snaulX closed 5 months ago

snaulX commented 5 months ago

I'm using CPM.cmake in my project and include RmlUi using it in own CMakeLists. But I can't set default font interface because it cannot find freetype. But I added freetype earlier in this cmakelists. So it can be linked without find_package.

CPMAddPackage(
        NAME freetype
        URL https://download.savannah.gnu.org/releases/freetype/freetype-2.13.2.tar.xz
        VERSION 2.13.2
        DOWNLOAD_ONLY
        OPTIONS
        "BUILD_SHARED_LIBS OFF"
        "FT_WITH_BROTLI OFF"
        "FT_WITH_HARFBUZZ OFF"
        "CMAKE_DISABLE_FIND_PACKAGE_ZLIB ON"
        "CMAKE_DISABLE_FIND_PACKAGE_BZip2 ON"
        "CMAKE_DISABLE_FIND_PACKAGE_PNG ON"
        "CMAKE_DISABLE_FIND_PACKAGE_HarfBuzz ON"
        "CMAKE_DISABLE_FIND_PACKAGE_BrotliDec ON"
)
CPMAddPackage(
        NAME RmlUi
        GITHUB_REPOSITORY mikke89/RmlUi
        GIT_TAG 5.1
        OPTIONS
        "BUILD_SAMPLES OFF"
        "BUILD_TESTING OFF"
        "DISABLE_RTTI_AND_EXCEPTIONS ON"
)

target_compile_definitions(UISystem PRIVATE RMLUI_USE_CUSTOM_RTTI RMLUI_STATIC_LIB)

target_link_libraries(UISystem
        PRIVATE
        RmlCore
        RmlDebugger
)

So maybe add an option to link freetype without find_package?

snaulX commented 5 months ago

Btw I saw the same problem in https://github.com/SamCZ/Aurora (author using own RmlUi fork just for commeting find_package line in cmakelists)

mikke89 commented 5 months ago

We're working on a full overhaul on the CMake lists. Can you test how it works out for you with the cmake branch? Take a look here: https://github.com/mikke89/RmlUi/blob/cmake/CMake/Dependencies.cmake

snaulX commented 5 months ago

No, it works for me only with full disabling of freetype find_package and additional checks

mikke89 commented 5 months ago

@hobyst - do you have some input about adding a check for target Freetype::Freetype before its find_package? I see there are some incode comments about it, but I don't remember if we discussed this aspect specifically.

I guess if we do that, we should also add a check before each of our dependencies.

mikke89 commented 5 months ago

I wonder if CPM is supposed to opt in to the FetchContent redirection mode? See here: https://cmake.org/cmake/help/v3.28/command/find_package.html#search-modes.

@snaulX Can you research into how this is supposed to work, or if you need to declare something on your end?

It would certainly be preferable if we didn't need special logic to handle this. But if it turns out we do, then we'll of course go ahead and do that.

snaulX commented 5 months ago

I suppose we need to extend functionality of CPM or there. I didn't found any possible solutions to fix this without modifications of RmlUi/CPM. But I'll continue researching. Maybe I will ask for PR to extend functionality in CPM.

hobyst commented 5 months ago

The new CMake branch does all calls to find_package() non-REQUIRED and only checks for targets with the intention of making dependency management much more flexible.

With the new CMake project and CPM, it shouldn't get more complex than this:

CPMAddPackage(
        NAME freetype
        URL https://download.savannah.gnu.org/releases/freetype/freetype-2.13.2.tar.xz
        VERSION 2.13.2
        DOWNLOAD_ONLY
        OPTIONS
        "BUILD_SHARED_LIBS OFF"
        "FT_WITH_BROTLI OFF"
        "FT_WITH_HARFBUZZ OFF"
        "CMAKE_DISABLE_FIND_PACKAGE_ZLIB ON"
        "CMAKE_DISABLE_FIND_PACKAGE_BZip2 ON"
        "CMAKE_DISABLE_FIND_PACKAGE_PNG ON"
        "CMAKE_DISABLE_FIND_PACKAGE_HarfBuzz ON"
        "CMAKE_DISABLE_FIND_PACKAGE_BrotliDec ON"
)

if (freetype_ADDED)
  # Add alias target to make the target provided by CPM conform to the standard naming
  # brought by the CMake-bundled FindFreetype.cmake find module
  add_library(Freetype::Freetype ALIAS freetype)
endif()

# Add the RmlUi source code as a subdirectory to the CMake project
add_subdirectory(rmlui)

target_link_libraries(foo PRIVATE RmlUi::RmlUi RmlUi::Debugger)

If CMake complains about the alias, this can be done instead:

if (freetype_ADDED)
  # Add alias target to make the target provided by CPM conform to the standard naming
  # brought by the CMake-bundled FindFreetype.cmake find module
  add_library(Freetype::Freetype INTERFACE IMPORTED)
  target_link_libraries(Freetype::Freetype INTERFACE freetype)
endif()

Based on the snippet included in the first post of the issue discussion thread, I have a question for @snaulX concerning the attempt at integrating the code from the cmake branch into their project: Did you alias the target provided by CPM as Freetype::Freetype, as indicated in the CPM wiki? Freetype::Freetype is the target name RmlUi tries to link against as that is the name standardized by upstream CMake, so if the package manager provides Freetype under a different name, you will need to create an alias for it. This can be done without the need to alter the RmlUi CMake code.

Edit: There's also the possibility of a target name clash/mess between CPM and the Freetype find module included with CMake. In which case, the CMake code might need to be changed to make the target checks before the find_package() calls or even check twice: one to check if the user has provided it and another one and another one to check if the find_package() call has found it successfully.

snaulX commented 5 months ago

Yes, this works. I forget that CPM has sample about freetype and about cmake aliasing. Thanks for the answer.