Copilot commented on code in PR #50195:
URL: https://github.com/apache/arrow/pull/50195#discussion_r4003038737
##########
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}
Review Comment:
This target now installs `libarrow_s3` plus the ArrowS3 CMake/pkg-config
metadata, but `dev/tasks/linux-packages/apache-arrow/yum/arrow.spec.in` still
has no S3 runtime or devel `%package`/`%files` entries. Since the RPM build
enables `ARROW_S3`, these files will be left unpackaged (or make the RPM build
fail), so the Linux package split is incomplete. Please update the RPM spec
with the corresponding packages and dependencies.
##########
c_glib/arrow-glib/meson.build:
##########
@@ -225,6 +225,9 @@ if not gio.found()
gio = dependency('gio-2.0')
endif
dependencies = [arrow_acero, arrow_compute, arrow, gobject, gio]
+if arrow_s3.found()
+ dependencies += arrow_s3
+endif
Review Comment:
`arrow_s3` is optional here, but `file-system.cpp` unconditionally gets the
`ARROW_S3` feature macro from `arrow/filesystem/api.h` when the C++ Arrow build
has S3 enabled and therefore still emits calls to `InitializeS3`, `FinalizeS3`,
and S3 types. If the separate ArrowS3 package/library is absent, this target
can be built without that dependency and then has unresolved S3 symbols (or
fails when linked statically); please make ArrowS3 required whenever the
detected Arrow build has S3 enabled, or gate the S3 GLib implementation
consistently.
##########
dev/tasks/linux-packages/apache-arrow/debian/control.in:
##########
@@ -126,6 +126,19 @@ Description: Apache Arrow is a data processing library for
analysis
.
This package provides C++ library files for Dataset module.
+Package: libarrow-s3-2600
+Section: libs
+Architecture: any
+Multi-Arch: same
+Pre-Depends: ${misc:Pre-Depends}
Review Comment:
These Debian entries add the new S3 packages, but
`dev/tasks/linux-packages/apache-arrow/yum/arrow.spec.in` has no corresponding
`%package`/`%files` entries for `libarrow_s3.so`, `libarrow_s3.a`, `ArrowS3`,
or `arrow-s3.pc`. Since the RPM build enables ARROW_S3, the new installed files
will be unpackaged (or the RPM build will fail); add matching runtime and devel
RPM subpackages and dependencies.
--
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]
