Skip to content

Update CMakeLists.txt replace find_package with aws_use_package for aws-c-event-stream #1441

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Update CMakeLists.txt replace find_package with aws_use_package for aws-c-event-stream #1441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
73e1ac7
Update CMakeLists.txt replace find_package with aws_use_package for a…
mattparks Aug 21, 2020
9012eea
Provide aws_use_package incase building outside aws-c-common
mattparks Aug 21, 2020
48b4e06
Fixed documentation typo in aws_use_package docs
mattparks Mar 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions aws-cpp-sdk-core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
add_project(aws-cpp-sdk-core "Core http and utility library for the AWS C++ SDK")

include(AwsFindPackage)

if(VERSION_STRING)
set(AWSSDK_VERSION_STRING ${VERSION_STRING})
message(STATUS "Updating version info to ${VERSION_STRING}")
Expand Down Expand Up @@ -493,9 +495,9 @@ endif()

target_link_libraries(${PROJECT_NAME} PRIVATE ${PLATFORM_DEP_LIBS} ${CLIENT_LIBS} ${CRYPTO_LIBS} ${AWS_SDK_ADDITIONAL_LIBRARIES})

find_package(aws-c-event-stream REQUIRED)
aws_use_package(aws-c-event-stream)

target_link_libraries(${PROJECT_NAME} PUBLIC AWS::aws-c-event-stream)
target_link_libraries(${PROJECT_NAME} PUBLIC ${DEP_AWS_LIBS})


if(USE_WINDOWS_DLL_SEMANTICS)
Expand Down
22 changes: 22 additions & 0 deletions cmake/AwsFindPackage.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
# SPDX-License-Identifier: Apache-2.0.

option(IN_SOURCE_BUILD "If the CRT libs are being built from your source tree (add_subdirectory), set this to ON" OFF)

# This function handles dependency list building based on if traditional CMAKE modules via. find_package should be
# used, vs if this is an in source build via. something like git submodules and add_subdirectory.
# This is largely because CMake was not well planned out, and as a result, in-source and modules don't play well
# together. Only use this on CRT libraries (including S2N), libcrypto will stay as an assumed external dependency.
Comment on lines +6 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rather tortured English. And, presumably there is no need to comment on whether CMake, etc. was "well planned" or not. And, it's unclear (at least to me) what you mean by "CRT libraries" (i.e., presumably you don't mean the C Language Run-Time Library or any of the analogous C++ Language ones).

How about something like:

This function handles building a list of external dependencies, taking into account whether the dependency is built as a submodule or found elsewhere on the build platform. It should be used as a replacement for find_package() in cases where the dependency might satisfied either by using an installed library or by building it as part of the build. The IN_SOURCE_BUILD option determines how the dependency will be satisfied. For dependencies which are never satisfied as part of the build (such as libcrypto), find_package() should be used instead of this function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is from https://github.com/awslabs/aws-c-common/blob/master/cmake/AwsFindPackage.cmake
If you want me to update the comment I can add to this pr.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry -- Github's presentation made it look like this was a new file that you were adding, so I figured it would be best if it had better verbiage before it was merged in.

In any case, I was just making a suggestion -- I'm not an aws-sdk-cpp maintainer, so I'll defer to them on what they want.

#
# package_name: is the name of the package to find
# DEP_AWS_LIBS: output variable will be appended after each call to this function. You don't have to use it,
# but it can be passed directly target_link_libraries and it will be the properly qualified library
# name and namespace based on configuration.
function(aws_use_package package_name)
if (IN_SOURCE_BUILD)
set(DEP_AWS_LIBS ${DEP_AWS_LIBS} ${package_name} PARENT_SCOPE)
else()
find_package(${package_name} REQUIRED)
set(DEP_AWS_LIBS ${DEP_AWS_LIBS} AWS::${package_name} PARENT_SCOPE)
endif()
endfunction()