From cd65abacaf8c3a6259a8447971c7b64ebfa1c5d1 Mon Sep 17 00:00:00 2001 From: Wan-Teh Chang Date: Wed, 24 Jul 2024 15:35:56 -0700 Subject: [PATCH] Compile avif_apps_internal without -DAVIF_DLL In shared library builds (-DBUILD_SHARED_LIBS=ON), compile the source files in the avif_apps_internal static library without -DAVIF_DLL. Also link avif_obj with the m and Threads::Threads libraries in the PRIVATE scope instead of the PUBLIC scope. Fix https://github.com/AOMediaCodec/libavif/issues/2339. --- CMakeLists.txt | 58 +++++++++++++++++++++----------------------- tests/CMakeLists.txt | 29 +++++++++++++--------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fcbcba5747..b8ad48ffa3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -455,7 +455,7 @@ if(UNIX OR MINGW) # Find out if we have threading available set(THREADS_PREFER_PTHREAD_FLAG ON) find_package(Threads) - target_link_libraries(avif_obj PUBLIC m Threads::Threads) + target_link_libraries(avif_obj PRIVATE m Threads::Threads) endif() if(NOT AVIF_LIBYUV_ENABLED) @@ -676,45 +676,43 @@ if(AVIF_BUILD_APPS OR (AVIF_BUILD_TESTS AND (AVIF_ENABLE_FUZZTEST OR AVIF_ENABLE find_package(JPEG REQUIRED) endif() - add_library( - avif_apps_obj OBJECT apps/shared/avifexif.c apps/shared/avifjpeg.c apps/shared/avifpng.c apps/shared/avifutil.c - apps/shared/iccmaker.c apps/shared/y4m.c third_party/iccjpeg/iccjpeg.c - ) - # Instead of building avif_apps/avif_apps_internal and linking to avif/avif_internal, avif_apps_obj only does one compilation. - # Still, the compile definitions needs to be passed from avif to avif_apps. The following also passes them to avif_apps_internal but AVIF_DLL does not impact avif_apps_internal. - target_compile_definitions(avif_apps_obj PRIVATE $>) - target_link_libraries(avif_apps_obj PUBLIC avif_obj PNG::PNG ZLIB::ZLIB JPEG::JPEG) - target_link_libraries(avif_apps_obj PRIVATE avif_enable_warnings) - if(CMAKE_SYSTEM_NAME STREQUAL "Linux") - target_link_libraries(avif_apps_obj PRIVATE m) - endif() - # In GitHub CI's macos-latest os image, /usr/local/include has not only the headers of libpng - # and libjpeg but also the headers of an older version of libavif. Put the avif include - # directory before ${PNG_PNG_INCLUDE_DIR} ${JPEG_INCLUDE_DIR} to prevent picking up old libavif - # headers from /usr/local/include. - target_include_directories(avif_apps_obj PRIVATE third_party/iccjpeg) - target_include_directories(avif_apps_obj SYSTEM PRIVATE ${PNG_PNG_INCLUDE_DIR} ${JPEG_INCLUDE_DIR}) - if(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP) if(TARGET LibXml2::LibXml2) set(AVIF_ENABLE_EXPERIMENTAL_JPEG_GAIN_MAP_CONVERSION TRUE) add_compile_definitions(AVIF_ENABLE_EXPERIMENTAL_JPEG_GAIN_MAP_CONVERSION) - target_link_libraries(avif_apps_obj PRIVATE LibXml2::LibXml2) else() message(STATUS "libavif: libxml2 not found; avifenc will ignore any gain map in jpeg files") endif() endif() + set(AVIF_APPS_SRCS apps/shared/avifexif.c apps/shared/avifjpeg.c apps/shared/avifpng.c apps/shared/avifutil.c + apps/shared/iccmaker.c apps/shared/y4m.c third_party/iccjpeg/iccjpeg.c + ) + + macro(add_avif_apps_library suffix) + add_library(avif_apps${suffix} STATIC ${AVIF_APPS_SRCS}) + target_link_libraries(avif_apps${suffix} PUBLIC avif${suffix} PRIVATE PNG::PNG ZLIB::ZLIB JPEG::JPEG avif_enable_warnings) + if(CMAKE_SYSTEM_NAME STREQUAL "Linux") + target_link_libraries(avif_apps${suffix} PRIVATE m) + endif() + if(AVIF_ENABLE_EXPERIMENTAL_JPEG_GAIN_MAP_CONVERSION) + target_link_libraries(avif_apps${suffix} PRIVATE LibXml2::LibXml2) + endif() + target_include_directories(avif_apps${suffix} INTERFACE apps/shared) + # In GitHub CI's macos-latest os image, /usr/local/include has not only the headers of libpng + # and libjpeg but also the headers of an older version of libavif. Put the avif include + # directory before ${PNG_PNG_INCLUDE_DIR} ${JPEG_INCLUDE_DIR} to prevent picking up old libavif + # headers from /usr/local/include. + target_include_directories(avif_apps${suffix} PRIVATE third_party/iccjpeg) + target_include_directories(avif_apps${suffix} SYSTEM PRIVATE ${PNG_PNG_INCLUDE_DIR} ${JPEG_INCLUDE_DIR}) + endmacro() + # Main avif_apps library. - add_library(avif_apps STATIC) - target_link_libraries(avif_apps PUBLIC avif PRIVATE avif_apps_obj) - target_include_directories(avif_apps INTERFACE apps/shared) + add_avif_apps_library("") # avif_apps_internal is to use when linking to avif_internal. if(BUILD_SHARED_LIBS) - add_library(avif_apps_internal STATIC) - target_link_libraries(avif_apps_internal PUBLIC avif_internal PRIVATE avif_apps_obj) - target_include_directories(avif_apps_internal INTERFACE apps/shared) + add_avif_apps_library(_internal) else() add_library(avif_apps_internal ALIAS avif_apps) endif() @@ -739,7 +737,7 @@ if(AVIF_BUILD_APPS) if(AVIF_LIB_USE_CXX) set_target_properties(avifenc PROPERTIES LINKER_LANGUAGE "CXX") endif() - target_link_libraries(avifenc avif_apps avif_enable_warnings) + target_link_libraries(avifenc avif_apps avif avif_enable_warnings) add_executable(avifdec apps/avifdec.c) if(WIN32) target_sources(avifdec PRIVATE apps/utf8.rc) @@ -750,7 +748,7 @@ if(AVIF_BUILD_APPS) if(AVIF_LIB_USE_CXX) set_target_properties(avifdec PROPERTIES LINKER_LANGUAGE "CXX") endif() - target_link_libraries(avifdec avif_apps avif_enable_warnings) + target_link_libraries(avifdec avif_apps avif avif_enable_warnings) if(NOT SKIP_INSTALL_APPS AND NOT SKIP_INSTALL_ALL) install( @@ -785,7 +783,7 @@ if(AVIF_BUILD_APPS) endif() set_target_properties(avifgainmaputil PROPERTIES LINKER_LANGUAGE "CXX") target_include_directories(avifgainmaputil PRIVATE apps/avifgainmaputil/) - target_link_libraries(avifgainmaputil libargparse avif_apps avif_enable_warnings) + target_link_libraries(avifgainmaputil libargparse avif_apps avif avif_enable_warnings) # Don't add avifgainmaputil to installed apps for now. endif() endif() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index be184780d8..4a4be470cb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -26,7 +26,7 @@ add_executable(aviftest aviftest.c) if(AVIF_CODEC_LIBGAV1_ENABLED OR AVIF_LIBYUV_ENABLED) set_target_properties(aviftest PROPERTIES LINKER_LANGUAGE "CXX") endif() -target_link_libraries(aviftest avif) +target_link_libraries(aviftest avif avif_enable_warnings) add_test(NAME aviftest COMMAND aviftest ${CMAKE_CURRENT_SOURCE_DIR}/data) register_test_for_coverage(aviftest ${CMAKE_CURRENT_SOURCE_DIR}/data/) @@ -35,23 +35,25 @@ if(AVIF_CODEC_LIBGAV1_ENABLED OR AVIF_LIBYUV_ENABLED) set_target_properties(avifyuv PROPERTIES LINKER_LANGUAGE "CXX") endif() -target_link_libraries(avifyuv avif) +target_link_libraries(avifyuv avif avif_enable_warnings) foreach(AVIFYUV_MODE limited rgb) # Modes drift and premultiply take more than 2 minutes each so they are disabled. add_test(NAME avifyuv_${AVIFYUV_MODE} COMMAND avifyuv -m ${AVIFYUV_MODE}) endforeach() if(AVIF_ENABLE_FUZZTEST OR AVIF_ENABLE_GTEST OR AVIF_BUILD_APPS) add_library(aviftest_helpers OBJECT gtest/aviftest_helpers.cc) - target_link_libraries(aviftest_helpers avif_apps avif) + target_link_libraries(aviftest_helpers PUBLIC avif_apps avif) + target_link_libraries(aviftest_helpers PRIVATE avif_enable_warnings) add_library(aviftest_helpers_internal OBJECT gtest/aviftest_helpers.cc) - target_link_libraries(aviftest_helpers_internal avif_apps_internal avif_internal) + target_link_libraries(aviftest_helpers_internal PUBLIC avif_apps_internal avif_internal) + target_link_libraries(aviftest_helpers_internal PRIVATE avif_enable_warnings) endif() if(CMAKE_CXX_COMPILER_LOADED) # Fuzz target without any fuzzing engine dependency. For easy reproduction of oss-fuzz issues. add_executable(repro_avif_decode_fuzzer oss-fuzz/avif_decode_fuzzer.cc oss-fuzz/repro_fuzz.cc) set_target_properties(repro_avif_decode_fuzzer PROPERTIES LINKER_LANGUAGE "CXX") - target_link_libraries(repro_avif_decode_fuzzer avif) + target_link_libraries(repro_avif_decode_fuzzer avif avif_enable_warnings) # The test below exists for coverage and as a usage example: repro_avif_decode_fuzzer [reproducer file path] add_test(NAME repro_avif_decode_fuzzer COMMAND repro_avif_decode_fuzzer ${CMAKE_CURRENT_SOURCE_DIR}/data/color_grid_alpha_nogrid.avif @@ -65,25 +67,25 @@ endif() # are considered as extra linked libraries. macro(add_avif_gtest TEST_NAME) add_executable(${TEST_NAME} gtest/${TEST_NAME}.cc) - target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers GTest::GTest GTest::Main ${ARGN}) + target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers GTest::GTest GTest::Main ${ARGN} avif_enable_warnings) add_test(NAME ${TEST_NAME} COMMAND ${TEST_NAME}) register_test_for_coverage(${TEST_NAME}) endmacro() macro(add_avif_internal_gtest TEST_NAME) add_executable(${TEST_NAME} gtest/${TEST_NAME}.cc) - target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers_internal GTest::GTest GTest::Main ${ARGN}) + target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers_internal GTest::GTest GTest::Main ${ARGN} avif_enable_warnings) add_test(NAME ${TEST_NAME} COMMAND ${TEST_NAME}) register_test_for_coverage(${TEST_NAME}) endmacro() macro(add_avif_gtest_with_data TEST_NAME) add_executable(${TEST_NAME} gtest/${TEST_NAME}.cc) - target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers GTest::GTest ${ARGN}) + target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers GTest::GTest ${ARGN} avif_enable_warnings) add_test(NAME ${TEST_NAME} COMMAND ${TEST_NAME} ${CMAKE_CURRENT_SOURCE_DIR}/data/) register_test_for_coverage(${TEST_NAME} ${CMAKE_CURRENT_SOURCE_DIR}/data/) endmacro() macro(add_avif_internal_gtest_with_data TEST_NAME) add_executable(${TEST_NAME} gtest/${TEST_NAME}.cc) - target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers_internal GTest::GTest ${ARGN}) + target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers_internal GTest::GTest ${ARGN} avif_enable_warnings) add_test(NAME ${TEST_NAME} COMMAND ${TEST_NAME} ${CMAKE_CURRENT_SOURCE_DIR}/data/) register_test_for_coverage(${TEST_NAME} ${CMAKE_CURRENT_SOURCE_DIR}/data/) endmacro() @@ -91,9 +93,9 @@ endmacro() if(AVIF_ENABLE_GTEST) check_avif_option(AVIF_GTEST TARGET GTest::GTest PKG_NAME GTest) add_library(avifincrtest_helpers OBJECT gtest/avifincrtest_helpers.cc) - target_link_libraries(avifincrtest_helpers avif GTest::GTest) + target_link_libraries(avifincrtest_helpers avif GTest::GTest avif_enable_warnings) add_library(avifincrtest_helpers_internal OBJECT gtest/avifincrtest_helpers.cc) - target_link_libraries(avifincrtest_helpers_internal avif_internal GTest::GTest) + target_link_libraries(avifincrtest_helpers_internal avif_internal GTest::GTest avif_enable_warnings) endif() if(AVIF_ENABLE_GTEST) @@ -185,6 +187,9 @@ if(AVIF_ENABLE_FUZZTEST) macro(add_avif_fuzztest TEST_NAME) add_executable(${TEST_NAME} gtest/${TEST_NAME}.cc gtest/avif_fuzztest_helpers.cc ${ARGN}) # FuzzTest bundles GoogleTest so no need to link to gtest librairies. + # NOTE: FuzzTest and Abseil headers have compiler warnings (mostly + # -Wsign-compare and some -Wunused-parameter and -Wshorten-64-to-32, + # so we don't enable compiler warnings on the fuzztest targets. target_link_libraries(${TEST_NAME} PRIVATE aviftest_helpers_internal) link_fuzztest(${TEST_NAME}) add_test(NAME ${TEST_NAME} COMMAND ${TEST_NAME}) @@ -238,7 +243,7 @@ if(AVIF_BUILD_APPS) target_link_options(are_images_equal PRIVATE /MANIFEST:NO) endif() endif() - target_link_libraries(are_images_equal aviftest_helpers) + target_link_libraries(are_images_equal aviftest_helpers avif_enable_warnings) add_test(NAME test_cmd COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/test_cmd.sh ${CMAKE_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/data )