-
Notifications
You must be signed in to change notification settings - Fork 14.7k
cmake: add option to build and link BoringSSL #17205
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
|
This does not build on Mac: cmake -B build-boring -DLLAMA_BUILD_BORINGSSL=ON
cmake --build build-boring -j |
|
It worked for me because I tested OpenSSL just before I guess.. |
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
|
Fixed, I thought I could put the library in |
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <angt@huggingface.co>
|
I've added the ci for windows and macos @ggerganov do you want to update release in the same PR? |
CISC
left a comment
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.
On an (un)related note I see we have these:
llama.cpp/.github/workflows/build.yml
Line 92 in 2ba8b25
| ctest -L 'main|curl' --verbose --timeout 900 |
llama.cpp/.github/workflows/build.yml
Line 232 in 2ba8b25
| ctest -L 'main|curl' --verbose --timeout 900 |
But I can't see that we are using the
curl test label anywhere?
|
I've found this: llama.cpp/examples/eval-callback/CMakeLists.txt Lines 7 to 15 in 845f200
|
|
I've used OpenSSL for linux: #17235 For windows and macos, I really recommend BoringSSL. I know the message from Google is kind of scary, but the OpenSSL API has always been respected (they can't patch the entire world to link against BoringSSL anyway) and it is the best choice in my opinion, for safety and portability. That's said, I plan to add other libraries in the future, mostly to reduce compilation time by using the native OS stack when available. |
ggerganov
left a comment
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 see, I missed #17235
I think this change for the CI is OK.
For my understanding, what is the reason for preferring BoringSSL over OpenSSL for Windows and Mac?
|
I use OpenSSL on Linux only because it's super easy to have the |
|
If the only concern is build time, I think that for the releases we'll want to use OpenSSL as it is not that important to reduce the build time there. |
|
@ggerganov, just thinking, why not keep |
|
The other ggufs in The |
* cmake: add option to build and link BoringSSL Signed-off-by: Adrien Gallouët <angt@huggingface.co> * cmake : fix typo Signed-off-by: Adrien Gallouët <angt@huggingface.co> * cmake : disable boringssl test and asm by default Signed-off-by: Adrien Gallouët <angt@huggingface.co> * cmake : skip bssl Signed-off-by: Adrien Gallouët <angt@huggingface.co> * cmake : disable fips Signed-off-by: Adrien Gallouët <angt@huggingface.co> * cmake : fix cmake --install Signed-off-by: Adrien Gallouët <angt@huggingface.co> * ci : use boringssl for windows and mac Signed-off-by: Adrien Gallouët <angt@huggingface.co> --------- Signed-off-by: Adrien Gallouët <angt@huggingface.co>

No description provided.