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

This commit changes the naming conventioins of the shared object file… #48

Merged
merged 3 commits into from
Jul 7, 2023

Conversation

sirajperson
Copy link

@sirajperson sirajperson commented Jul 6, 2023

…s. This modificatioin will produce libfalcon.so instead of liblibfalcon.so which would create downstream bugs. Please review the following build results:

cmake -DLLAMA_CUBLAS=1 -DCUDAToolkit_ROOT=/usr/local/cuda/ -DBUILD_SHARED_LIBS=1 ..

... cmake config output ...

cmake --build . --config Release

... cmake build output ...

ll *.so 
-rwxrwxr-x 1 siraj siraj  44K Jul  6 14:37 libcmpnct_unicode.so
-rwxrwxr-x 1 siraj siraj 932K Jul  6 14:38 libfalcon.so
-rwxrwxr-x 1 siraj siraj 1.4M Jul  6 14:37 libggml_shared.so
-rwxrwxr-x 1 siraj siraj 825K Jul  6 14:37 libllama.so

instead of the existing shared object output:

ll *.so
-rwxrwxr-x 1 siraj siraj  44K Jul  6 14:46 libcmpnct_unicode.so
-rwxrwxr-x 1 siraj siraj 1.4M Jul  6 14:46 libggml_shared.so
-rwxrwxr-x 1 siraj siraj 932K Jul  6 14:46 liblibfalcon.so
-rwxrwxr-x 1 siraj siraj 825K Jul  6 14:46 libllama.so

…s. This modificatioin will produce libfalcon.so instead of liblibfalcon.so which would create downstreem bugs.
@cmp-nct
Copy link
Owner

cmp-nct commented Jul 7, 2023

I didn't notice that there is a second lib added beforehand (for .a and .so)
I'm not super happy to change the file names though, is there a middle ground just for the linker step so it generates the .a and .so binaries without liblib ?

…pp and libfalcon.h. It produces shared object files that maintain the standard file format of libfalcon.so
@sirajperson
Copy link
Author

Sure. I've gone ahead and refactored to keep the libfalcon.cpp and libfalcon.h file names, while still producing the correct shared object files.

@sirajperson
Copy link
Author

No troubles with the merge. Let me know if everything looks good.

@cmp-nct cmp-nct merged commit a9602c0 into cmp-nct:master Jul 7, 2023
@cmp-nct
Copy link
Owner

cmp-nct commented Jul 7, 2023

It's all merged by now, had some issues but good now.
Thanks

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