Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix static libs INTERFACE_INCLUDE_DIRECTORIES #2505

Conversation

WangWeiLin-MV
Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV commented Nov 14, 2024

Fix issue microsoft/vcpkg#42112, error with fatal error: 'avif/avif.h' file not found that target avif (i.e. avif_static) lost INTERFACE_INCLUDE_DIRECTORIES for static build.

Let merge_static_libs.cmake copy INTERFACE_INCLUDE_DIRECTORIES too.

@WangWeiLin-MV WangWeiLin-MV marked this pull request as ready for review November 14, 2024 08:37
@vrabaud
Copy link
Collaborator

vrabaud commented Nov 14, 2024

Thx !

@WangWeiLin-MV WangWeiLin-MV force-pushed the cmake/merge_static_libs/copy-include-dirs branch from d12cde0 to 281a6d7 Compare November 15, 2024 02:53
@vrabaud
Copy link
Collaborator

vrabaud commented Nov 15, 2024

@WangWeiLin-MV , does that new version of the patch actually fix the original bug ?

@WangWeiLin-MV
Copy link
Contributor Author

WangWeiLin-MV commented Nov 15, 2024

@vrabaud Yes, in my tests, it was generated correctly in packages/libavif_x64-windows-static/share/libavif/libavif-config.cmake

add_library(avif STATIC IMPORTED)

set_target_properties(avif PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
)

and invoke avifVersion() successfully.

@vrabaud vrabaud merged commit 6fac47f into AOMediaCodec:main Nov 15, 2024
34 checks passed
@vrabaud
Copy link
Collaborator

vrabaud commented Nov 15, 2024

Great, thx !

@WangWeiLin-MV WangWeiLin-MV deleted the cmake/merge_static_libs/copy-include-dirs branch November 15, 2024 10:12
@@ -52,6 +52,11 @@ function(merge_static_libs target in_target)
set(source_file ${CMAKE_CURRENT_BINARY_DIR}/${target}_depends.c)
add_library(${target} STATIC ${source_file})

get_target_property(include_dirs ${in_target} INTERFACE_INCLUDE_DIRECTORIES)
if(include_dirs)
target_include_directories(${target} PUBLIC ${include_dirs})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @WangWeiLin-MV,

INTERFACE seems more appropriate than PUBLIC because ${include_dirs} comes from the INTERFACE_INCLUDE_DIRECTORIES property of ${in_target}.

Could you see if INTERFACE also works? Thanks!

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.

3 participants