Update CMakeLists.txt replace find_package with aws_use_package for aws-c-event-stream #1441
Conversation
…ws-c-event-stream NOTE: this only works if this project is used as a submodule alongside aws-c-common, AwsFindPackage.cmake might be required to be copied into this project.
cmake/AwsFindPackage.cmake
Outdated
| # together. Only use this on CRT libraries (including S2N), libcrypto will stay as an assumed external dependency. | ||
| # | ||
| # package_name: is the name of the package to find | ||
| # DEP_CRT_LIBS: output variable will be appended after each call to this function. You don't have to use it, |
There was a problem hiding this comment.
Presumably you meant DEP_AWS_LIBS instead of DEP_CRT_LIBS, here.
| # 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. |
There was a problem hiding this comment.
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. TheIN_SOURCE_BUILDoption determines how the dependency will be satisfied. For dependencies which are never satisfied as part of the build (such aslibcrypto),find_package()should be used instead of this function.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Hi there, thank you for your pull request. |
KaibaLopez
added
the
pr/work-in-progress
|
Hi @mattparks We've updated a lot in |
jmklix
added
needs-review
|
Closing this PR. Thanks for taking the time to make this PR, but it is now outdated thanks to some changes in the build process of this sdk. |
Issue #, if available:
When building alongside aws-c-event-stream find_package pulls in the system library instead of the in source target.
Description of changes:
This change brings over the concept of
aws_use_packagefrom aws-c-common to aws-cpp-sdk-core. I did not add any testing because withIN_SOURCE_BUILD=OFFthe same fine_package behaviour is maintained.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.