This is an automated email from the ASF dual-hosted git repository. kou pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push: new 2235a7ed40 GH-40228: [C++][CMake] Improve description why we need to initialize AWS C++ SDK in arrow-s3fs-test (#40229) 2235a7ed40 is described below commit 2235a7ed40b999d919d7d17cbb34097e819a5acf Author: Sutou Kouhei <k...@clear-code.com> AuthorDate: Tue Feb 27 13:54:26 2024 +0900 GH-40228: [C++][CMake] Improve description why we need to initialize AWS C++ SDK in arrow-s3fs-test (#40229) ### Rationale for this change Only static linking isn't important. Static linking + private symbols are important. ### What changes are included in this PR? Improve comment and macro name. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. * GitHub Issue: #40228 Authored-by: Sutou Kouhei <k...@clear-code.com> Signed-off-by: Sutou Kouhei <k...@clear-code.com> --- cpp/src/arrow/filesystem/CMakeLists.txt | 18 +++++++----------- cpp/src/arrow/filesystem/s3fs_test.cc | 6 +++--- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/cpp/src/arrow/filesystem/CMakeLists.txt b/cpp/src/arrow/filesystem/CMakeLists.txt index a42a8d0f8c..77e93223cd 100644 --- a/cpp/src/arrow/filesystem/CMakeLists.txt +++ b/cpp/src/arrow/filesystem/CMakeLists.txt @@ -71,17 +71,13 @@ if(ARROW_S3) get_target_property(AWS_CPP_SDK_S3_TYPE aws-cpp-sdk-s3 TYPE) # We need to initialize AWS C++ SDK for direct use (not via # arrow::fs::S3FileSystem) in arrow-s3fs-test if we use static AWS - # C++ SDK. Because AWS C++ SDK has internal static variables that - # aren't shared in libarrow and arrow-s3fs-test. It means that - # arrow::fs::InitializeS3() doesn't initialize AWS C++ SDK that is - # directly used in arrow-s3fs-test. - # - # But it seems that internal static variables in AWS C++ SDK are - # shared on macOS even if we link static AWS C++ SDK to both - # libarrow and arrow-s3fs-test. So we don't need to initialize AWS - # C++ SDK in arrow-s3fs-test on macOS. - if(AWS_CPP_SDK_S3_TYPE STREQUAL "STATIC_LIBRARY" AND NOT APPLE) - list(APPEND ARROW_S3FS_TEST_COMPILE_DEFINITIONS "AWS_CPP_SDK_S3_NOT_SHARED") + # C++ SDK and hide symbols of them. Because AWS C++ SDK has + # internal static variables that aren't shared in libarrow and + # arrow-s3fs-test. It means that arrow::fs::InitializeS3() doesn't + # initialize AWS C++ SDK that is directly used in arrow-s3fs-test. + if(AWS_CPP_SDK_S3_TYPE STREQUAL "STATIC_LIBRARY" + AND CXX_LINKER_SUPPORTS_VERSION_SCRIPT) + list(APPEND ARROW_S3FS_TEST_COMPILE_DEFINITIONS "AWS_CPP_SDK_S3_PRIVATE_STATIC") endif() target_compile_definitions(arrow-s3fs-test PRIVATE ${ARROW_S3FS_TEST_COMPILE_DEFINITIONS}) diff --git a/cpp/src/arrow/filesystem/s3fs_test.cc b/cpp/src/arrow/filesystem/s3fs_test.cc index 33e9712a66..394f59e91a 100644 --- a/cpp/src/arrow/filesystem/s3fs_test.cc +++ b/cpp/src/arrow/filesystem/s3fs_test.cc @@ -150,7 +150,7 @@ class ShortRetryStrategy : public S3RetryStrategy { class AwsTestMixin : public ::testing::Test { public: void SetUp() override { -#ifdef AWS_CPP_SDK_S3_NOT_SHARED +#ifdef AWS_CPP_SDK_S3_PRIVATE_STATIC auto aws_log_level = Aws::Utils::Logging::LogLevel::Fatal; aws_options_.loggingOptions.logLevel = aws_log_level; aws_options_.loggingOptions.logger_create_fn = [&aws_log_level] { @@ -161,13 +161,13 @@ class AwsTestMixin : public ::testing::Test { } void TearDown() override { -#ifdef AWS_CPP_SDK_S3_NOT_SHARED +#ifdef AWS_CPP_SDK_S3_PRIVATE_STATIC Aws::ShutdownAPI(aws_options_); #endif } private: -#ifdef AWS_CPP_SDK_S3_NOT_SHARED +#ifdef AWS_CPP_SDK_S3_PRIVATE_STATIC Aws::SDKOptions aws_options_; #endif };