-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Adding version detection function to new build #1931
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
@@ -0,0 +1,20 @@ | |||
include_guard() |
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.
Equivalent to pragma_once in C++, no need to include this twice.
set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE | ||
STRING "Choose the type of build." FORCE) | ||
# cmake-gui helper | ||
set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" "MinSizeRel" "RelWithDebInfo") |
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.
This helps gui tools as cmake to show only a list of valid options to configure the CMAKE_BUILD_TYPE variable in place of having a free form string as input.
endif () | ||
|
||
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake") | ||
include(sdk_versioning) |
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 module has no side effects, it contains only function/macro definitions.
|
||
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake") | ||
include(sdk_versioning) | ||
obtain_project_version(SDK_PROJECT_VERSION) |
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 first argument passed to the function is the name of the return value variable expected.
message(STATUS "Detecting toolchain and external dependencies.") | ||
message(STATUS "Prepare CRT dependency.") | ||
message(STATUS "Setting up core library.") | ||
message(STATUS "Building core library.") | ||
message(STATUS "Building core library tests.") | ||
message(STATUS "Add support for static analysis.") | ||
message(STATUS "Building client libraries.") | ||
message(STATUS "Setting up tests.") | ||
message(STATUS "Setting up installation.") | ||
message(STATUS "Setting up packaging. ") | ||
message(STATUS "Setting up docs.") | ||
message(STATUS "Add support support for SDK flags.") | ||
message(STATUS "Add previously available options.") |
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.
TODO items
find_package(Git QUIET) | ||
if (GIT_FOUND) | ||
execute_process(COMMAND ${GIT_EXECUTABLE} --git-dir=${CMAKE_CURRENT_SOURCE_DIR}/.git describe --abbrev=0 --tags | ||
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} | ||
OUTPUT_VARIABLE VERSION_STRING | ||
OUTPUT_STRIP_TRAILING_WHITESPACE) | ||
endif () |
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.
Trying to infer version from git tags if available.
if (NOT VERSION_STRING) | ||
# extract it from the existing generated header file | ||
file(STRINGS "${CMAKE_CURRENT_SOURCE_DIR}/src/aws-cpp-sdk-core/include/aws/core/VersionConfig.h" __SDK_VERSION_LINE LIMIT_COUNT 1 REGEX "AWS_SDK_VERSION_STRING.*[0-9]+\\.[0-9]+\\.[0-9]+") | ||
string(REGEX MATCH "([0-9]+\\.[0-9]+\\.[0-9]+)" VERSION_STRING "${__SDK_VERSION_LINE}") | ||
endif () |
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.
Obtained version from previously generated config file.
endif () | ||
|
||
set(${resultVar} ${VERSION_STRING} PARENT_SCOPE) | ||
message(STATUS "SDK project version: ${VERSION_STRING}") |
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.
This should be DEBUG message but they are not available at the minimal cmake version we using
string(REGEX MATCH "([0-9]+\\.[0-9]+\\.[0-9]+)" VERSION_STRING "${__SDK_VERSION_LINE}") | ||
endif () | ||
|
||
set(${resultVar} ${VERSION_STRING} PARENT_SCOPE) |
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.
returning the version to the parent scope
@@ -0,0 +1,20 @@ | |||
include_guard() | |||
|
|||
function(obtain_project_version resultVar) |
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.
Preferring function over macro to keep parent scope clean
Issue #, if available:
Description of changes:
Since this is the first new function we introducing in the build system and a few things are different to those in the LEGACY implementation I will comment on most of its lines in this PR to highlight the changes and their reasons.
This particular PR is the best place to discuss about those decisions, that may seem small here, but will affect the whole build system code style and practices long term.
Check all that applies:
README changes coming in a follow up PR after the build system is completely refactored.
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.