-
Notifications
You must be signed in to change notification settings - Fork 14.1k
add version to all shared object files #17091
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
Conversation
ggml/src/ggml-blas/CMakeLists.txt
Outdated
| set_target_properties(ggml-blas PROPERTIES | ||
| VERSION ${GGML_VERSION} | ||
| SOVERSION ${GGML_VERSION_MAJOR} | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this in ggml_add_backend_library, rather than duplicating it for every backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, I'll get that fixed on Monday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slaren I've cleaned that up and updated the PR.
When compiling llama.cpp in Yocto, it fails QA checks because the generated so files aren't versioned. This applies a version to all generated so files, allowing the package to build without errors.
ec3d4ce to
2719f80
Compare
…et_target_properties) Cause by ggml-org/llama.cpp#17091
|
This merged pull request broke all |
|
Hello @furrysalamander. Please consider the following command And the same after your commit: Why have you removed |
|
Hello @slaren, I see we have an excellent docker workflow for github actions. Why this workflow is not used for validating pull requests? Please just enable it in github settings. |
|
I'm taking a look at this now. |
|
This change has no effect on compiling dynamic link libraries for Windows; in fact, it may cause CMake variables to be unavailable. You should consider the compilation requirements for different platforms. |
PR ggml-org#17091 set the VERSION of various libraries to 0.0.abcd, where abcd is the LLAMA_BUILD_NUMBER. That build number is too large to fit in the Mach-O 'current version' field's 'micro' part, which only goes up to 255. This just sets the Mach-O current version to 0 to get it building properly again. Fixes ggml-org#17258.
PR #17091 set the VERSION of various libraries to 0.0.abcd, where abcd is the LLAMA_BUILD_NUMBER. That build number is too large to fit in the Mach-O 'current version' field's 'micro' part, which only goes up to 255. This just sets the Mach-O current version to 0 to get it building properly again. Fixes #17258.
PR ggml-org#17091 set the VERSION of various libraries to 0.0.abcd, where abcd is the LLAMA_BUILD_NUMBER. That build number is too large to fit in the Mach-O 'current version' field's 'micro' part, which only goes up to 255. This just sets the Mach-O current version to 0 to get it building properly again. Fixes ggml-org#17258.



While working on a Yocto recipe for llama.cpp, I got some QA errors because the so files don't have a version set. This is an easy enough fix, you just have to specify one in Cmake.
I've done my best to set these with what I understand are appropriate values, but if I'm pulling in inappropriate variables for any of these, by all means let's fix them. The mtmd stuff in particular seemed a bit ambiguous, but even though it's a "subproject" I imagine the desire (for now) is to still release (and version) it together with llama.cpp as a whole?