pthom / hello_imgui

Hello, Dear ImGui: unleash your creativity in app development and prototyping
https://pthom.github.io/hello_imgui
MIT License
678 stars 103 forks source link

Linking Issues with nlohmann_json in HelloImGui: Conflicts and Solutions #119

Closed cian-dev10 closed 3 months ago

cian-dev10 commented 3 months ago

Issue Description

I have encountered a problem where nlohmann_json is being included in the project in two different ways, which might lead to conflicts or duplication. Currently, nlohmann_json is being fetched directly and also set up manually as an external dependency. I would like to consolidate this setup to use an option that allows toggling between fetching the library and using an external version.

Current Configuration (mine on top-level)

Fetching nlohmann_json using FetchContent In the top-level CMakeLists.txt, nlohmann_json is fetched from GitHub using FetchContent if it is not already a target:

if (NOT TARGET nlohmann_json)
    ## JSON library
    FetchContent_Declare(
        json
        URL https://github.com/nlohmann/json/releases/download/v3.11.3/json.tar.xz
    )
    FetchContent_MakeAvailable(json)
    set_property(TARGET nlohmann_json PROPERTY CXX_STANDARD 17)
    set_property(TARGET nlohmann_json PROPERTY FOLDER "Third Party")
endif()

Manual Setup of nlohmann_json

In the him_add_nlohmann_json function, nlohmann_json is manually added as an external dependency if it is not found via find_package:

###################################################################################################
# Add nlohmann_json: API = him_add_nlohmann_json
###################################################################################################
function(him_add_nlohmann_json)
    find_package(nlohmann_json CONFIG QUIET)
    if(nlohmann_json_FOUND)
        message(STATUS "HelloImGui: Using nlohmann_json from find_package(nlohmann_json)")
        target_link_libraries(${HELLOIMGUI_TARGET} PUBLIC nlohmann_json::nlohmann_json)
        set(HELLOIMGUI_NLOHMANN_JSON_SELECTED_INFO "Found via find_package(nlohmann_json)" CACHE INTERNAL "" FORCE)
    else()
        message(STATUS "HelloImGui: Using nlohmann_json from external/nlohmann_json")
        set(nlohmann_json_dir ${HELLOIMGUI_BASEPATH}/external/nlohmann_json)
        add_library(nlohmann_json INTERFACE)
        target_include_directories(nlohmann_json INTERFACE $<BUILD_INTERFACE:${nlohmann_json_dir}>)
        # target_compile_definitions(nlohmann_json INTERFACE NLOHMANN_JSON_NOEXCEPTION)
        target_link_libraries(${HELLOIMGUI_TARGET} PUBLIC nlohmann_json)
        set(HELLOIMGUI_NLOHMANN_JSON_SELECTED_INFO "Using external/nlohmann_json" CACHE INTERNAL "" FORCE)

        him_add_installable_dependency(nlohmann_json)
        if(HELLOIMGUI_INSTALL)
            install(FILES ${nlohmann_json_dir}/nlohmann/json.hpp DESTINATION include/nlohmann/json.hpp)
            install(FILES ${nlohmann_json_dir}/nlohmann/json_fwd.hpp DESTINATION include/nlohmann/json_fwd.hpp)
        endif()
    endif()
endfunction()

Proposed Solution To resolve the duplication issue and make it easier to manage the inclusion of nlohmann_json, I propose introducing a CMake option to choose between using the fetched nlohmann_json or the external version. This option will allow users to specify their preference.

Add the following option to the CMakeLists.txt

option(HELLOIMGUI_USE_EXTERNAL_JSON "Use external nlohmann_json library" OFF)

Is it possible?

Best regards

pthom commented 3 months ago

Hello,

I see that you closed this issue. Did you find an alternative way to solve it? Your idea of using an option was interesting anyhow.