Skip to content

Nix: Build shared library #1067

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

Closed
wants to merge 2 commits into from

Conversation

MostAwesomeDude
Copy link

I couldn't get CMake to cooperate, so I ended up using the plain Makefile instead. I've tested the shared library as well as llama and embedding binaries. I already have this flake as an input in a personal repository, and this change works for me when integrated.

I do need an aarch64 Darwin user to verify that the CFLAGS work properly. @niklaskorz, does this work for you?

See also NixOS/nixpkgs#225058, since I know that it's controversial to have a flake.

I can't test whether this works on Darwin, but it can build libllama.so.

I did try building via CMake with -DBUILD_SHARED_LIBS=ON, but it did not work and I could not make it work.
Whoops! I didn't notice this at first because I'm not on Darwin. Can
somebody test this, please?
@prusnak
Copy link
Collaborator

prusnak commented Apr 19, 2023

We should really investigate how to build the shared library via cmake and not merge this PR as is

@MostAwesomeDude
Copy link
Author

Sure, whatever works for you. I'm going to keep using this change locally. Feel free to add more commits, close this PR and open another one, etc.

@prusnak
Copy link
Collaborator

prusnak commented Apr 20, 2023

on macOS using cmake -DBUILD_SHARED_LIBS=ON builds liblamma.dylib

@MostAwesomeDude
Copy link
Author

on macOS using cmake -DBUILD_SHARED_LIBS=ON builds liblamma.dylib

Nice! On Linux, this builds libllama.so, but then the tests and examples don't build; I already explored this path before deciding to use the Makefile instead.

If you can figure out how to unbreak the Linux build, then I happily agree with your CMake plan.

@prusnak
Copy link
Collaborator

prusnak commented Apr 22, 2023

Maybe symbols need to be exported on Linux similarly to Windows (#1100)?

What error do you see when building examples?

@MostAwesomeDude
Copy link
Author

[ 45%] Linking C executable ../bin/test-quantize
/nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: CMakeFiles/test-quantize.dir/test-quantize.c.o: undefined reference to symbol 'roundf@@GLIBC_2.2.5'
/nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: /nix/store/76l4v99sk83ylfwkz8wmwrm4s8h73rhd-glibc-2.35-224/lib/libm.so.6: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
make[2]: *** [tests/CMakeFiles/test-quantize.dir/build.make:98: bin/test-quantize] Error 1

Like it's not being linked with -lm or something like that.

@prusnak
Copy link
Collaborator

prusnak commented Apr 23, 2023

Like it's not being linked with -lm or something like that.

Does this help?

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 11ebe9e..ba5904f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -81,6 +81,7 @@ set(CMAKE_C_STANDARD 11)
 set(CMAKE_C_STANDARD_REQUIRED true)
 set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads REQUIRED)
+set(LLAMA_EXTRA_LIBS ${LLAMA_EXTRA_LIBS} m)
 
 if (NOT MSVC)
     if (LLAMA_SANITIZE_THREAD)

@MostAwesomeDude
Copy link
Author

Does this help?

I tried it, and the build is still broken with the same error in the same place. I double-checked.

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

Successfully merging this pull request may close these issues.

2 participants