aws / amazon-chime-sdk-cpp

Apache License 2.0
13 stars 5 forks source link

Several build system issues #17

Open tomadamatkinson opened 9 months ago

tomadamatkinson commented 9 months ago

Hi,

I've been attempting to use the SDK locally and have spotted several build system issues. There is a hard dependency with BoringSSL which isn't needed. Please let me know if there is a reason behind this

I also noticed that you the CMake is performing a manual combine step

# Combine binaries
# Currently supporting Mac and Linux

set(SDK_BINARY_LIB_NAME "libamazon_chime_signaling_sdk")

if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
    set(SDK_BINARY_NAME "${SDK_BINARY_LIB_NAME}.a")

    # TODO @hokyungh: might be better to use mri script
    add_custom_command(
            TARGET ${LIBRARY_NAME}
            POST_BUILD
            COMMAND ar -x $<TARGET_FILE:${LIBRARY_NAME}>
        COMMAND ar -x $<TARGET_FILE:protobuf::libprotobuf>
            COMMAND ar -x $<TARGET_FILE:websockets>
            COMMAND ar -qc ${SDK_BINARY_NAME} *.o
    )
elseif(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
    set(SDK_BINARY_NAME "${SDK_BINARY_LIB_NAME}.a")
    add_custom_command(
            TARGET ${LIBRARY_NAME}
            POST_BUILD
            COMMAND
            libtool -static -o ${SDK_BINARY_NAME}
            $<TARGET_FILE:${LIBRARY_NAME}>
            $<TARGET_FILE:protobuf::libprotobuf>
            $<TARGET_FILE:websockets>
            $<TARGET_FILE:websockets_shared>
    )
endif()

I'm confused as to why this is being carried out as CMake builds these libraries under the hood when you use target_link_libraries

Is there a reason behind manually doing this step?

I am assuming that due to #16 the SDK is a few versions behind the internal repository?

Happy to discuss this further. These are a few of the issues I experienced building locally and can be solved with a few small tweaks of the cmake :smile:

hokyungh commented 9 months ago

If i remember correctly, target_link_libraries does not actually combine library. The main reason for this combining is to use it in the demo application as one library. See here.

tomadamatkinson commented 9 months ago

target_link_libraries is quite powerful. It abstracts any need to deal with the low-level compilation steps. If defined as a static library CMake will package the object files from .a into a shared library or executable depending on what type the final target is.

For example

add_library(lib1 STATIC ${LIB1_SRC})
add_library(lib2 STATIC ${LIB2_SRC})
target_link_libraries(lib1 PUBLIC lib2)

add_library(lib3 STATIC ${LIB3_SRC})

add_executable(exe1 ${EXE1_SRC})
target_link_libraries(exe1 PRIVATE lib2 lib3)

The above will generate a single exe1 executable which will contain the object files of the three libraries defined above. lib1 is included as the target_link_libraries between itself and lib2 are marked as PUBLIC. If this is PRIVATE then the lib wont be automatically added as a dependency

I believe you are linking as PRIVATE in many places which may explain why you need to do the manual combine step