Skip to content

Commit

Permalink
Disable s3fs-test with bundled aws-sdk-cpp
Browse files Browse the repository at this point in the history
  • Loading branch information
kou committed Feb 8, 2023
1 parent 0f80ec8 commit 8d2eebe
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 48 deletions.
8 changes: 1 addition & 7 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -4821,17 +4821,11 @@ macro(build_awssdk)
DEPENDS aws_c_common_ep)
add_dependencies(AWS::aws-checksums aws_checksums_ep)

# When arrow is dynamically linked, arrow-s3fs-test and libarrow.so each has its own copy of AWS SDK.
# Need S2N_TLS to statically link OpenSSL::Crypto and internalize it to avoid conflicts.
set(S2N_TLS_CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS})
if(ARROW_BUILD_TESTS AND ARROW_TEST_LINKAGE STREQUAL "shared")
list(APPEND S2N_TLS_CMAKE_ARGS -DS2N_INTERN_LIBCRYPTO=ON)
endif()
externalproject_add(s2n_tls_ep
${EP_COMMON_OPTIONS}
URL ${S2N_TLS_SOURCE_URL}
URL_HASH "SHA256=${ARROW_S2N_TLS_BUILD_SHA256_CHECKSUM}"
CMAKE_ARGS ${S2N_TLS_CMAKE_ARGS}
CMAKE_ARGS ${AWSSDK_COMMON_CMAKE_ARGS}
BUILD_BYPRODUCTS ${S2N_TLS_STATIC_LIBRARY})
add_dependencies(AWS::s2n-tls s2n_tls_ep)

Expand Down
50 changes: 23 additions & 27 deletions cpp/src/arrow/filesystem/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,26 @@ if(ARROW_GCS)
Boost::system)
endif()

set(ARROW_S3_BUILD_TESTS ${ARROW_BUILD_TESTS})
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)
set(ARROW_S3_BUILD_TESTS OFF)
endif()
endif()

if(ARROW_S3_BUILD_TESTS)
add_arrow_test(s3fs_test
SOURCES
s3fs_test.cc
Expand All @@ -57,33 +76,10 @@ if(ARROW_S3)
EXTRA_LINK_LIBS
Boost::filesystem
Boost::system)
if(TARGET arrow-s3fs-test)
set(ARROW_S3FS_TEST_COMPILE_DEFINITIONS)
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")
endif()
target_compile_definitions(arrow-s3fs-test
PRIVATE ${ARROW_S3FS_TEST_COMPILE_DEFINITIONS})
endif()

if(ARROW_BUILD_TESTS)
add_executable(arrow-s3fs-narrative-test s3fs_narrative_test.cc)
target_link_libraries(arrow-s3fs-narrative-test ${ARROW_TEST_LINK_LIBS}
${GFLAGS_LIBRARIES} GTest::gtest)
add_dependencies(arrow-tests arrow-s3fs-narrative-test)
endif()
add_executable(arrow-s3fs-narrative-test s3fs_narrative_test.cc)
target_link_libraries(arrow-s3fs-narrative-test ${ARROW_TEST_LINK_LIBS}
${GFLAGS_LIBRARIES} GTest::gtest)
add_dependencies(arrow-tests arrow-s3fs-narrative-test)

if(ARROW_BUILD_BENCHMARKS AND ARROW_PARQUET)
add_arrow_benchmark(s3fs_benchmark
Expand Down
14 changes: 0 additions & 14 deletions cpp/src/arrow/filesystem/s3fs_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -151,27 +151,13 @@ class AwsTestMixin : public ::testing::Test {
AwsTestMixin() : ec2_metadata_disabled_guard_("AWS_EC2_METADATA_DISABLED", "true") {}

void SetUp() override {
#ifdef AWS_CPP_SDK_S3_NOT_SHARED
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] {
return std::make_shared<Aws::Utils::Logging::ConsoleLogSystem>(aws_log_level);
};
Aws::InitAPI(aws_options_);
#endif
}

void TearDown() override {
#ifdef AWS_CPP_SDK_S3_NOT_SHARED
Aws::ShutdownAPI(aws_options_);
#endif
}

private:
EnvVarGuard ec2_metadata_disabled_guard_;
#ifdef AWS_CPP_SDK_S3_NOT_SHARED
Aws::SDKOptions aws_options_;
#endif
};

class S3TestMixin : public AwsTestMixin {
Expand Down

0 comments on commit 8d2eebe

Please sign in to comment.