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 8ba3264e5a GH-37394: [C++][S3] Use AWS_SDK_VERSION_* instead of
try_compile() (#37395)
8ba3264e5a is described below
commit 8ba3264e5a647d817f17be29f744b55c87e6fff7
Author: Sutou Kouhei <[email protected]>
AuthorDate: Wed Aug 30 09:11:52 2023 +0900
GH-37394: [C++][S3] Use AWS_SDK_VERSION_* instead of try_compile() (#37395)
### Rationale for this change
I don't know why but `try_compile()` based `Aws::SDKOptions::ioOptions`
detection seems not work on some environments. Some unrelated errors may be
happen.
### What changes are included in this PR?
Using `AWS_SDK_VERSION_{MAJOR,MINOR,PATCH}` instead of `try_compile()`.
Downside: `AWS_SDK_VERSION_*` are available since 1.9.7 or later but
`Aws::SDKOptions::ioOptions` is available since 1.9.0. So we can't detect
`Aws::SDKOptions::ioOptions` with [1.9.0,1.9.6]. I think that it's acceptable.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #37394
Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
cpp/src/arrow/CMakeLists.txt | 10 ------
cpp/src/arrow/filesystem/s3fs.cc | 39 +++++++++++++++++-----
.../arrow/filesystem/try_compile/check_s3fs_crt.cc | 28 ----------------
3 files changed, 31 insertions(+), 46 deletions(-)
diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt
index a398e790de..ba3f0ef30d 100644
--- a/cpp/src/arrow/CMakeLists.txt
+++ b/cpp/src/arrow/CMakeLists.txt
@@ -497,16 +497,6 @@ if(ARROW_FILESYSTEM)
list(APPEND ARROW_SRCS filesystem/hdfs.cc)
endif()
if(ARROW_S3)
- try_compile(S3_HAS_CRT ${CMAKE_CURRENT_BINARY_DIR}/try_compile
- SOURCES
"${CMAKE_CURRENT_SOURCE_DIR}/filesystem/try_compile/check_s3fs_crt.cc"
- CMAKE_FLAGS
"-DINCLUDE_DIRECTORIES=${CURRENT_INCLUDE_DIRECTORIES}"
- LINK_LIBRARIES ${AWSSDK_LINK_LIBRARIES} CXX_STANDARD 17)
-
- if(S3_HAS_CRT)
- message(STATUS "AWS SDK is new enough to have CRT support")
- add_definitions(-DARROW_S3_HAS_CRT)
- endif()
-
list(APPEND ARROW_SRCS filesystem/s3fs.cc)
set_source_files_properties(filesystem/s3fs.cc
PROPERTIES SKIP_PRECOMPILE_HEADERS ON
diff --git a/cpp/src/arrow/filesystem/s3fs.cc b/cpp/src/arrow/filesystem/s3fs.cc
index c67f7668ff..29f8882225 100644
--- a/cpp/src/arrow/filesystem/s3fs.cc
+++ b/cpp/src/arrow/filesystem/s3fs.cc
@@ -44,6 +44,7 @@
#include <aws/core/Aws.h>
#include <aws/core/Region.h>
+#include <aws/core/VersionConfig.h>
#include <aws/core/auth/AWSCredentials.h>
#include <aws/core/auth/AWSCredentialsProviderChain.h>
#include <aws/core/auth/STSCredentialsProvider.h>
@@ -53,11 +54,6 @@
#include <aws/core/utils/logging/ConsoleLogSystem.h>
#include <aws/core/utils/stream/PreallocatedStreamBuf.h>
#include <aws/core/utils/xml/XmlSerializer.h>
-#ifdef ARROW_S3_HAS_CRT
-#include <aws/crt/io/Bootstrap.h>
-#include <aws/crt/io/EventLoopGroup.h>
-#include <aws/crt/io/HostResolver.h>
-#endif
#include <aws/identity-management/auth/STSAssumeRoleCredentialsProvider.h>
#include <aws/s3/S3Client.h>
#include <aws/s3/S3Errors.h>
@@ -80,6 +76,35 @@
#include <aws/s3/model/PutObjectRequest.h>
#include <aws/s3/model/UploadPartRequest.h>
+// AWS_SDK_VERSION_{MAJOR,MINOR,PATCH} are available since 1.9.7.
+#if defined(AWS_SDK_VERSION_MAJOR) && defined(AWS_SDK_VERSION_MINOR) && \
+ defined(AWS_SDK_VERSION_PATCH)
+// Redundant "(...)" are for suppressing "Weird number of spaces at
+// line-start. Are you using a 2-space indent? [whitespace/indent]
+// [3]" errors...
+#define ARROW_AWS_SDK_VERSION_CHECK(major, minor, patch) \
+ ((AWS_SDK_VERSION_MAJOR > (major) || \
+ (AWS_SDK_VERSION_MAJOR == (major) && AWS_SDK_VERSION_MINOR > (minor)) || \
+ ((AWS_SDK_VERSION_MAJOR == (major) && AWS_SDK_VERSION_MINOR == (minor) && \
+ AWS_SDK_VERSION_PATCH >= (patch)))))
+#else
+#define ARROW_AWS_SDK_VERSION_CHECK(major, minor, patch) 0
+#endif
+
+// This feature is available since 1.9.0 but
+// AWS_SDK_VERSION_{MAJOR,MINOR,PATCH} are available since 1.9.7. So
+// we can't use this feature for [1.9.0,1.9.6]. If it's a problem,
+// please report it to our issue tracker.
+#if ARROW_AWS_SDK_VERSION_CHECK(1, 9, 0)
+#define ARROW_S3_HAS_CRT
+#endif
+
+#ifdef ARROW_S3_HAS_CRT
+#include <aws/crt/io/Bootstrap.h>
+#include <aws/crt/io/EventLoopGroup.h>
+#include <aws/crt/io/HostResolver.h>
+#endif
+
#include "arrow/util/windows_fixup.h"
#include "arrow/buffer.h"
@@ -2913,9 +2938,7 @@ struct AwsInstance {
return std::make_shared<Aws::Utils::Logging::ConsoleLogSystem>(
aws_options_.loggingOptions.logLevel);
};
-#if (defined(AWS_SDK_VERSION_MAJOR) && \
- (AWS_SDK_VERSION_MAJOR > 1 || AWS_SDK_VERSION_MINOR > 9 || \
- (AWS_SDK_VERSION_MINOR == 9 && AWS_SDK_VERSION_PATCH >= 272)))
+#if ARROW_AWS_SDK_VERSION_CHECK(1, 9, 272)
// ARROW-18290: escape all special chars for compatibility with non-AWS S3
backends.
// This configuration options is only available with AWS SDK 1.9.272 and
later.
aws_options_.httpOptions.compliantRfc3986Encoding = true;
diff --git a/cpp/src/arrow/filesystem/try_compile/check_s3fs_crt.cc
b/cpp/src/arrow/filesystem/try_compile/check_s3fs_crt.cc
deleted file mode 100644
index 83240effdb..0000000000
--- a/cpp/src/arrow/filesystem/try_compile/check_s3fs_crt.cc
+++ /dev/null
@@ -1,28 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied. See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-// Dummy file for checking if IOOptions exists in SDKOptions.
-// This was introduced when the AWS SDK switched to using the
-// CRT for I/O.
-
-#include <aws/core/Aws.h>
-
-int main() {
- Aws::SDKOptions aws_options;
- auto io_options = aws_options.ioOptions;
- return 0;
-}
