-
Notifications
You must be signed in to change notification settings - Fork 206
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
Fix static libs INTERFACE_INCLUDE_DIRECTORIES #2505
Conversation
Thx ! |
d12cde0
to
281a6d7
Compare
@WangWeiLin-MV , does that new version of the patch actually fix the original bug ? |
@vrabaud Yes, in my tests, it was generated correctly in add_library(avif STATIC IMPORTED)
set_target_properties(avif PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
) and invoke |
Great, thx ! |
@@ -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}) |
There was a problem hiding this comment.
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!
Fix issue microsoft/vcpkg#42112, error with
fatal error: 'avif/avif.h' file not found
that targetavif
(i.e.avif_static
) lostINTERFACE_INCLUDE_DIRECTORIES
for static build.Let
merge_static_libs.cmake
copyINTERFACE_INCLUDE_DIRECTORIES
too.