You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/09/22 22:58:11 UTC
[GitHub] [arrow] nealrichardson opened a new pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
nealrichardson opened a new pull request #8243:
URL: https://github.com/apache/arrow/pull/8243
I've tried enabling this in the R linux builds and have made some progress, but I'm hitting some issues that someone more experienced with cmake might be able to help with.
* In the everything bundled static build that is the default for R on Linux (the rstudio/r-base centos jobs), the addition of aws-sdk-cpp means that openssl is required, but the system doesn't have it, and we don't have a bundled openssl external project. TileDB does: https://github.com/TileDB-Inc/TileDB/blob/dev/cmake/Modules/FindOpenSSL_EP.cmake#L85-L118 so maybe we can use that for inspiration.
* In the ubuntu R jobs that are based on the ubuntu-cpp image, openssl is present so the build doesn't fail, but with ARROW_DEPENDENCY_SOURCE=AUTO, cmake fails unless you set AWSSDK_SOURCE=BUNDLED because it can't "find" it. We should fix that.
* Setting AWSSDK_SOURCE=BUNDLED, [awssdk_ep builds successfully](https://github.com/nealrichardson/arrow/runs/1151722752?check_suite_focus=true#step:8:1379), but Arrow s3fs.cc compilation fails because it cant find the aws-sdk headers. Looking at build_awssdk and comparing with other ep's, it's surely incomplete, and among other things it probably needs `-DBUILD_SHARED_LIBS=OFF`. This is where I'm not sure how to finish wiring this ep up. Once again, TileDB may be a point of reference: https://github.com/TileDB-Inc/TileDB/blob/dev/cmake/Modules/FindAWSSDK_EP.cmake
Solving this will also let us enable S3 in more Python wheels; cf. #7997
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou edited a comment on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
kou edited a comment on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697040532
For the aws-sdk headers, the following patch will fix them:
```diff
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 0180ef912..033e18491 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -2629,10 +2629,19 @@ macro(build_awssdk)
set(AWSSDK_BUILD_TYPE Release)
endif()
+ set(AWSSDK_LIST_SEPARATOR "|")
+ string(JOIN
+ ${AWSSDK_LIST_SEPARATOR}
+ AWSSDK_BUILD_ONLY
+ s3
+ core
+ config
+ identity-management
+ sts)
set(AWSSDK_CMAKE_ARGS
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_INSTALL_LIBDIR=lib
- -DBUILD_ONLY=s3;core;config;identity-management;sts
+ -DBUILD_ONLY=${AWSSDK_BUILD_ONLY}
-DENABLE_UNITY_BUILD=on
-DENABLE_TESTING=off
"-DCMAKE_C_FLAGS=${EP_C_FLAGS}"
@@ -2661,7 +2670,8 @@ macro(build_awssdk)
${EP_LOG_OPTIONS}
URL ${AWSSDK_SOURCE_URL}
CMAKE_ARGS ${AWSSDK_CMAKE_ARGS}
- BUILD_BYPRODUCTS ${AWSSDK_SHARED_LIBS})
+ BUILD_BYPRODUCTS ${AWSSDK_SHARED_LIBS}
+ LIST_SEPARATOR ${AWSSDK_LIST_SEPARATOR})
file(MAKE_DIRECTORY ${AWSSDK_INCLUDE_DIR})
```
We need to escape `;` in `BUILD_ONLY`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou edited a comment on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
kou edited a comment on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697040532
For the aws-sdk headers, the following patch will fix them:
```diff
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 0180ef912..033e18491 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -2629,10 +2629,19 @@ macro(build_awssdk)
set(AWSSDK_BUILD_TYPE Release)
endif()
+ set(AWSSDK_LIST_SEPARATOR "|")
+ string(JOIN
+ ${AWSSDK_LIST_SEPARATOR}
+ AWSSDK_BUILD_ONLY
+ s3
+ core
+ config
+ identity-management
+ sts)
set(AWSSDK_CMAKE_ARGS
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_INSTALL_LIBDIR=lib
- -DBUILD_ONLY=s3;core;config;identity-management;sts
+ -DBUILD_ONLY=${AWSSDK_BUILD_ONLY}
-DENABLE_UNITY_BUILD=on
-DENABLE_TESTING=off
"-DCMAKE_C_FLAGS=${EP_C_FLAGS}"
@@ -2661,7 +2670,8 @@ macro(build_awssdk)
${EP_LOG_OPTIONS}
URL ${AWSSDK_SOURCE_URL}
CMAKE_ARGS ${AWSSDK_CMAKE_ARGS}
- BUILD_BYPRODUCTS ${AWSSDK_SHARED_LIBS})
+ BUILD_BYPRODUCTS ${AWSSDK_SHARED_LIBS}
+ LIST_SEPARATOR ${AWSSDK_LIST_SEPARATOR})
file(MAKE_DIRECTORY ${AWSSDK_INCLUDE_DIR})
```
We need to escape `;` in `BUILD_ONLY`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697040532
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-701016718
Closing in favor of #8304
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697054182
Ok, with that SEMICOLON change, the Ubuntu R job successfully compiles the C++ library, but the R package can't use it because it created a shared library for aws-sdk-cpp (or several), and those are stuck inside the build/awssdk_ep directory: https://github.com/apache/arrow/pull/8243/checks?check_run_id=1152416386#step:9:61
I think this means the bundled ep needs to build aws-sdk-cpp static, like it does for e.g. jemalloc--right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697027877
https://issues.apache.org/jira/browse/ARROW-10068
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697151804
Right.
It seems that we need to specify `-DBUILD_SHARED_LIBS=off`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697049024
Ah, we can use `$<SEMICOLON>` https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html#escaped-characters instead of `LIST_SEPARATOR` like TileDB does: https://github.com/TileDB-Inc/TileDB/blob/dev/cmake/Modules/FindAWSSDK_EP.cmake#L104
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697040532
For the ubuntu R jobs, the following patch will fix them:
```diff
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 0180ef912..033e18491 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -2629,10 +2629,19 @@ macro(build_awssdk)
set(AWSSDK_BUILD_TYPE Release)
endif()
+ set(AWSSDK_LIST_SEPARATOR "|")
+ string(JOIN
+ ${AWSSDK_LIST_SEPARATOR}
+ AWSSDK_BUILD_ONLY
+ s3
+ core
+ config
+ identity-management
+ sts)
set(AWSSDK_CMAKE_ARGS
-DCMAKE_BUILD_TYPE=Release
-DCMAKE_INSTALL_LIBDIR=lib
- -DBUILD_ONLY=s3;core;config;identity-management;sts
+ -DBUILD_ONLY=${AWSSDK_BUILD_ONLY}
-DENABLE_UNITY_BUILD=on
-DENABLE_TESTING=off
"-DCMAKE_C_FLAGS=${EP_C_FLAGS}"
@@ -2661,7 +2670,8 @@ macro(build_awssdk)
${EP_LOG_OPTIONS}
URL ${AWSSDK_SOURCE_URL}
CMAKE_ARGS ${AWSSDK_CMAKE_ARGS}
- BUILD_BYPRODUCTS ${AWSSDK_SHARED_LIBS})
+ BUILD_BYPRODUCTS ${AWSSDK_SHARED_LIBS}
+ LIST_SEPARATOR ${AWSSDK_LIST_SEPARATOR})
file(MAKE_DIRECTORY ${AWSSDK_INCLUDE_DIR})
```
We need to escape `;` in `BUILD_ONLY`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697989129
Ah, wheel...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697027877
https://issues.apache.org/jira/browse/ARROW-10068
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697352808
> OpenSSL is a security related product. I'm not sure we can release a new version ASAP when bundled OpenSSL has a vulnerability.
I agree, but I think we already do that with Flight...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson closed pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
nealrichardson closed pull request #8243:
URL: https://github.com/apache/arrow/pull/8243
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697047650
It's not a strong opinion but I'm negative we bundle OpenSSL.
OpenSSL is a security related product. I'm not sure we can release a new version ASAP when bundled OpenSSL has a vulnerability.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #8243: ARROW-10068: [C++] Add bundled external project for aws-sdk-cpp
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #8243:
URL: https://github.com/apache/arrow/pull/8243#issuecomment-697054182
Ok, with that SEMICOLON change, the Ubuntu R job successfully compiles the C++ library, but the R package can't use it because it created a shared library for aws-sdk-cpp (or several), and those are stuck inside the build/awssdk_ep directory: https://github.com/apache/arrow/pull/8243/checks?check_run_id=1152416386#step:9:61
I think this means the bundled ep needs to build aws-sdk-cpp static, like it does for e.g. jemalloc--right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org