tadeja commented on code in PR #50195:
URL: https://github.com/apache/arrow/pull/50195#discussion_r4007292657
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -1042,21 +1021,59 @@ if(ARROW_FILESYSTEM)
endforeach()
endif()
if(ARROW_S3)
- foreach(ARROW_FILESYSTEM_TARGET ${ARROW_FILESYSTEM_TARGETS})
- target_link_libraries(${ARROW_FILESYSTEM_TARGET} PRIVATE
${AWSSDK_LINK_LIBRARIES})
- endforeach()
-
- if(ARROW_S3_MODULE)
- if(NOT ARROW_BUILD_SHARED)
- message(FATAL_ERROR "ARROW_S3_MODULE without shared libarrow
(-DARROW_BUILD_SHARED=ON) is not supported"
- )
+ # If libarrow_s3.a is only built, "pkg-config --cflags --libs
+ # arrow-s3" outputs build flags for static linking not shared
+ # linking. ARROW_S3_PC_* except ARROW_S3_PC_*_PRIVATE are for the
+ # static linking case.
+ if(NOT ARROW_BUILD_SHARED AND ARROW_BUILD_STATIC)
+ string(APPEND ARROW_S3_PC_CFLAGS "${ARROW_S3_PC_CFLAGS_PRIVATE}")
+ set(ARROW_S3_PC_CFLAGS_PRIVATE "")
+ set(ARROW_S3_PC_LIBS "${ARROW_S3_PC_LIBS_PRIVATE}")
+ set(ARROW_S3_PC_LIBS_PRIVATE "")
+ set(ARROW_S3_PC_REQUIRES "${ARROW_S3_PC_REQUIRES_PRIVATE}")
+ set(ARROW_S3_PC_REQUIRES_PRIVATE "")
+ else()
+ set(ARROW_S3_PC_LIBS "")
+ set(ARROW_S3_PC_REQUIRES "")
+ endif()
+ list(APPEND ARROW_S3_LIB_SRCS filesystem/s3fs_module.cc filesystem/s3fs.cc)
+ set(ARROW_S3_STATIC_INSTALL_INTERFACE_LIBS Arrow::arrow_static)
+ if(AWSSDK_SOURCE STREQUAL "SYSTEM")
+ list(APPEND ARROW_S3_STATIC_INSTALL_INTERFACE_LIBS
${AWSSDK_LINK_LIBRARIES})
+ elseif(AWSSDK_SOURCE STREQUAL "BUNDLED")
+ if(UNIX)
+ list(APPEND ARROW_S3_STATIC_INSTALL_INTERFACE_LIBS CURL::libcurl)
endif()
-
- add_library(arrow_s3fs MODULE filesystem/s3fs_module.cc
filesystem/s3fs.cc)
- target_link_libraries(arrow_s3fs PRIVATE ${AWSSDK_LINK_LIBRARIES}
arrow_shared)
- set_source_files_properties(filesystem/s3fs.cc filesystem/s3fs_module.cc
- PROPERTIES SKIP_UNITY_BUILD_INCLUSION ON)
endif()
+ add_arrow_lib(arrow_s3
+ CMAKE_PACKAGE_NAME
+ ArrowS3
+ PKG_CONFIG_NAME
+ arrow-s3
+ SOURCES
+ ${ARROW_S3_LIB_SRCS}
+ SHARED_LINK_LIBS
+ arrow_shared
+ SHARED_PRIVATE_LINK_LIBS
+ ${AWSSDK_LINK_LIBRARIES}
+ SHARED_INSTALL_INTERFACE_LIBS
+ Arrow::arrow_shared
+ STATIC_LINK_LIBS
+ arrow_static
+ ${AWSSDK_LINK_LIBRARIES}
+ STATIC_INSTALL_INTERFACE_LIBS
+ ${ARROW_S3_STATIC_INSTALL_INTERFACE_LIBS}
Review Comment:
SHARED_LINK_FLAGS seem to be still needed for bundled builds, as without
version-script `symbols.map` this shows **8308 symbols**
```console
nm -D --defined-only --demangle libarrow_s3.so | grep -Ec ' [A-Z] Aws::'
```
after applying '${ARROW_VERSION_SCRIPT_FLAGS}' below, the same shows **0
symbols**
(Arrow S3 symbols like `arrow::fs::EnsureS3Initialized()` and
`arrow_filesystem_get_registry` export OK)
```suggestion
${ARROW_S3_STATIC_INSTALL_INTERFACE_LIBS}
SHARED_LINK_FLAGS
${ARROW_VERSION_SCRIPT_FLAGS}
```
That's including moving the existing
[if(CXX_LINKER_SUPPORTS_VERSION_SCRIPT)](https://github.com/raulcd/arrow/blob/040bce12b21fdc40ffb4b9604bc71029a061dd2e/cpp/src/arrow/CMakeLists.txt#L1142)
section just before current line 970 `if(ARROW_FILESYSTEM)` so it gets placed
before `add_arrow_lib(arrow_s3` section.
( Potentially
[add_arrow_lib(arrow_compute](https://github.com/raulcd/arrow/blame/040bce12b21fdc40ffb4b9604bc71029a061dd2e/cpp/src/arrow/CMakeLists.txt#L947)
also needs that as `ARROW_VERSION_SCRIPT_FLAGS` variable in compute seciton
seems empty since compute move? However, checking and potentially moving before
compute section should be a separate issue, 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
