-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
ggml: link MATH_LIBRARY not by its full path #9339
Conversation
The proposed solution generates the following warning on my system:
Isn't it better to do something like this instead: diff --git a/ggml/src/CMakeLists.txt b/ggml/src/CMakeLists.txt
index cd2dcd06..0c7b5cad 100644
--- a/ggml/src/CMakeLists.txt
+++ b/ggml/src/CMakeLists.txt
@@ -1334,6 +1334,9 @@ find_library(MATH_LIBRARY m)
if (MATH_LIBRARY)
if (NOT WIN32 OR NOT GGML_SYCL)
target_link_libraries(ggml PRIVATE ${MATH_LIBRARY})
+ if (BUILD_SHARED_LIBS)
+ set_target_properties(ggml PROPERTIES INTERFACE_LINK_LIBRARIES "m")
+ endif()
endif()
endif() |
I think I found one problem. Even now the default build from the master branch on MacOS results in a duplicate problem.
But when Metal (
Thus, all libraries and frameworks end up being duplicated. Moreover, this problem appears only with all three variables ( |
I'm also not 100% confident about the correct CMake way, though here is an attempt to remove fix the double linkage: #9463. PTAL |
@ggerganov yes, that was exactly my solution! Rebased my branch onto |
b7d6eb0
to
ab82cad
Compare
Waiting for #9463 to be merged. |
Also added a change to avoid duplicating the |
c9ae421
to
6953305
Compare
Let's rebase on latest |
6953305
to
7c7294a
Compare
7c7294a
to
603a3f8
Compare
Finally the work is finished😊 |
The current
MATH_LIBRARY
linkage method works if the llama.cpp library is installed on the same machine where it will be used. If the built and installed artefacts were transferred to another machine, an error like this may occur:due to the fact that the
llama-config.cmake
specifies the full path to the library. The proposed fix changes the config toset(_llama_link_deps "${ggml_LIBRARY}" "Threads::Threads;m")
. Which helps with portability.