Skip to content

Only INSTALL_INTERFACE for static.#2269

Merged
vrabaud merged 3 commits into
AOMediaCodec:mainfrom
vrabaud:pkgconfig
Jul 16, 2024
Merged

Only INSTALL_INTERFACE for static.#2269
vrabaud merged 3 commits into
AOMediaCodec:mainfrom
vrabaud:pkgconfig

Conversation

@vrabaud

@vrabaud vrabaud commented Jul 15, 2024

Copy link
Copy Markdown
Contributor

This fixes #2264

Comment thread CMakeLists.txt
Comment thread CMakeLists.txt
get_target_property(target_type ${target} TYPE)
if(target_type STREQUAL "SHARED_LIBRARY" OR NOT BUILD_SHARED_LIBS)
# The transitive dependency is an export link library if it is a static library in a static build.
if(NOT BUILD_SHARED_LIBS)

@wantehchang wantehchang Jul 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wanted to note that we can't test this code (lines 172-174) right now, because in a static build, the cmake config files are not installed:

    # Enable CMake configs in VCPKG mode
    if(BUILD_SHARED_LIBS OR VCPKG_TARGET_TRIPLET)   
        install(EXPORT ${PROJECT_NAME}-config DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME})
        
        include(CMakePackageConfigHelpers)
        write_basic_package_version_file(
            ${PROJECT_NAME}-config-version.cmake VERSION ${PROJECT_VERSION} COMPATIBILITY SameMajorVersion
        )
        install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}-config-version.cmake
                DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}
        )
    endif()

I tried to test this code last week but could not make it work. Another difficulty is that in a static build, the library we install is avif_static, not avif:

    if(BUILD_SHARED_LIBS)
        set(LIBAVIF_INSTALL_TARGET avif)
    else()
        set(LIBAVIF_INSTALL_TARGET avif_static)
    endif()

where avif_static is the merged archive library:

    merge_static_libs(avif_static avif)

I am wondering if avif_static does not inherit the link libraries of avif that we set here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am playing with an installed static CI right now to debug this.

@wantehchang wantehchang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@vrabaud vrabaud merged commit 1a1c778 into AOMediaCodec:main Jul 16, 2024
@vrabaud vrabaud deleted the pkgconfig branch July 16, 2024 12:52
@wantehchang wantehchang added this to the v1.1.1 milestone Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v1.1.0 CMake config leaks shared library dependencies into consumers

2 participants