Revert 7e53955 (#542) and fix properly

[anzz1]

Reverting 7e53955 (#542)

ggml shouldn't be linked twice as it's already linked to llama (probably common should be linked to ggml instead) .
ref: 7e53955#commitcomment-106187813

• Reverts 7e53955 (Fix missing ggml link in cmake for executables in examples on w64-mingw32 #542)
• Needs someone with minGW to post a proper solution that works (thanks @marcom).

[marcom]

Edit: just to clarify, I have no problem with the revert, as I can ultimately work around this problem in the BinaryBuilder repo https://github.com/JuliaPackaging/Yggdrasil/blob/master/L/llama_cpp/build_tarballs.jl

I wasn't able to get it to work any other way, but not sure if I'm doing anything wrong.

The weird thing is that on all other platforms that are cross-compiled for ({x86_64,i686,aarch64}-{apple,linux} and x86_64-freebsd), the change in #542 wasn't necessary, only on {x86_64,i686}-w64-mingw32.

I don't use mingw32 myself, so I am not experienced with anything that might be different on this platform.

I tried unsuccessfully:

--- a/examples/CMakeLists.txt
+++ b/examples/CMakeLists.txt
@@ -21,7 +21,7 @@ endif()
 
 target_include_directories(${TARGET} PUBLIC .)
 target_compile_features(${TARGET} PUBLIC cxx_std_11)
-target_link_libraries(${TARGET} PRIVATE llama)
+target_link_libraries(${TARGET} PRIVATE llama ggml)

--- a/examples/CMakeLists.txt
+++ b/examples/CMakeLists.txt
@@ -21,7 +21,7 @@ endif()
 
 target_include_directories(${TARGET} PUBLIC .)
 target_compile_features(${TARGET} PUBLIC cxx_std_11)
-target_link_libraries(${TARGET} PRIVATE llama)
+target_link_libraries(${TARGET} PUBLIC llama ggml)

--- a/examples/CMakeLists.txt
+++ b/examples/CMakeLists.txt
@@ -21,7 +21,7 @@ endif()
 
 target_include_directories(${TARGET} PUBLIC .)
 target_compile_features(${TARGET} PUBLIC cxx_std_11)
-target_link_libraries(${TARGET} PRIVATE llama)
+target_link_libraries(${TARGET} llama ggml)

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -240,7 +240,7 @@ add_library(llama
 
 target_include_directories(llama PUBLIC .)
 target_compile_features(llama PUBLIC cxx_std_11) # don't bump
-target_link_libraries(llama PRIVATE ggml ${LLAMA_EXTRA_LIBS})
+target_link_libraries(llama PUBLIC ggml PRIVATE ${LLAMA_EXTRA_LIBS})

[anzz1]

Hmm, that definitely sounds like a bug in minGW or CMake. 🤔

In any case it would be important to find a workaround which doesn't involve using a hack. Or if there isn't, make the hack target only mingw32 and keep the working setups as-is.

Changing the setup for every compiler/platform because one of them has a bug was definitely the wrong approach, and that's my bad for jumping the gun merging it.

[marcom]

With the patch below it now cross-compiles for me in BinaryBuilder on mingw by using the gcc visibility rules. I can submit an extra PR if you think this is the right fix. I read somewhere that on mingw __declspec(dllexport) is supposed to be understood, but it didn't seem to be equivalent to __attribute__ ((visibility ("default"))) here (that is in the else case not visible in the diff).

diff --git a/llama.h b/llama.h
index ebf55f4..b7530dc 100644
--- a/llama.h
+++ b/llama.h
@@ -6,7 +6,7 @@
 #include <stdbool.h>
 
 #ifdef LLAMA_SHARED
-#    ifdef _WIN32
+#    if defined _WIN32 && !defined __MINGW32__
 #        ifdef LLAMA_BUILD
 #            define LLAMA_API __declspec(dllexport)
 #        else

[anzz1]

@marcom
Thanks for taking time to look into this more!

That is definitely the right way to fix it instead of changing the linkings. As mingw is based on GCC while __declspec(dllexport/dllimport) is more of a MSVC thing, there probably was some compatibility mismatch. I don't have too much experience with mingw, but in my limited experience using the GCC or Clang definitions instead of MSVC ones for MinGW usually yields the proper results.

I've pushed the change to this PR, no need to submit a new PR.

[CoderRC]

@anzz1 _POSIX_MAPPED_FILES is defined in unistd.h.