-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
build dev 1.10 pr8 #1935
build dev 1.10 pr8 #1935
Conversation
@@ -40,9 +40,14 @@ else () | |||
set(CMAKE_CXX_EXTENSIONS OFF) | |||
set(CMAKE_CXX_STANDARD_REQUIRED ON) | |||
|
|||
message(STATUS "Building ${PROJECT_NAME} ${CMAKE_PROJECT_VERSION}") | |||
# Setting build to hide symbols in targets by default | |||
set(CMAKE_CXX_VISIBILITY_PRESET hidden) |
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.
will it also hide all C-symbols not marked as static from C translation units?
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.
It seems like this only affected C++ until version 3.12 of CMAKE, not applies also to C.
https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html?highlight=cmake_cxx_visibility_preset
I will leave it like this for now, we shouldn't expose anything C at this SDK right now, but worth to dig into this further for future use cases.
# Project dependencies http library (curl or OS), tinyxml2, cJSON, ssl (open or other), CRT | ||
find_package(CURL REQUIRED) | ||
find_package(ZLIB REQUIRED) | ||
find_package(OpenSSL REQUIRED COMPONENTS Crypto) |
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.
What about aws-lc and s2n?
There are so much done in CRT to use aws-lc.
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.
The comment below, this is a "basic Linux" build to start exploring full build, more dependencies will be added for support more platforms along the path before this branch is merged we want parity with what we supported before.
985ca76
to
b9a2259
Compare
Issue #1888 , if available:
Description of changes:
Added logic to detect dependencies and setup compiler basic options. This is an early draft, still needs more dependencies to deal with other platforms.
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.