kou commented on code in PR #33808:
URL: https://github.com/apache/arrow/pull/33808#discussion_r1084831937
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -4673,7 +4761,19 @@ macro(build_awssdk)
aws-cpp-sdk-core
aws-c-event-stream
aws-checksums
- aws-c-common)
+ aws-c-common
+ aws-c-auth
+ aws-c-cal
+ aws-c-compression
+ aws-c-http
+ aws-c-io
+ aws-c-mqtt
+ aws-c-s3
+ aws-c-sdkutils
+ s2n
+ crypto
+ ssl
Review Comment:
Are they artifacts by `s2n-tls`?
Then we should not create `AWS::crypto` and `AWS::ssl`. We should create
`AWS::s2n-tls` and associate `libcrypto.a` and `libssl.a` with `AWS::s2n-tls`.
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -455,6 +455,93 @@ else()
)
endif()
+if(DEFINED ENV{ARROW_AWS_C_AUTH_URL})
+ set(AWS_C_AUTH_SOURCE_URL "$ENV{ARROW_AWS_C_AUTH_URL}")
+else()
+ set_urls(AWS_C_AUTH_SOURCE_URL
+
"https://github.com/awslabs/aws-c-auth/archive/${ARROW_AWS_C_AUTH_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_AWS_C_CAL_URL})
+ set(AWS_C_CAL_SOURCE_URL "$ENV{ARROW_AWS_C_CAL_URL}")
+else()
+ set_urls(AWS_C_CAL_SOURCE_URL
+
"https://github.com/awslabs/aws-c-cal/archive/${ARROW_AWS_C_CAL_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_AWS_C_COMPRESSION_URL})
+ set(AWS_C_COMPRESSION_SOURCE_URL "$ENV{ARROW_AWS_C_COMPRESSION_URL}")
+else()
+ set_urls(AWS_C_COMPRESSION_SOURCE_URL
+
"https://github.com/awslabs/aws-c-compression/archive/${ARROW_AWS_C_COMPRESSION_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_AWS_C_HTTP_URL})
+ set(AWS_C_HTTP_SOURCE_URL "$ENV{ARROW_AWS_C_HTTP_URL}")
+else()
+ set_urls(AWS_C_HTTP_SOURCE_URL
+
"https://github.com/awslabs/aws-c-http/archive/${ARROW_AWS_C_HTTP_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_AWS_C_IO_URL})
+ set(AWS_C_IO_SOURCE_URL "$ENV{ARROW_AWS_C_IO_URL}")
+else()
+ set_urls(AWS_C_IO_SOURCE_URL
+
"https://github.com/awslabs/aws-c-io/archive/${ARROW_AWS_C_IO_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_AWS_C_MQTT_URL})
+ set(AWS_C_MQTT_SOURCE_URL "$ENV{ARROW_AWS_C_MQTT_URL}")
+else()
+ set_urls(AWS_C_MQTT_SOURCE_URL
+
"https://github.com/awslabs/aws-c-mqtt/archive/${ARROW_AWS_C_MQTT_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_AWS_C_S3_URL})
+ set(AWS_C_S3_SOURCE_URL "$ENV{ARROW_AWS_C_S3_URL}")
+else()
+ set_urls(AWS_C_S3_SOURCE_URL
+
"https://github.com/awslabs/aws-c-s3/archive/${ARROW_AWS_C_S3_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_AWS_C_SDKUTILS_URL})
+ set(AWS_C_SDKUTILS_SOURCE_URL "$ENV{ARROW_AWS_C_SDKUTILS_URL}")
+else()
+ set_urls(AWS_C_SDKUTILS_SOURCE_URL
+
"https://github.com/awslabs/aws-c-sdkutils/archive/${ARROW_AWS_C_SDKUTILS_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_AWS_LC_URL})
+ set(AWS_LC_SOURCE_URL "$ENV{ARROW_AWS_LC_URL}")
+else()
+ set_urls(AWS_LC_SOURCE_URL
+
"https://github.com/awslabs/aws-lc/archive/${ARROW_AWS_LC_BUILD_VERSION}.tar.gz";
+ )
+endif()
+
+if(DEFINED ENV{ARROW_S2N_URL})
+ set(S2N_SOURCE_URL "$ENV{ARROW_S2N_URL}")
+else()
+ set_urls(S2N_SOURCE_URL
+
"https://github.com/awslabs/s2n/archive/${ARROW_S2N_BUILD_VERSION}.tar.gz";)
+endif()
Review Comment:
`s2n` -> `s2n-tls`?
https://github.com/aws/s2n is redirected to https://github.com/aws/s2n-tls .
##########
cpp/thirdparty/versions.txt:
##########
@@ -107,6 +129,17 @@ DEPENDENCIES=(
"ARROW_AWS_CHECKSUMS_URL
aws-checksums-${ARROW_AWS_CHECKSUMS_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-checksums/archive/${ARROW_AWS_CHECKSUMS_BUILD_VERSION}.tar.gz";
"ARROW_AWS_C_COMMON_URL
aws-c-common-${ARROW_AWS_C_COMMON_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-common/archive/${ARROW_AWS_C_COMMON_BUILD_VERSION}.tar.gz";
"ARROW_AWS_C_EVENT_STREAM_URL
aws-c-event-stream-${ARROW_AWS_C_EVENT_STREAM_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-event-stream/archive/${ARROW_AWS_C_EVENT_STREAM_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_C_AUTH_URL aws-c-auth-${ARROW_AWS_C_AUTH_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-auth/archive/${ARROW_AWS_C_AUTH_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_C_CAL_URL aws-c-cal-${ARROW_AWS_C_CAL_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-cal/archive/${ARROW_AWS_C_CAL_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_C_COMPRESSION_URL
aws-c-compression-${ARROW_AWS_C_COMPRESSION_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-compression/archive/${ARROW_AWS_C_COMPRESSION_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_C_HTTP_URL aws-c-http-${ARROW_AWS_C_HTTP_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-http/archive/${ARROW_AWS_C_HTTP_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_C_IO_URL aws-c-io-${ARROW_AWS_C_IO_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-io/archive/${ARROW_AWS_C_IO_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_C_MQTT_URL aws-c-mqtt-${ARROW_AWS_C_MQTT_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-mqtt/archive/${ARROW_AWS_C_MQTT_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_C_S3_URL aws-c-s3-${ARROW_AWS_C_S3_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-s3/archive/${ARROW_AWS_C_S3_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_C_SDKUTILS_URL
aws-c-sdkutils-${ARROW_AWS_C_SDKUTILS_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-c-sdkutils/archive/${ARROW_AWS_C_SDKUTILS_BUILD_VERSION}.tar.gz";
+ "ARROW_AWS_LC_URL aws-lc-${ARROW_AWS_LC_BUILD_VERSION}.tar.gz
https://github.com/awslabs/aws-lc/archive/${ARROW_AWS_LC_BUILD_VERSION}.tar.gz";
+ "ARROW_S2N_URL s2n-${ARROW_S2N_BUILD_VERSION}.tar.gz
https://github.com/awslabs/s2n/archive/${ARROW_S2N_BUILD_VERSION}.tar.gz";
Review Comment:
Could you keep alphabetical order?
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -4612,6 +4699,7 @@ endif()
macro(build_awssdk)
message(STATUS "Building AWS C++ SDK from source")
set(AWSSDK_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/awssdk_ep-install")
+ set(AWSSDK_SOURCE
"${CMAKE_CURRENT_BINARY_DIR}/awssdk_ep-prefix/src/awssdk_ep")
Review Comment:
```suggestion
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -4714,35 +4814,145 @@ macro(build_awssdk)
DEPENDS aws_c_common_ep)
add_dependencies(AWS::aws-checksums aws_checksums_ep)
+ externalproject_add(s2n_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${S2N_SOURCE_URL}
+ URL_HASH "SHA256=${ARROW_S2N_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${S2N_STATIC_LIBRARY})
+ add_dependencies(AWS::s2n s2n_ep)
+
+ externalproject_add(aws_c_cal_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_C_CAL_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_AWS_C_CAL_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_C_CAL_STATIC_LIBRARY}
+ DEPENDS aws_c_common_ep)
+ add_dependencies(AWS::aws-c-cal aws_c_cal_ep)
+
+ externalproject_add(aws_c_io_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_C_IO_SOURCE_URL}
+ URL_HASH "SHA256=${ARROW_AWS_C_IO_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_C_IO_STATIC_LIBRARY}
+ DEPENDS aws_c_common_ep s2n_ep aws_c_cal_ep)
+ add_dependencies(AWS::aws-c-io aws_c_io_ep)
+
externalproject_add(aws_c_event_stream_ep
${EP_COMMON_OPTIONS}
URL ${AWS_C_EVENT_STREAM_SOURCE_URL}
URL_HASH
"SHA256=${ARROW_AWS_C_EVENT_STREAM_BUILD_SHA256_CHECKSUM}"
CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
BUILD_BYPRODUCTS ${AWS_C_EVENT_STREAM_STATIC_LIBRARY}
- DEPENDS aws_checksums_ep)
+ DEPENDS aws_checksums_ep s2n_ep aws_c_io_ep)
add_dependencies(AWS::aws-c-event-stream aws_c_event_stream_ep)
- set(AWSSDK_PATCH_COMMAND)
- if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION
VERSION_GREATER
- "10")
- # Workaround for https://github.com/aws/aws-sdk-cpp/issues/1750
- set(AWSSDK_PATCH_COMMAND "sed" "-i.bak" "-e" "s/\"-Werror\"//g"
- "<SOURCE_DIR>/cmake/compiler_settings.cmake")
- endif()
+ externalproject_add(aws_c_sdkutils_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_C_SDKUTILS_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_AWS_C_SDKUTILS_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_C_SDKUTILS_STATIC_LIBRARY}
+ DEPENDS aws_c_common_ep)
+ add_dependencies(AWS::aws-c-sdkutils aws_c_sdkutils_ep)
+
+ externalproject_add(aws_c_compression_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_C_COMPRESSION_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_AWS_C_COMPRESSION_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_C_COMPRESSION_STATIC_LIBRARY}
+ DEPENDS aws_c_common_ep)
+ add_dependencies(AWS::aws-c-compression aws_c_compression_ep)
+
+ externalproject_add(aws_c_http_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_C_HTTP_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_AWS_C_HTTP_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_C_HTTP_STATIC_LIBRARY}
+ DEPENDS aws_c_common_ep s2n_ep aws_c_io_ep)
+ add_dependencies(AWS::aws-c-http aws_c_http_ep)
+
+ externalproject_add(aws_c_mqtt_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_C_MQTT_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_AWS_C_MQTT_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_C_MQTT_STATIC_LIBRARY}
+ DEPENDS aws_c_common_ep s2n_ep aws_c_io_ep aws_c_http_ep)
+ add_dependencies(AWS::aws-c-mqtt aws_c_mqtt_ep)
+
+ externalproject_add(aws_c_auth_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_C_AUTH_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_AWS_C_AUTH_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_C_AUTH_STATIC_LIBRARY}
+ DEPENDS aws_c_common_ep
+ aws_c_sdkutils_ep
+ s2n_ep
+ aws_c_io_ep
+ aws_c_http_ep)
+ add_dependencies(AWS::aws-c-auth aws_c_auth_ep)
+
+ externalproject_add(aws_c_s3_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_C_S3_SOURCE_URL}
+ URL_HASH "SHA256=${ARROW_AWS_C_S3_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_C_S3_STATIC_LIBRARY}
+ DEPENDS aws_c_common_ep
+ aws_c_cal_ep
+ s2n_ep
+ aws_c_io_ep
+ aws_c_http_ep
+ aws_c_auth_ep)
+ add_dependencies(AWS::aws-c-s3 aws_c_s3_ep)
+
+ externalproject_add(aws_lc_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_LC_SOURCE_URL}
+ URL_HASH "SHA256=${ARROW_AWS_LC_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${CRYPTO_STATIC_LIBRARY}
${SSL_STATIC_LIBRARY})
+ add_dependencies(AWS::crypto aws_lc_ep)
+ add_dependencies(AWS::ssl aws_lc_ep)
+
+ externalproject_add(aws_crt_cpp_ep
+ ${EP_COMMON_OPTIONS}
+ URL ${AWS_CRT_CPP_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_AWS_CRT_CPP_BUILD_SHA256_CHECKSUM}"
+ CMAKE_ARGS ${AWSSDK_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${AWS_CRT_CPP_STATIC_LIBRARY}
+ DEPENDS aws_c_auth_ep
+ aws_c_cal_ep
+ aws_c_common_ep
+ aws_c_compression_ep
+ aws_c_event_stream_ep
+ aws_c_http_ep
+ aws_c_io_ep
+ aws_c_mqtt_ep
+ aws_c_s3_ep
+ aws_c_sdkutils_ep
+ aws_checksums_ep
+ aws_lc_ep
+ s2n_ep)
+ add_dependencies(AWS::aws-crt-cpp aws_crt_cpp_ep)
externalproject_add(awssdk_ep
${EP_COMMON_OPTIONS}
URL ${AWSSDK_SOURCE_URL}
URL_HASH "SHA256=${ARROW_AWSSDK_BUILD_SHA256_CHECKSUM}"
CMAKE_ARGS ${AWSSDK_CMAKE_ARGS}
- PATCH_COMMAND ${AWSSDK_PATCH_COMMAND}
BUILD_BYPRODUCTS
${AWS_CPP_SDK_COGNITO_IDENTITY_STATIC_LIBRARY}
${AWS_CPP_SDK_CORE_STATIC_LIBRARY}
${AWS_CPP_SDK_IDENTITY_MANAGEMENT_STATIC_LIBRARY}
${AWS_CPP_SDK_S3_STATIC_LIBRARY}
${AWS_CPP_SDK_STS_STATIC_LIBRARY}
- DEPENDS aws_c_event_stream_ep)
+ DEPENDS aws_c_event_stream_ep aws_crt_cpp_ep)
Review Comment:
Can we remove `aws_c_event_stream_ep` here because `aws_crt_cpp_ep` depends
on `aws_c_event_stream_ep`?
```suggestion
DEPENDS aws_crt_cpp_ep)
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
