-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
aws-c-compression: add missing interface definition if shared + modernize more for conan v2 #17109
aws-c-compression: add missing interface definition if shared + modernize more for conan v2 #17109
Conversation
df6102b
to
b5d36e7
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 6 (
Conan v2 pipeline ✔️
All green in build 5 ( |
find_package(aws-c-compression REQUIRED CONFIG) | ||
|
||
add_executable(${PROJECT_NAME} ../test_package/test_package.c) | ||
target_link_libraries(${PROJECT_NAME} PRIVATE AWS::aws-c-compression) |
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.
Why removing this find_package()? I mean, if we are still generating the alias targets to maintain compatibility, shouldn't we still keep this?
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 don't understand, it's a common pattern we have in all modern recipes. It avoids code duplication and mistakes by ensuring a common CMakeLists in test package & test_v1_package, so find_package is not removed at all, but delegated to CMakeLists of test_package.
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.
oh sorry, I didn't read it well. It makes sense
…if shared + modernize more for conan v2 * add AWS_COMPRESSION_USE_IMPORT_EXPORT interface definition if shared * modernize more for conan v2 * more elegant way to define target for legacy generators * aws-c-common is public
see https://github.com/awslabs/aws-c-compression/blob/v0.2.16/include/aws/compression/exports.h