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
};
