You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/07/26 09:58:46 UTC
[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1140: MINIFICPP-1611 Update AWS SDK to latest released version
szaszm commented on a change in pull request #1140:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1140#discussion_r676455014
##########
File path: cmake/BundledAwsSdkCpp.cmake
##########
@@ -30,87 +30,59 @@ function(use_bundled_libaws SOURCE_DIR BINARY_DIR)
set(SUFFIX "a")
set(PREFIX "lib")
endif()
- set(BYPRODUCTS
- "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-cpp-sdk-core.${SUFFIX}"
- "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-cpp-sdk-s3.${SUFFIX}")
+
+ if (WIN32 OR APPLE)
+ set(BYPRODUCTS
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-checksums.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-event-stream.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-s3.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-crt-cpp.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-common.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-mqtt.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-io.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-http.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-auth.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-cal.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-compression.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-cpp-sdk-core.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-cpp-sdk-s3.${SUFFIX}")
+ else()
+ set(BYPRODUCTS
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}s2n.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-checksums.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-event-stream.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-s3.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-crt-cpp.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-common.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-mqtt.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-io.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-http.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-auth.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-cal.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-compression.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-cpp-sdk-core.${SUFFIX}"
+ "${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-cpp-sdk-s3.${SUFFIX}")
+ endif()
Review comment:
Let's extract the common part!
Do we actually need all of these libs by the way?
```suggestion
if (NOT WIN32 AND NOT APPLE)
list(APPEND BYPRODUCTS "${CMAKE_INSTALL_LIBDIR}/${PREFIX}s2n.${SUFFIX}")
endif()
list(APPEND BYPRODUCTS
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-checksums.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-event-stream.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-s3.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-crt-cpp.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-common.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-mqtt.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-io.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-http.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-auth.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-cal.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-compression.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-cpp-sdk-core.${SUFFIX}"
"${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-cpp-sdk-s3.${SUFFIX}")
```
##########
File path: .github/workflows/ci.yml
##########
@@ -80,6 +80,8 @@ jobs:
env:
CLCACHE_DIR: ${{ GITHUB.WORKSPACE }}\clcache
steps:
+ - name: Support longpaths
+ run: git config --system core.longpaths true
Review comment:
Did you run into issues? What were they? If it's an easy change, I would prefer avoiding long paths, as there may be tools that don't support them.
##########
File path: cmake/BundledAwsSdkCpp.cmake
##########
@@ -120,71 +92,100 @@ function(use_bundled_libaws SOURCE_DIR BINARY_DIR)
)
# Set dependencies
- add_dependencies(aws-c-common-external CURL::libcurl OpenSSL::Crypto OpenSSL::SSL ZLIB::ZLIB)
- add_dependencies(aws-checksum-external aws-c-common-external CURL::libcurl OpenSSL::Crypto OpenSSL::SSL ZLIB::ZLIB)
- add_dependencies(aws-c-event-stream-external CURL::libcurl OpenSSL::Crypto OpenSSL::SSL ZLIB::ZLIB)
- add_dependencies(aws-c-event-stream-external aws-c-common-external aws-checksum-external)
add_dependencies(aws-sdk-cpp-external CURL::libcurl OpenSSL::Crypto OpenSSL::SSL ZLIB::ZLIB)
- add_dependencies(aws-sdk-cpp-external aws-c-event-stream-external aws-c-common-external aws-checksum-external)
# Set variables
set(LIBAWS_FOUND "YES" CACHE STRING "" FORCE)
set(LIBAWS_INCLUDE_DIR "${BINARY_DIR}/thirdparty/libaws-install/include" CACHE STRING "" FORCE)
set(LIBAWS_LIBRARIES
${AWSSDK_LIBRARIES_LIST}
- "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-event-stream.${SUFFIX}"
- "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-common.${SUFFIX}"
- "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-checksums.${SUFFIX}"
CACHE STRING "" FORCE)
# Create imported targets
file(MAKE_DIRECTORY ${LIBAWS_INCLUDE_DIR})
add_library(AWS::aws-c-common STATIC IMPORTED)
set_target_properties(AWS::aws-c-common PROPERTIES IMPORTED_LOCATION "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-common.${SUFFIX}")
- add_dependencies(AWS::aws-c-common aws-c-common-external)
- set_property(TARGET AWS::aws-c-common APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${LIBAWS_INCLUDE_DIR})
- set_property(TARGET AWS::aws-c-common APPEND PROPERTY INTERFACE_LINK_LIBRARIES CURL::libcurl OpenSSL::Crypto OpenSSL::SSL ZLIB::ZLIB Threads::Threads)
- if (APPLE)
- set_property(TARGET AWS::aws-c-common APPEND PROPERTY INTERFACE_LINK_LIBRARIES "-framework CoreFoundation")
+ add_dependencies(AWS::aws-c-common aws-sdk-cpp-external)
+ target_include_directories(AWS::aws-c-common INTERFACE ${LIBAWS_INCLUDE_DIR})
+
+ if (NOT WIN32 AND NOT APPLE)
+ add_library(AWS::s2n STATIC IMPORTED)
+ set_target_properties(AWS::s2n PROPERTIES IMPORTED_LOCATION "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}s2n.${SUFFIX}")
+ add_dependencies(AWS::s2n aws-sdk-cpp-external)
+ target_include_directories(AWS::s2n INTERFACE ${LIBAWS_INCLUDE_DIR})
endif()
+ add_library(AWS::aws-c-io STATIC IMPORTED)
+ set_target_properties(AWS::aws-c-io PROPERTIES IMPORTED_LOCATION "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-io.${SUFFIX}")
+ add_dependencies(AWS::aws-c-io aws-sdk-cpp-external)
+ target_include_directories(AWS::aws-c-io INTERFACE ${LIBAWS_INCLUDE_DIR})
+ target_link_libraries(AWS::aws-c-io INTERFACE AWS::aws-c-common)
+
add_library(AWS::aws-checksums STATIC IMPORTED)
set_target_properties(AWS::aws-checksums PROPERTIES IMPORTED_LOCATION "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-checksums.${SUFFIX}")
- add_dependencies(AWS::aws-checksums aws-checksums-external)
- set_property(TARGET AWS::aws-checksums APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${LIBAWS_INCLUDE_DIR})
- set_property(TARGET AWS::aws-checksums APPEND PROPERTY INTERFACE_LINK_LIBRARIES CURL::libcurl OpenSSL::Crypto OpenSSL::SSL ZLIB::ZLIB Threads::Threads)
- if (APPLE)
- set_property(TARGET AWS::aws-checksums APPEND PROPERTY INTERFACE_LINK_LIBRARIES "-framework CoreFoundation")
- endif()
+ add_dependencies(AWS::aws-checksums aws-sdk-cpp-external)
+ target_include_directories(AWS::aws-checksums INTERFACE ${LIBAWS_INCLUDE_DIR})
add_library(AWS::aws-c-event-stream STATIC IMPORTED)
set_target_properties(AWS::aws-c-event-stream PROPERTIES IMPORTED_LOCATION "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-event-stream.${SUFFIX}")
- add_dependencies(AWS::aws-c-event-stream aws-c-event-stream-external)
- set_property(TARGET AWS::aws-c-event-stream APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${LIBAWS_INCLUDE_DIR})
- set_property(TARGET AWS::aws-c-event-stream APPEND PROPERTY INTERFACE_LINK_LIBRARIES AWS::aws-c-common AWS::aws-checksums CURL::libcurl OpenSSL::Crypto OpenSSL::SSL ZLIB::ZLIB Threads::Threads)
- if (APPLE)
- set_property(TARGET AWS::aws-c-event-stream APPEND PROPERTY INTERFACE_LINK_LIBRARIES "-framework CoreFoundation")
- endif()
+ add_dependencies(AWS::aws-c-event-stream aws-sdk-cpp-external)
+ target_include_directories(AWS::aws-c-event-stream INTERFACE ${LIBAWS_INCLUDE_DIR})
+ target_link_libraries(AWS::aws-c-event-stream INTERFACE AWS::aws-checksums AWS::aws-c-io)
+
+ add_library(AWS::aws-c-auth STATIC IMPORTED)
+ set_target_properties(AWS::aws-c-auth PROPERTIES IMPORTED_LOCATION "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-auth.${SUFFIX}")
+ add_dependencies(AWS::aws-c-auth aws-sdk-cpp-external)
+ target_include_directories(AWS::aws-c-auth INTERFACE ${LIBAWS_INCLUDE_DIR})
+
+ add_library(AWS::aws-c-s3 STATIC IMPORTED)
+ set_target_properties(AWS::aws-c-s3 PROPERTIES IMPORTED_LOCATION "${BINARY_DIR}/thirdparty/libaws-install/${CMAKE_INSTALL_LIBDIR}/${PREFIX}aws-c-s3.${SUFFIX}")
+ add_dependencies(AWS::aws-c-s3 aws-sdk-cpp-external)
+ target_include_directories(AWS::aws-c-s3 INTERFACE ${LIBAWS_INCLUDE_DIR})
+ target_link_libraries(AWS::aws-c-s3 INTERFACE AWS::aws-c-auth)
Review comment:
Did you consider switching to s3-crt? https://github.com/aws/aws-sdk-cpp/wiki/Improving-S3-Throughput-with-AWS-SDK-for-CPP-v1.9#working-with-the-new-s3-crt-client
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org