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

Missing symbols when building tests on Windows #1374

Closed
kmilos opened this issue Nov 7, 2024 · 5 comments
Closed

Missing symbols when building tests on Windows #1374

kmilos opened this issue Nov 7, 2024 · 5 comments

Comments

@kmilos
Copy link
Contributor

kmilos commented Nov 7, 2024

Building 1.19.2 now gives:

 FAILED: tests/encode.exe 
  C:\Windows\system32\cmd.exe /C "cd . && D:\M\msys64\ucrt64\bin\g++.exe -march=nocona -msahf -mtune=generic -O2 -pipe -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wp,-D__USE_MINGW_ANSI_STDIO=1 -O3 -DNDEBUG  tests/CMakeFiles/encode.dir/encode.cc.obj -o tests\encode.exe -Wl,--out-implib,tests\libencode.dll.a -Wl,--major-image-version,0,--minor-image-version,0  libheif/libheif.dll.a  tests/libtestframework.a  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."
  D:/M/msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/14.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: tests/libtestframework.a(test_utils.cc.obj):test_utils.cc:(.text+0x6b): undefined reference to `__imp_heif_context_get_number_of_top_level_images'

1.19.1 patched for symbol export was building ok.

I guess one could just turn off building the tests, but looks like Windows CI should be generally improved...

@farindk
Copy link
Contributor

farindk commented Nov 7, 2024

That is because of the switch to the new catch2 testing framework.
Does it work when you add this line:

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 0dbae17..c69debb 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -12,6 +12,7 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "GN
 endif()
 
 add_library(testframework STATIC ${CMAKE_BINARY_DIR}/generated/test-config.cc test_utils.cc catch_amalgamated.cpp)
+target_link_libraries(testframework PRIVATE heif)
 
 macro(add_libheif_test TEST_FILE)
     set(TEST_NAME ${TEST_FILE})

@kmilos
Copy link
Contributor Author

kmilos commented Nov 8, 2024

Does it work when you add this line

Works indeed (haven't checked if they actually run though), but perhaps you really want more formally:

--- a/tests/CMakeLists.txt	2024-11-08 14:45:29.053186000 +0100
+++ b/tests/CMakeLists.txt	2024-11-08 15:07:37.327068900 +0100
@@ -12,11 +12,12 @@
 endif()
 
 add_library(testframework STATIC ${CMAKE_BINARY_DIR}/generated/test-config.cc test_utils.cc catch_amalgamated.cpp)
+target_link_libraries(testframework PUBLIC heif)
 
 macro(add_libheif_test TEST_FILE)
     set(TEST_NAME ${TEST_FILE})
     add_executable(${TEST_NAME} ${TEST_FILE}.cc)
-    target_link_libraries(${TEST_NAME} PRIVATE heif testframework)
+    target_link_libraries(${TEST_NAME} PRIVATE testframework)
     add_test(NAME ${TEST_NAME} COMMAND ./${TEST_NAME})
     set_tests_properties(${TEST_NAME} PROPERTIES SKIP_REGULAR_EXPRESSION "[1-9][0-9]* skipped")
 endmacro()
@@ -24,7 +25,7 @@
 macro(add_heifio_test TEST_FILE)
     set(TEST_NAME ${TEST_FILE})
     add_executable(${TEST_NAME} main.cc ${TEST_FILE}.cc)
-    target_link_libraries(${TEST_NAME} PRIVATE heif heifio testframework)
+    target_link_libraries(${TEST_NAME} PRIVATE heifio testframework)
     target_include_directories(${TEST_NAME} PRIVATE ${libheif_SOURCE_DIR})
     add_test(NAME ${TEST_NAME} COMMAND ./${TEST_NAME})
     set_tests_properties(${TEST_NAME} PROPERTIES SKIP_REGULAR_EXPRESSION "[1-9][0-9]* skipped")

Also, maybe have BUILD_TESTING off by default?

@farindk
Copy link
Contributor

farindk commented Nov 8, 2024

but perhaps you really want more formally:

I have the feeling that we have to link heif into all targets that use functions from heif. Duplicates will be removed by the linker, but if we only link it into the testframework, we might get errors like the one because of which you opened this issue.

Also, maybe have BUILD_TESTING off by default?

That is what the cmake presets are for. When building for a release package, use the release preset, which has testing switched off.

@kmilos
Copy link
Contributor Author

kmilos commented Nov 8, 2024

Ok, thanks.

@farindk
Copy link
Contributor

farindk commented Nov 8, 2024

Thanks for the report.

@farindk farindk closed this as completed Nov 8, 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

No branches or pull requests

2 participants